Skip to content
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

Wasm module fails with memory bound issue (runs on other runtimes, or when compiled with emscripten) #6695

Closed
matsbror opened this issue Jul 6, 2023 · 4 comments
Labels
bug Incorrect behavior in the current implementation that needs fixing

Comments

@matsbror
Copy link

matsbror commented Jul 6, 2023

Test Case

shootout-heapsort.zip

Steps to Reproduce

  • Unzip the example
  • run heapsort with wasmtime heapsort.wasm

To rebuild the program:

  • Run Make (assumes clang installed and with WASI_SYSROOT set to the location of the wasi sysroot (or located at /opt/wasi-sdk/share/wasi-sysroot

Expected Results

I would expect wasmtime to run the program without any errors. It runs well using other runtimes such as iwasm (WAMR). However, wasmer has a similar issue.

Actual Results

$ wasmtime heapsort.wasm
Error: failed to run main module `heapsort.wasm`

Caused by:
    0: failed to invoke command default
    1: error while executing at wasm backtrace:
           0:  0x28c - <unknown>!__original_main
           1:  0x17b - <unknown>!_start
    2: memory fault at wasm address 0x20000 in linear memory of size 0x20000
    3: wasm trap: out of bounds memory access

Versions and Environment

Wasmtime version or commit: wasmtime-cli 9.0.3

Operating system: WSL2 Ubuntu-2204, kernel 5.15.90

Architecture: x86-64

Extra Info

Anything else you'd like to add?

If I compile with emscripten instead of regular clang (or the clang included in wasi-sdk), then the resulting webassembly module runs fine also with wasmtime.

@matsbror matsbror added the bug Incorrect behavior in the current implementation that needs fixing label Jul 6, 2023
@alexcrichton
Copy link
Member

Can you clarify more why you think this program should work? The program itself appears to exhibit undefined behavior at the C level as you're allocating an array double* elements but storing double elements into it meaning that your allocation is half as large as it needs to be (pointers are 4 bytes and double is 8 bytes). I can additionally reproduce the out-of-bounds behavior in node as well.

Given the debugging I'm doing it appears that iwasm or WAMR may not be executing this wasm correctly because it's indeed attempting a store at address 0x20000 which is out-of-bounds for this wasm instance. I'm not sure why WAMR would not be reporting the out-of-bounds.

@matsbror
Copy link
Author

matsbror commented Jul 6, 2023

Thanks @alexcrichton , changing that line to allocate sizeof(double) instead clearly corrected the issue.

The reason why I thought the program should work is because it is part of sightglass and when running it there it seems to work with wasmtime, albeit called in a different way than with the cli.

The error is in sightglass https://github.com/bytecodealliance/sightglass/blob/04546548e60dcce4b6691f8ddb75442c4d251ad9/benchmarks/shootout-heapsort/benchmark.c#L62 so I should make a pull request to fix it there.

@matsbror matsbror closed this as completed Jul 6, 2023
@alexcrichton
Copy link
Member

Ah ok, in that case it seems like this may be compiler differences. Using emcc seems to create a linear memory with an initial and maximum size of 256, but using clang natively seems to create a linear memory of initial size 2 and no maximum size. This means that the UB is papered over since the end of the array still fits in linear memory with emscripten, but it's detected when memory is more minimally fit.

@abrown
Copy link
Contributor

abrown commented Jul 6, 2023

@matsbror, just a heads up: I also ran into this issue when I was working on bytecodealliance/sightglass#260 and ended up fixing it in bytecodealliance/sightglass@8b9855b. Once that PR is merged this should be fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect behavior in the current implementation that needs fixing
Projects
None yet
Development

No branches or pull requests

3 participants