Repository navigation
Exclude __stack_chk_guard where the loader owns it, and finish the records - #39
Conversation
The generated shim emitted __stack_chk_guard as a SHIM(void) stub on every architecture, because the generator picked FUNC versus OBJECT from the x86-64 target inventory and the name is musl-only there. On aarch64, riscv64 and loongarch64 the dynamic loader exports that name as the process-wide stack canary, glibc's lookup lets an unversioned preload definition win for the versioned reference, and every __stack_chk_guard GOT slot in the process, libc's own included, bound the stub in read-only text. Chromium built as an anylinux AppImage died on aarch64 with SIGSEGV in its GPU process under the preload with CROSS_LIBC_DLOPEN=0, because the damage is done at relocation time; x86-64 and ppc64le were unaffected, their loaders keep the canary in thread storage and never export the name. Issue #37; report 3.7 has the per-architecture measurement table. Three layers. The generator excludes the name on the three architectures whose loader exports it and emits zeroed data of the real size where musl guests need it, and now decides the type from the merged kind table, which also re-types six other musl objects from stubs to data. forward-shim.c and its manifest are regenerated. scripts/verify-artifacts.sh refuses any build whose preload exports a name the target's own libc family also exports, beyond dlopen and version-compat.c's forwarders; CLD_SYSROOT names the sysroot for verification outside a build, and the released v0.2.5 aarch64 object makes it refuse naming the symbol, measured on an x86-64 machine against the real bullseye arm64 cross libc. E102 is the suite's case, run on both rows: on the aarch64 runner it failed before this change and passes after, and the total moves to 65/65 and 61/61, with every one-home record moved with it. The fixed x86-64 build exports the same 140 names as before, so the musl bridge keeps every symbol it had, including __stack_chk_guard itself on the architectures whose loader does not provide it.
The one-home gate refused the clean tree because this file had spelt the new suite totals out. It is the gate working as designed and it is why the totals live in report 08 alone.
E102's logic is proven against the floor cross libc on this machine; the runner row is the PR's CI run, the same way E101's aarch64 number was measured. The silicon run lives in the consumer's CI, which this repository cannot cite as a file.
The loongarch64 exclusion spelled __loongarch64__, which GCC never defines: the compiler's own loongarch-c.cc defines __loongarch__ and __loongarch64, without the trailing underscores. The exclusion never fired, the artefact still exported the canary, and the gate refused the loongarch64 build, which is the gate's first live catch. Measured against gcc-mirror's loongarch-c.cc, loongarch_define_unconditional_macros. The gate's refusal now prints the shared names verbatim through IFS= read, with the artefact's defining dynsym line beside each. The ppc64le build was refused over a name that printed as 8], which word splitting can produce and a verbatim print cannot, so this is the diagnostic that says which it was. PROGRESS: fix the report 03 link, relative to docs/todo/.
PPC64 ELFv2 readelf prints a st_other annotation, [<localentry>: 8], between the visibility and the index. With the name shifted out of column 8, every symbol on both sides of the comparison parsed as the two characters 8], the intersection found it, and the gate refused the ppc64le build over a name that is not a name. The bullseye cross binutils prints the column and the newer x86-64 binutils does not for the same file, which is why the local rehearsal saw nothing and the runner did. Stripped in both parsers, the gate's and E102's, with the measurement in the comment. Re-proven after the change: the planted v0.2.5 aarch64 object is refused naming __stack_chk_guard with its defining dynsym line printed beside it.
E102 moved both evidence-table totals by one, and report 08 was the only record updated. The summary, section 9 and section 10 still asserted the old numbers as fact, and the report index still called section 3 six defects after it became seven. The one-home gate cannot see this: it checks only the current numbers.
The header still said THREE PROPERTIES while the new block introduced the fourth. Spelled out beside the other three, with the __stack_chk_guard measurement as the reason it exists.
Every other architecture test in src/cross-libc-dlopen.c, src/runtime-select.c and src/gl-fwd.c uses __loongarch64, and GCC defines it, so the comment that claims the spellings match is made true. The generated guard is regenerated.
The E102 claim rested on a fork run and the consumer's silicon report. The upstream matrix run is durable and green on all six builds and both evidence rows, so it is the citable one.
|
End-to-end check on the consumer, with the preload present rather than deleted. Built this PR's artefacts (the green matrix run rm ./AppDir/lib/sharun-preload/cross-libc-dlopen.sowith a copy of this PR's Workflow run
The aarch64 row is the one that died with SIGSEGV before, with only the preload |
|
Reviewed by building and measuring both sides rather than reading. Cross-built aarch64 and loongarch64 and native x86-64 from this branch and from The fix is correct and I found no regression. The issues below are all in the gate that is meant to stop this recurring, not in the fix itself. Regression check, both pathsEvery defined dynamic symbol, this branch versus
Undefined-import set unchanged on both. Export count 74 before and 74 after. The removal is safe on the excluded architectures because the reference still resolves. Measured against That is 3.7 in one screenful. The loader defines it, libc.so.6 imports it, and the unversioned preload definition was winning that binding. Same shape confirmed on riscv64 and on loongarch64 ( The loongarch64 cross build from this branch passes the new gate and exports zero Findings1. The new gate can pass without measuring anything. Green, on a check that read nothing. The file header two screens up says each property "fails silently if it is wrong rather than loudly, which is why they are checked rather than assumed". Suggest asserting that the target-side name list is non-empty, since a glibc that exports nothing is not a thing, and calling 2. 3. 4. The refusal diagnostic names innocent symbols. Given this block exists because a mis-parsed name cost a debugging cycle, 5. E102 prints nothing when it fires. Smaller
On the loongarch64 spellingSettled by measurement, on the exact compiler the build uses (
The correction in the PR description is also right, and I checked it rather than taking it on trust. VerdictThe diagnosis is right, including the non-obvious half of it, which is why x86-64 and ppc64le were never affected. Finding 1 through 3 are what I would want fixed before this merges, because for the three architectures with no runner this script is the release criterion and it currently has three paths that pass without measuring anything. The rest are nits. |
Review finding: readelf's errors were swallowed, so a broken readelf or a wrong file set made both name lists empty, the intersection vacuously empty, and the check print a clean result without measuring. The empty target case returned through say() and exit 0, which for the three architectures with no runner is the whole gate evaporating. Both are bad() now, CLD_SYSROOT also looks in usr/lib and the multiarch directories of an extracted rootfs, the diagnostic matches a name exactly instead of as a substring, and the exemption file is a mktemp rather than a dotfile in the artefact directory.
Review finding: the case asserted only that /work/e102-collisions was empty, so a failure was E102 MISMATCH naming nothing. It now prints them the way verify-artifacts.sh does, and echoes the needle only when the list is empty.
… the re-typing Review nits: 08 still carried '64 minus 4 is 60' beside the current accounting; PROGRESS named the superseded branch; 3.7 now says what the merged-kind-table fix did to the six other data symbols and to the canary a musl guest reads on x86-64 and ppc64le.
|
Thanks — all fair, and findings 1-3 are exactly the failure mode the file header claims to prevent. Pushed 1. An empty measurement now refuses. Both name lists must be non-empty before the intersection runs; either one being empty is instead of 2. No target libc found now refuses. 3. 4. Exact match. 5. E102 prints the names. It prints Smaller:
On loongarch64, agreed and thanks for measuring it on the exact cross compiler — Re-ran locally after the edits: |
The unverified path now refuses the build, so the notes that described it as a skip and the build doc that counted three properties were both stale. The script header says what the fourth property needs to run.
The Helium anylinux AppImages bundle their libc family, so they are the runtime the preload lands in. aarch64 glibc 2.43 exports the canary and x86-64 glibc 2.44 does not, and the gate refuses v0.2.5 against that AppDir and accepts this branch's build.
|
Re-verified the five fixes on The five fixes hold
No regression from the two new refusals: Agreed on Pushed: b253b36, the four leftovers
Pushed: 5a536fc, 3.7 measured against a real consumerThe table in 3.7 was measured on So the split is a property of the architecture rather than of the 2.31 cross packages it was first measured on, which is the part 3.7 could not previously say. A/B against that AppDir with The released object defines the canary as a 20-byte FUNC in read-only x86-64 runtime, both pathsRan the real browser rather than only the objects, with a real GPU process:
Byte-identical PNGs across all three. Confirmed the preload was genuinely in the process rather than silently skipped: under Not measured here: aarch64 at runtime. This machine has no ARM silicon and no qemu binfmt, so the aarch64 evidence stays the static A/B above plus the ARM runner's E102 row in CI. |
|
Thanks — reviewed both. The consumer-glibc measurement is the piece 3.7 needed: it shows the split is architectural, not an artefact of the 2.31 cross packages, and the |
3 participants
Supersedes #38. This branch carries #38's five commits unchanged and adds three.
The core fix is unchanged: the generated shim no longer defines
__stack_chk_guardon aarch64, riscv64 and loongarch64, where the dynamicloader exports it as the process-wide stack canary; the generator decides
FUNC versus OBJECT from the merged kind table rather than the x86-64
target's;
scripts/verify-artifacts.shrefuses any build whose preloadexports a name the target's own libc family also exports; and E102 is the
suite case.
Added on top of #38
docs: carry the 65/65 and 61/61 totals into every record— report 01,section 9 and section 10 still asserted the old totals as fact, and the
report index still called section 3 six defects after it became seven. The
one-home gate cannot see this because it only checks the current numbers.
verify-artifacts: the name-collision check is the fourth property— thescript header still said
THREE PROPERTIES.gen_forward_shim: spell the loongarch64 gate as the tree does— the guardnow uses
defined(__loongarch64), the spelling every other architecturetest in
src/uses. GCC defines both__loongarch__and__loongarch64;this makes the comment that claims the spellings match true. Regenerated.
Verification
makebuilds all five artefacts clean.sh scripts/verify-artifacts.sh src: all checks pass, andcross-libc-dlopen.so: no name reexported from the target libc family.libc.so.6+ld-linux-aarch64.so.1viaCLD_SYSROOT, the gate flags__stack_chk_guardwhen the object exportsit (the pre-fix shape) and prints the loader's defining symbol.
sh scripts/check-drift.shandsh scripts/check-charset.shpass.make shimregeneratessrc/forward-shim.cand the manifestbyte-identically.
collisions.
run 34748021128:
all six build rows green and both evidence tables green, E102 MATCH on both.
Not changed, and why
src/cross-libc-dlopen.c(CLD_TRIPLET) andsrc/runtime-select.c(RS_LDSO) gate on__loongarch64__does not matchthe tree: both use
defined(__loongarch64)with no trailing underscores,which GCC defines (
gcc/config/loongarch/loongarch-c.cc), so both branchesfire. The only
__loongarch64__in the tree is the generator commentexplaining GCC never defines it. No follow-up branch is needed; changing
those two lines would introduce the bug the note claims to fix.
shim_covered[]still lists__stack_chk_guardon the architectures whereit is no longer defined.
forward_shim_covered()has no in-tree consumer,so this is cosmetic and is left as-is.
Closes #37. Supersedes #38.