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

Turn the wasmtime-runtime crate into the wasmtime::runtime::vm module #8501

Merged
merged 6 commits into from
Apr 30, 2024

Conversation

fitzgen
Copy link
Member

@fitzgen fitzgen commented Apr 29, 2024

We want to merge them together because there is no abstraction or layering or boundary between them in practice, and our architectures and designs have, at times, been constrained by this artificial crate boundary and what is visible in which crate and this has forced us to use nasty workarounds like defining traits in wasmtime-runtime that get implemented in wasmtime and used as dynamic trait objects inside wasmtime-runtime. Merging them will allow us to remove these hacks.

@fitzgen fitzgen requested review from a team as code owners April 29, 2024 15:05
@github-actions github-actions bot added wasmtime:api Related to the API of the `wasmtime` crate itself wasmtime:config Issues related to the configuration of Wasmtime labels Apr 29, 2024

This comment was marked as off-topic.

@fitzgen

This comment was marked as outdated.

@fitzgen fitzgen changed the title Use wasmtime-runtime as crate::runtime::vm internally in the wasmtime crate Turn the wasmtime-runtime crate into the crate::runtime::vm module Apr 29, 2024
@fitzgen fitzgen force-pushed the wasmtime-runtime-as-vm branch 5 times, most recently from e7401eb to 371aeba Compare April 29, 2024 19:10
@fitzgen fitzgen changed the title Turn the wasmtime-runtime crate into the crate::runtime::vm module Turn the wasmtime-runtime crate into the wasmtime::runtime::vm module Apr 29, 2024
@fitzgen fitzgen added this pull request to the merge queue Apr 30, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 30, 2024
prtest:full
@fitzgen fitzgen enabled auto-merge April 30, 2024 18:31
@fitzgen fitzgen added this pull request to the merge queue Apr 30, 2024
Merged via the queue into bytecodealliance:main with commit 72004aa Apr 30, 2024
47 checks passed
@fitzgen fitzgen deleted the wasmtime-runtime-as-vm branch April 30, 2024 19:15
@kaimast
Copy link

kaimast commented May 20, 2024

How do I call wasmtime_runtime::raise_user_trap after this change?

@alexcrichton
Copy link
Member

Ah it's no longer possible to call the function after this change as it's private to the wasmtime crate. Could you detail what you were doing with the function? We can try to help out to find a workaround perhaps

@fitzgen
Copy link
Member Author

fitzgen commented May 20, 2024

Adding to what Alex said: that function was never intended to be exposed to or used by Wasmtime embedders. The wasmtime-runtime crate was an implementation detail of Wasmtime itself. Generally, to raise a trap in Wasm from a host API, you want to return an Err(..) from the host call.

@kaimast
Copy link

kaimast commented May 20, 2024

Generally, to raise a trap in Wasm from a host API, you want to return an Err(..) from the host call.

Oh, I did not realize that is possible. I fairly recently ported my code over from Wasmer, so I am not super familiar with API yet. That is indeed a nicer way of doing it...

Thanks for the quick responses!

@fitzgen
Copy link
Member Author

fitzgen commented May 21, 2024

@kaimast no problem! Feel free to pop into our Zulip or file an issue if you have any more questions.

jameysharp added a commit to jameysharp/wasmtime that referenced this pull request Jul 12, 2024
In the past, the wasmtime-runtime crate couldn't directly call
`get_wasm_trap` because the registry of loaded modules was in the
wasmtime crate, so it called through a global function pointer
registered with `init_traps` instead.

Since the two crates were merged in bytecodealliance#8501, we no longer need this
indirection.

While I'm here, I've also split the former `get_wasm_trap` function into
two parts: `lookup_code` finds a loaded module that had been previously
registered with `register_code`, and the `lookup_trap_code` step is now
done at the call site in `test_if_trap`. This makes the module registry
more broadly useful.
jameysharp added a commit to jameysharp/wasmtime that referenced this pull request Jul 12, 2024
In the past, the wasmtime-runtime crate couldn't directly call
`get_wasm_trap` because the registry of loaded modules was in the
wasmtime crate, so it called through a global function pointer
registered with `init_traps` instead.

Since the two crates were merged in bytecodealliance#8501, we no longer need this
indirection.

While I'm here, I've also split the former `get_wasm_trap` function into
two parts: `lookup_code` finds a loaded module that had been previously
registered with `register_code`, and the `lookup_trap_code` step is now
done by a helper on `CodeMemory`. This makes the module registry more
broadly useful.

I also simplified the code lookup step in two ways:
- I removed a redundant check from the code lookup. `BTreeMap::range`
  will only return entries where `end >= pc`, so the `end < pc`
  condition is always false.
- I used checked_sub instead of writing both the comparison and
  subtraction explicitly.
jameysharp added a commit to jameysharp/wasmtime that referenced this pull request Jul 12, 2024
In the past, the wasmtime-runtime crate couldn't directly call
`get_wasm_trap` because the registry of loaded modules was in the
wasmtime crate, so it called through a global function pointer
registered with `init_traps` instead.

Since the two crates were merged in bytecodealliance#8501, we no longer need this
indirection.

While I'm here, I've also split the former `get_wasm_trap` function into
two parts: `lookup_code` finds a loaded module that had been previously
registered with `register_code`, and the `lookup_trap_code` step is now
done by a helper on `CodeMemory`. This makes the module registry more
broadly useful.

I also simplified the code lookup step in two ways:
- I removed a redundant check from the code lookup. `BTreeMap::range`
  will only return entries where `end >= pc`, so the `end < pc`
  condition is always false.
- I used checked_sub instead of writing both the comparison and
  subtraction explicitly.
github-merge-queue bot pushed a commit that referenced this pull request Jul 15, 2024
In the past, the wasmtime-runtime crate couldn't directly call
`get_wasm_trap` because the registry of loaded modules was in the
wasmtime crate, so it called through a global function pointer
registered with `init_traps` instead.

Since the two crates were merged in #8501, we no longer need this
indirection.

While I'm here, I've also split the former `get_wasm_trap` function into
two parts: `lookup_code` finds a loaded module that had been previously
registered with `register_code`, and the `lookup_trap_code` step is now
done by a helper on `CodeMemory`. This makes the module registry more
broadly useful.

I also simplified the code lookup step in two ways:
- I removed a redundant check from the code lookup. `BTreeMap::range`
  will only return entries where `end >= pc`, so the `end < pc`
  condition is always false.
- I used checked_sub instead of writing both the comparison and
  subtraction explicitly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wasmtime:api Related to the API of the `wasmtime` crate itself wasmtime:config Issues related to the configuration of Wasmtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants