JS interop fuzz bug with GTO and externref

[kripken]

$ bin/wasm-opt -all --closed-world --gto t.wat --fuzz-exec
[fuzz-exec] export func_invoker
[trap unreachable]
[fuzz-exec] export func_31
[fuzz-exec] note result: func_31 => object(null)
[fuzz-exec] export func_invoker
[trap unreachable]
[fuzz-exec] export func_31
[fuzz-exec] note result: func_31 => object(externref(105))
[fuzz-exec] comparing func_31
values not identical! [ref (type $struct.0 (descriptor $struct.1) (struct)) ()desc=[ref (type $struct.0 (sub (describes $struct.1) (struct (field externref)))) externref(105)]] != [ref (type $struct.0 (descriptor $struct.1) (struct (field (mut (ref null (shared func)))) (field v128) (field i8) (field (mut (ref $func.0))) (field (mut (ref struct))))) (shared funcref(1), i32x4 0x00000000 0x00000000 0x00000000 0x00000000, 0, shared funcref(1), [ref (type $struct.0 (sub (describes $struct.1) (struct (field (mut externref)) (field (mut (ref $func.0))) (field v128) (field f64) (field v128) (field (mut exnref))))) (externref(105), shared funcref(1), i32x4 0x00000000 0x00000000 0x00000000 0x00000000, 0, i32x4 0x00000000 0x00000000 0x00000000 0x00000000, exnref)]), desc=[ref (type $struct.0 (sub (describes $struct.1) (struct (field (mut externref)) (field (mut (ref $func.0))) (field v128) (field f64) (field v128) (field (mut exnref))))) (externref(105), shared funcref(1), i32x4 0x00000000 0x00000000 0x00000000 0x00000000, 0, i32x4 0x00000000 0x00000000 0x00000000 0x00000000, nullexnref)]]
[fuzz-exec] optimization passes changed results

(module
 (rec
  (type $0 (descriptor $2) (struct (field (mut (ref null (shared func)))) (field v128) (field i8) (field (mut (ref $1))) (field (mut (ref struct)))))
  (type $5 (func (result f32 f64 f64 v128 (ref null $3) (ref $2))))
  (type $3 (shared (struct (field i16) (field (mut v128)) (field (mut (ref null (shared none)))))))
  (type $2 (sub (describes $0) (struct (field (mut externref)) (field (mut (ref $1))) (field v128) (field f64) (field v128) (field (mut exnref)))))
  (type $15 (func (param (ref null $2)) (result (ref $0))))
  (type $4 (sub (func (param i64) (result f32 f64 f64 v128 (ref null $3) (ref $2)))))
  (type $1 (shared (func (result f64 v128))))
  (type $11 (func (param f32)))
 )
 (type $9 (func))
 (type $17 (func (result structref)))
 (import "__fuzz_import" "extern$" (global $gimport$0 (ref extern)))
 (table $0 i64 55 funcref (ref.null nofunc))
 (elem $0 (i64.const 0) $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $5 $11)
 (elem declare func $1)
 (tag $tag$0 (type $11) (param f32))
 (tag $tag$1 (type $9))
 (export "func_invoker" (func $4))
 (export "func_31" (func $9))
 (func $1 (type $1) (result f64 v128)
  (tuple.make 2
   (f64.const 0)
   (v128.const i32x4 0x00000000 0x00000000 0x00000000 0x00000000)
  )
 )
 (@binaryen.js.called)
 (func $2 (type $4) (param $0 i64) (result f32 f64 f64 v128 (ref null $3) (ref $2))
  (local $2 (ref null $4))
  (drop
   (try (result v128)
    (do
     (v128.const i32x4 0x00000000 0x00000000 0x00000000 0x00000000)
    )
    (catch $tag$0
     (drop
      (pop f32)
     )
     (block (result v128)
      (table.set $0
       (i64.const 0)
       (ref.as_non_null
        (local.get $2)
       )
      )
      (v128.const i32x4 0x00000000 0x00000000 0x00000000 0x00000000)
     )
    )
    (catch_all
     (unreachable)
    )
   )
  )
  (unreachable)
 )
 (func $4 (type $9)
  (drop
   (block (result f32)
    (drop
     (block (result f32)
      (drop
       (tuple.extract 6 0
        (call $2
         (i64.const 0)
        )
       )
      )
      (f32.const 0)
     )
    )
    (f32.const 0)
   )
  )
 )
 (@binaryen.js.called)
 (func $5 (type $15) (param $0 (ref null $2)) (result (ref $0))
  (unreachable)
 )
 (func $9 (type $17) (result structref)
  (local $0 exnref)
  (struct.new_desc $0
   (ref.func $1)
   (v128.const i32x4 0x00000000 0x00000000 0x00000000 0x00000000)
   (i32.const 0)
   (ref.func $1)
   (struct.new $2
    (global.get $gimport$0)
    (ref.func $1)
    (v128.const i32x4 0x00000000 0x00000000 0x00000000 0x00000000)
    (f64.const 0)
    (v128.const i32x4 0x00000000 0x00000000 0x00000000 0x00000000)
    (block $block (result (ref exn))
     (try_table (catch_all_ref $block)
      (throw $tag$1)
     )
    )
   )
   (struct.new $2
    (global.get $gimport$0)
    (ref.func $1)
    (v128.const i32x4 0x00000000 0x00000000 0x00000000 0x00000000)
    (f64.const 0)
    (v128.const i32x4 0x00000000 0x00000000 0x00000000 0x00000000)
    (local.get $0)
   )
  )
 )
 (@binaryen.js.called)
 (func $11 (type $4) (param $0 i64) (result f32 f64 f64 v128 (ref null $3) (ref $2))
  (local $1 stringref)
  (drop
   (struct.get $2 0
    (struct.new $2
     (local.get $1)
     (ref.func $1)
     (v128.const i32x4 0x00000000 0x00000000 0x00000000 0x00000000)
     (f64.const 0)
     (v128.const i32x4 0x00000000 0x00000000 0x00000000 0x00000000)
     (ref.null noexn)
    )
   )
  )
  (unreachable)
 )
)

