Stream: git-wasmtime

Topic: wasmtime / issue #4881 fuzz: improve the spec interpreter

Wasmtime GitHub notifications bot (Sep 07 2022 at 22:56):

abrown commented on issue #4881:

@conrad-watt, I see occasional errors when running the wasm-spec-interpreter tests. I am not sure what is going on, but I wonder if the global state on the OCaml side? E.g.:

$ cargo test --features build-libinterpret
    Finished test [unoptimized + debuginfo] target(s) in 0.03s
     Running unittests src/lib.rs (/.../wasmtime/target/debug/deps/wasm_spec_interpreter-f7e7d72240eb2160)

running 10 tests
test with_library::tests::order_of_params ... ok
test with_library::tests::invalid_function_name ... ok
test with_library::tests::oob_legacy ... ok
test with_library::tests::oob ... ok
test with_library::tests::multiple_invocation_legacy ... ok
test with_library::tests::simd_not_legacy ... ok
test with_library::tests::load_store_and_export ... ok
test with_library::tests::simd_not ... ok
test with_library::tests::order_of_params_legacy ... ok
test with_library::tests::multiple_invocation ... FAILED

failures:

---- with_library::tests::multiple_invocation stdout ----
thread 'with_library::tests::multiple_invocation' panicked at 'called `Result::unwrap()` on an `Err` value: "Error(_, \"(Isabelle) error: type system violation\")"', crates/fuzzing/wasm-spec-interpreter/src/with_library.rs:212:10
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    with_library::tests::multiple_invocation

test result: FAILED. 9 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

I will see more than 75% of test runs like the above fail, but often for different reasons. I wondered if the locking is an issue, but it continues to happen even when I run the tests in single-threaded mode (e.g., cargo test --features build-libinterpret -j 1). Any ideas?

Wasmtime GitHub notifications bot (Sep 07 2022 at 22:56):

abrown edited a comment on issue #4881:

@conrad-watt, I see occasional errors when running the wasm-spec-interpreter tests. I am not sure what is going on, but I wonder if the global state on the OCaml side? E.g.:

$ cargo test --features build-libinterpret
    Finished test [unoptimized + debuginfo] target(s) in 0.03s
     Running unittests src/lib.rs (/.../wasmtime/target/debug/deps/wasm_spec_interpreter-f7e7d72240eb2160)

running 10 tests
test with_library::tests::order_of_params ... ok
test with_library::tests::invalid_function_name ... ok
test with_library::tests::oob_legacy ... ok
test with_library::tests::oob ... ok
test with_library::tests::multiple_invocation_legacy ... ok
test with_library::tests::simd_not_legacy ... ok
test with_library::tests::load_store_and_export ... ok
test with_library::tests::simd_not ... ok
test with_library::tests::order_of_params_legacy ... ok
test with_library::tests::multiple_invocation ... FAILED

failures:

---- with_library::tests::multiple_invocation stdout ----
thread 'with_library::tests::multiple_invocation' panicked at 'called `Result::unwrap()` on an `Err` value: "Error(_, \"(Isabelle) error: type system violation\")"', crates/fuzzing/wasm-spec-interpreter/src/with_library.rs:212:10
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    with_library::tests::multiple_invocation

test result: FAILED. 9 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

I will see more than 75% of test runs like the above fail, but often for different reasons. I wondered if the locking is an issue, but it continues to happen even when I run the tests in single-threaded mode (e.g., cargo test --features build-libinterpret -j 1). Any ideas?

Wasmtime GitHub notifications bot (Sep 07 2022 at 22:57):

abrown edited a comment on issue #4881:

@conrad-watt, I see occasional errors when running the wasm-spec-interpreter tests. I am not sure what is going on, but I wonder if something is wrong with the global state on the OCaml side? E.g.:

$ cargo test --features build-libinterpret
    Finished test [unoptimized + debuginfo] target(s) in 0.03s
     Running unittests src/lib.rs (/.../wasmtime/target/debug/deps/wasm_spec_interpreter-f7e7d72240eb2160)

