fix(test): keep the runtime's symbols global in the ELF provider link - #8127
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe ELF provider version script now explicitly exports the 16 provider symbols without applying ChangesELF provider export binding
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
`gc-native-roots` is red on main, on both ELF platforms only, with
Error: "stdlib provider is bound to a different runtime image"
Mach-O and Windows pass. Bisected by re-running the last-good job on its own
commit: 5c27d1a passes today, so the environment is fine and the gate is not
flaky. The break is the next commit, #8089, which added the ELF branch of this
linker shim.
That branch writes a version script listing the 16 provider exports and then
`local: *`. The provider statically links the runtime rlib as well as loading
the runtime `.so`, so it carries its own definition of `js_gc_init` and
friends, and `local: *` binds those internally — a local symbol is not
preemptible. The stdlib therefore stops resolving stateful runtime calls to the
image the host loaded first, which is exactly the condition this fixture exists
to detect, and it duly detected it.
The Mach-O branch never had the problem because it re-exports the runtime's
whole symbol set (`nm -gU "$runtime_library"`) alongside the provider exports.
Do the same for ELF. `local: *` still hides everything else, so #8089's intent
— export the Web Fetch/Streams surface the later-loaded app needs, and nothing
more — is preserved.
`nm -D --defined-only` rather than the Mach-O branch's `nm -gU`: preemption is
governed by the dynamic symbol table, not the static symtab that a stripped
`.so` need not carry at all.
Second attempt; the first was wrong in a way CI caught. The failure is real: `gc-native-roots` is red on both ELF platforms only, with "stdlib provider is bound to a different runtime image". Re-running the last-good job on its own commit shows that commit still passes, so the environment is not at fault — the break came with the ELF branch of this shim added in #8089. The cause is the `local: *` in the version script that branch writes. The provider statically links the runtime rlib as well as loading the runtime `.so`, so it carries its own `js_gc_init` and friends; `local: *` binds those internally, and a local symbol is not preemptible. The stdlib then resolves stateful runtime calls to its OWN copy rather than the image the host loaded first — precisely the condition the fixture detects. Before this shim parsed `--version-script` at all, the rustc-generated script was passed through and the gate passed: the regression is the hiding, not the export list. My first fix mirrored the Mach-O branch and re-exported the runtime's whole symbol table via `nm`. That cannot work here: rustc also passes `--no-undefined-version`, so naming a symbol the output does not define is a hard lld error, and the link died with hundreds of "version script assignment of 'global' to symbol ... failed: symbol not defined". ld64 only warns, which is why the Mach-O branch gets away with it. Drop the `local: *` instead. Every symbol not named keeps its default global, preemptible binding — restoring the pre-#8089 behaviour — while the 16 provider exports are still named explicitly, so #8089's actual goal (the later-loaded app resolves the Web Fetch/Streams surface) is unchanged.
cb3d164 to
cb7b49a
Compare
Independently verified on real ELF, outside CI — this PR's head does fix itThe PR body says "the real oracle is this PR's own Method. Linux/aarch64 container ( This is not the full gate — no Perry compiler, no app dylib, no GC assertions. It exercises exactly the chain this PR changes: rlib-vs-
Every claim in your root-cause section checks out, including the one that is easy to get wrong: Two things the numbers add: 1. rustc's own version script already lists every 2. libstd is the same rlib for both images, so these hash-match and really will bind to the runtime provider at load. The provider profile is Suggestion: delete the interception insteadPassing rustc's script through untouched restores the last-good export surface exactly — 19 exports, the same 2 shared with the runtime provider, byte-identical output size — and is a deletion, so there is no new invariant to maintain: @@
-Wl,-exported_symbols_list,*)
original_export_list=${argument#-Wl,-exported_symbols_list,}
;;
- -Wl,--version-script=*)
- original_version_script=${argument#-Wl,--version-script=}
- ;;
*) arguments+=("$argument") ;;
esac
done
@@
-if [[ -n "$original_version_script" ]]; then
- if [[ "$saw_runtime_rlib" == true ]]; then
- custom_version_script=$(mktemp "${TMPDIR:-/tmp}/perry-8075-version.XXXXXX")
- {
- echo '{ global:'
- printf ' %s;\n' "${stdlib_provider_exports[@]}"
- echo 'local: *; };'
- } > "$custom_version_script"
- arguments+=("-Wl,--version-script=$custom_version_script")
- else
- arguments+=("-Wl,--version-script=$original_version_script")
- fi
-fi(plus dropping the now-unused Either way, this class should not have to travel through a 90-minute gate to a runtime error string again. A link-time assertion in nm -D "$stdlib_library" | grep -q ' js_gc_init$' || {
echo "stdlib provider hides js_gc_init: the bundled runtime copy is not preemptible" >&2
exit 1
}Happy to open that as a follow-up, or fold it in here — say which. |
1 participant
gc-native-rootsis red on main, on both ELF platforms only (Mach-O and Windows pass), with:Established by experiment, not inference
I re-ran the last-good job on its own commit:
5c27d1ad9passes today. So the environment is fine and the gate is not flaky — the break is real and belongs to the next commit, #8089, which introduced the ELF branch of this linker shim.Root cause
The ELF branch writes a version script listing the 16 provider exports and then
local: *.The provider statically links the runtime rlib and loads the runtime
.so, so it carries its own definition ofjs_gc_initand friends.local: *binds those internally, and a local symbol is not preemptible — so the stdlib stops resolving stateful runtime calls to the image the host loaded first. That is precisely the condition the fixture exists to detect (probe() != api.gc_init), and it detected it correctly.The Mach-O branch never had this problem because it re-exports the runtime's whole symbol set (
nm -gU "$runtime_library") alongside the provider exports. The two branches were asymmetric.Fix
Do the same for ELF.
local: *still hides everything else, so #8089's intent — export the Web Fetch/Streams surface the later-loaded app needs, and nothing more — is preserved.One deliberate difference:
nm -D --defined-onlyrather thannm -gU. Preemption is governed by the dynamic symbol table, not the static symtab that a stripped.soneed not carry at all. (Plainnm -gUis correct for Mach-O, where that is the table plain nm reads.)Validation
Shell syntax checked, and the awk emits well-formed version-script entries (
name;). The real oracle is this PR's owngc-native-rootsrun — the failure is ELF-only and cannot be reproduced on a macOS host, which is exactly how it reached main.Summary by CodeRabbit