Skip to content

wasm linker lowers undefined and 0 incorrectly with respect to the bss section #14834

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
andrewrk opened this issue Mar 7, 2023 · 2 comments · Fixed by #14838
Closed

wasm linker lowers undefined and 0 incorrectly with respect to the bss section #14834

andrewrk opened this issue Mar 7, 2023 · 2 comments · Fixed by #14838
Labels
arch-wasm 32-bit and 64-bit WebAssembly bug Observed behavior contradicts documented or intended behavior linking miscompilation The compiler reports success but produces semantically incorrect code.
Milestone
0.11.0

Comments

@andrewrk
Copy link
Member

andrewrk commented Mar 7, 2023 •
edited
Loading

Zig Version

0.11.0-dev.1898+36d47dd19

Steps to Reproduce and Observed Behavior

This test case is incorrect: https://github.com/ziglang/zig/blob/36d47dd1991f0ccd7a9673075624f09500cc415e/test/link/wasm/bss/build.zig

The global variable has value undefined:

zig/test/link/wasm/bss/lib.zig

Line 1 in 36d47dd

pub var bss: u32 = undefined;

So it should not go into the bss section in debug mode.

Furthermore, when I change the value to 0 it should definitely go into bss, however in this case the test fails.

Expected Behavior

Zig is technically allowed to lower undefined in any way it wants to, however the linker test can test the actual behavior of the linker. It should do the following for undefined:

  • Debug, ReleaseSafe: lower to 0xaa bytes
  • ReleaseFast, ReleaseSmall: lower to bss

For zero values, it must lower to bss.

cc @Luukdegram and @kubkon
@andrewrk andrewrk added bug Observed behavior contradicts documented or intended behavior arch-wasm 32-bit and 64-bit WebAssembly miscompilation The compiler reports success but produces semantically incorrect code. linking labels Mar 7, 2023
@andrewrk andrewrk added this to the 0.11.0 milestone Mar 7, 2023
@Luukdegram
Copy link
Contributor

Thanks for reporting and clarifying this with me. I'll have a fix ready for both undefined and zero values this week :)

@andrewrk
Copy link
Member Author

andrewrk commented Mar 7, 2023

If you make it .build_modes = false and handle each of the build modes internally inside the build.zig file, that will make it easier for me to port the commit from master branch into #14647 🙂

@Luukdegram Luukdegram mentioned this issue Mar 8, 2023
Merged
kivikakk added a commit to charlottia/ava that referenced this issue Aug 31, 2024
@kivikakk
core: zero-init remainder of DMEM.
d1d0c3e 
Unclear if Zig will always do this for us or what; it appears that it/
Andrew always wants to treat bss as explicitly zero-initted:
ziglang/zig#14834 (comment).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
arch-wasm 32-bit and 64-bit WebAssembly bug Observed behavior contradicts documented or intended behavior linking miscompilation The compiler reports success but produces semantically incorrect code.
Projects
None yet
Milestone
0.11.0
Development

Successfully merging a pull request may close this issue.

wasm-linker: fix storing atoms in the correct segment Luukdegram/zig
2 participants
@andrewrk @Luukdegram