Merge wasmerio#2728

2728: Replace RuntimeError::raise with RuntimeError::custom r=syrusakbary a=Amanieu `RuntimeError::raise` should not be exposed to user code since it perfoms a `longjmp` internally which is unsound if there are any destructors on the stack. Instead a custom error type should be returned using `RuntimeError::custom` which can be passed through WASM code and later retrieved using `RuntimeError::downcast`. Co-authored-by: Amanieu d'Antras <[email protected]>
gtheric · Dec 18, 2021 · c68393a · c68393a
2 parents c9eab66 + 17c0834
commit c68393a
Show file tree

Hide file tree

Showing 7 changed files with 52 additions and 25 deletions.
diff --git a/examples/early_exit.rs b/examples/early_exit.rs
@@ -62,9 +62,9 @@ fn main() -> anyhow::Result<()> {
     let module = Module::new(&store, wasm_bytes)?;
 
     // We declare the host function that we'll use to terminate execution.
-    fn early_exit() {
+    fn early_exit() -> Result<(), RuntimeError> {
         // This is where it happens.
-        RuntimeError::raise(Box::new(ExitCode(1)));
+        Err(RuntimeError::custom(Box::new(ExitCode(1))))
     }
 
     // Create an import object.

diff --git a/lib/api/src/js/externals/function.rs b/lib/api/src/js/externals/function.rs
@@ -644,6 +644,7 @@ impl wasmer_types::WasmValueType for Function {
 /// This private inner module contains the low-level implementation
 /// for `Function` and its siblings.
 mod inner {
+    use super::RuntimeError;
     use super::VMFunctionBody;
     use std::array::TryFromSliceError;
     use std::convert::{Infallible, TryInto};
@@ -1125,6 +1126,7 @@ mod inner {
                         }));
                         match result {
                             Ok(Ok(result)) => return result.into_c_struct(),
+                            Ok(Err(trap)) => RuntimeError::raise(Box::new(trap)),
                             _ => unimplemented!(),
                             // Ok(Err(trap)) => unsafe { raise_user_trap(Box::new(trap)) },
                             // Err(panic) => unsafe { resume_panic(panic) },
@@ -1170,6 +1172,7 @@ mod inner {
                         }));
                         match result {
                             Ok(Ok(result)) => return result.into_c_struct(),
+                            Ok(Err(trap)) => RuntimeError::raise(Box::new(trap)),
                             _ => unimplemented!(),
                             // Ok(Err(trap)) => unsafe { raise_user_trap(Box::new(trap)) },
                             // Err(panic) => unsafe { resume_panic(panic) },

diff --git a/lib/api/src/js/trap.rs b/lib/api/src/js/trap.rs
@@ -34,6 +34,10 @@ enum RuntimeErrorSource {
     Js(JsValue),
 }
 
+/// This is a hack to ensure the error type is Send+Sync
+unsafe impl Send for RuntimeErrorSource {}
+unsafe impl Sync for RuntimeErrorSource {}
+
 impl fmt::Display for RuntimeErrorSource {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
@@ -58,26 +62,28 @@ impl RuntimeError {
         }
     }
 
-    /// Creates a new user `RuntimeError` with the given `error`.
-    pub fn user(error: impl Error + Send + Sync + 'static) -> Self {
-        RuntimeError {
-            inner: Arc::new(RuntimeErrorSource::User(Box::new(error))),
-        }
-    }
-
     /// Raises a custom user Error
+    #[deprecated(since = "2.1.1", note = "prefer using RuntimeError::custom instead")]
     pub fn raise(error: Box<dyn Error + Send + Sync>) -> ! {
-        let error = if error.is::<RuntimeError>() {
-            *error.downcast::<RuntimeError>().unwrap()
-        } else {
-            RuntimeError {
-                inner: Arc::new(RuntimeErrorSource::User(error)),
-            }
-        };
+        let error = Self::custom(error);
         let js_error: JsValue = error.into();
         wasm_bindgen::throw_val(js_error)
     }
 
+    /// Creates a custom user Error.
+    ///
+    /// This error object can be passed through Wasm frames and later retrieved
+    /// using the `downcast` method.
+    pub fn custom(error: Box<dyn Error + Send + Sync>) -> Self {
+        match error.downcast::<Self>() {
+            // The error is already a RuntimeError, we return it directly
+            Ok(runtime_error) => *runtime_error,
+            Err(error) => RuntimeError {
+                inner: Arc::new(RuntimeErrorSource::User(error)),
+            },
+        }
+    }
+
     /// Returns a reference the `message` stored in `Trap`.
     pub fn message(&self) -> String {
         format!("{}", self.inner)

diff --git a/lib/api/tests/js_instance.rs b/lib/api/tests/js_instance.rs
@@ -690,8 +690,8 @@ mod js {
 
         impl std::error::Error for ExitCode {}
 
-        fn early_exit() {
-            RuntimeError::raise(Box::new(ExitCode(1)));
+        fn early_exit() -> Result<(), RuntimeError> {
+            Err(RuntimeError::custom(Box::new(ExitCode(1))))
         }
 
         let import_object = imports! {

diff --git a/lib/emscripten/src/jmp.rs b/lib/emscripten/src/jmp.rs
@@ -59,16 +59,14 @@ impl Error for LongJumpRet {}
 /// _longjmp
 // This function differs from the js implementation, it should return Result<(), &'static str>
 #[allow(unreachable_code)]
-pub fn _longjmp(ctx: &EmEnv, env_addr: i32, val: c_int) {
+pub fn _longjmp(ctx: &EmEnv, env_addr: i32, val: c_int) -> Result<(), RuntimeError> {
     let val = if val == 0 { 1 } else { val };
     get_emscripten_data(ctx)
         .set_threw_ref()
         .expect("set_threw is None")
         .call(env_addr, val)
         .expect("set_threw failed to call");
-    // TODO: return Err("longjmp")
-    RuntimeError::raise(Box::new(LongJumpRet));
-    unreachable!();
+    Err(RuntimeError::custom(Box::new(LongJumpRet)))
 }
 
 // extern "C" {

diff --git a/lib/engine/src/trap/error.rs b/lib/engine/src/trap/error.rs
@@ -107,10 +107,31 @@ impl RuntimeError {
     }
 
     /// Raises a custom user Error
+    #[deprecated(since = "2.1.1", note = "prefer using RuntimeError::custom instead")]
     pub fn raise(error: Box<dyn Error + Send + Sync>) -> ! {
         unsafe { raise_user_trap(error) }
     }
 
+    /// Creates a custom user Error.
+    ///
+    /// This error object can be passed through Wasm frames and later retrieved
+    /// using the `downcast` method.
+    pub fn custom(error: Box<dyn Error + Send + Sync>) -> Self {
+        match error.downcast::<Self>() {
+            // The error is already a RuntimeError, we return it directly
+            Ok(runtime_error) => *runtime_error,
+            Err(error) => {
+                let info = FRAME_INFO.read().unwrap();
+                Self::new_with_trace(
+                    &info,
+                    None,
+                    RuntimeErrorSource::User(error),
+                    Backtrace::new_unresolved(),
+                )
+            }
+        }
+    }
+
     fn new_with_trace(
         info: &GlobalFrameInfo,
         trap_pc: Option<usize>,

diff --git a/lib/wasi/src/syscalls/mod.rs b/lib/wasi/src/syscalls/mod.rs
@@ -2521,10 +2521,9 @@ pub fn poll_oneoff(
     unimplemented!();
 }
 
-pub fn proc_exit(env: &WasiEnv, code: __wasi_exitcode_t) {
+pub fn proc_exit(env: &WasiEnv, code: __wasi_exitcode_t) -> Result<(), RuntimeError> {
     debug!("wasi::proc_exit, {}", code);
-    RuntimeError::raise(Box::new(WasiError::Exit(code)));
-    unreachable!();
+    Err(RuntimeError::custom(Box::new(WasiError::Exit(code))))
 }
 
 pub fn proc_raise(env: &WasiEnv, sig: __wasi_signal_t) -> __wasi_errno_t {