Skip to content

lib.sh prints "matches the binary under test" after comparing two source fingerprints, so a prefix another process wrote is certified #959

Description

@OffgridwithJD

lib.sh prints "source <hash> matches the binary under test" after comparing two source fingerprints. It never looks at the installed library. So when another process has written the shared prefix, the harness asserts a positive fact about a binary it has not examined, and then reports checks against it.

This is the shell twin of #956, which #957 fixed for the pytest layer. I had concluded the shell path was free of this class because it degrades honestly to freshness UNVERIFIED — that is true of the unknown branch and false of the fresh branch, which makes the positive claim. @jdatcmd's logs showed it first; this is an independent reproduction with the mechanism.

Measured, deliberately constructed

Two trees whose src/ differs by 5 files. B builds through the harness, so B gets a stamp. A then installs its library into the same prefix. B runs #945's own suite with PGC_SKIP_BUILD=1.

A (foreign installer)  a2e189d7   pre-#945
B (runs the suite)     b90dda90   main
src/ differs           5 files

1. B builds+installs via pgc_build_and_install   .so a6bc267f693a, B stamps: 1
2. A installs into the same prefix               .so d312a10c0cfb  (A's), B's stamp untouched
3. B runs with PGC_SKIP_BUILD=1:

   -- PGC_SKIP_BUILD=1: not building AND NOT INSTALLING;
   -- .so: d312a10c0cfb /usr/local/pg18a/lib/postgresql/pgcolumnar.so
   -- source: a0e6afc3e13e matches the binary under test        <-- FALSE
   FAIL  plan has runtime coordinator: got [0] want [1]
   FAIL  plan has build tap: got [0] want [1]
   FAIL  build rows omit NULL: got [] want [201]
   FAIL  filter ready before scan: got [0] want [1]
   FAIL  clustered groups removed: got [] want [19]
   FAIL  clustered reads fewer groups: got [20] want [1]
   FAIL  scattered plan has coordinator: got [0] want [1]
   FAIL  scattered bloom rejects most non-matches: got [0] want [1]

Nine failures, the code entirely innocent, under a printed assurance that the binary matched. @jdatcmd measured the same shape on PG 17: the identical sentence above two different libraries with opposite outcomes — 9edef563e34d giving 25 passed + 19 FAILED, 2c9559d087b0 giving 44 passed + 0 failed.

The mechanism

pgc_freshness_verdict (lib.sh:768) compares recorded against current. Both are source fingerprints. The caller then prints a sentence about the binary.

And the stamp is keyed per source tree:

A: /root/wv3/.pgc_source_stamp.18.d9e24bec
B: /root/wfpB/.pgc_source_stamp.18.d9e24bec

Same suffix — major plus installation — but the file lives in the source directory. So a tree's stamp records only what that tree last built. It is structurally unable to observe another tree overwriting the shared prefix, which is why B's stamp still matched while the prefix held A's library.

This also answers the question @jdatcmd explicitly declined to guess at — why their recorded stamp held main's fingerprint when a pre-#945 install had happened in between. Both their runs did build through the harness; the later one wrote its stamp in its own tree, leaving main's untouched and still matching. No mis-modelled write path, just per-tree keying.

What is and is not wrong here

The unknown branch is right and should not change. It prints freshness UNVERIFIED (no stamp for major N), and its comment is correct that refusing would break a documented hand-install workflow. Measured separately: with no stamp, a foreign binary gives exactly that line and no false claim.

The fresh branch is the defect. It has the evidence for "the source has not changed since this tree last built" and prints a claim about the binary.

Worth noting what already works: pgc_so_line prints the library's md5 unconditionally, and it is how both of us eventually diagnosed this. The fingerprint is there; it just sits beside a sentence that contradicts it.

Proposed

Record the installed library's md5 in the stamp beside the source fingerprint, and compare the file on disk before claiming a match. #957 did exactly this for the pytest layer.

  • fresh requires both the source fingerprint and the recorded library digest to match what is on disk.
  • A library that changed under the stamp is not fresh. It should be refused, the way a changed source already is — the binary is demonstrably not the one this tree built.
  • unknown is unchanged: a hand install records no digest, so it still reports UNVERIFIED and still works.

The trap for the implementation, from #956: the digest is not a function of the source, because the build path is compiled in. @jdatcmd measured 2c9559d087b0 and 757591c69d32 from one commit with only the build directory differing. So the stamp must record the digest that was installed when it was written, never a per-source constant.

Not established, by either of us

The stale -> FATAL branch is read but unexercised. I tried to reach it and landed in unknown, because a bare make install writes no stamp — pgc_write_source_stamp is called from exactly one place, inside pgc_build_and_install. So neither of us has demonstrated the FATAL path fires; we have only read it.

Claiming it

I will take this unless @jdatcmd or @linuxhikerpm would rather. It is shell-side, so it lands ledger rows and moves checks_never_observed_red — with the board empty that composes with nothing, which is the opposite of the situation that had me defer #901's drift guard earlier today.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions