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

Crash when wasmtime invokes specific function #2099

Closed
itowlson opened this issue Oct 18, 2021 · 20 comments · Fixed by #2157
Closed

Crash when wasmtime invokes specific function #2099

itowlson opened this issue Oct 18, 2021 · 20 comments · Fixed by #2157

Comments

@itowlson
Copy link

I am encountering a crash in Wasm generated with recent versions of AssemblyScript when I invoke a function other than the default entry point.

Repro setup

I am testing with AssemblyScript 0.19.17 and Wasmtime 0.30.0.

My index.ts is as follows:

import "wasi";

import { Console } from "as-wasi";

export function foo(): void {
    Console.log("foo");
}

My package.json contains the following scripts:

    "build:untouched": "asc assembly/index.ts --target debug --use abort=wasi_abort",
    "build:optimized": "asc assembly/index.ts --target release --use abort=wasi_abort",
    "build": "npm run build:untouched && npm run build:optimized"

Symptoms

What I ran:

  • npm run build
  • wasmtime run --invoke foo ./build/untouched.wasm

Expected output: foo

Actual output: many (stack overflow amount?) lines of error dump which repeats the following lines:

         21386: 0x242f - <unknown>!~lib/as-wasi/as-wasi/wasi_abort
         21387: 0x1044 - <unknown>!~lib/rt/itcms/visitRoots
         21388: 0x1970 - <unknown>!~lib/rt/itcms/step
         21389: 0x1a4c - <unknown>!~lib/rt/itcms/interrupt
         21390: 0x1fda - <unknown>!~lib/rt/itcms/__new
         21391: 0x2564 - <unknown>!~lib/string/String#concat
         21392:  0xcd0 - <unknown>!~lib/string/String.__concat

Additional information

wasmtime run ./build/untouched.wasm works fine without crashing. (If I add some top level statements and wasmtime run then these are executed correctly.) It's only when I pass the --invoke option that things go wrong.

The same problem occurs if I embed wasmtime and call its API to invoke a function other than _start.

The same problem occurs if I use ./build/optimized.wasm, or the .wat build output.

The problem does not occur with AssemblyScript 0.17.2/as-wasi 0.4.3. With these versions wasmtime run --invoke foo runs correctly.

Please let me know if you need any more info - thanks!

@dcodeIO
Copy link
Member

dcodeIO commented Oct 18, 2021

Just an initial guess: Is it possible that _start isn't called before other functions are invoked? Calling _start is important to set up all the runtime internals, and not doing so can lead to issues like these.

@itowlson
Copy link
Author

From the Wasmtime side I think that's correct. --invoke just calls the given exported function directly as far as I know. (I'm not a Wasmtime expert though.)

If that's the case, is this a fix on the Wasmtime side or the AssemblyScript side? As I mentioned it used to work okay...

@dcodeIO
Copy link
Member

dcodeIO commented Oct 18, 2021

I see that in the command line arguments --explicitStart is not specified. Perhaps specifying it fixes the problem, as it will force a _start function and not generate an implicit (start (function ...)), which may not work? With it, I guess invoking _start first and other exports after should do the right thing.

@dcodeIO
Copy link
Member

dcodeIO commented Oct 18, 2021

Looking at the linked issue, with recent versions of the compiler it should be unnecessary to specify --use abort=wasi_abort. Is injected automatically upon import "wasi";. Also, recent versions have console integration built into the standard library, and one doesn't necessarily need as-wasi for it.

import "wasi";

export function foo(): void {
    console.log("foo");
}
    "build:untouched": "asc assembly/index.ts --explicitStart --debug",
    "build:optimized": "asc assembly/index.ts --explicitStart -O",

