1747145 - Configure should check that linking wasm modules works

Status: RESOLVED FIXED

(Firefox Build System :: General, defect, P3)

Description

[Sylvestre Ledru [:Sylvestre]]

using apt.llvm.org packages:

sudo debootstrap bullseye bullseye.dir
sudo mount -t proc /proc bullseye.dir/proc
sudo mount -t proc /dev/pts bullseye.dir/dev/pts
sudo mount -t tmpfs tmpfs  bullseye.dir/dev/shm
sudo chroot bullseye.dir
apt install mercurial wget lsb-release wget software-properties-common gnupg2 python3-distutils python3-pip
wget https://apt.llvm.org/llvm.sh
chmod +x llvm.sh
./llvm.sh 14
hg clone https://hg.mozilla.org/mozilla-central/
cd mozilla-central
echo "export CC=clang-14
export  CXX=clang++-14
ac_add_options --enable-bootstrap
" > .mozconfig

./mach bootstrap
./mach build

 0:11.90 /usr/bin/clang++-14 -std=gnu++17 --target=wasm32-wasi --sysroot=/root/.mozbuild/sysroot-wasm32-wasi -o rlbox.wasm -Wl,--export-all -Wl,--stack-first -Wl,-z,stack-size=262144 -Wl,--no-entry -Wl,--growable-table ogg_alloc.wasm ogg_bitwise.wasm ogg_framing.wasm xmlparse.wasm xmlrole.wasm xmltok.wasm wasm2c_sandbox_wrapper.wasm mozHunspellRLBoxSandbox.wasm affentry.wasm affixmgr.wasm csutil.wasm hashmgr.wasm hunspell.wasm phonet.wasm replist.wasm suggestmgr.wasm GraphiteExtra.wasm CmapCache.wasm Code.wasm Collider.wasm Decompressor.wasm Face.wasm FeatureMap.wasm FileFace.wasm Font.wasm GlyphCache.wasm GlyphFace.wasm Intervals.wasm Justifier.wasm NameTable.wasm Pass.wasm Position.wasm Segment.wasm Silf.wasm Slot.wasm Sparse.wasm TtfUtil.wasm UtfCodec.wasm call_machine.wasm gr_char_info.wasm gr_face.wasm gr_features.wasm gr_font.wasm gr_logging.wasm gr_segment.wasm gr_slot.wasm json.wasm RLBoxWOFF2Sandbox.wasm table_tags.wasm variable_length.wasm woff2_common.wasm woff2_dec.wasm woff2_out.wasm -lwasi-emulated-process-clocks
 0:11.93 clang: error: unable to execute command: Executable "wasm-ld" doesn't exist!
 0:11.93 clang: error: linker command failed with exit code 1 (use -v to see invocation)

Comment 1

[Mike Hommey [:glandium]]

wasm-ld is supposed to come with clang. There's a case to be made that we should check in configure rather than failing later, but there's a problem in your packages if wasm-ld is not there.

Comment 2

[Sylvestre Ledru [:Sylvestre]]

ok,
it is:
lld-14: /usr/bin/wasm-ld-14

lld has it:

lld:amd64: /usr/bin/lld
lrwxrwxrwx 1 root root 6 Oct  6 17:55 /usr/bin/lld -> lld-14

it is possible to provide it like CXX=clang++-14 ?

Comment 3

[Mike Hommey [:glandium]]

It's clang that is calling it.

Comment 4

[Mike Hommey [:glandium]]

Coming from: https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/WebAssembly.h#L70

Comment 5

[Sylvestre Ledru [:Sylvestre]]

Cool, I will tell the debian packager. thanks ;)

Comment 6

[Mike Hommey [:glandium]]

