wasmtime / PR #10455 Keep Config options for disabling co... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #10455 Keep Config options for disabling co...

Wasmtime GitHub notifications bot (Mar 21 2025 at 20:13):

Stebalien requested alexcrichton for a review on PR #10455.

Wasmtime GitHub notifications bot (Mar 21 2025 at 20:13):

Stebalien requested wasmtime-core-reviewers for a review on PR #10455.

Wasmtime GitHub notifications bot (Mar 21 2025 at 20:13):

Stebalien opened PR #10455 from Stebalien:steb/wasm-config to bytecodealliance:main:

This patch keeps config options for disabling default-enabled wasmtime features, even when those features are disabled via crate feature flags. That way, these wasm features can be disabled by libraries even if the crate-level features are later enabled by some other crate in the build-tree.

Specifically, the following methods are available on the wasmtime::Config type, regardless of the enabled crate features:

Config::wasm_threads.

Config::wasm_reference_types.

Config::wasm_component_model.

Config::parallel_compilation.

Of these, enabling any of the first three while the corresponding crate feature is disabled will lead to a runtime error when constructing the wasmtime::Engine. Enabling the last one, parallel compilation, will simply do nothing.

fixes #10454

Wasmtime GitHub notifications bot (Mar 21 2025 at 20:17):

Stebalien commented on PR #10455:

I've tried to keep this change as minimal as possible and stuck to default-enabled features. I'm also happy to extend it to (any of the) features that are currently default-disabled:

Config::async_support

Config::wasm_function_references

Config::wasm_gc

Config::component_model_async

Config::coredump_on_trap

Config::wmemcheck

Config::debug_adapter_modules

Also note: I didn't bother with cranelift/winch options. As I understand it, those will only matter if you actually use cranelift/winch so they shouldn't affect any creates that aren't using the cranelift/winch features.

Wasmtime GitHub notifications bot (Mar 21 2025 at 20:22):

Stebalien submitted PR review.

Wasmtime GitHub notifications bot (Mar 21 2025 at 20:22):

Stebalien created PR review comment:

Judging by the test failures, this feature flag shouldn't be here in the first place and this feature is enabled by-default regardless, right?

Wasmtime GitHub notifications bot (Mar 21 2025 at 20:47):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Mar 21 2025 at 20:47):

alexcrichton created PR review comment:

Oh this is a bit subtle, and this also changed historically with Wasmtime. In essence though REFERENCE_TYPES should always be enabled regardless of what the crate features are, it's the GC_TYPES feature which is actually gated based on feature = "gc". There's no wasm_gc_types(...) configuration method, though.

I think this can be fixed by updating the validator to allow reference-types being enabled with feature = "gc" being turned off. It's feature = "gc" being turned off plus GC_TYPES being turned on that should be an error.

Wasmtime GitHub notifications bot (Mar 21 2025 at 20:48):

alexcrichton commented on PR #10455:

I think it might make sense to un-gate wasm_* configuration while you're here, but for the others I think it's ok to leave them as-is and deal with them as they come up later. Although if you'd like to handle them here that's also ok.

Wasmtime GitHub notifications bot (Mar 21 2025 at 22:37):

Stebalien submitted PR review.

Wasmtime GitHub notifications bot (Mar 21 2025 at 22:37):

Stebalien created PR review comment:

What happens if I turn off wasm_reference_types but enable the GC feature?

Should wasm_gc toggle GC_TYPES?

Wasmtime GitHub notifications bot (Mar 21 2025 at 22:38):

Stebalien updated PR #10455.

Wasmtime GitHub notifications bot (Mar 21 2025 at 22:43):

Stebalien submitted PR review.

Wasmtime GitHub notifications bot (Mar 21 2025 at 22:43):

Stebalien created PR review comment:

Also, does this apply to function references? That is, can I enable function references _without_ the GC feature?

Wasmtime GitHub notifications bot (Mar 21 2025 at 22:48):

Stebalien updated PR #10455.

Wasmtime GitHub notifications bot (Mar 21 2025 at 22:49):

Stebalien edited PR #10455:

This patch keeps config options for disabling default-enabled wasmtime features, even when those features are disabled via crate feature flags. That way, these wasm features can be disabled by libraries even if the crate-level features are later enabled by some other crate in the build-tree.

Specifically, the following wasmtime::Config methods are now enabled regardless of compile-time features (removed dependency on the GC feature):

Config::wasm_reference_types.

Config::wasm_function_references.

