tolerate undefined data symbols in emitted modules - #59
Conversation
An input module can carry undefined data symbols: rustc incremental builds sometimes leave references to promoted anonymous globals (anon.<hash>.N.llvm.<id>) whose content-derived names changed between compilations, and wasm-ld links them under --allow-undefined. The emit phase treated any such symbol in a module's included set as fatal. An undefined symbol has no definition to place or relocate, so skip it with a warning and keep its references exactly as the linker resolved them - the same treatment the zero-sized symbol case already gets.
|
I don't understand the full context here. Do you have a reproducing example - non-synthetic preferably - so a test case can catch this? |
Builds a minimal splittable module whose symbol table carries an undefined data symbol referenced by a R_WASM_MEMORY_ADDR_LEB relocation from a function kept in the main module - the shape rustc incremental builds produce via renamed promoted anonymous globals, linked by wasm-ld under --allow-undefined. Asserts the transform succeeds, that the defined data symbol next to it is still relocated to the output segment base (proving the relocation plumbing in the test attaches to the dependency pass), and that the undefined reference keeps the linker-resolved address 0. Fails against the previous emit code with "Expected data symbol dep node to ref to defined data symbol".
|
@svieujot does rust-lang/rust#81280 look relevant to you? My current assumption is that you are more likely seeing the result of a compiler bug, and I would rather have that tracked upstream somewhere before masking it here and seeing more downstream issues as a result. |
The producer of these symbols is tracked as rust-lang/rust#81280: incremental builds - particularly interrupted ones - reuse objects that reference promoted anonymous globals whose content-derived names changed. Name the issue and the remediation (a clean rebuild of the defining crate) in both the code comment and the warning, so the tolerance does not hide the compiler bug from anyone who hits it.
|
I added an additional commit with a unit test to reproduce the problem (it needs to be cherry picked on a branch without the fix). And yes, rust-lang/rust#81280 is exactly it. I believe we should still have this prevention in place as the issue has been open since 2021 and has no timeline. |
|
Do you manually have to pass I would prefer an test case that just links to an undefined So, perhaps we can just fix the overall behaviour with undefined symbols and the fix then doesn't have to be special. |
An undefined data symbol has no definition, so there is nothing to place and no address to relocate; the linker already resolved its references (to 0 under --allow-undefined) and reloc_value leaves such references untouched. Dropping the symbol from the emit state is the general rule, not a workaround for one producer, so the warning that pointed at the rustc incremental-build issue becomes a trace and the comment states the invariant. The hand-built module test goes away in favour of an integration fixture that links a real undefined extern data symbol.
The crate references an extern data symbol that nothing defines and links under --allow-undefined (re-added by its build script, since rustc no longer passes the flag - rust-lang/rust#149868), so wasm-ld keeps the symbol undefined in the symbol table, resolves the reference to address 0 and leaves a relocation against it. The split must accept the module and the reference must keep the linker's address; the fixture also carries a split point so the whole pipeline runs. This is the shape rustc incremental builds leave behind under rust-lang/rust#81280. Before the previous commit the split failed with 'Expected data symbol dep node to ref to defined data symbol'.
|
Indeed, I did manually have to pass I now removed it and will report back. |
|
Oh wow, your test case is a fun way to trigger undefined behaviour without writing |
An input module can carry undefined data symbols: rustc incremental builds sometimes leave references to promoted anonymous globals (anon..N.llvm.) whose content-derived names changed between compilations, and wasm-ld links them under --allow-undefined. The emit phase treated any such symbol in a module's included set as fatal.
An undefined symbol has no definition to place or relocate, so skip it with a warning and keep its references exactly as the linker resolved them - the same treatment the zero-sized symbol case already gets.
We have this problem with cargo-leptos:
cargo-leptos runs this emit phase in-process for every watch --split rebuild, so without this fix a rustc incremental build that leaves undefined data symbols in the module (dangling references to renamed anon..llvm. promoted globals, accepted by wasm-ld under --allow-undefined) makes the whole front build fail with "Expected data symbol dep node to ref to defined data symbol" until the user wipes the target directory for a clean build.