Merge latest changes

.github/workflows/main.yml

@@ -740,23 +740,6 @@ jobs:
       env:
         RUST_BACKTRACE: 1
 
-    # Crude check for whether
-    # `unsafe_disable_continuation_linearity_check` makes the test
-    # `cont_twice` fail.
-    # We run it with continuation pooling enabled, so that the involved
-    # `VMContRef` should still be allocated, meaning that we can reliably detect
-    # that the test is resuming an already returned fiber.
-    # We expect the test case to trigger an internal assertion failure (rather
-    # than a proper Wasm trap), which turns into a SIGILL (signal 4, exit code
-    # 132).
-    - run: |
-        # By default, Github actions run commands with pipefail enabled. We don't want that, as the command below is supposed to fail:
-        set +e
-        # Let's keep the build step separate from the run step below, because the former should not fail:
-        cargo build --features=unsafe_disable_continuation_linearity_check,wasmfx_pooling_allocator
-        # We want this to fail and are happy with any non-zero exit code:
-        cargo run --features=unsafe_disable_continuation_linearity_check,wasmfx_pooling_allocator -- wast -W=exceptions,function-references,stack-switching tests/misc_testsuite/stack-switching/cont_twice.wast ; test $? -eq 1
-
     # NB: the test job here is explicitly lacking in cancellation of this run if
     # something goes wrong. These take the longest anyway and otherwise if
     # Windows fails GitHub Actions will confusingly mark the failed Windows job

Cargo.toml

@@ -414,18 +414,6 @@ memory-protection-keys = ["wasmtime-cli-flags/memory-protection-keys"]
 # throughout Wasmtime and its dependencies.
 disable-logging = ["log/max_level_off", "tracing/max_level_off"]
 
-# Until we implement proper reference counting for `VMContObj` objects,
-# we may use this flag to bypass the creation of VMContObj objects,
-# directly using the corresponding VMContRef instead.
-# This is to allow running benchmarks that may create a lot of such objects and
-# may otherwise run out of memory.
-# Note that enabling this is highly unsafe, as it makes it impossible to detect
-# at runtime when an already taken continuation is used again.
-unsafe_disable_continuation_linearity_check = [
-  "wasmtime-cranelift/unsafe_disable_continuation_linearity_check",
-  "wasmtime/unsafe_disable_continuation_linearity_check"
-]
-
 # The following toggles the baseline implementation of typed continuations.
 wasmfx_baseline = [
   "wasmtime-cranelift/wasmfx_baseline",

crates/c-api/Cargo.toml

@@ -56,10 +56,6 @@ gc = ["wasmtime/gc"]
 cranelift = ['wasmtime/cranelift']
 winch = ['wasmtime/winch']
 
-# Enable unsafe mutable continuations (temporary hack to workaround
-# the garbage generated by continuation objects).
-unsafe_disable_continuation_linearity_check = ["wasmtime/unsafe_disable_continuation_linearity_check"]
-
 # Toggle the baseline implementation of WasmFX
 wasmfx_baseline = [
   "wasmtime/wasmfx_baseline"

crates/c-api/artifact/Cargo.toml

@@ -55,12 +55,6 @@ gc = ["wasmtime-c-api/gc"]
 cranelift = ["wasmtime-c-api/cranelift"]
 winch = ["wasmtime-c-api/winch"]
 
-# Enable unsafe mutable continuations (temporary hack to workaround
-# the garbage generated by continuation objects).
-unsafe_disable_continuation_linearity_check = [
-  "wasmtime-c-api/unsafe_disable_continuation_linearity_check"
-]
-
 # Toggle the baseline implementation of WasmFX
 wasmfx_baseline = ["wasmtime-c-api/wasmfx_baseline"]
 

crates/continuations/src/lib.rs

@@ -71,6 +71,27 @@ pub struct StackLimits {
     pub last_wasm_entry_sp: usize,
 }
 
+/// This type represents "common" information that we need to save both for the
+/// main stack and each continuation.
+#[repr(C)]
+#[derive(Debug, Clone)]
+pub struct CommonStackInformation {
+    pub limits: StackLimits,
+    /// For the main stack, this field must only have one of the following values:
+    /// - Running
+    /// - Parent
+    pub state: State,
+}
+
+impl CommonStackInformation {
+    pub fn running_default() -> Self {
+        Self {
+            limits: StackLimits::default(),
+            state: State::Running,
+        }
+    }
+}
+
 impl StackLimits {
     pub fn with_stack_limit(stack_limit: usize) -> Self {
         Self {
@@ -171,12 +192,17 @@ pub const STACK_CHAIN_CONTINUATION_DISCRIMINANT: usize = 2;
 pub enum State {
     /// The `VMContRef` has been created, but `resume` has never been
     /// called on it. During this stage, we may add arguments using `cont.bind`.
-    Allocated,
-    /// `resume` has been invoked at least once on the `VMContRef`,
-    /// meaning that the function passed to `cont.new` has started executing.
-    /// Note that this state does not indicate whether the execution of this
-    /// function is currently suspended or not.
-    Invoked,
+    Fresh,
+    /// The continuation is running, meaning that it is the one currently
+    /// executing code.
+    Running,
+    /// The continuation is suspended because it executed a resume instruction
+    /// that has not finished yet. In other words, it became the parent of
+    /// another continuation (which may itself be `Running`, a `Parent`, or
+    /// `Suspended`).
+    Parent,
+    /// The continuation was suspended by a `suspend` instruction.
+    Suspended,
     /// The function originally passed to `cont.new` has returned normally.
     /// Note that there is no guarantee that a VMContRef will ever
     /// reach this status, as it may stay suspended until being dropped.
@@ -214,22 +240,21 @@ pub mod offsets {
     /// Offsets of fields in `wasmtime_runtime::continuation::VMContRef`.
     /// We uses tests there to ensure these values are correct.
     pub mod vm_cont_ref {
-        use crate::Payloads;
+        use crate::{CommonStackInformation, Payloads};
 
-        /// Offset of `limits` field
-        pub const LIMITS: usize = 0;
+        /// Offset of `common_stack_information` field
+        pub const COMMON_STACK_INFORMATION: usize = 0;
         /// Offset of `parent_chain` field
-        pub const PARENT_CHAIN: usize = LIMITS + 4 * core::mem::size_of::<usize>();
+        pub const PARENT_CHAIN: usize =
+            COMMON_STACK_INFORMATION + core::mem::size_of::<CommonStackInformation>();
         /// Offset of `stack` field
         pub const STACK: usize = PARENT_CHAIN + 2 * core::mem::size_of::<usize>();
         /// Offset of `args` field
         pub const ARGS: usize = STACK + super::FIBER_STACK_SIZE;
         /// Offset of `tag_return_values` field
         pub const TAG_RETURN_VALUES: usize = ARGS + core::mem::size_of::<Payloads>();
-        /// Offset of `state` field
-        pub const STATE: usize = TAG_RETURN_VALUES + core::mem::size_of::<Payloads>();
         /// Offset of `revision` field
-        pub const REVISION: usize = STATE + core::mem::size_of::<usize>();
+        pub const REVISION: usize = TAG_RETURN_VALUES + core::mem::size_of::<Payloads>();
     }
 
     pub mod stack_limits {
@@ -242,6 +267,14 @@ pub mod offsets {
         pub const LAST_WASM_ENTRY_SP: usize = offset_of!(StackLimits, last_wasm_entry_sp);
     }
 
+    pub mod common_stack_information {
+        use crate::CommonStackInformation;
+        use memoffset::offset_of;
+
+        pub const LIMITS: usize = offset_of!(CommonStackInformation, limits);
+        pub const STATE: usize = offset_of!(CommonStackInformation, state);
+    }
+
     /// Size of wasmtime_runtime::continuation::FiberStack.
     /// We test there that this value is correct.
     pub const FIBER_STACK_SIZE: usize = 3 * core::mem::size_of::<usize>();

crates/cranelift/Cargo.toml

@@ -43,6 +43,5 @@ incremental-cache = ["cranelift-codegen/incremental-cache"]
 wmemcheck = ["wasmtime-environ/wmemcheck"]
 gc = ["wasmtime-environ/gc"]
 threads = ["wasmtime-environ/threads"]
-unsafe_disable_continuation_linearity_check = []
 wasmfx_baseline = []
 

crates/cranelift/src/wasmfx/baseline.rs

@@ -18,12 +18,8 @@ fn get_revision<'a>(
     builder: &mut FunctionBuilder,
     contref: ir::Value,
 ) -> ir::Value {
-    if cfg!(feature = "unsafe_disable_continuation_linearity_check") {
-        builder.ins().iconst(I64, 0)
-    } else {
-        let mem_flags = ir::MemFlags::trusted();
-        builder.ins().load(I64, mem_flags, contref, 0)
-    }
+    let mem_flags = ir::MemFlags::trusted();
+    builder.ins().load(I64, mem_flags, contref, 0)
 }
 
 fn compare_revision_and_increment<'a>(
@@ -32,21 +28,17 @@ fn compare_revision_and_increment<'a>(
     contref: ir::Value,
     witness: ir::Value,
 ) -> ir::Value {
-    if cfg!(feature = "unsafe_disable_continuation_linearity_check") {
-        builder.ins().iconst(I64, 0)
-    } else {
-        let mem_flags = ir::MemFlags::trusted();
-        let revision = get_revision(env, builder, contref);
+    let mem_flags = ir::MemFlags::trusted();
+    let revision = get_revision(env, builder, contref);
 
-        let evidence = builder.ins().icmp(IntCC::Equal, witness, revision);
-        builder
-            .ins()
-            .trapz(evidence, crate::TRAP_CONTINUATION_ALREADY_CONSUMED);
+    let evidence = builder.ins().icmp(IntCC::Equal, witness, revision);
+    builder
+        .ins()
+        .trapz(evidence, crate::TRAP_CONTINUATION_ALREADY_CONSUMED);
 
-        let revision_plus1 = builder.ins().iadd_imm(revision, 1);
-        builder.ins().store(mem_flags, revision_plus1, contref, 0);
-        revision_plus1
-    }
+    let revision_plus1 = builder.ins().iadd_imm(revision, 1);
+    builder.ins().store(mem_flags, revision_plus1, contref, 0);
+    revision_plus1
 }
 
 fn typed_continuations_load_payloads<'a>(