1856733 - Wasm GC: global.get in initializer expression can access preceding global definitions

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, defect, P1)

Description

[jerome]

Steps to reproduce:

I have a Web assembly program that contains global definitions that refers to previous global definitions.

Actual results:

I get this error message in the console:
CompileError: wasm validation error: at offset 21410: global.get in initializer expression must reference a global immutable import

Expected results:

This should be allowed. (It works in Chrome 119.)
The GC proposal states the following:
"global.get is a constant instruction and can access preceding (immutable) global definitions, not just imports as in the MVP"
(https://github.com/WebAssembly/gc/blob/main/proposals/gc/MVP.md)

Comment 1

[Yury Delendik (:yury)]

The following wasm code doesn't trigger the validation error:

(module
  (global $g i32 (i32.const 1))
  (global $g2 i32 (global.get $g))
)

Can you provide the minimal example that fails the validation?

Comment 2

[Yury Delendik (:yury)]

I think that

(module
  (import "" "d" (global $g0 i32))
  (global $g i32 (i32.const 1))
  (global $g2 (mut i32) (global.get $g))
)

is the test case, but I want to make sure before changing the title

Comment 3

[jerome]

Here are two other different test cases, which do not involve mutable globals.

(module
 (import "xx" "d" (global $g0 i32))
 (global $int i32 (i32.const 251))
 (global $tbl2 (ref i31) (ref.i31
   (global.get $int)
 ))
)

(module
 (import "xx" "d" (global $g0 i32))
 (type $block (array (ref eq)))
 (global $len i32 (i32.const 256))
 (global $tbl (ref $block) (array.new $block
  (ref.i31
   (i32.const 0)
  )
  (global.get $len)
 ))
)

Comment 4

[Ryan Hunt [:rhunt]]

It looks like the arithmetic around valid globals for global.get is screwed up when there imported globals [1]. We should be adding the amount of imported globals to i.

[1] https://searchfox.org/mozilla-central/rev/1f5d04fed3631f97a84b589429419b83342d7c9a/js/src/wasm/WasmValidate.cpp#2383

Comment 5

[Yury Delendik (:yury)]

Attachment: Bug 1856733 - [wasm] Include imported globals in init_expr validation. r?rhunt

Change validation logic and cleanup around DecodeConstantExpression.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b5be2e34b80d
[wasm] Include imported globals in init_expr validation. r=rhunt

Comment 7

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/b5be2e34b80d