Skip to content

Commit c91c052

Browse files
authored
Fix table.init text format parsing (#7654)
`table.init` text format has two forms: - `table.init tableidx elemidx` - `table.init elemidx` (`tableidx` is implicitly 0) Because the optional `tableidx` is not the last (second) argument, we have to parse one or two immediate arguments, and depending on how many we parsed decide which one is `elemidx`. Currently the code assumes first immediate argument is always a table, which does not handle the second form. Before this PR, `wasm-shell` fails to parse this functions in spec tests: https://github.com/WebAssembly/testsuite/blob/e05365077e13a1d86ffe77acfb1a835b7aa78422/bulk.wast#L207-L211 After the PR we're able to parse it, but we still can't parse the whole file, because of other issues. Error when parsing `bulk.wast`, before this PR: ``` 209:6: error: expected elem index or identifier ``` After this PR: ``` 250:33: error: unrecognized instruction ``` (The new error is about `elem.drop`, which we don't support yet, see #7209)
1 parent c2b7a04 commit c91c052

File tree

2 files changed

+38
-6
lines changed

2 files changed

+38
-6
lines changed

scripts/test/shared.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -416,12 +416,12 @@ def get_tests(test_dir, extensions=[], recursive=False):
416416
'address.wast', # 64-bit offset allowed by memory64
417417
'align.wast', # Alignment bit 6 used by multi-memory
418418
'binary.wast', # memory.grow reserved byte a LEB in multi-memory
419-
'bulk.wast', # Requires table.init abbreviation with implicit table
419+
'bulk.wast', # Requires support for elem.drop
420420
'comments.wast', # Issue with carriage returns being treated as newlines
421421
'const.wast', # Hex float constant not recognized as out of range
422422
'conversions.wast', # Promoted NaN should be canonical
423423
'data.wast', # Constant global references allowed by GC
424-
'elem.wast', # Requires table.init abbreviation with implicit table
424+
'elem.wast', # Requires modeling empty declarative segments
425425
'f32.wast', # Adding -0 and -nan should give a canonical NaN
426426
'f64.wast', # Adding -0 and -nan should give a canonical NaN
427427
'float_exprs.wast', # Adding 0 and NaN should give canonical NaN

src/parser/parsers.h

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2130,10 +2130,33 @@ makeTableCopy(Ctx& ctx, Index pos, const std::vector<Annotation>& annotations) {
21302130
template<typename Ctx>
21312131
Result<>
21322132
makeTableInit(Ctx& ctx, Index pos, const std::vector<Annotation>& annotations) {
2133+
// Note: binary and text formats for `table.init` are different. In both
2134+
// formats the table index is optional (with 0 as the default). When both the
2135+
// table and elem index are specified, the elem index comes first in the
2136+
// binary format, but second in the text format.
2137+
2138+
auto reset = ctx.in.getPos();
2139+
2140+
auto retry = [&]() -> Result<> {
2141+
// We're unable to parse the two argument format. Try one argument format
2142+
// with just elem index.
2143+
WithPosition with(ctx, reset);
2144+
auto elem = elemidx(ctx);
2145+
CHECK_ERR(elem);
2146+
MaybeResult<typename Ctx::TableIdxT> table = ctx.getTableFromIdx(0);
2147+
return ctx.makeTableInit(pos, annotations, table.getPtr(), *elem);
2148+
};
2149+
21332150
auto table = maybeTableidx(ctx);
2134-
CHECK_ERR(table);
2135-
auto elem = elemidx(ctx);
2136-
CHECK_ERR(elem);
2151+
if (table.getErr()) {
2152+
return retry();
2153+
}
2154+
2155+
auto elem = maybeElemidx(ctx);
2156+
if (elem.getErr() || !elem) {
2157+
return retry();
2158+
}
2159+
21372160
return ctx.makeTableInit(pos, annotations, table.getPtr(), *elem);
21382161
}
21392162

@@ -2850,13 +2873,22 @@ template<typename Ctx> Result<typename Ctx::GlobalIdxT> globalidx(Ctx& ctx) {
28502873

28512874
// elemidx ::= x:u32 => x
28522875
// | v:id => x (if elems[x] = v)
2853-
template<typename Ctx> Result<typename Ctx::ElemIdxT> elemidx(Ctx& ctx) {
2876+
template<typename Ctx>
2877+
MaybeResult<typename Ctx::ElemIdxT> maybeElemidx(Ctx& ctx) {
28542878
if (auto x = ctx.in.takeU32()) {
28552879
return ctx.getElemFromIdx(*x);
28562880
}
28572881
if (auto id = ctx.in.takeID()) {
28582882
return ctx.getElemFromName(*id);
28592883
}
2884+
return {};
2885+
}
2886+
2887+
template<typename Ctx> Result<typename Ctx::ElemIdxT> elemidx(Ctx& ctx) {
2888+
if (auto idx = maybeElemidx(ctx)) {
2889+
CHECK_ERR(idx);
2890+
return *idx;
2891+
}
28602892
return ctx.in.err("expected elem index or identifier");
28612893
}
28622894

0 commit comments

Comments
 (0)