The Wasmtime command could be (with or without invoking _start, but haven't checked):

wasmtime run --invoke _start --invoke foo ./build/untouched.wasm

@itowlson
Copy link
Author

itowlson commented Oct 18, 2021

Thanks for the tips. Unfortunately it doesn't seem to help:

$ wasmtime run --invoke foo ./build/optimized.wasm
abort:  in ~lib/rt/itcms.ts(159:16)
Error: failed to run main module `./build/optimized.wasm`

Caused by:
    0: failed to invoke `foo`
    1: exit with invalid exit status outside of [0..126)
       wasm backtrace:
           0:  0x3dd - <unknown>!<wasm function 3>
           1:  0x41f - <unknown>!<wasm function 4>
           2:  0x9d2 - <unknown>!<wasm function 10>
           3:  0xcb6 - <unknown>!<wasm function 12>
           4: 0x11b1 - <unknown>!<wasm function 15>
       
$ wasmtime run --invoke _start --invoke foo ./build/optimized.wasm
error: The argument '--invoke <FUNCTION>' was provided more than once, but cannot be used multiple times

USAGE:
    wasmtime run <MODULE> --invoke <FUNCTION>

(By the way, it looks like I still need as-wasi or Console doesn't resolve, but that's by the by.) Duh, I missed that I needed to change it to console. Mea culpa.

@itowlson
Copy link
Author

It does change something though: I don't get the cycling in the concat/visitRoots/abort loop any more:

Caused by:
    0: failed to invoke `foo`
    1: exit with invalid exit status outside of [0..126)
       wasm backtrace:
           0:  0x5f9 - <unknown>!~lib/wasi/index/abort
           1:  0x672 - <unknown>!~lib/rt/itcms/visitRoots
           2:  0xf3b - <unknown>!~lib/rt/itcms/step
           3: 0x107a - <unknown>!~lib/rt/itcms/interrupt
           4: 0x1608 - <unknown>!~lib/rt/itcms/__new
           5: 0x1892 - <unknown>!~lib/string/String.UTF8.encode
           6: 0x166e - <unknown>!~lib/string/String.UTF8.encode@varargs
           7: 0x16a5 - <unknown>!~lib/as-wasi/as-wasi/Descriptor#writeStringLn
           8: 0x1720 - <unknown>!~lib/as-wasi/as-wasi/Descriptor#writeString
           9: 0x178d - <unknown>!~lib/as-wasi/as-wasi/Console.write
          10: 0x1796 - <unknown>!~lib/as-wasi/as-wasi/Console.log
          11: 0x1865 - <unknown>!assembly/index/foo

@itowlson
Copy link
Author

Whoa! So when I removed as-wasi and made the change to use console instead of Console, the simple code worked with or without --explicitStart!

Unfortunately, when I added a line that did a string operation (Math.random().toString()) instead of just literals, it failed again. So not yet a solution in practice, but progress...!

@dcodeIO
Copy link
Member

dcodeIO commented Oct 18, 2021

Math.random itself calls a WASI API to seed the RNG, perhaps there's something up with that. Some permission necessary maybe?

@itowlson
Copy link
Author

It's the toString call that fails, and again feels like it's failing in the allocator/GC:

Caused by:
    0: failed to invoke `foo`
    1: exit with invalid exit status outside of [0..126)
       wasm backtrace:
           0:  0x5a9 - <unknown>!~lib/wasi/index/abort
           1: 0x2c4a - <unknown>!~lib/rt/itcms/visitRoots
           2: 0x2e73 - <unknown>!~lib/rt/itcms/step
           3: 0x2fb2 - <unknown>!~lib/rt/itcms/interrupt
           4: 0x31ea - <unknown>!~lib/rt/itcms/__new
           5: 0x3441 - <unknown>!~lib/util/number/dtoa
           6: 0x322c - <unknown>!~lib/number/F64#toString
           7: 0x337b - <unknown>!assembly/index/foo

@dcodeIO
Copy link
Member

dcodeIO commented Oct 18, 2021

If this is the untouched build, the only assert than can be hit is when visitRoots visits pinned objects, which I guess there shouldn't be any. That would indicate perhaps that memory got corrupted, but hard to tell how that would even happen unless Wasmtime mutated the memory, which it shouldn't.

@itowlson
Copy link
Author

Yeah that was the untouched build (asc assembly/index.ts --target debug --explicitStart)

@MaxGraey
Copy link
Member

wasmtime run --invoke _start --invoke foo ./build/optimized.wasm

Are you sure you could use multiply invoke commands? For WASI you definitely should insert --invoke _start according wasmtime cli docs. But I'm not sure you could also call --invoke foo at the same time.

@dcodeIO
Copy link
Member

dcodeIO commented Oct 19, 2021

You are right, seems one cannot --invoke multiple functions, and it appears that Wasmtime doesn't invoke the implicit start function (when not compiling with --explicitStart) when using the --invoke argument. Seems one cannot do that...?

For reference, I tried the same with the Wasmer CLI, which also doesn't work. As soon as --invoke is specified, the WASI imports are not present. Makes me assume that both the Wasmtime and Wasmer CLIs are meant to run WASI executables only, but to do more with it one needs an embedding that supports, say, calling other exports as well.

@dcodeIO
Copy link
Member

dcodeIO commented Oct 19, 2021

Opened an issue at Wasmer. If there is interest in Wasmtime support as well, feel free to file an issue there as well.

@itowlson
Copy link
Author

Response from the Wasmtime folks (bytecodealliance/wasmtime#3474 (comment)):

There are two kinds of wasi programs:

  • commands: These are like executables and need _start invoked one time. Once it returns you are no longer allowed to call into it ever again.
  • reactors: These are like libraries and need _initialize invoked one time. Once it returns you are allowed to call any exported function as many times as you want. (subject to restrictions defined by the specific reactor you are running)

If assemblyscript wants to allow invoking functions other than _start, it will need to create wasi reactors AFAIK and thus provide an _initialize function instead of a _start function. It is not valid to provide both.

@dcodeIO
Copy link
Member

dcodeIO commented Oct 27, 2021

I guess we can alias _start with _initialize and see what happens, though I wonder what will happen in practice if both are present. There isn't really a main function in AS, likely making a distinction not very useful. Top-level code is pretty much initialize code, with one depending on the other in AS.

@itowlson
Copy link
Author

This does seem like a tricky one - it sounds like WASI is making a distinction between two types of program which it's not obvious how to distinguish in AssemblyScript.

We have also assumed that top-level statements do not run when we invoke foo directly, only when we invoke _start. This was how older versions of AssemblyScript behaved. I worry that aliasing _initialize and _start would change that behaviour. (And for our real use case we really do need to be able to invoke a function without causing the top level code to run.)

On top of that, other languages seem not to care if something is a command or reactor: if we have a WASM module built from Rust we can invoke _start or foo and it doesn't care, both work fine! This certainly wasn't a nuance of WASI/WASM that I knew about before...!

@dcodeIO
Copy link
Member

dcodeIO commented Oct 27, 2021

Invoking an export may require top-level code to have executed already, though, say

var a = 1; // but more complex

export function foo(): i32 {
  return a;
}

where top-level code leading to a = 1 can be arbitrarily complex, potentially involving the runtime, memory management, etc.

An alternative could be to deprecate _start and only use _initialize, where a user can export a custom _start function that is independent, but then again we would end up with both. Perhaps, if such a custom _start is present, we'd inline what would otherwise go into _initialize there to deduplicate. Not sure, though.

@itowlson
Copy link
Author

Update/clarification from a colleague who knows more about the WASI ABI than I do. It seems like the response from Wasmtime (quoted above) was slightly wrong.

  • If a module exports both _start and _initialize then that's an error and Wasmtime will reject the module.
  • If a module exports _start but not _initialize then it is a command but you can still invoke its exports and the ABI doc does not say that _start is called in this case (and indeed Wasmtime doesn't). You can only call one export on each instance/each call gets a new instance - not quite sure of the nuance here but the impact is it doesn't retain state from call to call.
  • Otherwise it's a reactor and Wasmtime automatically calls _initialize if present. But then you can call multiple exports on the same instance (and it retains module state from call to call).

https://github.com/bytecodealliance/wasmtime/blob/49133f62e69e43b01562141abfe9a0f1f957ab86/crates/wasmtime/src/linker.rs#L1252

(forgive me if this is all familiar ground to you folks, but I had copied a comment that implied something different, and wanted to clarify for anyone coming on this issue later)

@dcodeIO
Copy link
Member

dcodeIO commented Dec 13, 2021

Took at stab at this in #2157, replacing the --explicitStart option with a more general --exportStart NAME option. Additionally, when import "wasi" is used, but the option is omitted, the compiler tries to infer from whether there are custom function exports if the module is likely a reactor (has some) or a command (has none). Specifying the option overrides what the compiler would infer, so --exportStart _initialize makes it a reactor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants