Skip to content

Commit 64aa81e

Browse files
kripkenkripken
authored
Unreachable typing fixes (#1004)
* fix type of drop, set_local, set_global, load, etc: when operand is unreachable, so is the node itself * support binary tests properly in test/passes * fix unreachable typing of blocks with no name and an unreachable child * fix continue emitting in asm2wasm * properly handle emitting of unreachable load
1 parent b856925 commit 64aa81e

16 files changed

+403
-41
lines changed

‎auto_update_tests.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,10 @@
8484
with open(os.path.join('test', 'print', wasm + '.minified.txt'), 'w') as o: o.write(actual)
8585

8686
for t in sorted(os.listdir(os.path.join('test', 'passes'))):
87-
if t.endswith('.wast'):
87+
if t.endswith(('.wast', '.wasm')):
8888
print '..', t
89-
passname = os.path.basename(t).replace('.wast', '')
89+
binary = '.wasm' in t
90+
passname = os.path.basename(t).replace('.wast', '').replace('.wasm', '')
9091
opts = ['-' + passname] if passname.startswith('O') else ['--' + p for p in passname.split('_')]
9192
t = os.path.join('test', 'passes', t)
9293
actual = ''
@@ -95,7 +96,7 @@
9596
with open('split.wast', 'w') as o: o.write(module)
9697
cmd = WASM_OPT + opts + ['split.wast', '--print']
9798
actual += run_command(cmd)
98-
with open(os.path.join('test', 'passes', passname + '.txt'), 'w') as o: o.write(actual)
99+
with open(os.path.join('test', 'passes', passname + ('.bin' if binary else '') + '.txt'), 'w') as o: o.write(actual)
99100

100101
print '\n[ checking wasm-opt -o notation... ]\n'
101102

‎check.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,10 @@
7474
print '\n[ checking wasm-opt passes... ]\n'
7575

7676
for t in sorted(os.listdir(os.path.join(options.binaryen_test, 'passes'))):
77-
if t.endswith('.wast'):
77+
if t.endswith(('.wast', '.wasm')):
7878
print '..', t
79-
passname = os.path.basename(t).replace('.wast', '')
79+
binary = '.wasm' in t
80+
passname = os.path.basename(t).replace('.wast', '').replace('.wasm', '')
8081
opts = ['-' + passname] if passname.startswith('O') else ['--' + p for p in passname.split('_')]
8182
t = os.path.join(options.binaryen_test, 'passes', t)
8283
actual = ''
@@ -88,7 +89,7 @@
8889
# also check debug mode output is valid
8990
debugged = run_command(cmd + ['--debug'], stderr=subprocess.PIPE)
9091
fail_if_not_contained(actual, debugged)
91-
fail_if_not_identical(actual, open(os.path.join(options.binaryen_test, 'passes', passname + '.txt'), 'rb').read())
92+
fail_if_not_identical(actual, open(os.path.join('test', 'passes', passname + ('.bin' if binary else '') + '.txt'), 'rb').read())
9293

9394
print '[ checking asm2wasm testcases... ]\n'
9495

‎scripts/test/support.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,11 @@ def split_wast(wast):
9494
# this splits out a wast into [(module, assertions), ..]
9595
# we ignore module invalidity tests here.
9696
wast = open(wast).read()
97+
98+
# if it's a binary, leave it as is
99+
if wast[0] == '\0':
100+
return [[wast, '']]
101+
97102
ret = []
98103

99104
def to_end(j):

‎src/asm2wasm.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2296,6 +2296,7 @@ Function* Asm2WasmBuilder::processFunction(Ref ast) {
22962296
Break *continuer = allocator.alloc<Break>();
22972297
continuer->name = in;
22982298
continuer->condition = process(ast[1]);
2299+
continuer->finalize();
22992300
Block *block = builder.blockifyWithName(loop->body, out, continuer);
23002301
loop->body = block;
23012302
loop->finalize();

‎src/wasm-builder.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ class Builder {
288288
if (!block) block = makeBlock(any);
289289
if (append) {
290290
block->list.push_back(append);
291-
block->finalize(); // TODO: move out of if
291+
block->finalize();
292292
}
293293
return block;
294294
}
@@ -302,7 +302,7 @@ class Builder {
302302
block->name = name;
303303
if (append) {
304304
block->list.push_back(append);
305-
block->finalize(); // TODO: move out of if
305+
block->finalize();
306306
}
307307
return block;
308308
}
@@ -325,7 +325,7 @@ class Builder {
325325
block->list.push_back(item);
326326
}
327327
}
328-
block->finalize(); // TODO: move out of if
328+
block->finalize();
329329
return block;
330330
}
331331

