From 9bfade13c4cd5ebce7ae70681c655302f0f02ce7 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 22 Jul 2020 18:25:56 -0700 Subject: wasm2js: coerce function pointer indexes (#2979) We emit FUNCTION_TABLE[ptr], where FUNCTION_TABLE is a JS array. That is a rare case where true is handled differently than 1 (a typed array or an add would cast, etc.), so we must explicitly cast there. Fixes an issue that existed before, but became a problem due to #2869 which optimized some selects into a form that emitted a true or a false, and if that was a function pointer, it could be bad, see https://app.circleci.com/pipelines/github/emscripten-core/emscripten/6699/workflows/0c4da49c-75d0-4b0a-8fac-686a8330a3fe/jobs/336520 The new test/wasm2js/indirect-select.2asm.js.opt output shows what happened there. Verified as passing emscripten's wasm2js1 wasm2js2 test suites. --- src/wasm2js.h | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/wasm2js.h b/src/wasm2js.h index 3accfad75..33f70a4fc 100644 --- a/src/wasm2js.h +++ b/src/wasm2js.h @@ -1146,6 +1146,14 @@ Ref Wasm2JSBuilder::processFunctionBody(Module* m, } } } + // Ensure the function pointer is a number. In general in wasm2js we are + // ok with true/false being present, as they are immediately cast to a + // number anyhow on their use. However, FUNCTION_TABLE[true] is *not* the + // same as FUNCTION_TABLE[1], so we must cast. This is a rare exception + // because FUNCTION_TABLE is just a normal JS object, not a typed array + // or a mathematical operation (all of which coerce to a number for us). + auto target = visit(curr->target, EXPRESSION_RESULT); + target = makeAsmCoercion(target, ASM_INT); if (mustReorder) { Ref ret; ScopedTemp idx(Type::i32, parent, func); @@ -1155,7 +1163,9 @@ Ref Wasm2JSBuilder::processFunctionBody(Module* m, IString temp = temps.back()->temp; sequenceAppend(ret, visitAndAssign(operand, temp)); } - sequenceAppend(ret, visitAndAssign(curr->target, idx)); + sequenceAppend(ret, + ValueBuilder::makeBinary( + ValueBuilder::makeName(idx.getName()), SET, target)); Ref theCall = ValueBuilder::makeCall(ValueBuilder::makeSub( ValueBuilder::makeName(FUNCTION_TABLE), idx.getAstName())); for (size_t i = 0; i < temps.size(); i++) { @@ -1172,9 +1182,8 @@ Ref Wasm2JSBuilder::processFunctionBody(Module* m, return ret; } else { // Target has no side effects, emit simple code - Ref theCall = ValueBuilder::makeCall( - ValueBuilder::makeSub(ValueBuilder::makeName(FUNCTION_TABLE), - visit(curr->target, EXPRESSION_RESULT))); + Ref theCall = ValueBuilder::makeCall(ValueBuilder::makeSub( + ValueBuilder::makeName(FUNCTION_TABLE), target)); for (auto* operand : curr->operands) { theCall[2]->push_back(visit(operand, EXPRESSION_RESULT)); } -- cgit v1.2.3