Merge pull request #4492 from wasmerio/longjmp

No longer restoring the thread local memory when we longjmp
wasmerio · Mar 18, 2024 · 4d0f268 · 4d0f268
2 parents 3ab660e + 5a820ca
commit 4d0f268
Show file tree

Hide file tree

Showing 11 changed files with 36 additions and 20 deletions.
diff --git a/lib/wasix/src/bin_factory/exec.rs b/lib/wasix/src/bin_factory/exec.rs
@@ -199,7 +199,7 @@ fn call_module(
         if rewind_state.is_64bit {
             let res = rewind_ext::<Memory64>(
                 &mut ctx,
-                rewind_state.memory_stack,
+                Some(rewind_state.memory_stack),
                 rewind_state.rewind_stack,
                 rewind_state.store_data,
                 rewind_result,
@@ -212,7 +212,7 @@ fn call_module(
         } else {
             let res = rewind_ext::<Memory32>(
                 &mut ctx,
-                rewind_state.memory_stack,
+                Some(rewind_state.memory_stack),
                 rewind_state.rewind_stack,
                 rewind_state.store_data,
                 rewind_result,

diff --git a/lib/wasix/src/os/task/process.rs b/lib/wasix/src/os/task/process.rs
@@ -252,8 +252,13 @@ impl WasiProcessInner {
                 trace!("checkpoint finished");
 
                 // Rewind the stack and carry on
-                return match rewind_ext::<M>(&mut ctx, memory_stack, rewind_stack, store_data, None)
-                {
+                return match rewind_ext::<M>(
+                    &mut ctx,
+                    Some(memory_stack),
+                    rewind_stack,
+                    store_data,
+                    None,
+                ) {
                     Errno::Success => OnCalledAction::InvokeAgain,
                     err => {
                         tracing::warn!(

diff --git a/lib/wasix/src/os/task/thread.rs b/lib/wasix/src/os/task/thread.rs
@@ -198,8 +198,8 @@ pub struct WasiMemoryLayout {
 // Contains the result of a rewind operation
 #[derive(Clone, Debug)]
 pub(crate) struct RewindResult {
-    /// Memory stack used to restore the stack trace back to where it was
-    pub memory_stack: Bytes,
+    /// Memory stack used to restore the memory stack (thing that holds local variables) back to where it was
+    pub memory_stack: Option<Bytes>,
     /// Generic serialized object passed back to the rewind resumption code
     /// (uses the bincode serializer)
     pub rewind_result: Option<Bytes>,

diff --git a/lib/wasix/src/state/run.rs b/lib/wasix/src/state/run.rs
@@ -89,15 +89,15 @@ fn run_with_deep_sleep(
         let errno = if rewind_state.is_64bit {
             crate::rewind_ext::<wasmer_types::Memory64>(
                 &mut ctx,
-                rewind_state.memory_stack,
+                Some(rewind_state.memory_stack),
                 rewind_state.rewind_stack,
                 rewind_state.store_data,
                 rewind_result,
             )
         } else {
             crate::rewind_ext::<wasmer_types::Memory32>(
                 &mut ctx,
-                rewind_state.memory_stack,
+                Some(rewind_state.memory_stack),
                 rewind_state.rewind_stack,
                 rewind_state.store_data,
                 rewind_result,

diff --git a/lib/wasix/src/syscalls/mod.rs b/lib/wasix/src/syscalls/mod.rs
@@ -1162,18 +1162,18 @@ where
     let rewind_result = bincode::serialize(&result).unwrap().into();
     rewind_ext::<M>(
         &mut ctx,
-        memory_stack,
+        Some(memory_stack),
         rewind_stack,
         store_data,
         Some(rewind_result),
     )
 }
 
-#[instrument(level = "debug", skip_all, fields(memory_stack_len = memory_stack.len(), rewind_stack_len = rewind_stack.len(), store_data_len = store_data.len()))]
+#[instrument(level = "debug", skip_all, fields(rewind_stack_len = rewind_stack.len(), store_data_len = store_data.len()))]
 #[must_use = "the action must be passed to the call loop"]
 pub fn rewind_ext<M: MemorySize>(
     ctx: &mut FunctionEnvMut<WasiEnv>,
-    memory_stack: Bytes,
+    memory_stack: Option<Bytes>,
     rewind_stack: Bytes,
     store_data: Bytes,
     rewind_result: Option<Bytes>,
@@ -1265,15 +1265,15 @@ pub fn rewind_ext2(
         let errno = if rewind_state.is_64bit {
             crate::rewind_ext::<wasmer_types::Memory64>(
                 ctx,
-                rewind_state.memory_stack,
+                Some(rewind_state.memory_stack),
                 rewind_state.rewind_stack,
                 rewind_state.store_data,
                 rewind_result,
             )
         } else {
             crate::rewind_ext::<wasmer_types::Memory32>(
                 ctx,
-                rewind_state.memory_stack,
+                Some(rewind_state.memory_stack),
                 rewind_state.rewind_stack,
                 rewind_state.store_data,
                 rewind_result,
@@ -1338,7 +1338,9 @@ where
 
         // Restore the memory stack
         let (env, mut store) = ctx.data_and_store_mut();
-        set_memory_stack::<M>(env, &mut store, memory_stack);
+        if let Some(memory_stack) = memory_stack {
+            set_memory_stack::<M>(env, &mut store, memory_stack);
+        }
 
         if let Some(rewind_result) = result.rewind_result {
             let ret = bincode::deserialize(&rewind_result)

diff --git a/lib/wasix/src/syscalls/wasix/proc_fork.rs b/lib/wasix/src/syscalls/wasix/proc_fork.rs
@@ -250,7 +250,7 @@ fn run<M: MemorySize>(
         let mut ctx = ctx.env.clone().into_mut(&mut store);
         let res = rewind_ext::<M>(
             &mut ctx,
-            rewind_state.memory_stack,
+            Some(rewind_state.memory_stack),
             rewind_state.rewind_stack,
             rewind_state.store_data,
             Some(rewind_result),

diff --git a/lib/wasix/src/syscalls/wasix/stack_restore.rs b/lib/wasix/src/syscalls/wasix/stack_restore.rs
@@ -42,7 +42,16 @@ pub fn stack_restore<M: MemorySize>(
             // so that the execution can end here and continue elsewhere.
             let pid = ctx.data().pid();
             let tid = ctx.data().tid();
-            match rewind::<M, _>(ctx, memory_stack.freeze(), rewind_stack, store_data, val) {
+
+            let rewind_result = bincode::serialize(&val).unwrap().into();
+            let ret = rewind_ext::<M>(
+                &mut ctx,
+                None, // we do not restore the thread memory as `longjmp`` is not meant to do this
+                rewind_stack,
+                store_data,
+                Some(rewind_result),
+            );
+            match ret {
                 Errno::Success => OnCalledAction::InvokeAgain,
                 err => {
                     warn!("failed to rewind the stack - errno={}", err);

diff --git a/lib/wasix/src/syscalls/wasix/thread_spawn.rs b/lib/wasix/src/syscalls/wasix/thread_spawn.rs
@@ -220,7 +220,7 @@ fn call_module<M: MemorySize>(
         let mut ctx = ctx.env.clone().into_mut(&mut store);
         let res = rewind_ext::<M>(
             &mut ctx,
-            rewind_state.memory_stack,
+            Some(rewind_state.memory_stack),
             rewind_state.rewind_stack,
             rewind_state.store_data,
             Some(rewind_result),

diff --git a/tests/integration/cli/tests/snapshots/snapshot__snapshot_bash_dash.snap b/tests/integration/cli/tests/snapshots/snapshot__snapshot_bash_dash.snap
@@ -25,7 +25,7 @@ expression: snapshot
     "Success": {
       "stdout": "hi\n",
       "stderr": "test.wasm-5.1# # # test.wasm-5.1# exit\n",
-      "exit_code": 78
+      "exit_code": 0
     }
   }
 }
diff --git a/tests/integration/cli/tests/snapshots/snapshot__snapshot_dash_bash.snap b/tests/integration/cli/tests/snapshots/snapshot__snapshot_dash_bash.snap
@@ -25,7 +25,7 @@ expression: snapshot
     "Success": {
       "stdout": "hi\n",
       "stderr": "# bash-5.1# exit\n# # ",
-      "exit_code": 78
+      "exit_code": 0
     }
   }
 }
diff --git a/tests/integration/cli/tests/snapshots/snapshot__snapshot_dash_dash.snap b/tests/integration/cli/tests/snapshots/snapshot__snapshot_dash_dash.snap
@@ -25,7 +25,7 @@ expression: snapshot
     "Success": {
       "stdout": "hi\n",
       "stderr": "# # \n# # ",
-      "exit_code": 78
+      "exit_code": 0
     }
   }
 }