The following methods are available on the wasmtime::Config type, regardless of the enabled crate features but will lead to runtime errors when constructing a wasmtime::Engine:

Config::wasm_threads.

Config::wasm_gc.

Config::wasm_component_model.

Config::wasm_component_model_async.

Finally, Config::parallel_compilation is defined regardless of whether or not the parallel compilation feature is enabled. However, without the corresponding crate feature, this config option is a no-op.

fixes #10454

Wasmtime GitHub notifications bot (Mar 21 2025 at 22:51):

Stebalien commented on PR #10455:

I've updated the PR per the new description to:

Always define the Config::wasm_* functions.

Allow setting Config::wasm_reference_types and Config::wasm_function_references (I'm not sure if this second one is correct, pending discussion in https://github.com/bytecodealliance/wasmtime/pull/10455#discussion_r2008436743).

Wasmtime GitHub notifications bot (Mar 21 2025 at 22:51):

Stebalien updated PR #10455.

Wasmtime GitHub notifications bot (Mar 21 2025 at 23:45):

github-actions[bot] commented on PR #10455:

Label Messager: wasmtime:config

It looks like you are changing Wasmtime's configuration options. Make sure to
complete this check list:

[ ] If you added a new Config method, you wrote extensive documentation for
it.

<details>

Our documentation should be of the following form:

```text
Short, simple summary sentence.

More details. These details can be multiple paragraphs. There should be
information about not just the method, but its parameters and results as
well.

Is this method fallible? If so, when can it return an error?

Can this method panic? If so, when does it panic?

Example

Optional example here.
```

</details>

[ ] If you added a new Config method, or modified an existing one, you
ensured that this configuration is exercised by the fuzz targets.

<details>

For example, if you expose a new strategy for allocating the next instance
slot inside the pooling allocator, you should ensure that at least one of our
fuzz targets exercises that new strategy.

Often, all that is required of you is to ensure that there is a knob for this
configuration option in [wasmtime_fuzzing::Config][fuzzing-config] (or one
of its nested structs).

Rarely, this may require authoring a new fuzz target to specifically test this
configuration. See [our docs on fuzzing][fuzzing-docs] for more details.

</details>

[ ] If you are enabling a configuration option by default, make sure that it
has been fuzzed for at least two weeks before turning it on by default.

[fuzzing-config]: https://github.com/bytecodealliance/wasmtime/blob/ca0e8d0a1d8cefc0496dba2f77a670571d8fdcab/crates/fuzzing/src/generators.rs#L182-L194
[fuzzing-docs]: https://docs.wasmtime.dev/contributing-fuzzing.html

<details>

To modify this label's message, edit the <code>.github/label-messager/wasmtime-config.md</code> file.

To add new label messages or remove existing label messages, edit the
<code>.github/label-messager.json</code> configuration file.

Learn more.

</details>

Wasmtime GitHub notifications bot (Mar 21 2025 at 23:57):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Mar 21 2025 at 23:57):

alexcrichton created PR review comment:

If reference-types is turned off but GC is enabled, then that means GC-using modules will fail to load/compile because the feature is turned off, and the active feature at compile time is basically bloat/inert code and doesn't get used.

For now the hope is we can internalize GC_TYPES to exclusively the feature = "gc" toggle, so wasm_gc should not turn on GC_TYPES. If wasm_gc is turned on though and GC_TYPES is off then you can still load modules, but only those that don't use types that require a GC.

For function-references we don't require a GC for typed function references so you should be able to use such a module without the GC feature.

The end goal is to have wasm_gc on-by-default in the future, and in such a world we still want GC-disabled builds of Wasmtime to still load modules, so that's what GC_TYPES is doing, basically disallowing loading modules at runtime which use features disabled at compile time.

Wasmtime GitHub notifications bot (Mar 22 2025 at 00:00):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Mar 22 2025 at 00:00):

alexcrichton created PR review comment:

For this we should be able to remove this check entirely. I commented a bit above but on this specifically it's where gc is safe to enable and it's just GC-using-things that aren't safe to enable. This is mostly because the GC proposal was larger than just GC references, it also introduced many other highly-related but still somewhat orthogonal things. The goal is to use GC_TYPES to separate out the runtime requirement but otherwise all the other stuff in the GC proposal is still valid.

Wasmtime GitHub notifications bot (Mar 22 2025 at 00:01):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Mar 22 2025 at 00:01):

alexcrichton created PR review comment:

Mind adding a check below to return an error if this is explicitly enabled but disabled at compile time?

Last updated: Apr 17 2025 at 11:03 UTC