-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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.
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
Cranelift: assertion left == right
failed: the memory base pointer may be incorrect due to sharing memory
#8652
Comments
The assertion triggered is this one and is more-or-less this issue where the quality of the current implementation could be better. It might be easy enough to fix this particular assertion, but I don't know enough about the DWARF bits to know. In the meantime though dropping |
Yeah so I looked at the assertion and couldn't figure out what that meant at all, so I decided to file a bug instead. In the end I had to use wabt's |
When If it helps I added |
That's correct, and I figured it's a bug. |
FWIW, a number of us are frustrated at the default llvm behavior here to invoke wasm-opt automagically if it's found in the path (and only if -- nondeterministic builds!). You can find a (very) long thread of folks discovering this footgun and registering new reasons for it to be removed at llvm/llvm-project#55781 (nothing would be more welcome than to fix this imho; I've been bitten by it a handful of times too). In the meantime, a workaround a number of folks have found -- and even used in quasi-official places like WASI builds of SpiderMonkey in various SDKs -- is to prepend a fake |
@whitequark would you be able to share either the wasm binary itself (perhaps not here if it's too large but via some other means) or instructions on how to build a local copy of it? It sounds like the dwarf info may be corrupted and that could range from a build process thing to an llvm bug to a tooling issue in wasmtime/wasm-tools. |
I can do both! The binary is about half a gig but it's built with a simple script. I'm going to double-check the build to make sure you can reproduce the crash and then upload. I don't envy you looking at half a GB of DWARF though... |
I found it baffling and frustrating, yes. I assumed that it was (uncharacteristically!) a decision driven by the Wasm folks. If it's not, should we not just fix that? |
@alexcrichton This branch contains a reproducer--just run
Also here is an upload if you want to skip building it. |
Feel free to add more data upstream! I think several folks have pushed on this (see above thread) but additional anecdotes might add some motivation or spawn new interest... |
I mean, who's the code owner for wasm-ld? Should we not submit a patch and ping them? Adding an option to disable wasm-opt seems completely uncontroversial; making it on-by-default perhaps more so, but even that would make it more in line with other targets. |
Ah I think everything's actually working as intended in the dwarf bits there, albeit unfortunately. The faulting address there is in It looks like the bug here has to do with the protocol of spawning a thread as the second instruction in the thread probably shouldn't be the one faulting. I tested another address by running
I believe that the |
I'm personally under the impression it's Sam Clegg (not pinging here directly as it might be a bit out of context) |
Yeah, it's left me quite confused. Also, previously I got a fault on a similarly non-memory-accessing instruction, it was on |
Ah ok yeah given that it's still in wasi-libc that makes sense that there's no filename/line number there either because you haven't made it into clang/lld yet. That being said that instruction should also not be faulting, so my hypothesis would be some sort of corruption/bug in the threads part of wasi-threads. I'm not sure how well tested the whole port is (with threads), so this isn't necessarily altogether surprising |
By port here do you mean the wasi-libc port? |
Ah that explains it! Can we ship wasi-libc with debug symbols? |
Ah sorry, but yes by port I mean the implementation in wasi-libc. The implementation of pthread-style APIs there I don't believe has seen a large amount of testing. WebAssembly/wasi-libc#405 is an example older issue where I think WebAssembly/wasi-libc#409 fixed some things but that thread mentions known open issues. |
Er, I should keep reading before replaying, apologies!
Personally I don't see any reason why not, they're easy enough to strip out and otherwise difficult to retrofit and very useful in situations like this. Changes to the builds of wasi-libc would go in wasi-sdk around the wasi-libc build rules, probably appending to |
Actually if you run the exact example today from WebAssembly/wasi-libc#405 it is not fixed, and might be what's happening here. |
This is more-or-less a prerequisite for bytecodealliance#8652 and extends the generated dwarf with expressions to not only dereference owned memories but additionally imported memories which involve some extra address calculations to be emitted in the dwarf.
@whitequark btw if you hadn't seen already from WebAssembly/wasi-libc#405 this was enough for me to get the diff --git a/build.sh b/build.sh
index f99daf8..e659a9d 100755
--- a/build.sh
+++ b/build.sh
@@ -14,7 +14,7 @@ WASI_LDFLAGS="-O0" # "-flto -Wl,--strip-all"
# LLVM doesn't build without <mutex>, etc, even with -DLLVM_ENABLE_THREADS=OFF.
WASI_TARGET="${WASI_TARGET}-threads"
WASI_CFLAGS="${WASI_CFLAGS} -pthread"
-WASI_LDFLAGS="${WASI_LDFLAGS} -Wl,--max-memory=4294967296"
+WASI_LDFLAGS="${WASI_LDFLAGS} -Wl,--max-memory=4294967296,--import-memory,--export-memory"
# LLVM assumes the existence of mmap.
WASI_CFLAGS="${WASI_CFLAGS} -D_WASI_EMULATED_MMAN"
WASI_LDFLAGS="${WASI_LDFLAGS} -lwasi-emulated-mman" |
Ah that's interesting, I didn't realize that's the fix! |
This is more-or-less a prerequisite for #8652 and extends the generated dwarf with expressions to not only dereference owned memories but additionally imported memories which involve some extra address calculations to be emitted in the dwarf.
This commit resolves an assert in the dwarf generating of core wasm modules when the module has a defined linear memory which is flagged `shared`. This is represented slightly differently in the `VMContext` than owned memories that aren't `shared`, and looks more like an imported memory. With support in bytecodealliance#8740 it's now much easier to support this. Closes bytecodealliance#8652
With #8750 the assert here is resolved. I'll note that our DWARF bits are pretty unoptimized so the final
also noting that it takes quite some time to also load the debug info into LLDB. Using an updated wasi-sdk (from this CI build) I ended up getting the same thing. I think that's because this faulting function is in assembly which either isn't built with dwarf by default or doesn't get dwarf due to it being assembly. |
This commit resolves an assert in the dwarf generating of core wasm modules when the module has a defined linear memory which is flagged `shared`. This is represented slightly differently in the `VMContext` than owned memories that aren't `shared`, and looks more like an imported memory. With support in #8740 it's now much easier to support this. Closes #8652
Thanks! |
Steps to Reproduce
I have a binary (
wasm-ld
, which islld
from the LLVM distribution) built withwasi-sdk-22.0
for thewasm32-wasi-threads
target, withCFLAGS
of-pthread
andLDFLAGS
of-Wl,--max-memory=4294967296
. It is built with debug symbols using the normal LLVM mechanism (RelWithDebInfo
). It is too big to include here but I believe the relevant excerpt is:I run the binary with
RUST_BACKTRACE=full WASMTIME_BACKTRACE_DETAILS=1 wasmtime -S threads --dir . --dir /tmp -D debug-info -D address-map ./llvm-build/bin/wasm-ld <...arguments that cause a trap...>
.Expected Results
A backtrace with Wasm-level symbols is displayed.
Actual Results
An assertion failure in Cranelift.
Versions and Environment
Cranelift version or commit:
wasmtime-cranelift-20.0.2
Operating system: Linux
Architecture: x86_64
The text was updated successfully, but these errors were encountered: