[BUG] Compiling function failed with type error in range pattern case

[rickyvetter]

Describe the bug
I've been able to reproduce the following error:

Uncaught (in promise) CompileError: WebAssembly.instantiateStreaming(): Compiling function #366 failed: type error in fallthru[0] (expected (ref i31), got (ref eq)) @+118976

With a minimal set of code that looks like:

let fn_const r _ = r

let int_of_hex c =
  match c with
  | '0' .. '9' -> 1
  | 'a' -> 0
  | _ -> failwith "nope"
;;

let int64_of_hex c = Stdlib.Int64.of_int (int_of_hex c)
let combine = fn_const (int64_of_hex 'c') (int64_of_hex 'c')
let () = print_endline (Stdlib.Int64.to_string combine)

Expected behavior
I'd expect this code to compile and run (with errors)

Versions
This fails when passing --opt 3. If you don't pass an opt flag, the code and wasm both compile. We use a slightly different build of wasm_of_ocaml, so for clarity --opt 3 passes -O3 --skip-pass=inlining-optimizing --traps-never-happen to binaryen

[vouillon]

Could you try with the development version of Binaryen, or this PR: WebAssembly/binaryen#6510?
I suspect wasm-opt is generating some invalid code here, and the fix has not yet been released.

[rickyvetter]

Rebuilt with WebAssembly/binaryen@0c23394 and it does not resolve the error.

[vouillon]

I was not able to reproduce it, even with binaryen version 117.

Which version of node (or Chrome) are you using?

[rickyvetter]

I repro in Chrome 125.0.6422.141 (Official Build) (64-bit) on linux. I don't repro in node v22.0.0-v8-canary2024030314ed92e804

[vouillon]

The validation algorithm was tightened in V8 on March 5 to follow more closely the spec. This explains why you can repro with Chrome 125 but not with this version of node canary.

I thought that was fixed in the development version of Binaryen, but maybe they missed some cases.

Maybe you can share the .wat and .wasm files so that I can have a look?

[rickyvetter]

Oh - I realized that this actually isn't the full repo. I forgot I had some ppx's turned on. Here's a full repro with all ppx enabled (just one extra very special line at the top). Again - this validates with no opt but fails to validate with the optimization level listed above.

let _foo = ref (Stdlib.Gc.quick_stat ())
let fn_const r _ = r

let int_of_hex c =
  match c with
  | '0' .. '9' -> 1
  | 'a' -> 0
  | _ -> failwith "nope"
;;

let int64_of_hex c = Stdlib.Int64.of_int (int_of_hex c)
let combine = fn_const (int64_of_hex 'c') (int64_of_hex 'c')
let () = print_endline (Stdlib.Int64.to_string combine)

[vouillon]

I'm still not able to reproduce the issue.

[rickyvetter]

Interesting. I sent you the wasm and wat files directly!

[vouillon]

I can reproduce the issue with binaryen 117 with the following piece of code:

let fn_const r _ = r

let int_of_hex c =
  match c with
  | '0' .. '9' -> 1
  | 'a' -> 0
  | _ -> failwith "nope"
;;

let int64_of_hex c = Stdlib.Int64.of_int (int_of_hex c)
let combine = fn_const (int64_of_hex 'c') (int64_of_hex 'c')
let () = print_endline (Stdlib.Int64.to_string combine)

let () = failwith "foo"

(Function failwith should not get inlined by wasm_of_ocaml.)

However, the code generated by WebAssembly/binaryen@0c23394 is valid.

[rickyvetter]

Ahh yes - it does with the flags you're passing! In -O3 we are also passing --skip-pass=inlining-optimizing because if we don't we get pretty major regressions on startup times on many of our apps apps. If you pass this flag then even WebAssembly/binaryen@0c23394 output fails to validate.

Edit: I just tested the current tip of the main branch (WebAssembly/binaryen@0fa99fb) as well and this is still failing there. Given the custom opt args, it seems like it might make more sense for there to be an issue/tracking on the binaryen side, but I'm not sure how to structure that issue outside of the context of this wasm application.

I'm looking into re-enabling inline-optimizing but I think it's going to be pretty horrible for us on the performance side. Much worse than JavaScript startup.

[vouillon]

I noticed that --skip-pass=inlining-optimizing as an impact on startup time as well. I need to put it back.

[vouillon]

But even with this flag, I'm not able to reproduce the issue. Could you double check you are using the right binaryen binary?

Here is what I get when I compile the .wat file you sent me. I'm using the following command.

wasm-opt -g -all -O3 --skip-pass=inlining-optimizing --traps-never-happen  validation-error.bc.wasm.wat  -o - | wasm-dis -

The issue is that the result type of (br_if $label$1 ...) is (ref eq) (the type of the block labelled $label$1) , even though the returned expression is (ref.i31 (i32.const 0)), while the conditional has the more precise result type (ref i31). The development version of binaryen inserts a ref.cast (ref i31) to recover this more precise type.

 (func $int64_of_hex (param $0 (ref i31)) (result (ref $int64))
  (struct.new $int64
   (global.get $int64_ops)
   (i64.extend_i32_s
    (i31.get_s
     (ref.cast (ref i31)
      (block $label$1 (result (ref eq))
       (drop
        (if (result (ref i31))
         (i32.ge_s
          (i31.get_s
           (local.get $0)
          )
          (i32.const 58)
         )
         (then
          (ref.cast (ref i31)
           (br_if $label$1
            (ref.i31
             (i32.const 0)
            )
            (ref.eq
             (ref.i31
              (i32.const 97)
             )
             (local.get $0)
            )
           )
          )
         )
         [...]

[rickyvetter]

Ahh yes. You're right again. There was an issue with the way we were importing binaryen and it wasn't actually using the commit hash the way I thought. I've been building with an earlier version of binaryen this whole time!

Thanks for your patience - this problem looks resolved as of WebAssembly/binaryen@0c23394.