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

Wasmtime does not exit with the same code as the wasm main on Windows #6352

Closed
yowl opened this issue May 7, 2023 · 10 comments
Closed

Wasmtime does not exit with the same code as the wasm main on Windows #6352

yowl opened this issue May 7, 2023 · 10 comments
Labels
bug Incorrect behavior in the current implementation that needs fixing

Comments

@yowl
Copy link
Contributor

yowl commented May 7, 2023

Test Case

Create a wasm which as a main that exits with a return value > 1. E.g. the attached

Steps to Reproduce

  • wasmtime --wasm-features=threads --wasi-modules=experimental-wasi-threads HelloWasm.wasm
  • echo %ERROR_LEVEL%

Expected Results

should print 100

Actual Results

prints 1

Versions and Environment

Wasmtime version or commit: wasmtime-cli 9.0.0

Operating system: Win11

Architecture: x64

Extra Info

Node.js prints 100 for this test. Thanks!

HelloWasm.zip

@yowl yowl added the bug Incorrect behavior in the current implementation that needs fixing label May 7, 2023
@sunfishcode
Copy link
Member

On Windows, error codes have a different meaning than on Unix. Error code 100 on Windows means "Cannot create another system semaphore", which wouldn't correctly reflect the intent of a portable program, so Wasmtime normalizes error codes.

@yowl
Copy link
Contributor Author

yowl commented May 7, 2023

I see, this it the how the dotnet runtime tests work so would have thought they would be ok. Note that node.js behaves in line with how these tests are run. However if it is not consider wrong, then I can take it up with the dotnet team and perhaps change the how the test results are analysed.

@sunfishcode
Copy link
Member

There is some design tension here. On one hand there's a desire to have WASI programs be portable and avoid having assumptions about how they're being used. On the other hand, this dotnet example shows an example of programs that know how they're going to be used and have a special convention that only has meaning in that context.

We could potentially relax this, within some limits (Unix has more restrictions on the value). Perhaps one question is, how much of a need is there for real-world programs to use custom error code conventions, rather than just specialized situations like test harnesses?

@bjorn3
Copy link
Contributor

bjorn3 commented May 7, 2023

Rust uses 101 for panics. ls uses 1 for minor issues and 2 for major issues. tar uses 1 for when some files differ in comparison mode and uses 2 for fatal errors.

@yowl
Copy link
Contributor Author

yowl commented May 7, 2023

Error code 100 on Windows means

These are the code for GetLastError, i.e. calls to the Win32 API, which is not the same as the main return value. Interestingly it looks like *nix has more restrictions, yet wasmtime does indeed return the main exit code there (e.g. with $? in bash)

@yowl
Copy link
Contributor Author

yowl commented May 7, 2023

Some more investigation, wasmer on linux and windows shows 100 for the exit code. Out of the 4 that I've tried the odd one out so far is wasmtime on windows.

@sunfishcode
Copy link
Member

These are the code for GetLastError, i.e. calls to the Win32 API, which is not the same as the main return value.

There are programs which return the GetLastError as their exit status, though it appears this convention is not as widespread as I initially thought.

Wasmtime is starting to anticipate WASI Preview2 in some parts of its implementation, and Preview2 currently has only a boolean success-or-failure exit status from commands. I've now filed WebAssembly/wasi-cli#11 to discuss expanding it.

@yowl
Copy link
Contributor Author

yowl commented May 8, 2023

Thanks, @sunfishcode do you want to close this then and it can just be taken up in WebAssembly/wasi-cli#11 ? That would be fine with me.

@abrown
Copy link
Contributor

abrown commented May 8, 2023

Yeah, seems like we should have one issue for this: I also had WebAssembly/WASI#524 open which I believe is similar.

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

4 participants