Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inline element segment abbreviations omit necessary reftype #1656

Closed
tomstuart opened this issue May 27, 2023 · 1 comment · Fixed by #1657
Closed

Inline element segment abbreviations omit necessary reftype #1656

tomstuart opened this issue May 27, 2023 · 1 comment · Fixed by #1657

Comments

@tomstuart
Copy link
Contributor

Both cases of a table definition’s inline element segment abbreviation produce syntactically invalid element lists.

Case 1: element list contains element expressions

Problem

e.g. (table $t funcref (elem (ref.func $f) (ref.null func) (ref.func $g))) (from elem.wast) is allowed, but the inline element segment abbreviation would rewrite it into (table $t 3 3 funcref) (elem (table $t) (i32.const 0) (ref.func $f) (ref.null func) (ref.func $g)), which is a syntax error. The rewritten element list is missing a reftype.

Solution

Update the expanded syntax to prefix the element list with the reftype from the abbreviation syntax. The rewritten result for the above example is then (table $t 3 3 funcref) (elem (table $t) (i32.const 0) funcref (ref.func $f) (ref.null func) (ref.func $g)).

Case 2: element list contains function indices

Problem

e.g. (table $t1 funcref (elem $f $g)) (from call_indirect.wast) is allowed, but the inline element segment abbreviation would rewrite it into (table $t1 2 2 funcref) (elem (table $t) (i32.const 0) $f $g), which is a syntax error.

The rewritten element list is missing the ‘func’ keyword; when an element list is written as a sequence of function indices, it must generally be preceded by ‘func’.

Element segment syntax may allow ‘func’ to be omitted (“for backwards compatibility with earlier versions of WebAssembly”) only if the table use is also omitted, but the table use is present in this case.

(A minor point is that in this case it also doesn’t make sense for the abbreviation syntax to use the reftype production. While an element list consisting of a vector of element expressions can use either ‘funcref’ or ‘externref’ as its reference type, an element list consisting of a vector of function indices always uses ‘funcref, so ‘externref’ can never occur here.)

Solution

Update the abbreviation syntax to only allow the ‘funcref’ keyword as the reference type instead of the reftype production. Update the expanded syntax to prefix the element list with the ‘func’ keyword. The rewritten result for the above example is then (table $t1 2 2 funcref) (elem (table $t) (i32.const 0) func $f $g).

@rossberg
Copy link
Member

Ah, yes, likely a leftover from Wasm 1.0, when the kernel syntax for element segments was different.

tomstuart added a commit to tomstuart/wasminna that referenced this issue Jun 17, 2023
The expanded syntax for inline element segments [0] duplicates the
reftype in the resulting element list [1], but this should be replaced
by the `func` keyword when the element list contains function indexes so
that the function indexes abbreviation can be expanded correctly. We
distinguish that case by checking whether the first item is a list (i.e.
an element expression).

[0] https://webassembly.github.io/spec/core/text/modules.html#text-table-abbrev
[1] WebAssembly/spec#1656
tomstuart added a commit to tomstuart/wasminna that referenced this issue Jun 17, 2023
)

A table definition can include an inline element segment [0].
This commit implements desugaring for that abbreviation in the preprocessor
and removes support for it from the AST parser, AST nodes and
interpreter.

This work required reporting [1] and fixing [2] an issue in the
WebAssembly language specification to correct the expanded syntax of
element lists.

[0] https://webassembly.github.io/spec/core/text/modules.html#text-table-abbrev
[1] WebAssembly/spec#1656
[2] WebAssembly/spec#1657
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants