Skip to content

Commit 0a55f80

Browse files
authored
"Downgrade" threads support to tier 2, disable fuzzing (#12036)
* "Downgrade" threads support to tier 2, disable fuzzing This commit is borne out of a fuzz bug that was opened recently. The fuzz bug specifically has to do with fallout from #12022, specifically `SharedMemory` being used to allocated instead of `Memory`. In this situation the resource limiter is no longer consulted meaning that shared memories bypass this and aren't caught by OOM checks. This is currently by design because `SharedMemory` instances don't know which resource limiter to hook into per-store. More generally though the implementation of wasm threads, while workable in Wasmtime, has a number of known relatively large deficiencies. These were not resolved prior to ungating the wasm proposal (that's on me) but nevertheless the quality of implementation is not quite up to "tier 1 par" with the rest of what Wasmtime offers. Given this the threads proposal is now downgraded to tier 2. To help minimize the impact of this the wasm proposal is left enabled-by-default, but creation of a `SharedMemory` in the Rust API requires opting-in via a new `Config::shared_memory` method. This commit shuffles around some documentation of wasm proposals to split it into tier 1/2/3 instead of on/off-by-default and then adds a column for whether the proposal is on-by-default. * clangformat * Fix tests * Add tests for failed creation Fix an issue where defined shared memories weren't gated * Sync disabled threads stub * Fix another test prtest:full * Fix fuzzing tests * Fix dwarf tests
1 parent 17060bb commit 0a55f80

File tree

22 files changed

+165
-53
lines changed

22 files changed

+165
-53
lines changed

crates/c-api/include/wasmtime/config.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,13 +147,18 @@ WASMTIME_CONFIG_PROP(void, max_wasm_stack, size_t)
147147
* \brief Configures whether the WebAssembly threading proposal is enabled.
148148
*
149149
* This setting is `false` by default.
150-
*
151-
* Note that threads are largely unimplemented in Wasmtime at this time.
152150
*/
153151
WASMTIME_CONFIG_PROP(void, wasm_threads, bool)
154152

155153
#endif // WASMTIME_FEATURE_THREADS
156154

155+
/**
156+
* \brief Configures whether shared memories can be created.
157+
*
158+
* This setting is `false` by default.
159+
*/
160+
WASMTIME_CONFIG_PROP(void, shared_memory, bool)
161+
157162
/**
158163
* \brief Configures whether the WebAssembly tail call proposal is enabled.
159164
*

crates/c-api/include/wasmtime/config.hh

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,13 @@ class Config {
289289
}
290290
#endif // WASMTIME_FEATURE_THREADS
291291

292+
/// \brief Configures whether wasm shared memories can be created.
293+
///
294+
/// https://docs.wasmtime.dev/api/wasmtime/struct.Config.html#method.shared_memory
295+
void shared_memory(bool enable) {
296+
wasmtime_config_shared_memory_set(ptr.get(), enable);
297+
}
298+
292299
/// \brief Configures whether the WebAssembly tail call proposal is enabled
293300
///
294301
/// https://docs.wasmtime.dev/api/wasmtime/struct.Config.html#method.wasm_tail_call

crates/c-api/src/config.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,11 @@ pub extern "C" fn wasmtime_config_wasm_threads_set(c: &mut wasm_config_t, enable
7979
c.config.wasm_threads(enable);
8080
}
8181

82+
#[unsafe(no_mangle)]
83+
pub extern "C" fn wasmtime_config_shared_memory_set(c: &mut wasm_config_t, enable: bool) {
84+
c.config.shared_memory(enable);
85+
}
86+
8287
#[unsafe(no_mangle)]
8388
pub extern "C" fn wasmtime_config_wasm_tail_call_set(c: &mut wasm_config_t, enable: bool) {
8489
c.config.wasm_tail_call(enable);

crates/cli-flags/src/lib.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,8 @@ wasmtime_option_group! {
369369
pub tail_call: Option<bool>,
370370
/// Configure support for the threads proposal.
371371
pub threads: Option<bool>,
372+
/// Configure the ability to create a `shared` memory.
373+
pub shared_memory: Option<bool>,
372374
/// Configure support for the shared-everything-threads proposal.
373375
pub shared_everything_threads: Option<bool>,
374376
/// Configure support for the memory64 proposal.
@@ -1004,6 +1006,10 @@ impl CommonOptions {
10041006
config.gc_support(enable);
10051007
}
10061008

1009+
if let Some(enable) = self.wasm.shared_memory {
1010+
config.shared_memory(enable);
1011+
}
1012+
10071013
Ok(config)
10081014
}
10091015

crates/fuzzing/src/generators/config.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,7 @@ impl Config {
308308
Some(self.module_config.config.shared_everything_threads_enabled);
309309
cfg.wasm.wide_arithmetic = Some(self.module_config.config.wide_arithmetic_enabled);
310310
cfg.wasm.exceptions = Some(self.module_config.config.exceptions_enabled);
311+
cfg.wasm.shared_memory = Some(self.module_config.shared_memory);
311312
if !self.module_config.config.simd_enabled {
312313
cfg.wasm.relaxed_simd = Some(false);
313314
}

crates/fuzzing/src/generators/module.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ pub struct ModuleConfig {
2323
pub component_model_error_context: bool,
2424
pub component_model_gc: bool,
2525
pub legacy_exceptions: bool,
26+
pub shared_memory: bool,
2627
}
2728

2829
impl<'a> Arbitrary<'a> for ModuleConfig {
@@ -53,7 +54,11 @@ impl<'a> Arbitrary<'a> for ModuleConfig {
5354
config.custom_page_sizes_enabled = u.arbitrary()?;
5455
config.wide_arithmetic_enabled = u.arbitrary()?;
5556
config.memory64_enabled = u.ratio(1, 20)?;
56-
config.threads_enabled = u.ratio(1, 20)?;
57+
// Fuzzing threads is an open question. Even without actual parallel
58+
// threads `SharedMemory` still poses a problem where it isn't hooked
59+
// into resource limits the same way `Memory` is. Overall not clear what
60+
// to do so it's disabled for now.
61+
config.threads_enabled = false;
5762
// Allow multi-memory but make it unlikely
5863
if u.ratio(1, 20)? {
5964
config.max_memories = config.max_memories.max(2);
@@ -75,6 +80,7 @@ impl<'a> Arbitrary<'a> for ModuleConfig {
7580
component_model_error_context: false,
7681
component_model_gc: false,
7782
legacy_exceptions: false,
83+
shared_memory: false,
7884
function_references_enabled: config.gc_enabled,
7985
config,
8086
})

crates/fuzzing/src/oracles.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -709,6 +709,7 @@ pub fn wast_test(u: &mut arbitrary::Unstructured<'_>) -> arbitrary::Result<()> {
709709
crate::init_fuzzing();
710710

711711
let mut fuzz_config: generators::Config = u.arbitrary()?;
712+
fuzz_config.module_config.shared_memory = true;
712713
let test: generators::WastTest = u.arbitrary()?;
713714

714715
let test = &test.test;
@@ -1310,7 +1311,6 @@ mod tests {
13101311
| WasmFeatures::SIMD
13111312
| WasmFeatures::MULTI_MEMORY
13121313
| WasmFeatures::RELAXED_SIMD
1313-
| WasmFeatures::THREADS
13141314
| WasmFeatures::TAIL_CALL
13151315
| WasmFeatures::WIDE_ARITHMETIC
13161316
| WasmFeatures::MEMORY64

crates/wasmtime/src/config.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ pub struct Config {
164164
pub(crate) macos_use_mach_ports: bool,
165165
pub(crate) detect_host_feature: Option<fn(&str) -> Option<bool>>,
166166
pub(crate) x86_float_abi_ok: Option<bool>,
167+
pub(crate) shared_memory: bool,
167168
}
168169

169170
/// User-provided configuration for the compiler.
@@ -273,6 +274,7 @@ impl Config {
273274
#[cfg(not(feature = "std"))]
274275
detect_host_feature: None,
275276
x86_float_abi_ok: None,
277+
shared_memory: false,
276278
};
277279
#[cfg(any(feature = "cranelift", feature = "winch"))]
278280
{
@@ -2885,6 +2887,27 @@ impl Config {
28852887
self.x86_float_abi_ok = Some(enable);
28862888
self
28872889
}
2890+
2891+
/// Enable or disable the ability to create a
2892+
/// [`SharedMemory`](crate::SharedMemory).
2893+
///
2894+
/// The WebAssembly threads proposal, configured by [`Config::wasm_threads`]
2895+
/// is on-by-default but there are enough deficiencies in Wasmtime's
2896+
/// implementation and API integration that creation of a shared memory is
2897+
/// disabled by default. This cofiguration knob can be used to enable this.
2898+
///
2899+
/// When enabling this method be aware that wasm threads are, at this time,
2900+
/// a [tier 2
2901+
/// feature](https://docs.wasmtime.dev/stability-tiers.html#tier-2) in
2902+
/// Wasmtime meaning that it will not receive security updates or fixes to
2903+
/// historical releases. Additionally security CVEs will not be issued for
2904+
/// bugs in the implementation.
2905+
///
2906+
/// This option is `false` by default.
2907+
pub fn shared_memory(&mut self, enable: bool) -> &mut Self {
2908+
self.shared_memory = enable;
2909+
self
2910+
}
28882911
}
28892912

28902913
impl Default for Config {

crates/wasmtime/src/runtime/memory.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -777,8 +777,9 @@ pub unsafe trait MemoryCreator: Send + Sync {
777777
/// used concurrently by multiple agents. Because these agents may execute in
778778
/// different threads, [`SharedMemory`] must be thread-safe.
779779
///
780-
/// When the threads proposal is enabled, there are multiple ways to construct
781-
/// shared memory:
780+
/// When the [threads proposal is enabled](crate::Config::wasm_threads) and the
781+
/// [the creation of shared memories is enabled](crate::Config::shared_memory),
782+
/// there are multiple ways to construct shared memory:
782783
/// 1. for imported shared memory, e.g., `(import "env" "memory" (memory 1 1
783784
/// shared))`, the user must supply a [`SharedMemory`] with the
784785
/// externally-created memory as an import to the instance--e.g.,
@@ -797,6 +798,7 @@ pub unsafe trait MemoryCreator: Send + Sync {
797798
/// # fn main() -> anyhow::Result<()> {
798799
/// let mut config = Config::new();
799800
/// config.wasm_threads(true);
801+
/// config.shared_memory(true);
800802
/// # if Engine::new(&config).is_err() { return Ok(()); }
801803
/// let engine = Engine::new(&config)?;
802804
/// let mut store = Store::new(&engine, ());
@@ -825,9 +827,8 @@ impl SharedMemory {
825827
}
826828
debug_assert!(ty.maximum().is_some());
827829

828-
let tunables = engine.tunables();
829830
let ty = ty.wasmtime_memory();
830-
let memory = crate::runtime::vm::SharedMemory::new(ty, tunables)?;
831+
let memory = crate::runtime::vm::SharedMemory::new(engine, ty)?;
831832

832833
Ok(Self {
833834
vm: memory,

crates/wasmtime/src/runtime/vm/instance/allocator/on_demand.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ unsafe impl InstanceAllocator for OnDemandInstanceAllocator {
129129
let allocation_index = MemoryAllocationIndex::default();
130130
let memory = Memory::new_dynamic(
131131
ty,
132-
request.store.engine().tunables(),
132+
request.store.engine(),
133133
creator,
134134
image,
135135
request.limiter.as_deref_mut(),

0 commit comments

Comments
 (0)