wasmtime / issue #4671 fuzz: different results for `shr_s` · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / issue #4671 fuzz: different results for `shr_s`

Wasmtime GitHub notifications bot (Aug 10 2022 at 12:26):

Test Case
(module
  (type (;0;) (func (param i32 i32) (result i32)))
  (func (;0;) (type 0) (param i32 i32) (result i32)
    local.get 0
    local.get 1
    i32.shr_s
  )
  (export "test" (func 0))
)
Also see attached files (annoyingly renamed with .txt appended due to GitHub upload restrictions):
- crash-3be2c01861adcd71b08427e6ad1251de6fb3159b.txt
- testcase169.wat.txt
- testcase169.wasm.txt

Steps to Reproduce

On the abrown:meta-diff branch:
$ RUST_LOG=wasmtime_fuzzing=debug cargo +nightly fuzz run differential-new fuzz/artifacts/differential-new/crash-3be2c01861adcd71b08427e6ad1251de6fb3159b
Expected Results

Execution to match for both the Wasmtime and wasm-spec-interpreter run.

Actual Results

The results of the shift do not match:
[2022-08-10T12:14:39Z DEBUG wasmtime_fuzzing::oracles] Evaluating: test([I32(1795123818), I32(-2147483648)])
[2022-08-10T12:14:39Z DEBUG wasmtime_fuzzing::oracles]  -> results on spec: [I32(-2097152)]
[2022-08-10T12:14:39Z DEBUG wasmtime_fuzzing::oracles]  -> results on wasmtime: [I32(1795123818)]
Versions and Environment

Wasmtime version or commit: abrown:meta-diff branch

Operating system: Fedora 35

Architecture: x86-64

Other

I am reporting this to clean up any fuzz bugs found before trying to merge #4515. In talking to @alexcrichton, the first reaction seemed to be that this is a bug in the spec interpreter OCaml bindings (after all, Wasmtime passes all spec tests for this kind of simple operation as does the spec interpreter, I assume). @conrad-watt, any thoughts on this?

Wasmtime GitHub notifications bot (Aug 10 2022 at 13:08):

conrad-watt commented on issue #4671:

This is the result that would be obtained if the arguments were provided in reverse order. I think the solution is to change the following line of interpret.ml:

line 58:
let opt_params_ = Option.map (List.map convert_to_wasm) opt_params in

to
58:
let opt_params_ = Option.map (List.rev_map convert_to_wasm) opt_params in

This matches the existing use of List.rev_map in processing the execution result (line 64). The Some path was never exercised before, so sorry for missing this!

Wasmtime GitHub notifications bot (Aug 10 2022 at 13:08):

conrad-watt edited a comment on issue #4671:

This is the result that would be obtained if the arguments were provided in reverse order. I think the solution is to change the following line of interpret.ml:

line 58:
let opt_params_ = Option.map (List.map convert_to_wasm) opt_params in

to
line 58:
let opt_params_ = Option.map (List.rev_map convert_to_wasm) opt_params in

This matches the existing use of List.rev_map in processing the execution result (line 64). The Some path was never exercised before, so sorry for missing this!

Wasmtime GitHub notifications bot (Aug 10 2022 at 13:20):

conrad-watt edited a comment on issue #4671:

This is the result that would be obtained if the arguments were provided in reverse order. I think the solution is to change the following line of interpret.ml:

line 58:
let opt_params_ = Option.map (List.map convert_to_wasm) opt_params in

to
line 58:
let opt_params_ = Option.map (List.rev_map convert_to_wasm) opt_params in

This matches the existing use of List.rev_map in processing the execution result (line 64). The Some path was never exercised before, so sorry for missing this!

EDIT: it looks like this path _was_ exercised in the unit tests, but hilariously none of the current unit tests care about order of arguments!

Wasmtime GitHub notifications bot (Aug 10 2022 at 13:22):

conrad-watt edited a comment on issue #4671:

This is the result that would be obtained if the arguments were provided in reverse order. I think the solution is to change the following line of interpret.ml:

line 58:
let opt_params_ = Option.map (List.map convert_to_wasm) opt_params in

to

line 58:
let opt_params_ = Option.map (List.rev_map convert_to_wasm) opt_params in

This matches the existing use of List.rev_map in processing the execution result (line 64). The Some path was never exercised before, so sorry for missing this!

EDIT: it looks like this path _was_ exercised in the unit tests, but hilariously none of the current unit tests care about order of arguments!

Wasmtime GitHub notifications bot (Aug 10 2022 at 14:55):

alexcrichton closed issue #4671:

Test Case
(module
  (type (;0;) (func (param i32 i32) (result i32)))
  (func (;0;) (type 0) (param i32 i32) (result i32)
    local.get 0
    local.get 1
    i32.shr_s
  )
  (export "test" (func 0))
)
Also see attached files (annoyingly renamed with .txt appended due to GitHub upload restrictions):
- crash-3be2c01861adcd71b08427e6ad1251de6fb3159b.txt
- testcase169.wat.txt
- testcase169.wasm.txt

Steps to Reproduce

On the abrown:meta-diff branch:
$ RUST_LOG=wasmtime_fuzzing=debug cargo +nightly fuzz run differential-new fuzz/artifacts/differential-new/crash-3be2c01861adcd71b08427e6ad1251de6fb3159b
Expected Results

Execution to match for both the Wasmtime and wasm-spec-interpreter run.

Actual Results

The results of the shift do not match:
[2022-08-10T12:14:39Z DEBUG wasmtime_fuzzing::oracles] Evaluating: test([I32(1795123818), I32(-2147483648)])
[2022-08-10T12:14:39Z DEBUG wasmtime_fuzzing::oracles]  -> results on spec: [I32(-2097152)]
[2022-08-10T12:14:39Z DEBUG wasmtime_fuzzing::oracles]  -> results on wasmtime: [I32(1795123818)]
Versions and Environment

Wasmtime version or commit: abrown:meta-diff branch

Operating system: Fedora 35

Architecture: x86-64

Other

I am reporting this to clean up any fuzz bugs found before trying to merge #4515. In talking to @alexcrichton, the first reaction seemed to be that this is a bug in the spec interpreter OCaml bindings (after all, Wasmtime passes all spec tests for this kind of simple operation as does the spec interpreter, I assume). @conrad-watt, any thoughts on this?

Last updated: Nov 22 2024 at 17:03 UTC