From 138a76df5d2b43556705bb2e4a2b26e24beb568d Mon Sep 17 00:00:00 2001
From: Alex Crichton <alex@alexcrichton.com>
Date: Tue, 17 Jan 2023 13:14:06 -0600
Subject: [PATCH] Fix a debug assert with `wasm_backtrace(false)` (#5580)

This commit fixes an issue where when backtraces were disabled but a
host function returned an error it would trigger a debug assertion
within Wasmtime. The fix here is to update the condition of the debug
assertion and add a test doing this behavior to ensure it works in the
future.

I've also further taken the liberty in this commit to remove the
deprecation notice for `Config::wasm_backtrace`. We don't really have a
strong reason for removing this functionality at this time and users
have multiple times now reported issues with performance that seem
worthwhile to keep the option. The latest issue, #5577, has a use case
where it appears the quadratic behavior is back in a way that Wasmtime
won't be able to detect. Namely with lots of wasm interleaved with host
on the stack if the original error isn't threaded through the entire
time then each host error will trigger a new backtrace since it doesn't
see a prior backtrace in the error being returned.

While this could otherwise be fixed with only capturing one contiguous
backtrace perhaps this seems reasonable enough to leave the
`wasm_backtrace` config option for now.

Closes #5577
---
 crates/wasmtime/src/config.rs |  2 --
 crates/wasmtime/src/trap.rs   |  4 +++-
 tests/all/traps.rs            | 25 +++++++++++++++++++++++++
 3 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/crates/wasmtime/src/config.rs b/crates/wasmtime/src/config.rs
index dbd5f47a71..b3c32f2bc6 100644
--- a/crates/wasmtime/src/config.rs
+++ b/crates/wasmtime/src/config.rs
@@ -365,8 +365,6 @@ impl Config {
     /// This option is `true` by default.
     ///
     /// [`WasmBacktrace`]: crate::WasmBacktrace
-    #[deprecated = "Backtraces will always be enabled in future Wasmtime releases; if this \
-                    causes problems for you, please file an issue."]
     pub fn wasm_backtrace(&mut self, enable: bool) -> &mut Self {
         self.wasm_backtrace = enable;
         self
diff --git a/crates/wasmtime/src/trap.rs b/crates/wasmtime/src/trap.rs
index 41b59fe0d4..e1e3fdd9ad 100644
--- a/crates/wasmtime/src/trap.rs
+++ b/crates/wasmtime/src/trap.rs
@@ -98,7 +98,9 @@ pub(crate) fn from_runtime_box(
             error,
             needs_backtrace,
         } => {
-            debug_assert!(needs_backtrace == backtrace.is_some());
+            debug_assert!(
+                needs_backtrace == backtrace.is_some() || !store.engine().config().wasm_backtrace
+            );
             (error, None)
         }
         wasmtime_runtime::TrapReason::Jit(pc) => {
diff --git a/tests/all/traps.rs b/tests/all/traps.rs
index a8005e1053..ae62280c6e 100644
--- a/tests/all/traps.rs
+++ b/tests/all/traps.rs
@@ -1160,3 +1160,28 @@ fn standalone_backtrace_disabled() -> Result<()> {
     f.call(&mut store, ())?;
     Ok(())
 }
+
+#[test]
+fn host_return_error_no_backtrace() -> Result<()> {
+    let mut config = Config::new();
+    config.wasm_backtrace(false);
+    let engine = Engine::new(&config)?;
+    let mut store = Store::new(&engine, ());
+    let module = Module::new(
+        &engine,
+        r#"
+            (module
+                (import "" "" (func $host))
+                (func $foo (export "f") call $bar)
+                (func $bar call $host)
+            )
+        "#,
+    )?;
+    let func = Func::wrap(&mut store, |_cx: Caller<'_, ()>| -> Result<()> {
+        bail!("test")
+    });
+    let instance = Instance::new(&mut store, &module, &[func.into()])?;
+    let f = instance.get_typed_func::<(), ()>(&mut store, "f")?;
+    assert!(f.call(&mut store, ()).is_err());
+    Ok(())
+}