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

c#: dotnet Runtime not started correctly when using strings (or heap objects) as first call to exported function #777

Closed
Tracked by #713
jsturtevant opened this issue Dec 1, 2023 · 14 comments
Labels
gen-c# Related to the C# code generator

Comments

@jsturtevant
Copy link
Collaborator

One thing that doesn't seem to work quite right is the current .NET implementation of cabi_realloc. It doesn't seem to start the runtime correctly. So, if you're exporting a function that accepts a string, and the very first call into your preview2 component is to that method, the very first thing it will do is call cabi_realloc which will fail with something like this:

Caused by:
    0: failed to invoke `run` function
    1: error while executing at wasm backtrace:
           0: 0x18fa7d7 - wit-component:shim!indirect-wasi:cli/[email protected]
           1: 0x18ef789 - wit-component:adapter:wasi_snapshot_preview1!wasi_snapshot_preview1::State::get_environment::h3923f56a626a4100
           2: 0x18ef996 - wit-component:adapter:wasi_snapshot_preview1!environ_sizes_get
           3: 0x18fa80d - wit-component:shim!adapt-wasi_snapshot_preview1-environ_sizes_get
           4: 0x145aac8 - <unknown>!__wasi_environ_sizes_get
           5: 0x145b358 - <unknown>!__wasilibc_initialize_environ
           6: 0x145b3f8 - <unknown>!__wasilibc_initialize_environ_eagerly
           7: 0xcc784c - <unknown>!__wasm_call_ctors
           8: 0xcc787f - <unknown>!_initialize
           9: 0xccb10c - <unknown>!InitializeRuntime()
          10: 0xccb4c6 - <unknown>!Thread::ReversePInvokeAttachOrTrapThread(ReversePInvokeFrame*)
          11: 0xccb5e1 - <unknown>!RhpReversePInvokeAttachOrTrapThread2
          12: 0xccb658 - <unknown>!RhpReversePInvoke
          13: 0x13a86eb - Adder_wit_computer_Intrinsics__cabi_realloc

As a workaround, if you first call any other export that only deals with int or other non-heap-objects, that will initialize the runtime correctly, and then cabi_realloc will no longer crash so you can then call exports that accept string.

Originally posted by @SteveSandersonMS in #760 (comment)

@esoterra
Copy link
Contributor

esoterra commented Dec 4, 2023

So presumably we need some initialization to happen in the start function that sets up the allocator.

@yowl
Copy link
Collaborator

yowl commented Dec 5, 2023

It's not clear to me what is actually failing here. In frame 9 we can see that it is attempting to initialize the runtime, but it fails in wasmtime as part of calling the c++ class ctors. Would need to attach lldb to a debug build of wasmtime I think to understand more.

@jsturtevant
Copy link
Collaborator Author

@SteveSandersonMS we discussed this a bit in our meeting today. Do you have a sample that you used to produce this error?

@SteveSandersonMS
Copy link

@jsturtevant Yes, it's this sample: https://github.com/SteveSandersonMS/wasm-component-sdk/blob/main/samples/calculator/CalculatorHost/Program.cs

If you were to move line 8 to before line 5, so it calls the string function before the int function, then it will fail with the error above. The sample only works because it calls the int function before the string function, even though they shouldn't affect each other.

That repo should be reasonably straightforwards to build (just clone and dotnet build at the root, or see how it's built in CI at https://github.com/SteveSandersonMS/wasm-component-sdk/blob/main/.github/workflows/build.yml)

@jsturtevant
Copy link
Collaborator Author

One suggestion (by @ricochet) that came up during the call was we might be using the wrong adapter type (command vs reactor). It looks like this might be using reactor by default https://github.com/SteveSandersonMS/wasm-component-sdk/blob/4866a47faebd92db8e7450c4d17ae9d55129b663/src/WasmComponent.Sdk/build/WasmComponent.Sdk.targets#L31

@yowl
Copy link
Collaborator

yowl commented Dec 5, 2023

For NativeAOT-LLVM reactor is what we should use I think, as we have no main. We can't call back to the component functions like getting environment variables from withincabi_realloc. It was mentioned in the call today that _start would have the same problem, but if that's the case, when is it safe to call __wasilibc_initialize_environ_eagerly ?

@SteveSandersonMS
Copy link

One suggestion (by @ricochet) that came up during the call was we might be using the wrong adapter type (command vs reactor). It looks like this might be using reactor by default

I would expect to use command for <OutputType>exe</OutputType> and reactor for <OutputType>library</OutputType>. Both are valid use cases.

@dicej
Copy link
Collaborator

dicej commented Dec 5, 2023

For reference, this is how the Rust generator addresses it: https://github.com/bytecodealliance/wit-bindgen/blob/main/crates/guest-rust/src/lib.rs#L25-L52

@jsturtevant
Copy link
Collaborator Author

This was another example mentioned today: https://github.com/bytecodealliance/javy/pull/481/files

@jsturtevant jsturtevant added the gen-c# Related to the C# code generator label Dec 5, 2023
@yowl
Copy link
Collaborator

yowl commented Dec 6, 2023

That repo should be reasonably straightforwards to build (just clone and dotnet build at the root,

Confirmed! SDK works great. I had a minor issue with download emsdk, maybe because I already has the EMSDK env set up. Make sure to use wasmtime 14 and not 15 as some function types have changed. I will try the start idea next. Unfortunately I don't think LLVM can produce those yet, but some wat and wasm-merge should be enough to see if it is a solution or not

@SteveSandersonMS
Copy link

I was trying to investigate a bit more, and found the following. Is this issue actually just a duplicate of dotnet/runtimelab#2359 ?

@yowl
Copy link
Collaborator

yowl commented Dec 6, 2023

I don't think so. That initialization problem with the dotnet runtime was resolved. The problem here is that the dotnet runtime initialises the Wasi SDK, and it's the WASI SDK that cannot be initialised from cabi_realloc. Other languages either don't use the WASI SDK, or they treat cabi_realloc as "special". I suspect we would get push pack if we tried to treat the UCO (UnmanagedCallersOnly) cabi_realloc as special from upstream and I'm not convinced we could make cabi_realloc work without initialising the WASI SDK and dotnet runtime. I'm looking at the start solution but don't have any results to feedback yet.

@yowl
Copy link
Collaborator

yowl commented Dec 13, 2023

I've asked at WebAssembly/wasi-sdk#365 whether Wasi SDK would be open to accepting a change for this. Alternatively we might be able to disable https://learn.microsoft.com/en-us/dotnet/api/system.environment.getenvironmentvariables?view=net-8.0.

I think the real solution is to use start but that is not likely to be supported in wasm-tools/wasmtime/llvm anytime soon.

@yowl
Copy link
Collaborator

yowl commented Dec 18, 2023

This should be fixed by #791 where we are generating cabi_realloc from c, avoiding the need to initialise the Wasi SDK (and dotnet runtime)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gen-c# Related to the C# code generator
Projects
None yet
Development

No branches or pull requests

5 participants