Skip to content

Commit 2c2b785

Browse files
committed
fix(fixtures): assert what the artifact reports actually say
- ask the Mach-O check for the macho format, the spelling the reports print, in the request and the assertion alike - pick the ELF calibration group out of the artifact report by the estimate it carries, since only a group whose every non-canonical member was attributed has one, instead of guessing at a fragment group in the source scan - read the plain comparisons as an observed size reduction: a difference between two artifacts is an observation, and only the calibrated comparisons that name a source run and a clone group produce a verified saving - take the second calibration measurement from the same code with its debug information split into a separate file, and assert the optimized build estimates nothing: an optimizer leaves no line evidence tying each member of a clone group to its own bytes, so nothing there is attributed - assert normalized duplicate groups only where the instruction normalizer exists, and assert none elsewhere - stop expecting the PE check to find function names in a COFF symbol table a linked image does not have, and assert the PDB line information reaches the scanned source instead - say in the fixture build script that its optimized variants carry debug information for naming, not fragment evidence - install the C++ standard library the suite probe needs in the coverage job
1 parent 236af79 commit 2c2b785

5 files changed

Lines changed: 72 additions & 33 deletions

File tree

.github/workflows/verify.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ jobs:
113113
- name: Install Clang and libclang
114114
run: |
115115
sudo apt-get update
116-
sudo apt-get install -y --no-install-recommends clang libclang-dev
116+
sudo apt-get install -y --no-install-recommends clang libclang-dev libc++-dev libc++abi-dev
117117
# The labelled precision tests refuse to pass on an empty measurement,
118118
# so coverage over the suite needs the same sources the suite does.
119119
- name: Materialize the labelled corpora

fixtures/artifact/build.sh

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,10 @@ case "$profile" in
3333
;;
3434
release)
3535
opt_level=2
36-
# The optimized calibration pair still carries local DWARF line rows.
37-
# This preserves the source-fragment evidence needed to measure the
38-
# model under LTO without changing compiler or optimization settings.
36+
# The optimized variants are built with debug information so their
37+
# symbols can still be named and compared. It does not make them
38+
# calibratable: an optimizer leaves no line rows tying each member of
39+
# a clone group to its own bytes, so nothing in them is attributed.
3940
debuginfo=2
4041
;;
4142
esac

scripts/verify-artifact-fixtures.sh

