Decouple some more Config methods from each other (#4262)

* Decouple some more `Config` methods from each other

This commit decouples validation of stack sizes and guard sizes until
`Engine::new` to avoid odd interactions between the order of invocation
of `Config` methods.

* Fix C API

* Typos

RELEASES.md

@@ -8,16 +8,19 @@ Unreleased.
 
 ### Changed
 
-* The `Config::strategy`, `Config::cranelift_flag_enable`, `Config::cranelift_flag_set` 
+* Some methods on the `Config` structure now return `&mut Self` instead of
-  and `Config::profiler` APIs now return `&mut Self` instead of `Result<&mut Self>`
+  `Result<&mut Self>` since the validation is deferred until `Engine::new`:
-  since the validation is deferred until `Engine::new`.
+  `profiler`, `cranelift_flag_enable`, `cranelift_flag_set`, `max_wasm_stack`,
+  `async_stack_size`, and `strategy`.
   [#4252](https://github.com/bytecodealliance/wasmtime/pull/4252)
+  [#4262](https://github.com/bytecodealliance/wasmtime/pull/4262)
 
 ### Fixed
 
 * A refactor of `Config` was made to fix an issue that the order of calls to `Config`
   matters now, which may lead to unexpected behavior.
   [#4252](https://github.com/bytecodealliance/wasmtime/pull/4252)
+  [#4262](https://github.com/bytecodealliance/wasmtime/pull/4262)
 
 --------------------------------------------------------------------------------
 

crates/c-api/src/config.rs

@@ -60,8 +60,8 @@ pub extern "C" fn wasmtime_config_epoch_interruption_set(c: &mut wasm_config_t,
 }
 
 #[no_mangle]
-pub extern "C" fn wasmtime_config_max_wasm_stack_set(c: &mut wasm_config_t, size: usize) -> bool {
+pub extern "C" fn wasmtime_config_max_wasm_stack_set(c: &mut wasm_config_t, size: usize) {
-    c.config.max_wasm_stack(size).is_ok()
+    c.config.max_wasm_stack(size);
 }
 
 #[no_mangle]

crates/wasmtime/src/config.rs

@@ -2,14 +2,11 @@ use crate::memory::MemoryCreator;
 use crate::trampoline::MemoryCreatorProxy;
 use anyhow::{bail, Result};
 use serde::{Deserialize, Serialize};
+use std::collections::{HashMap, HashSet};
 use std::fmt;
 #[cfg(feature = "cache")]
 use std::path::Path;
 use std::sync::Arc;
-use std::{
-    cmp,
-    collections::{HashMap, HashSet},
-};
 use wasmparser::WasmFeatures;
 #[cfg(feature = "cache")]
 use wasmtime_cache::CacheConfig;
@@ -90,10 +87,11 @@ impl Default for ModuleVersionStrategy {
 pub struct Config {
     #[cfg(compiler)]
     compiler_config: CompilerConfig,
+    profiling_strategy: ProfilingStrategy,
+
     pub(crate) tunables: Tunables,
     #[cfg(feature = "cache")]
     pub(crate) cache_config: CacheConfig,
-    pub(crate) profiling_strategy: ProfilingStrategy,
     pub(crate) mem_creator: Option<Arc<dyn RuntimeMemoryCreator>>,
     pub(crate) allocation_strategy: InstanceAllocationStrategy,
     pub(crate) max_wasm_stack: usize,
@@ -504,18 +502,14 @@ impl Config {
     /// abort the process.
     ///
     /// By default this option is 512 KiB.
-    pub fn max_wasm_stack(&mut self, size: usize) -> Result<&mut Self> {
+    ///
-        #[cfg(feature = "async")]
+    /// # Errors
-        if size > self.async_stack_size {
+    ///
-            bail!("wasm stack size cannot exceed the async stack size");
+    /// The `Engine::new` method will fail if the `size` specified here is
-        }
+    /// either 0 or larger than the [`Config::async_stack_size`] configuration.
-
+    pub fn max_wasm_stack(&mut self, size: usize) -> &mut Self {
-        if size == 0 {
-            bail!("wasm stack size cannot be zero");
-        }
-
         self.max_wasm_stack = size;
-        Ok(self)
+        self
     }
 
     /// Configures the size of the stacks used for asynchronous execution.
@@ -529,14 +523,16 @@ impl Config {
     /// stack and abort the process.
     ///
     /// By default this option is 2 MiB.
+    ///
+    /// # Errors
+    ///
+    /// The `Engine::new` method will fail if the value for this option is
+    /// smaller than the [`Config::max_wasm_stack`] option.
     #[cfg(feature = "async")]
     #[cfg_attr(nightlydoc, doc(cfg(feature = "async")))]
-    pub fn async_stack_size(&mut self, size: usize) -> Result<&mut Self> {
+    pub fn async_stack_size(&mut self, size: usize) -> &mut Self {
-        if size < self.max_wasm_stack {
-            bail!("async stack size cannot be less than the maximum wasm stack size");
-        }
         self.async_stack_size = size;
-        Ok(self)
+        self
     }
 
     /// Configures whether the WebAssembly threads proposal will be enabled for
@@ -1062,14 +1058,12 @@ impl Config {
     /// immediate offset of less than 2GB. On 32-bit platforms this defaults to
     /// 64KB.
     ///
-    /// ## Static vs Dynamic Guard Size
+    /// ## Errors
     ///
-    /// Note that for now the static memory guard size must be at least as large
+    /// The `Engine::new` method will return an error if this option is smaller
-    /// as the dynamic memory guard size, so configuring this property to be
+    /// than the value configured for [`Config::dynamic_memory_guard_size`].
-    /// smaller than the dynamic memory guard size will have no effect.
     pub fn static_memory_guard_size(&mut self, guard_size: u64) -> &mut Self {
         let guard_size = round_up_to_pages(guard_size);
-        let guard_size = cmp::max(guard_size, self.tunables.dynamic_memory_offset_guard_size);
         self.tunables.static_memory_offset_guard_size = guard_size;
         self
     }
@@ -1096,11 +1090,14 @@ impl Config {
     /// ## Default
     ///
     /// This value defaults to 64KB.
+    ///
+    /// ## Errors
+    ///
+    /// The `Engine::new` method will return an error if this option is larger
+    /// than the value configured for [`Config::static_memory_guard_size`].
     pub fn dynamic_memory_guard_size(&mut self, guard_size: u64) -> &mut Self {
         let guard_size = round_up_to_pages(guard_size);
         self.tunables.dynamic_memory_offset_guard_size = guard_size;
-        self.tunables.static_memory_offset_guard_size =
-            cmp::max(guard_size, self.tunables.static_memory_offset_guard_size);
         self
     }
 
@@ -1335,6 +1332,19 @@ impl Config {
         if self.features.threads && !self.features.bulk_memory {
             bail!("feature 'threads' requires 'bulk_memory' to be enabled");
         }
+        #[cfg(feature = "async")]
+        if self.max_wasm_stack > self.async_stack_size {
+            bail!("max_wasm_stack size cannot exceed the async_stack_size");
+        }
+        if self.max_wasm_stack == 0 {
+            bail!("max_wasm_stack size cannot be zero");
+        }
+        if self.tunables.static_memory_offset_guard_size
+            < self.tunables.dynamic_memory_offset_guard_size
+        {
+            bail!("static memory guard size cannot be smaller than dynamic memory guard size");
+        }
+
         Ok(())
     }