[compiler] Enable inlining of JS-to-Wasm calls inside try/catch

Fixes a problem with the inlining of JS-to-Wasm call wrappers into a
surrounding exception handler and re-enables this case.

Bug: v8:11092
Change-Id: I4937838c2b4a199e21f5ac90bee5b8e8de2470be
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2678341
Commit-Queue: Paolo Severini <paolosev@microsoft.com>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73086}

src/compiler/js-call-reducer.cc

@@ -3462,11 +3462,6 @@ Reduction JSCallReducer::ReduceCallWasmFunction(
     return NoChange();
   }
 
-  // TODO(paolosev@microsoft.com): Enable inlining for calls in try/catch.
-  if (NodeProperties::IsExceptionalCall(node)) {
-    return NoChange();
-  }
-
   const wasm::FunctionSig* wasm_signature = shared.wasm_function_signature();
   if (!CanInlineJSToWasmCall(wasm_signature)) {
     return NoChange();

src/compiler/js-inlining.cc

@@ -388,6 +388,7 @@ Reduction JSInliner::ReduceJSWasmCall(Node* node) {
   // Create the subgraph for the inlinee.
   Node* start_node;
   Node* end;
+  size_t subgraph_min_node_id;
   {
     Graph::SubgraphScope scope(graph());
 
@@ -404,6 +405,13 @@ Reduction JSInliner::ReduceJSWasmCall(Node* node) {
             jsgraph(), n.context(), n.frame_state(),
             wasm_call_params.signature());
     JSWasmCallData js_wasm_call_data(wasm_call_params.signature());
+
+    // All the nodes inserted by the inlined subgraph will have
+    // id >= subgraph_min_node_id. We use this later to avoid wire nodes that
+    // are not inserted by the inlinee but were already part of the graph to the
+    // surrounding exception handler, if present.
+    subgraph_min_node_id = graph()->NodeCount();
+
     BuildInlinedJSToWasmWrapper(
         graph()->zone(), jsgraph(), wasm_call_params.signature(),
         wasm_call_params.module(), source_positions_,
@@ -427,6 +435,9 @@ Reduction JSInliner::ReduceJSWasmCall(Node* node) {
     // Find all uncaught 'calls' in the inlinee.
     AllNodes inlined_nodes(local_zone_, end, graph());
     for (Node* subnode : inlined_nodes.reachable) {
+      // Ignore nodes that are not part of the inlinee.
+      if (subnode->id() < subgraph_min_node_id) continue;
+
       // Every possibly throwing node should get {IfSuccess} and {IfException}
       // projections, unless there already is local exception handling.
       if (subnode->op()->HasProperty(Operator::kNoThrow)) continue;

test/cctest/test-js-to-wasm.cc

@@ -68,6 +68,9 @@ struct ExportedFunction {
 
 DECLARE_EXPORTED_FUNCTION(nop, sigs.v_v(), WASM_CODE({WASM_NOP}))
 
+DECLARE_EXPORTED_FUNCTION(unreachable, sigs.v_v(),
+                          WASM_CODE({WASM_UNREACHABLE}))
+
 DECLARE_EXPORTED_FUNCTION(i32_square, sigs.i_i(),
                           WASM_CODE({WASM_LOCAL_GET(0), WASM_LOCAL_GET(0),
                                      kExprI32Mul}))
@@ -457,9 +460,9 @@ class FastJSWasmCallTester {
         ";"
         "function test() {"
         "  try {"
-        "    return " +
+        "    return %ObserveNode(" +
         exported_function_name +
-        "(arg);"
+        "(arg));"
         "  } catch (e) {"
         "    return 0;"
         "  }"
@@ -485,13 +488,19 @@ class FastJSWasmCallTester {
 
   // Executes a test function with a try/catch calling a Wasm function returning
   // void.
-  void CallAndCheckWithTryCatch_void(const std::string& exported_function_name,
-                                     const v8::Local<v8::Value> arg0,
-                                     const v8::Local<v8::Value> arg1) {
+  void CallAndCheckWithTryCatch_void(
+      const std::string& exported_function_name,
+      const std::vector<v8::Local<v8::Value>>& args) {
     LocalContext env;
-    CHECK((*env)->Global()->Set(env.local(), v8_str("arg0"), arg0).FromJust());
-    CHECK((*env)->Global()->Set(env.local(), v8_str("arg1"), arg1).FromJust());
+    for (size_t i = 0; i < args.size(); i++) {
+      CHECK((*env)
+                ->Global()
+                ->Set(env.local(), v8_str(("arg" + std::to_string(i)).c_str()),
+                      args[i])
+                .FromJust());
+    }
 
+    std::string js_args = ArgsToString(args.size());
     std::string js_code =
         "const importObj = {"
         "  env: {"
@@ -509,9 +518,9 @@ class FastJSWasmCallTester {
         ";"
         "function test() {"
         "  try {"
-        "    " +
-        exported_function_name +
-        "(arg0, arg1);"
+        "    %ObserveNode(" +
+        exported_function_name + "(" + js_args +
+        "));"
         "    return 1;"
         "  } catch (e) {"
         "    return 0;"
@@ -928,6 +937,13 @@ TEST(TestFastJSWasmCall_EagerDeopt) {
 
 // Exception handling tests
 
+TEST(TestFastJSWasmCall_Unreachable) {
+  v8::HandleScope scope(CcTest::isolate());
+  FastJSWasmCallTester tester;
+  tester.AddExportedFunction(k_unreachable);
+  tester.CallAndCheckWithTryCatch_void("unreachable", {});
+}
+
 TEST(TestFastJSWasmCall_Trap_i32) {
   v8::HandleScope scope(CcTest::isolate());
   FastJSWasmCallTester tester;
@@ -960,8 +976,8 @@ TEST(TestFastJSWasmCall_Trap_void) {
   v8::HandleScope scope(CcTest::isolate());
   FastJSWasmCallTester tester;
   tester.AddExportedFunction(k_store_i32);
-  tester.CallAndCheckWithTryCatch_void("store_i32", v8_int(0x7fffffff),
-                                       v8_int(42));
+  tester.CallAndCheckWithTryCatch_void("store_i32",
+                                       {v8_int(0x7fffffff), v8_int(42)});
 }
 
 // BigInt

test/mjsunit/regress/wasm/regress-crbug-1168612.js

+// Copyright 2021 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax --turbo-inline-js-wasm-calls
+
+load('test/mjsunit/wasm/wasm-module-builder.js');
+
+function getMain() {
+  var builder = new WasmModuleBuilder();
+  builder.addFunction("main", kSig_v_v)
+    .addBody([kExprUnreachable])
+    .exportAs("main");
+  return builder.instantiate().exports.main;
+}
+let foo = getMain();
+
+function loop() {
+  for (let i = 0; i < 2; i++) {
+    try {
+       foo();
+    } catch (e) {
+      if (i) {
+        throw e;
+      }
+    }
+  }
+}
+%PrepareFunctionForOptimization(loop);
+assertThrows(loop, WebAssembly.RuntimeError, "unreachable");
+%OptimizeFunctionOnNextCall(loop);
+assertThrows(loop, WebAssembly.RuntimeError, "unreachable");