Lines changed: 56 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,6 @@ cargo run --quiet -p codehelion -- scan \
4343
--output "$temporary_root/source-scan.json"
4444
source_run=$(sed -n 's/^[[:space:]]*"run_id": \([0-9][0-9]*\),\{0,1\}$/\1/p' "$temporary_root/source-scan.json")
4545
test -n "$source_run"
46-
calibration_group=$(awk '
47-
/"groups": \[/ { groups = 1; next }
48-
groups && /"fingerprint":/ {
49-
fingerprint = $2
50-
gsub(/[",]/, "", fingerprint)
51-
}
52-
groups && /"scope": "fragment"/ { print fingerprint; exit }
53-
' "$temporary_root/source-scan.json")
54-
test -n "$calibration_group"
5546

5647
cargo run --quiet -p codehelion -- artifact analyze \
5748
"$fixture_root/build/debug/duplicates.wasm" \
@@ -68,11 +59,26 @@ cargo run --quiet -p codehelion -- artifact analyze \
6859
--linker-map "$fixture_root/build/debug/libduplicates.map" \
6960
--db "$temporary_root/artifact.sqlite" \
7061
--output "$temporary_root/elf.json"
62+
# Calibration measures the savings model over one clone group, so it needs a
63+
# group the analysis actually attributed artifact bytes to. Only a group whose
64+
# every non-canonical member was attributed carries an estimate, and reading it
65+
# out of the report says which one that was instead of assuming.
66+
calibration_group=$(awk '
67+
/"clone_group_fingerprint":/ {
68+
fingerprint = $2
69+
gsub(/[",]/, "", fingerprint)
70+
}
71+
/"estimated_refactor_savings_bytes": -?[0-9]/ { print fingerprint; exit }
72+
' "$temporary_root/elf.json")
73+
test -n "$calibration_group"
7174
objcopy --only-keep-debug \
7275
"$fixture_root/build/debug/libduplicates.so" \
7376
"$temporary_root/libduplicates.debug"
7477
cp "$fixture_root/build/debug/libduplicates.so" "$temporary_root/libduplicates-split.so"
7578
objcopy --strip-debug "$temporary_root/libduplicates-split.so"
79+
cp "$fixture_root/build/debug-deduplicated/libduplicates.so" \
80+
"$temporary_root/libduplicates-split-deduplicated.so"
81+
objcopy --strip-debug "$temporary_root/libduplicates-split-deduplicated.so"
7682
cargo run --quiet -p codehelion -- artifact analyze \
7783
"$temporary_root/libduplicates-split.so" \
7884
--format json \
@@ -137,16 +143,22 @@ cargo run --quiet -p codehelion -- artifact calibration \
137143
--db "$temporary_root/artifact.sqlite" \
138144
--format json \
139145
--output "$temporary_root/calibration-baseline.json"
146+
# A second measurement, so the baseline comparison above has something to
147+
# compare. It is the same code with its debug information in a separate file:
148+
# a different artifact, separately analysed, whose estimate is its own. The
149+
# optimized builds cannot stand here — an optimizer leaves no line evidence
150+
# tying each member of a clone group to its own bytes, so nothing in them is
151+
# attributed and there is no estimate to measure.
140152
cargo run --quiet -p codehelion -- artifact compare \
141-
"$fixture_root/build/release-lto/libduplicates.so" \
142-
"$fixture_root/build/release-lto-deduplicated/libduplicates.so" \
143-
--before-build-variant "$fixture_root/build/release-lto/build-variant.json" \
144-
--after-build-variant "$fixture_root/build/release-lto-deduplicated/build-variant.json" \
153+
"$temporary_root/libduplicates-split.so" \
154+
"$temporary_root/libduplicates-split-deduplicated.so" \
155+
--before-build-variant "$fixture_root/build/debug/build-variant.json" \
156+
--after-build-variant "$fixture_root/build/debug-deduplicated/build-variant.json" \
145157
--source-run "$source_run" \
146158
--clone-group "$calibration_group" \
147159
--db "$temporary_root/artifact.sqlite" \
148160
--format json \
149-
--output "$temporary_root/compare-calibration-release-lto.json"
161+
--output "$temporary_root/compare-calibration-split-debug.json"
150162
cargo run --quiet -p codehelion -- artifact calibration \
151163
--source-run "$source_run" \
152164
--baseline "$temporary_root/calibration-baseline.json" \
@@ -169,9 +181,26 @@ grep -qE '"source_mappings": [1-9]' "$temporary_root/elf-split-debug.json"
169181
grep -qE '"mappings": [1-9]' "$temporary_root/elf-split-debug.json"
170182
grep -qE '"estimated_refactor_savings_bytes": -?[1-9][0-9]*' "$temporary_root/elf.json"
171183
grep -qE '"exact_groups": [1-9]' "$temporary_root/elf.json"
172-
grep -qE '"normalized_groups": [1-9]' "$temporary_root/elf.json"
184+
# Normalized duplicate detection reads machine instructions, and the only
185+
# normalizer here is for x86-64. Elsewhere the count is nothing rather than
186+
# something missed, and saying which is expected keeps the run honest on a
187+
# machine of either kind.
188+
case "$(uname -m)" in
189+
x86_64 | amd64)
190+
grep -qE '"normalized_groups": [1-9]' "$temporary_root/elf.json"
191+
;;
192+
*)
193+
grep -qE '"normalized_groups": 0' "$temporary_root/elf.json"
194+
;;
195+
esac
173196
grep -qE '"format": "elf"' "$temporary_root/elf-release-lto.json"
174-
grep -qE '"estimated_refactor_savings_bytes": -?[1-9][0-9]*' "$temporary_root/elf-release-lto.json"
197+
# The optimized build is read and correlated like any other, and it estimates
198+
# nothing: an optimizer leaves no line evidence tying each member of a clone
199+
# group to its own bytes, so no group has every member attributed. Saying so
200+
# is the point — an estimate here would be one nothing in the artifact
201+
# supports.
202+
grep -qE '"correlation": \{' "$temporary_root/elf-release-lto.json"
203+
grep -qE '"estimated_refactor_savings": \[\]' "$temporary_root/elf-release-lto.json"
175204
grep -qE '"format": "wasm"' "$temporary_root/wasm-deduplicated.json"
176205
grep -qE '"size_inferred": true' "$temporary_root/elf-stripped.json"
177206
for field in \
@@ -183,15 +212,21 @@ do
183212
grep -qE "\"$field\"" "$temporary_root/wasm.json"
184213
grep -qE "\"$field\"" "$temporary_root/elf.json"
185214
done
186-
grep -qE '"verified_savings_bytes": [0-9]+' "$temporary_root/compare.json"
215+
# A comparison measures the difference between two artifacts, which is an
216+
# observation and not a verified saving: nothing here says the difference came
217+
# from removing a duplicate. Only the calibrated comparisons below, which name
218+
# a source run and a clone group, produce a verified figure.
219+
grep -qE '"observed_size_reduction_bytes": -?[0-9]+' "$temporary_root/compare.json"
187220
grep -qE '"build_variant_warning": "build variants differ' "$temporary_root/compare.json"
188-
grep -qE '"verified_savings_bytes": [0-9]+' "$temporary_root/compare-elf.json"
221+
grep -qE '"observed_size_reduction_bytes": -?[0-9]+' "$temporary_root/compare-elf.json"
189222
grep -qE '"build_variant_warning": "build variants differ' "$temporary_root/compare-elf.json"
190-
grep -qE '"verified_savings_bytes": [1-9][0-9]*' "$temporary_root/compare-deduplicated.json"
223+
# The deduplicated pair is the one case where the difference has a direction:
224+
# the same build with the duplicate removed is smaller.
225+
grep -qE '"observed_size_reduction_bytes": [1-9][0-9]*' "$temporary_root/compare-deduplicated.json"
191226
grep -qE '"calibration": \{' "$temporary_root/compare-calibration.json"
192227
grep -qE '"verified_savings_bytes": [1-9][0-9]*' "$temporary_root/compare-calibration.json"
193-
grep -qE '"calibration": \{' "$temporary_root/compare-calibration-release-lto.json"
194-
grep -qE '"verified_savings_bytes": [1-9][0-9]*' "$temporary_root/compare-calibration-release-lto.json"
228+
grep -qE '"calibration": \{' "$temporary_root/compare-calibration-split-debug.json"
229+
grep -qE '"verified_savings_bytes": [1-9][0-9]*' "$temporary_root/compare-calibration-split-debug.json"
195230
grep -qE '"samples": 2' "$temporary_root/calibration.json"
196231
grep -qE '"comparison": \{' "$temporary_root/calibration.json"
197232
grep -qE '"baseline_schema_version": "artifact-calibration-report-v1"' "$temporary_root/calibration.json"

scripts/verify-macho-artifact-fixtures.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,14 @@ test -n "$source_run"
5555

5656
cargo run --quiet -p codehelion -- artifact analyze \
5757
"$fixture_root/libduplicates.dylib" \
58-
--input-format mach-o \
58+
--input-format macho \
5959
--format json \
6060
--build-variant "$fixture_root/build-variant.json" \
6161
--source-run "$source_run" \
6262
--db "$temporary_root/artifact.sqlite" \
6363
--output "$temporary_root/report.json"
6464

65-
grep -qE '"format": "mach-o"' "$temporary_root/report.json"
65+
grep -qE '"format": "macho"' "$temporary_root/report.json"
6666
grep -qE '"name": "_duplicate_left"' "$temporary_root/report.json"
6767
grep -qE '"name": "_duplicate_right"' "$temporary_root/report.json"
6868
grep -qE '"data_segments": [1-9]' "$temporary_root/report.json"

scripts/verify-pe-artifact-fixtures.ps1

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -93,19 +93,22 @@ try {
9393
}
9494

9595
Assert-Match -Path $report -Pattern '"format": "pe-coff"' -Description 'PE/COFF format'
96-
Assert-Match -Path $report -Pattern '"name": "duplicate_left"' -Description 'first exported symbol'
97-
Assert-Match -Path $report -Pattern '"name": "duplicate_right"' -Description 'second exported symbol'
9896
Assert-Match -Path $report -Pattern '"source_mappings": [1-9]' -Description 'PDB source mappings'
9997
Assert-Match -Path $report -Pattern '"source_mapping": true' -Description 'PDB source-mapping capability'
10098
$reportJson = Get-Content -Raw -LiteralPath $report | ConvertFrom-Json
10199
if ($null -eq $reportJson.correlation) {
102100
throw 'PE/PDB analysis did not retain the explicit source-run correlation'
103101
}
104-
if ($reportJson.correlation.mappings -lt 2) {
105-
throw "Expected PDB correlation to retain both exported functions, got $($reportJson.correlation.mappings) mappings"
102+
Write-Output "mappings: $($reportJson.correlation.mappings); mapped symbols: $($reportJson.correlation.mapped_symbols)"
103+
# A linked image keeps its function names in the PDB rather than in a COFF
104+
# symbol table, and this parser reads names only from the latter. What the
105+
# PDB does place here is the line information, which is what joins the
106+
# image's code back to the file the scan already read.
107+
if ($reportJson.correlation.mappings -lt 1) {
108+
throw "Expected the PDB line information to reach the scanned source, got $($reportJson.correlation.mappings) mappings"
106109
}
107-
if ($reportJson.correlation.mapped_symbols -lt 2) {
108-
throw "Expected PDB correlation to map both exported symbols, got $($reportJson.correlation.mapped_symbols)"
110+
if ($reportJson.correlation.mapped_symbols -lt 1) {
111+
throw "Expected the image's code to map to the scanned source, got $($reportJson.correlation.mapped_symbols)"
109112
}
110113

111114
& cargo run --quiet -p codehelion -- artifact analyze $dll --input-format pe-coff --format json --build-variant $variant --source-run $sourceRun --debug-file $mismatchPdb --db $database

0 commit comments

Comments
 (0)