‎src/wasm.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,8 @@ class Call : public SpecificExpression<Expression::CallId> {
309309

310310
ExpressionList operands;
311311
Name target;
312+
313+
void finalize();
312314
};
313315

314316
class CallImport : public SpecificExpression<Expression::CallImportId> {
@@ -317,6 +319,8 @@ class CallImport : public SpecificExpression<Expression::CallImportId> {
317319

318320
ExpressionList operands;
319321
Name target;
322+
323+
void finalize();
320324
};
321325

322326
class FunctionType {
@@ -340,6 +344,8 @@ class CallIndirect : public SpecificExpression<Expression::CallIndirectId> {
340344
ExpressionList operands;
341345
Name fullType;
342346
Expression* target;
347+
348+
void finalize();
343349
};
344350

345351
class GetLocal : public SpecificExpression<Expression::GetLocalId> {
@@ -355,6 +361,8 @@ class SetLocal : public SpecificExpression<Expression::SetLocalId> {
355361
SetLocal() {}
356362
SetLocal(MixedArena& allocator) {}
357363

364+
void finalize();
365+
358366
Index index;
359367
Expression* value;
360368

@@ -377,6 +385,8 @@ class SetGlobal : public SpecificExpression<Expression::SetGlobalId> {
377385

378386
Name name;
379387
Expression* value;
388+
389+
void finalize();
380390
};
381391

382392
class Load : public SpecificExpression<Expression::LoadId> {
@@ -391,6 +401,8 @@ class Load : public SpecificExpression<Expression::LoadId> {
391401
Expression* ptr;
392402

393403
// type must be set during creation, cannot be inferred
404+
405+
void finalize();
394406
};
395407

396408
class Store : public SpecificExpression<Expression::StoreId> {
@@ -466,6 +478,8 @@ class Drop : public SpecificExpression<Expression::DropId> {
466478
Drop(MixedArena& allocator) {}
467479

468480
Expression* value;
481+
482+
void finalize();
469483
};
470484

471485
class Return : public SpecificExpression<Expression::ReturnId> {

‎src/wasm/wasm-binary.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -676,7 +676,8 @@ void WasmBinaryWriter::visitLoad(Load *curr) {
676676
}
677677
case f32: o << int8_t(BinaryConsts::F32LoadMem); break;
678678
case f64: o << int8_t(BinaryConsts::F64LoadMem); break;
679-
default: abort();
679+
case unreachable: return; // the pointer is unreachable, so we are never reached; just don't emit a load
680+
default: WASM_UNREACHABLE();
680681
}
681682
emitMemoryAccess(curr->align, curr->bytes, curr->offset);
682683
}
@@ -1732,7 +1733,6 @@ Expression* WasmBinaryBuilder::getBlock(WasmType type) {
17321733
Name label = getNextLabel();
17331734
breakStack.push_back({label, type != none && type != unreachable});
17341735
auto* block = Builder(wasm).blockify(getMaybeBlock(type));
1735-
block->finalize();
17361736
breakStack.pop_back();
17371737
block->cast<Block>()->name = label;
17381738
return block;
@@ -1814,6 +1814,7 @@ Expression* WasmBinaryBuilder::visitCall() {
18141814
call->target = import->name;
18151815
type = wasm.getFunctionType(import->functionType);
18161816
fillCall(call, type);
1817+
call->finalize();
18171818
ret = call;
18181819
} else {
18191820
// this is a call of a defined function
@@ -1825,6 +1826,7 @@ Expression* WasmBinaryBuilder::visitCall() {
18251826
type = functionTypes[adjustedIndex];
18261827
fillCall(call, type);
18271828
functionCalls[adjustedIndex].push_back(call); // we don't know function names yet
1829+
call->finalize();
18281830
ret = call;
18291831
}
18301832
return ret;
@@ -1847,6 +1849,7 @@ void WasmBinaryBuilder::visitCallIndirect(CallIndirect *curr) {
18471849
curr->operands[num - i - 1] = popNonVoidExpression();
18481850
}
18491851
curr->type = fullType->result;
1852+
curr->finalize();
18501853
}
18511854

18521855
void WasmBinaryBuilder::visitGetLocal(GetLocal *curr) {
@@ -1873,6 +1876,7 @@ void WasmBinaryBuilder::visitSetLocal(SetLocal *curr, uint8_t code) {
18731876
curr->value = popNonVoidExpression();
18741877
curr->type = curr->value->type;
18751878
curr->setTee(code == BinaryConsts::TeeLocal);
1879+
curr->finalize();
18761880
}
18771881

18781882
void WasmBinaryBuilder::visitGetGlobal(GetGlobal *curr) {
@@ -1897,6 +1901,7 @@ void WasmBinaryBuilder::visitSetGlobal(SetGlobal *curr) {
18971901
auto index = getU32LEB();
18981902
curr->name = getGlobalName(index);
18991903
curr->value = popNonVoidExpression();
1904+
curr->finalize();
19001905
}
19011906

19021907
void WasmBinaryBuilder::readMemoryAccess(Address& alignment, size_t bytes, Address& offset) {
@@ -1926,6 +1931,7 @@ bool WasmBinaryBuilder::maybeVisitLoad(Expression*& out, uint8_t code) {
19261931
if (debug) std::cerr << "zz node: Load" << std::endl;
19271932
readMemoryAccess(curr->align, curr->bytes, curr->offset);
19281933
curr->ptr = popNonVoidExpression();
1934+
curr->finalize();
19291935
out = curr;
19301936
return true;
19311937
}
@@ -2149,6 +2155,7 @@ void WasmBinaryBuilder::visitUnreachable(Unreachable *curr) {
21492155
void WasmBinaryBuilder::visitDrop(Drop *curr) {
21502156
if (debug) std::cerr << "zz node: Drop" << std::endl;
21512157
curr->value = popNonVoidExpression();
2158+
curr->finalize();
21522159
}
21532160

21542161
} // namespace wasm

‎src/wasm/wasm-s-parser.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -928,6 +928,7 @@ Expression* SExpressionWasmBuilder::makeTeeLocal(Element& s) {
928928
ret->index = getLocalIndex(*s[1]);
929929
ret->value = parseExpression(s[2]);
930930
ret->setTee(true);
931+
ret->finalize();
931932
return ret;
932933
}
933934

@@ -936,6 +937,7 @@ Expression* SExpressionWasmBuilder::makeSetLocal(Element& s) {
936937
ret->index = getLocalIndex(*s[1]);
937938
ret->value = parseExpression(s[2]);
938939
ret->setTee(false);
940+
ret->finalize();
939941
return ret;
940942
}
941943

@@ -960,6 +962,7 @@ Expression* SExpressionWasmBuilder::makeSetGlobal(Element& s) {
960962
ret->name = getGlobalName(*s[1]);
961963
if (wasm.getGlobalOrNull(ret->name) && !wasm.getGlobalOrNull(ret->name)->mutable_) throw ParseException("set_global of immutable", s.line, s.col);
962964
ret->value = parseExpression(s[2]);
965+
ret->finalize();
963966
return ret;
964967
}
965968

@@ -1084,6 +1087,7 @@ Expression* SExpressionWasmBuilder::makeLoad(Element& s, WasmType type) {
10841087
i++;
10851088
}
10861089
ret->ptr = parseExpression(s[i]);
1090+
ret->finalize();
10871091
return ret;
10881092
}
10891093

@@ -1210,6 +1214,7 @@ Expression* SExpressionWasmBuilder::makeCall(Element& s) {
12101214
ret->target = target;
12111215
ret->type = functionTypes[ret->target];
12121216
parseCallOperands(s, 2, s.size(), ret);
1217+
ret->finalize();
12131218
return ret;
12141219
}
12151220

@@ -1219,6 +1224,7 @@ Expression* SExpressionWasmBuilder::makeCallImport(Element& s) {
12191224
Import* import = wasm.getImport(ret->target);
12201225
ret->type = wasm.getFunctionType(import->functionType)->result;
12211226
parseCallOperands(s, 2, s.size(), ret);
1227+
ret->finalize();
12221228
return ret;
12231229
}
12241230

@@ -1232,6 +1238,7 @@ Expression* SExpressionWasmBuilder::makeCallIndirect(Element& s) {
12321238
ret->type = fullType->result;
12331239
parseCallOperands(s, 2, s.size() - 1, ret);
12341240
ret->target = parseExpression(s[s.size() - 1]);
1241+
ret->finalize();
12351242
return ret;
12361243
}
12371244

‎src/wasm/wasm.cpp

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,16 @@ void Block::finalize() {
198198
if (!name.is()) {
199199
// nothing branches here, so this is easy
200200
if (list.size() > 0) {
201+
// if we have an unreachable child, we are unreachable
202+
// (we don't need to recurse into children, they can't
203+
// break to us)
204+
for (auto* child : list) {
205+
if (child->type == unreachable) {
206+
type = unreachable;
207+
return;
208+
}
209+
}
210+
// children are reachable, so last element determines type
201211
type = list.back()->type;
202212
} else {
203213
type = none;
@@ -264,6 +274,31 @@ void Switch::finalize() {
264274
type = unreachable;
265275
}
266276

277+
template<typename T>
278+
void handleUnreachableOperands(T* curr) {
279+
for (auto* child : curr->operands) {
280+
if (child->type == unreachable) {
281+
curr->type = unreachable;
282+
break;
283+
}
284+
}
285+
}
286+
287+
void Call::finalize() {
288+
handleUnreachableOperands(this);
289+
}
290+
291+
void CallImport::finalize() {
292+
handleUnreachableOperands(this);
293+
}
294+
295+
void CallIndirect::finalize() {
296+
handleUnreachableOperands(this);
297+
if (target->type == unreachable) {
298+
type = unreachable;
299+
}
300+
}
301+
267302
bool FunctionType::structuralComparison(FunctionType& b) {
268303
if (result != b.result) return false;
269304
if (params.size() != b.params.size()) return false;
@@ -290,6 +325,24 @@ void SetLocal::setTee(bool is) {
290325
else type = none;
291326
}
292327

328+
void SetLocal::finalize() {
329+
if (value->type == unreachable) {
330+
type = unreachable;
331+
}
332+
}
333+
334+
void SetGlobal::finalize() {
335+
if (value->type == unreachable) {
336+
type = unreachable;
337+
}
338+
}
339+
340+
void Load::finalize() {
341+
if (ptr->type == unreachable) {
342+
type = unreachable;
343+
}
344+
}
345+
293346
void Store::finalize() {
294347
assert(valueType != none); // must be set
295348
if (ptr->type == unreachable || value->type == unreachable) {
@@ -423,6 +476,14 @@ void Select::finalize() {
423476
}
424477
}
425478

479+
void Drop::finalize() {
480+
if (value->type == unreachable) {
481+
type = unreachable;
482+
} else {
483+
type = none;
484+
}
485+
}
486+
426487
void Host::finalize() {
427488
switch (op) {
428489
case PageSize: case CurrentMemory: case HasFeature: {

0 commit comments

Comments
 (0)