WebAssembly / gc

Branch of the spec repo scoped to discussion of GC integration in WebAssembly
https://webassembly.github.io/gc/
Other
997 stars 72 forks source link

Fix ref.null semantics #478

Closed ShinWonho closed 11 months ago

ShinWonho commented 11 months ago

Problem

Screenshot 2023-11-10 at 5 47 05 PM

This is the semantics of ref.null ht in the current specification. But I think simply pushing ref.null ht is problematic.

Example

Consider the following example, which is simplified version of "test-sub" in ref_test.wast.

(module
  (type $t0 (sub (struct)))

  (func (export "test-sub")
    ;; must hold
    (ref.test (ref null $t0) (ref.null $t0))
    (drop)))

(assert_return (invoke "test-sub"))
Screenshot 2023-11-10 at 6 08 14 PM

According to the semantics of ref.test, we first need to get the reference type of ref.null $t0.

Screenshot 2023-11-10 at 6 10 50 PM

However, the typeindex $t0 is not ok under empty context.

Suggested change

In the reference interpreter, type index of ref.null $t0 is substituted before pushed.

let rec step (c : config) : config =

      ....

      | RefNull t, vs' ->
        Ref (NullRef (subst_heap_type (subst_of c.frame.inst) t)) :: vs', []

Therefore, I fixed the semantics according to the reference interpreter.

Screenshot 2023-11-10 at 5 46 24 PM
zapashcanon commented 11 months ago

Because (ref.null ht) is defined to be a value, and values must never reduce, otherwise the Progress statement is compromised.

Wouldn't having null.new $t to be an instruction and ref.null $t a value fix the issue ?

rossberg commented 11 months ago

See my reply on the other issue. Also, ref.null is already standardised as part of Wasm 2.0 (as is ref.func, btw), so we cannot change it anymore.

ShinWonho commented 11 months ago
Screenshot 2023-11-13 at 5 33 26 PM Screenshot 2023-11-13 at 5 33 56 PM

I changed the syntax of ref.null value so that ref.null with type index cannot be a value. Is this right fix?