running 10 tests
test with_library::tests::order_of_params ... ok
test with_library::tests::invalid_function_name ... ok
test with_library::tests::oob_legacy ... ok
test with_library::tests::oob ... ok
test with_library::tests::multiple_invocation_legacy ... ok
test with_library::tests::simd_not_legacy ... ok
test with_library::tests::load_store_and_export ... ok
test with_library::tests::simd_not ... ok
test with_library::tests::order_of_params_legacy ... ok
test with_library::tests::multiple_invocation ... FAILED

failures:

---- with_library::tests::multiple_invocation stdout ----
thread 'with_library::tests::multiple_invocation' panicked at 'called `Result::unwrap()` on an `Err` value: "Error(_, \"(Isabelle) error: type system violation\")"', crates/fuzzing/wasm-spec-interpreter/src/with_library.rs:212:10
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    with_library::tests::multiple_invocation

test result: FAILED. 9 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

I will see more than 75% of test runs like the above fail, but often for different reasons. I wondered if the locking is an issue, but it continues to happen even when I run the tests in single-threaded mode (e.g., cargo test --features build-libinterpret -j 1). Any ideas?

Wasmtime GitHub notifications bot (Sep 07 2022 at 22:58):

abrown commented on issue #4881:

Here's another one that shows how the same error state is being seen across multiple tests:

$ cargo test --features build-libinterpret -j 1
    Finished test [unoptimized + debuginfo] target(s) in 0.04s
     Running unittests src/lib.rs (/home/abrown/Code/wasmtime/target/debug/deps/wasm_spec_interpreter-f7e7d72240eb2160)

running 10 tests
test with_library::tests::invalid_function_name ... ok
test with_library::tests::oob_legacy ... ok
test with_library::tests::order_of_params ... ok
test with_library::tests::order_of_params_legacy ... ok
test with_library::tests::oob ... ok
test with_library::tests::simd_not_legacy ... ok
test with_library::tests::simd_not ... FAILED
test with_library::tests::multiple_invocation ... FAILED
test with_library::tests::multiple_invocation_legacy ... ok
test with_library::tests::load_store_and_export ... FAILED

failures:

---- with_library::tests::simd_not stdout ----
thread 'with_library::tests::simd_not' panicked at 'called `Result::unwrap()` on an `Err` value: "Error(_, \"(Isabelle) trap: load\")"', crates/fuzzing/wasm-spec-interpreter/src/with_library.rs:268:68
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- with_library::tests::multiple_invocation stdout ----
thread 'with_library::tests::multiple_invocation' panicked at 'called `Result::unwrap()` on an `Err` value: "Error(_, \"(Isabelle) trap: load\")"', crates/fuzzing/wasm-spec-interpreter/src/with_library.rs:212:10

---- with_library::tests::load_store_and_export stdout ----
thread 'with_library::tests::load_store_and_export' panicked at 'called `Result::unwrap()` on an `Err` value: "Error(_, \"(Isabelle) trap: load\")"', crates/fuzzing/wasm-spec-interpreter/src/with_library.rs:340:27


failures:
    with_library::tests::load_store_and_export
    with_library::tests::multiple_invocation
    with_library::tests::simd_not

test result: FAILED. 7 passed; 3 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

Wasmtime GitHub notifications bot (Sep 07 2022 at 22:58):

abrown edited a comment on issue #4881:

Here's another one that shows how the same error state is being seen across multiple tests:

$ cargo test --features build-libinterpret -j 1
    Finished test [unoptimized + debuginfo] target(s) in 0.04s
     Running unittests src/lib.rs (/home/abrown/Code/wasmtime/target/debug/deps/wasm_spec_interpreter-f7e7d72240eb2160)

running 10 tests
test with_library::tests::invalid_function_name ... ok
test with_library::tests::oob_legacy ... ok
test with_library::tests::order_of_params ... ok
test with_library::tests::order_of_params_legacy ... ok
test with_library::tests::oob ... ok
test with_library::tests::simd_not_legacy ... ok
test with_library::tests::simd_not ... FAILED
test with_library::tests::multiple_invocation ... FAILED
test with_library::tests::multiple_invocation_legacy ... ok
test with_library::tests::load_store_and_export ... FAILED

