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.

1 files changed, 13 insertions, 4 deletions

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));
         }