Workaround: use /usr/lib/llvm-14/bin/clang as CC (you can unset CXX, it's simpler than also setting it to /usr/lib/llvm-14/bin/clang++)

which fails on the same command, but with a different error:

wasm-ld: error: unable to find library -lgcc

Comment 7

[Mike Hommey [:glandium]]

With llvm-11, the error was:

wasm-ld: error: cannot open /usr/lib/llvm-11/lib/clang/11.1.0/lib/wasi/libclang_rt.builtins-wasm32.a: No such file or directory

I suspect it's actually the same problem, but when it doesn't find the clang runtime, it falls back to libgcc. As a matter of fact, if I switch to clang-13, which gives the same unable to find library -lgcc error and copy libclang_rt.builtins-wasm32.a from the mozilla clang into /usr/lib/llvm-13/lib/clang/13.0.1/lib/wasi, it succeeds.

So as far as the error itself is concerned, this is RESOLVED INVALID, but we should definitely emit an error during configure.

Comment 8

[Mike Hommey [:glandium]]

(sidenote: we'll need that wasi clang runtime to enable rlbox in Debian too ; I thought I had filed a bug, but I can't find it so maybe I didn't)

Comment 9

[Sylvestre Ledru [:Sylvestre]]

ok, thanks

For people facing this issue, as workaround just add:

ac_add_options --without-wasm-sandboxed-libraries

Comment 10

[Sylvestre Ledru [:Sylvestre]]

here is the ugly patch I made for Debian:
https://salsa.debian.org/pkg-llvm-team/llvm-toolchain/-/blob/13/debian/patches/wasm-ld-path.diff

Comment 12

[Alex Vincent [:WeirdAl]]

[Tracking Requested - why for this release]: this caused me build bustage on ESR 102.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

I'll take a patch on ESR102 to make life easier for downstream embedders, but I don't see a need to track this either.

Comment 14

[Mike Hommey [:glandium]]

Attachment: Bug 1747145 - Add more configure checks for the wasm toolchain setup.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b5d3998c113c
Add more configure checks for the wasm toolchain setup. r=firefox-build-system-reviewers,ahochheiden

Comment 16

[Norisz Fay [:noriszfay]]

Backed out for causing build bustages

Backout link

Push with failures

Failure log

Comment 17

[Mike Hommey [:glandium]]

Attachment: Bug 1747145 - Only apply stlport flags to configure compiler executions for target.

(as opposed to host or wasm)

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2cd1b61ddd24
Only apply stlport flags to configure compiler executions for target. r=firefox-build-system-reviewers,andi
https://hg.mozilla.org/integration/autoland/rev/3be300874393
Add more configure checks for the wasm toolchain setup. r=firefox-build-system-reviewers,ahochheiden

Comment 19

[Noemi Erli[:noemi_erli]]

Backed out 3 changesets (Bug 1747145, Bug 1810627) for causing mochitest failures in /test_webassembly_compile.html

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&collapsedPushes=842382&selectedTaskRun=PY_yiSmZRHWHdwtpz1AReg.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&searchStr=linux%2C18.04%2Cx64%2Cwebrender%2Ctsan%2Copt%2Cmochitests%2Ctest-linux1804-64-tsan-qr%2Fopt-mochitest-plain%2C12&revision=b49ff59bb2cd5eabb09913c8f6d685f8382dff48

Log: https://treeherder.mozilla.org/logviewer?job_id=402747935&repo=autoland&lineNumber=2975

Backout: https://hg.mozilla.org/integration/autoland/rev/3057a2275d31d605e8cade763c03d08fca0806e5

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b1326c01f28f
Only apply stlport flags to configure compiler executions for target. r=firefox-build-system-reviewers,andi
https://hg.mozilla.org/integration/autoland/rev/6c47d6018555
Add more configure checks for the wasm toolchain setup. r=firefox-build-system-reviewers,ahochheiden

Comment 21

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/b1326c01f28f
https://hg.mozilla.org/mozilla-central/rev/6c47d6018555

Comment 22

[Mike Hommey [:glandium]]

Comment on attachment 9312536 [details]
Bug 1747145 - Add more configure checks for the wasm toolchain setup.

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Build-only change that is a no-op in practice for most builds. It is preliminary to bug 1810627, which allows downstreams like Debian to enable wasm sandboxing with their packaged wasi SDK.
• User impact if declined: See above
• Fix Landed on Version: 111
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): See above

Beta/Release Uplift Approval Request

• User impact if declined: See ESR request
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: Bug 1810627
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): See ESR request
• String changes made/needed: N/A
• Is Android affected?: No

Comment 23

[Mike Hommey [:glandium]]

Comment on attachment 9312762 [details]
Bug 1747145 - Only apply stlport flags to configure compiler executions for target.

Beta/Release Uplift Approval Request

• User impact if declined: See other request in the same bug
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky):
• String changes made/needed:
• Is Android affected?: Yes

Comment 24

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9312536 [details]
Bug 1747145 - Add more configure checks for the wasm toolchain setup.

Approved for 110 beta 4, thanks.

Comment 25

[Pascal Chevrel:pascalc (PTO until April 26)]

https://hg.mozilla.org/releases/mozilla-beta/rev/022726db3a2c
https://hg.mozilla.org/releases/mozilla-beta/rev/3a6ceb29e56b

Comment 26

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9312536 [details]
Bug 1747145 - Add more configure checks for the wasm toolchain setup.

Approved for 102.8esr.

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr102/rev/76878dedfc38
https://hg.mozilla.org/releases/mozilla-esr102/rev/ecb52b4afec3