failures:

---- with_library::tests::simd_not stdout ----
thread 'with_library::tests::simd_not' panicked at 'called `Result::unwrap()` on an `Err` value: "Error(_, \"(Isabelle) trap: load\")"', crates/fuzzing/wasm-spec-interpreter/src/with_library.rs:268:68
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- with_library::tests::multiple_invocation stdout ----
thread 'with_library::tests::multiple_invocation' panicked at 'called `Result::unwrap()` on an `Err` value: "Error(_, \"(Isabelle) trap: load\")"', crates/fuzzing/wasm-spec-interpreter/src/with_library.rs:212:10

---- with_library::tests::load_store_and_export stdout ----
thread 'with_library::tests::load_store_and_export' panicked at 'called `Result::unwrap()` on an `Err` value: "Error(_, \"(Isabelle) trap: load\")"', crates/fuzzing/wasm-spec-interpreter/src/with_library.rs:340:27


failures:
    with_library::tests::load_store_and_export
    with_library::tests::multiple_invocation
    with_library::tests::simd_not

test result: FAILED. 7 passed; 3 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

Note that multiple_invocation and simd_not do not do a load.

Wasmtime GitHub notifications bot (Sep 07 2022 at 23:09):

abrown commented on issue #4881:

Update: -j 1 may not be doing what I expect it to. When I forcibly limit execution to a single CPU, taskset --cpu-list 7 cargo test --features build-libinterpret, I do not see the errors. And this makes sense, now that I think about it (and don't trust -j 1): different test executions were sneaking in between instantiate and interpret, re-using the single global instance.

@alexcrichton, perhaps it makes sense to make SpecInstance hold on to the global lock for its lifetime so that no other threads can access its state?

Wasmtime GitHub notifications bot (Sep 07 2022 at 23:39):

github-actions[bot] commented on issue #4881:

Subscribe to Label Action

cc @fitzgen

<details>
This issue or pull request has been labeled: "fuzzing"

Thus the following users have been cc'd because of the following labels:

• fitzgen: fuzzing

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

Wasmtime GitHub notifications bot (Sep 07 2022 at 23:40):

conrad-watt commented on issue #4881:

Update: -j 1 may not be doing what I expect it to. When I forcibly limit execution to a single CPU, taskset --cpu-list 7 cargo test --features build-libinterpret, I do not see the errors. And this makes sense, now that I think about it (and don't trust -j 1): different test executions were sneaking in between instantiate and interpret, re-using the single global instance.

Ahhh, I'm sorry I didn't think of this. The way I currently handle the global store isn't thread safe as it gets globally reset every time instantiate is called.

@alexcrichton, perhaps it makes sense to make SpecInstance hold on to the global lock for its lifetime so that no other threads can access its state?

I think I can actually fix this with an OCaml-internal hack that would allow us to interleave instantiate and interpret properly - I'll have a quick play in my branch.

Wasmtime GitHub notifications bot (Sep 08 2022 at 00:34):

conrad-watt commented on issue #4881:

@abrown I can pick this up tomorrow if there's an issue, but I've pushed what _should_ be a fix to https://github.com/conrad-watt/wasmtime/tree/abrown-merge, as a commit on top of your PR branch here.

I was able to reproduce your errors above, and after a few runs the latest commit on that branch seems to have the tests passing.

Wasmtime GitHub notifications bot (Sep 08 2022 at 00:39):

conrad-watt edited a comment on issue #4881:

@abrown I can pick this up tomorrow if there's an issue, but I've pushed what _should_ be a fix to https://github.com/conrad-watt/wasmtime/tree/abrown-merge, as a commit on top of your PR branch here.

I was able to reproduce your errors above, and after a few runs the latest commit on that branch seems to have the tests passing.

EDIT: to go into a little detail, I've changed the OCaml-side definition of the instance so that each instance carries round a reference to a store that's specific to that instantiation. Because everything is updated by reference there should be no visible behavioural change on the Rust side, apart from everything suddenly being thread-safe. This fix will need to be generalised slightly in future if we want to allow multiple modules to be instantiated in the same store.

Wasmtime GitHub notifications bot (Sep 08 2022 at 00:42):

conrad-watt edited a comment on issue #4881:

@abrown I can pick this up tomorrow if there's an issue, but I've pushed what _should_ be a fix to https://github.com/conrad-watt/wasmtime/tree/abrown-merge, as a commit on top of your PR branch here.

I was able to reproduce your errors above, and after a few runs the latest commit on that branch seems to have the tests passing.

EDIT: to go into a little detail, I've changed the OCaml-side definition of the instance so that each instance carries round a reference to a "global store" that's specific to that instantiation. Because everything is updated by reference there should be no visible behavioural change on the Rust side, apart from everything suddenly being thread-safe. This fix will need to be generalised slightly in future if we want to allow multiple modules to be instantiated in the same store.

Wasmtime GitHub notifications bot (Sep 08 2022 at 00:43):

conrad-watt edited a comment on issue #4881:

@abrown I can pick this up tomorrow if there's an issue, but I've pushed what _should_ be a fix to https://github.com/conrad-watt/wasmtime/tree/abrown-merge, as a commit on top of your PR branch here.

I was able to reproduce your errors above, and after a few runs the latest commit on that branch seems to have the tests passing.

EDIT: to go into a little detail, I've changed the OCaml-side definition of the instance so that each instance carries round a reference to a "global store" that's specific to that instantiation. Because everything is updated by reference there should be no visible behavioural change on the Rust side, apart from everything suddenly being thread-safe (modulo the fact that access to the OCaml runtime still needs to be locked). This fix will need to be generalised slightly in future if we want to allow multiple modules to be instantiated in the same store.

Wasmtime GitHub notifications bot (Sep 08 2022 at 13:48):

abrown commented on issue #4881:

Thanks! I applied your changes in 19206a3 and I no longer see any test failures. I think this is good to go.

Wasmtime GitHub notifications bot (Sep 08 2022 at 14:44):

abrown commented on issue #4881:

Unfortunately, after d29cdbd when I run ALLOWED_ENGINES=spec cargo +nightly fuzz run -s none differential the following test case fails:

(module
  (type (;0;) (func))
  (func (;0;) (type 0)
    global.get 0
    i32.eqz
    if  ;; label = @1
      unreachable
    end
    global.get 0
    i32.const 1
    i32.sub
    global.set 0
  )
  (func (;1;) (type 0)
    global.get 0
    i32.eqz
    if  ;; label = @1
      unreachable
    end
    global.get 0
    i32.const 1
    i32.sub
    global.set 0
  )
  (global (;0;) (mut i32) i32.const 1000)
  (export "\00[\00" (func 0))
  (export "" (func 1))
)

The error is:

[2022-09-08T14:40:47Z DEBUG wasmtime_fuzzing::oracles] Evaluating: `[` with []
[2022-09-08T14:40:47Z DEBUG wasmtime_fuzzing::oracles]  -> results on spec: Err(Not_found)
[2022-09-08T14:40:47Z DEBUG wasmtime_fuzzing::oracles]  -> results on wasmtime: Ok([])
thread '<unnamed>' panicked at 'only the `lhs` (spec) failed for this input', crates/fuzzing/src/oracles.rs:387:28
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

@conrad-watt, what are the chances that we aren't handling the empty string name?

Wasmtime GitHub notifications bot (Sep 08 2022 at 14:44):

abrown edited a comment on issue #4881:

Unfortunately, after d29cdbd when I run ALLOWED_ENGINES=spec cargo +nightly fuzz run -s none differential the following test case fails:

(module
  (type (;0;) (func))
  (func (;0;) (type 0)
    global.get 0
    i32.eqz
    if  ;; label = @1
      unreachable
    end
    global.get 0
    i32.const 1
    i32.sub
    global.set 0
  )
  (func (;1;) (type 0)
    global.get 0
    i32.eqz
    if  ;; label = @1
      unreachable
    end
    global.get 0
    i32.const 1
    i32.sub
    global.set 0
  )
  (global (;0;) (mut i32) i32.const 1000)
  (export "\00[\00" (func 0))
  (export "" (func 1))
)

The error is:

[2022-09-08T14:40:47Z DEBUG wasmtime_fuzzing::oracles] Evaluating: `[` with []
[2022-09-08T14:40:47Z DEBUG wasmtime_fuzzing::oracles]  -> results on spec: Err(Not_found)
[2022-09-08T14:40:47Z DEBUG wasmtime_fuzzing::oracles]  -> results on wasmtime: Ok([])
thread '<unnamed>' panicked at 'only the `lhs` (spec) failed for this input', crates/fuzzing/src/oracles.rs:387:28
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

@conrad-watt, what are the chances that we aren't handling strings with weird bytes?

Wasmtime GitHub notifications bot (Sep 08 2022 at 14:45):

alexcrichton commented on issue #4881:

That actually looks like it maybe related to the FFI boundary and communicating strings with nul bytes in them since the function is called "\00[\00" and the Evaluating: ... there is printing the nul byte which I guess the terminal does nothing with.

Wasmtime GitHub notifications bot (Sep 08 2022 at 14:49):

abrown commented on issue #4881:

Yeah, maybe we need to pass bytes across to OCaml instead of a string?

Wasmtime GitHub notifications bot (Sep 08 2022 at 14:54):

alexcrichton edited a comment on issue #4881:

That actually looks like it may be related to the FFI boundary and communicating strings with nul bytes in them since the function is called "\00[\00" and the Evaluating: ... there is printing the nul byte which I guess the terminal does nothing with.

Wasmtime GitHub notifications bot (Sep 08 2022 at 15:08):

alexcrichton commented on issue #4881:

This looks like a bug in the ocaml-interop crate:

https://github.com/tezedge/ocaml-interop/blob/550dae57bddb8ae9e3838594bcccc18970f56e99/src/memory.rs#L69-L77

there the string_val function is documented here as:

String_val(v) returns a pointer to the first byte of the string v, with type char * or, when OCaml is configured with -force-safe-string, with type const char *. This pointer is a valid C string: there is a null byte after the last byte in the string. However, OCaml strings can contain embedded null bytes, which will confuse the usual C functions over strings.

Wasmtime GitHub notifications bot (Sep 08 2022 at 15:10):

alexcrichton commented on issue #4881:

that beings said ocaml_sys::string_val looks like this:

/// Extracts a machine `ptr` to the bytes making up an OCaml `string`
#[inline]
pub const unsafe fn string_val(val: Value) -> *mut u8 {
    val as *mut u8
}

so I dunno what's going on (not really that familiar with ocaml and FFI at all myself, this is just what I found looking around). It might be worth trying to use a byte list instead yeah.

Wasmtime GitHub notifications bot (Sep 08 2022 at 15:17):

conrad-watt commented on issue #4881:

FWIW my first reaction (in the spirit of what was discussed above) would be to try using OCaml's bytes type to pass the string from Rust -> OCaml and then calling Bytes.to_string in interpret.ml. LMK if it would be helpful for me to try this locally.

Wasmtime GitHub notifications bot (Sep 08 2022 at 15:18):

conrad-watt edited a comment on issue #4881:

FWIW my first reaction (in the spirit of what was discussed above) would be to try using OCaml's bytes type to pass the string function name from Rust -> OCaml and then calling Bytes.to_string in interpret.ml. LMK if it would be helpful for me to try this locally.

Wasmtime GitHub notifications bot (Sep 08 2022 at 17:01):

abrown commented on issue #4881:

@conrad-watt, yeah, if you try out the "bytes across the boundary" approach that would be great; otherwise I can try this later in a day or two when I get back to this.

Wasmtime GitHub notifications bot (Sep 08 2022 at 23:49):

alexcrichton commented on issue #4881:

I was curious to dig into this a bit. I actually don't think that the problem is with ocaml-interop, instead the e_name function to get the name of an export from the ocaml AST is returning the name of the export as \u{0}[\u{0}. It looks like the spec interpreter is mangling the name of the export so we're trying to lookup the raw name in a list of mangled names which then fails to find the export. As a quick-and-dirty hack I hooked up the same name-mangling in Rust:

<details>

diff --git a/crates/fuzzing/wasm-spec-interpreter/src/with_library.rs b/crates/fuzzing/wasm-spec-interpreter/src/with_library.rs
index 15be5b5ab..b02f2d6e3 100644
--- a/crates/fuzzing/wasm-spec-interpreter/src/with_library.rs
+++ b/crates/fuzzing/wasm-spec-interpreter/src/with_library.rs
@@ -35,6 +35,7 @@
 use crate::{SpecExport, SpecInstance, SpecValue};
 use ocaml_interop::{BoxRoot, OCamlRuntime, ToOCaml};
 use once_cell::sync::Lazy;
+use std::fmt::Write;
 use std::sync::Mutex;

 static INTERPRET: Lazy<Mutex<()>> = Lazy::new(|| Mutex::new(()));
@@ -62,7 +63,7 @@ pub fn interpret(

     // Prepare the box-rooted parameters.
     let instance = instance.to_boxroot(ocaml_runtime);
-    let name = name.to_string().to_boxroot(ocaml_runtime);
+    let name = mangle_name(name).to_boxroot(ocaml_runtime);
     let parameters = parameters.to_boxroot(ocaml_runtime);

     // Interpret the function.
@@ -96,13 +97,29 @@ pub fn export(instance: &SpecInstance, name: &str) -> Result<SpecExport, String>

     // Prepare the box-rooted parameters.
     let instance = instance.to_boxroot(ocaml_runtime);
-    let name = name.to_string().to_boxroot(ocaml_runtime);
+    let name = mangle_name(name).to_string().to_boxroot(ocaml_runtime);

     // Export the value.
     let results = ocaml_bindings::export(ocaml_runtime, &instance, &name);
     results.to_rust(&ocaml_runtime)
 }

+fn mangle_name(name: &str) -> String {
+    let mut mangled = String::new();
+    for c in name.chars() {
+        let val = u32::from(c);
+        if val < 0x20 || val >= 0x7f {
+            write!(mangled, "\\u{{{val:02x}}}").unwrap();
+        } else {
+            if c == '"' || c == '\\' {
+                mangled.push('\\');
+            }
+            mangled.push(c);
+        }
+    }
+    mangled
+}
+
 // Here we declare which functions we will use from the OCaml library. See
 // https://docs.rs/ocaml-interop/0.8.4/ocaml_interop/index.html#example.
 mod ocaml_bindings {

</details>

and I got past that particular crashing test case. @conrad-watt do you know if this is intentional or perhaps an accident of using the wrong function to find the name of an export? If it's intentional then it might be best to do the name mangling in ocaml to avoid duplicating it in Rust as I did above (which was just the fastest route for me)

---

Otherwise though I applied this diff:

diff --git a/crates/fuzzing/src/oracles/diff_spec.rs b/crates/fuzzing/src/oracles/diff_spec.rs
index 753a38014..2fd285508 100644
--- a/crates/fuzzing/src/oracles/diff_spec.rs
+++ b/crates/fuzzing/src/oracles/diff_spec.rs
@@ -16,6 +16,8 @@ impl SpecInterpreter {

         config.min_memories = config.min_memories.min(1);
         config.max_memories = config.max_memories.min(1);
+        config.min_tables = config.min_tables.min(1);
+        config.max_tables = config.max_tables.min(1);

         config.memory64_enabled = false;
         config.threads_enabled = false;
@@ -43,9 +45,7 @@ impl DiffEngine for SpecInterpreter {
     }

     fn is_stack_overflow(&self, err: &Error) -> bool {
-        // TODO: implement this for the spec interpreter
-        drop(err);
-        false
+        err.to_string().contains("(Isabelle) call stack exhausted")
     }
 }

and I've got 400k executions with no errors so far. (the is_stack_overflow check is required for correctness since the spec interpreter can stack overflow at a different time than wasmtime stack overflows)

Wasmtime GitHub notifications bot (Sep 09 2022 at 02:31):

conrad-watt commented on issue #4881:

@alexcrichton ahh you're absolutely right! I was inadvertently mangling names in the spec interpreter. I've just pushed a new commit of the spec interpreter that _should_ fix this - If the spec interpreter commit is bumped to 763f960264fe62a0128de173b8e03cd187579596 does everything work without the need to mangle Rust-provided names?

Wasmtime GitHub notifications bot (Sep 12 2022 at 14:40):

alexcrichton commented on issue #4881:

Works well for me locally, thanks!

Wasmtime GitHub notifications bot (Sep 12 2022 at 15:36):

alexcrichton commented on issue #4881:

Oh actually I may have spoken too soon. I forgot about the fuzzer I left running and it hit this crash:

thread '<unnamed>' panicked at 'failed to instantiate only one side: Some(failed to instantiate in spec interpreter: Invalid_argument("Char.chr")) != None', fuzz/fuzz_targets/differential.rs:128:13

which I think means that the test case failed to get instantiated in the spec interpreter but succeeded in instantiation within Wasmtime. The input module was:

(module
  (type (;0;) (func))
  (func (;0;) (type 0)
    global.get 0
    i32.eqz
    if  ;; label = @1
      unreachable
    end
    global.get 0
    i32.const 1
    i32.sub
    global.set 0
  )
  (global (;0;) (mut i32) i32.const 1000)
  (export "+1\c8\8d\0d\02\00\00\00" (func 0))
)

where the name of the export in Rust-debug-print looks like "+1ȍ\r\u{2}\0\0\0"

Wasmtime GitHub notifications bot (Sep 12 2022 at 16:34):

conrad-watt commented on issue #4881:

It's great that the fuzzer is able to torture this corner of the implementation :)

Here's a new fix - commit c6bab4461e10229e557aae2e1027cadfce0161ce. I did some testing locally, and fingers crossed no further issues...

Wasmtime GitHub notifications bot (Sep 12 2022 at 17:13):

abrown commented on issue #4881:

Just to make sure I get this straight: all that's left to do is to 1) apply the second diff @alexcrichton provided (min_tables + is_stack_overflow) and 2) point the repository to the second commit @conrad-watt provided. Is that right?

Wasmtime GitHub notifications bot (Sep 12 2022 at 17:14):

abrown edited a comment on issue #4881:

Just to make sure I get this straight: all that's left to do is to 1) apply the second diff @alexcrichton provided (min_tables + is_stack_overflow) and 2) point the repository to the second commit @conrad-watt provided (c6bab44). Is that right?

Wasmtime GitHub notifications bot (Sep 12 2022 at 18:17):

alexcrichton commented on issue #4881:

I believe so, yeah, but so far my fixes have all been driven from the fuzzer so I'd run it for a bit locally as well to ensure nothing shows up

Wasmtime GitHub notifications bot (Sep 12 2022 at 20:36):

abrown commented on issue #4881:

Ok, I ran this target for a while with the latest two commits and it seems to be good to go:

$ ALLOWED_ENGINES=-v8 cargo +nightly fuzz run differential
...
#556973 NEW    cov: 73704 ft: 392590 corp: 12777/4426Kb lim: 743 exec/s: 90 rss: 1082Mb L: 626/743 MS: 1 CopyPart-
=== Execution rate (438172 successes / 557000 attempted modules): 78.67% ===
        wasmi: 22.82%, spec: 23.46%, wasmtime: 53.71%, v8: 0.00%
        wasm-smith: 75.49%, single-inst: 24.51%
#557027 REDUCE cov: 73704 ft: 392590 corp: 12777/4426Kb lim: 743 exec/s: 90 rss: 1082Mb L: 339/743 MS: 4 ChangeASCIIInt-CrossOver-CopyPart-EraseBytes-

Last updated: Nov 22 2024 at 16:03 UTC