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

Question: Would the maintainers be receptive to a pull request to remove the eager loading of the environment? #452

Closed
yowl opened this issue Dec 14, 2023 · 11 comments

Comments

@yowl
Copy link

yowl commented Dec 14, 2023

Hi,

For the component model, eagerly loading the environment presents a problem for runtimes that call _initialize and __wasm_call_ctors This sequence results in the stack and failure:

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

When calling the component function cabi_realloc it is not permitted to make another component call , but that is what is happening here. Eagerly loading the environment (as I understand it), depends on the presence of extern char** environ which the dotnet runtime includes to support https://learn.microsoft.com/en-us/dotnet/api/system.environment.getenvironmentvariables?view=net-8.0 .

If I make the change here, would it be likely to be flat out rejected?

Update, there is one other call in wasm_call_ctors:

(func $__wasm_call_ctors (;26;) (type 12)
    call $__wasilibc_populate_preopens

This one I don't understand why it needs to be called here, seem that it is designed to only be called when needed?

@sbc100
Copy link
Member

sbc100 commented Dec 14, 2023

Rather than try to fix/remove/refactor static contructors it seems like maybe real solution here is to ensure that the module is initialized before the first call to __cabi_realloc.

User code can also run in __wasm_call_ctors (e.g. C++ static constructors) so trying to limit what can occurs during __wasm_call_ctors is probably not that right answer here.

I guess you could have a chicken and egg problem though since static constructors could (in theory) call out of the component, which could then trigger the calling of the __cabi_realloc export.

@yowl
Copy link
Author

yowl commented Dec 14, 2023

real solution here is to ensure that the module is initialized before the first call to __cabi_realloc.

I believe that would require a change the wasm component design which seems unlikely. There is the start section which I think would also be a real solution, but it isi not implemented in various places, yet.

@sbc100
Copy link
Member

sbc100 commented Dec 14, 2023

I'm not sure what you mean by the "start" section not being implemented. I don't know of any engines that don't implement it. It used by llvm (and emscripten) for certain things (e.g. loading passive data segments in multi-threading, and performs certain fixup in dyanmic linking). Perhaps you are referring to the fact that we don't use the "start" section to run arbitrary user code (e.g. startic ctors) because it runs before the exports are made available to the embedder?

@yowl
Copy link
Author

yowl commented Dec 14, 2023

Oh, that's interesting, what would the llvm look like to emit a start section, is there an attribute for it?

For start in the component model, I'm coming from this remark in the ByteAlliance zulip:

"
If you require the component-model start function to work it won't be easy and I'd recommend trying to avoid it. It's got some pretty fundamental design questions associated with it so it's not as simple as just implementing a few unimplemented!() blocks in Wasmtime
"

@sbc100
Copy link
Member

sbc100 commented Dec 14, 2023

I guess "component-model start function" is different from the start section in a core module.

For the core wasm start section: wasm-ld will generate a start section under certain circumstances, but you can't opt into or out it explicitly and you can't run your own code there: https://github.com/llvm/llvm-project/blob/ef3f476097c7a13c0578e331e44b584b706089ed/lld/wasm/Writer.cpp#L1410-L1432

@yowl
Copy link
Author

yowl commented Dec 14, 2023

Component start is similar but yes, is different, https://github.com/WebAssembly/component-model/blob/main/design/mvp/Explainer.md#-start-definitions

I suppose I might have to write cabi_realloc in LLVM so I can void wasm_call_ctors. I wanted to avoid that build toolchain complication, but seems there is no way around it. Shame as I wanted to use managed code 100% and https://learn.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.marshal.allochglobal?view=net-8.0

@sbc100
Copy link
Member

sbc100 commented Dec 14, 2023

@yowl IIRC if you export _initialize and have _initialize call __wasm_call_ctors then other exports (e.g. cabi_realloc) should not can an automatic call to __wasm_call_ctors injected.

@yowl
Copy link
Author

yowl commented Dec 14, 2023

But what would call _initialize ? In the component model reactor, the first call to the component can be cabi_realloc.

@sbc100
Copy link
Member

sbc100 commented Dec 14, 2023

Ensuring that _initialize is called before cabi_realloc would be something that component model would need to handle I think. I don't know enough about the state of the component model spec to know how best to achieve that. Perhaps open an issue in the repo? @lukewagner will likely know that answer.

@yowl
Copy link
Author

yowl commented Dec 14, 2023

I could be wrong, not an expert in this area either, but I think that is what start would do. So I'm going to look at implementing cabi_realloc outside of dotnet. The other workaround we have is for the user to call a method that does not require cabi_realloc first, e.g. some void f() would do to initialise libc. However if there's a better way I'm happy to try other things.

bytecodealliance/wit-bindgen#777 is the original issue. Other runtimes such as rust workaround this by treating the cabi_realloc export specially, which is where I'm leaning to for dotnet. E.g. https://github.com/bytecodealliance/wit-bindgen/blob/main/crates/guest-rust/src/lib.rs#L25-L52

@yowl
Copy link
Author

yowl commented Dec 15, 2023

THanks for the input, I will close this now.

@yowl yowl closed this as completed Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants