fix(release): pin collation on the manifest hashes - #1298
Merged
Conversation
justinjoy
force-pushed
the
fix/1294-manifest-collation
branch
from
September 2, 2026 05:55
16618ed to
1d15537
Compare
Two release gates hash a sorted file list and compare the result against a pinned or committed value, and neither pinned the collation: scripts/ci/run-doop-perf-gate.sh vs WIRELOG_DOOP_DATASET_MANIFEST_SHA256 scripts/release/run-downstream-matrix.sh vs downstream-matrix-oracles.tsv glibc's en_US collation ignores the hyphen in Method-Modifier.facts, moving it after MethodHandleConstant.facts, so a runner with a non-C locale computed a different manifest and the gate failed claiming the dataset had been substituted. That is the worst available message for the actual cause, and run-downstream-matrix.sh is invoked from release-tag.yml, so it gates GA. Verified against the real zxing dataset rather than reasoned about. Downloaded it through bench/data/doop/download.sh, archive checksum confirmed, then computed the manifest both ways: C 215ddcc50bca70c0089e0ced9298274aec4edb6741736d03cc3c4386b96f831c en_US 472346fb234ac0610e2a0b83b7076e8279525aeeade628261fd98433d54d62b6 The committed oracle is the C-order value, so pinning to C is corrective rather than a re-baseline. The other five workloads were checked the same way: their file sets do not discriminate, they reproduce their oracles under C, and no oracle anywhere needs updating. That check was the precondition -- pinning a sort whose oracle came from a different collation would have converted an intermittent failure into a permanent one, and looked like silently re-baselining a security-relevant hash. C is also the only collation a committed hash can safely depend on: it is byte order and invariant, while en_US collation is not stable across glibc versions -- 2.28 rewrote it -- so an en_US-derived oracle is pinned to a moving target. check-manifest-collation.sh guards both sites. It lifts the two manifest functions out of run-downstream-matrix.sh with sed rather than retyping them, so unpinning either one in that script makes the test fail; a retyped copy would assert only that GNU sort honours an explicit prefix, which nothing here can break. The perf gate's manifest is an inline expression and cannot be lifted, so a static pin check covers it. WIRELOG_DOOP_DATASET_MANIFEST_SHA256 has no committed value anywhere in this repository, so any existing pin is out-of-tree; its owner must recompute it under LC_ALL=C. And "C == en_US today" for the five non-discriminating workloads is an accident of their filenames, not a property to rely on elsewhere. The perf gate's manifest also embeds the directory path it is given, so relative and absolute paths yield different hashes -- the same tampering-shaped message from a second cause. That is #1297. Refs: #1294, #1297
justinjoy
force-pushed
the
fix/1294-manifest-collation
branch
from
September 2, 2026 05:56
1d15537 to
af2d0d0
Compare
The en_US branch exited 0, so on a host without that locale meson recorded the gate as passing while it asserted nothing. Same defect as #1301, introduced in a gate written to fix a different one, and caught by that issue's shared guard only when the two branches were merged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1294. Two release gates hashed a sorted file list and compared the
result against a pinned or committed value, with the collation unpinned.
The defect is live on the GA path, not latent
run-downstream-matrix.shis invoked fromrelease-tag.yml:279, so itsmanifest check gates GA. glibc's
en_UScollation ignores the hyphen inMethod-Modifier.facts, moving it afterMethodHandleConstant.facts. On arunner with a non-C locale the manifest differs and the gate fails with
— which reads as dataset tampering. The worst available message for the actual
cause.
Verified against the real dataset, not reasoned about
Downloaded the zxing dataset through
bench/data/doop/download.sh(archivechecksum confirmed) and computed the manifest both ways:
The committed oracle is the C-order value, so pinning is corrective rather
than a re-baseline.
The other five workloads were checked the same way before touching anything —
cspa-fast,galen,polonius,ddisasm,crdtall reproduce theircommitted oracles under C and are collation-invariant. That check was the
precondition: pinning a sort whose oracle came from a different collation would
convert an intermittent failure into a permanent one, and would look like
silently re-baselining a security-relevant hash.
C is also the only collation a committed hash can safely depend on — it is byte
order and invariant, whereas en_US collation is not stable across glibc
versions (2.28 rewrote it), so an en_US-derived oracle is pinned to a moving
target.
The guard
check-manifest-collation.shlifts the two manifest functions out ofrun-downstream-matrix.shwithsedrather than retyping them, so unpinningeither one in that script makes the test fail. A retyped copy would assert only
that GNU
sorthonours an explicitLC_ALL=Cprefix — a coreutils propertynothing here can break. The perf gate's manifest is an inline expression and
cannot be lifted, so a static pin check covers that site.
Between the three review gates, 12 mutations, 12 caught, each with its own
message: unpinning either function, unpinning or removing the perf-gate sort, a
partial rewrite to
--zero-terminatedwith the sibling still pinned, a plainunpinned
sortadded beside a pinned one, a renamed or deleted function, anunparseable body, and a truncated extraction that parses but returns nothing.
That last one is worth flagging to reviewers: the guard passed three earlier
revisions while a truncating
}would have left it comparing two empty strings.Both the review and risk gates found it independently.
Operator note
WIRELOG_DOOP_DATASET_MANIFEST_SHA256has no committed value anywhere in thisrepository, so any existing pin is out-of-tree. Its owner must recompute it
under
LC_ALL=C— otherwise this change flips that runner red.Also: "C == en_US today" for the five non-discriminating workloads is an
accident of their filenames, not a property to rely on elsewhere.
Follow-up
#1297 — the perf gate's manifest embeds the directory path it is given, so
relative, absolute and trailing-slash forms yield three hashes from one dataset:
the same tampering-shaped message from a second cause.
manifest_for_doopisimmune because it strips the prefix with
sed, so the two computations have nowdrifted twice — on collation and on path handling — which argues for one shared
helper taking the downstream matrix as reference.