Vacuum produces invalid get on non-nullable local

[tlively]

;; a.wast
(module
 (type $array (array (mut i32)))
 (func $0 (param $x (ref $array)) (result i32)
  (local $0 (ref $array))
  (block $label$1 (result i32)
   (block $label$2
    (drop
     (i32.add
      (array.len
       (local.tee $0
        (local.get $x)
       )
      )
      (br_if $label$1
       (i32.const 0)
       (i32.const 0)
      )
     )
    )
    (if
     (array.len
      (local.get $0)
     )
     (nop)
    )
    (unreachable)
   )
   (unreachable)
  )
 )
)

wasm-opt -all a.wast -o b.wasm --vacuum produces an invalid b.wast.

[kripken]

In what sense is it invalid? What error do you get? I get this:

(module
 (type $array (array (mut i32)))
 (type $ref|$array|_=>_i32 (func (param (ref $array)) (result i32)))
 (func $0 (type $ref|$array|_=>_i32) (param $x (ref $array)) (result i32)
  (local $0 (ref $array))
  (block $label$1 (result i32)
   (block $label$2
    (block
     (drop
      (array.len
       (local.tee $0
        (local.get $x)
       )
      )
     )
     (drop
      (br_if $label$1
       (i32.const 0)
       (i32.const 0)
      )
     )
    )
    (drop
     (array.len
      (local.get $0)
     )
    )
    (unreachable)
   )
  )
 )
)

That looks ok (the tee is a little nested, but in an unnamed block which is not emitted in the binary format, so we ignore it). And Binaryen reports no error.

[tlively]

V8 thinks it has a non-nullability validation error even though Binaryen thinks it's ok.

[kripken]

Looking into this, the issue is that indeed that third inner block is a problem for validation. It does not need to be emitted, and in optimized builds we do remove it: adding --generate-stack-ir --optimize-stack-ir is enough to avoid this problem. But in an unoptimized build, or one with -O1 or such, we don't do stack IR opts.

This seems a low priority to fix as it only affects unoptimized builds, but we should still fix it. I guess we could add logic to the binary writer for this - to avoid emitting blocks without a name - but that does feel a little unfortunate.

[kripken]

Actually -O1 here also works since it does MergeBlocks which removes the block entirely. So this is really just a problem for quite unoptimized code.

[tlively]

I just ran into this (or something very similar to it) again after almost 600k fuzzer iterations.

[tlively]

Just ran into this again after almost 90k iterations.