[tlively]

Ah, this is unfortunate. Here's a smaller reproducer:

(module
 (rec
  (type $struct (descriptor $desc) (struct))
  (type $desc (describes $struct) (struct (field (mut externref))))
 )
 (func (export "test") (result structref)
  (struct.new_desc $struct
   (struct.new $desc
    (extern.convert_any
     (struct.new_default $desc)
    )
   )
  )
 )
 (func (param $d (ref $desc))
  (drop
   (struct.get $desc 0
    (local.get $d)
   )
  )
 )
)

The problem is that GTO sees that the mutable externref field has no writes, so makes it immutable. But then once it is immutable, JSUtils::hasPossibleJSPrototypeField starts returning true, which changes how the struct is printed by execution-results.h.

The only easy fixes are to artificially restrict GTO from optimizing in ways that make the type look like it might have a prototype where it didn't before, or to stop printing prototype values in the interpreter, which would reduce its bug finding power.

The only other fix I can think of would be to cache a set of heap types known to have or not have JS prototypes on the Module. This would be yet another map to update whenever we rewrite types. Then the interpreter could skip printing prototype values for objects that look like they should have them if the original types did not have configurable prototypes. Then we would probably need an annotation to let us serialize this information, etc. etc.

[kripken]

Can we perhaps see in fuzz-exec that the field was not a prototype before, but became one? I.e. hasPossibleJSPrototypeField flips. We could then ignore that case directly?

[tlively]

Yeah, possibly. I'll look into it.

[tlively]

Or, on second thought, maybe there really is a bug with GTO? If a struct has a non-prototype externref field (because it is mutable or not the first field), should we consider it a bug if that field suddenly starts exposing a prototype? It seems like we should.

[kripken]

I admit I don't fully know what "exposing a prototype" means here. Does the JS engine treat such fields semantically different? I mean, does turning this field from mutable to immutable cause the JS engine to do anything other than know it is immutable?

[tlively]

Yes, if field 0 becomes an immutable externref referencing a JS object, that object will be the struct's JS prototype, so e.g. Object.getPrototypeOf(wasmStruct) will start returning that JS object and JS property accesses will start going to that JS object as well.

[kripken]

Hmm, should this perhaps be more explicit in the spec? Some marker "this is a prototype"?

[tlively]

We had a mechanism to make it more explicit, but then we got rid of it because it was extra complexity and overhead :) WebAssembly/custom-descriptors#53