[wasm-imported-strings] Fix IsArrayNewSegment in unreachable code

The node we query is invalid in unreachable code, so we must check
for that before fetching it from the graph.

Bonus: running the regression test in Turbofan revealed an unrelated
issue having to do with reachability of catch blocks after replacing
a well-known import, so this CL includes a fix for that too.

Fixed: chromium:324690505
Change-Id: If648faf7dd466ed857d723eec44fc01fc7a6a6fc
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5300801
Auto-Submit: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Matthias Liedtke <mliedtke@chromium.org>
Cr-Commit-Position: refs/heads/main@{#92362}

src/wasm/graph-builder-interface.cc

@@ -964,6 +964,25 @@ class WasmGraphBuildingInterface {
     }
     assumptions_->RecordAssumption(index, import);
     SetAndTypeNode(&returns[0], result);
+    // The decoder assumes that any call might throw, so if we are in a try
+    // block, it marks the associated catch block as reachable, and will
+    // later ask the graph builder to build the catch block's graph.
+    // However, we just replaced the call with a sequence that doesn't throw,
+    // which might make the catch block unreachable as far as the graph builder
+    // is concerned, which would violate assumptions when trying to build a
+    // graph for it. So we insert a fake branch to the catch block to make it
+    // reachable. Later phases will optimize this out.
+    if (decoder->current_catch() != -1) {
+      TryInfo* try_info = current_try_info(decoder);
+      if (try_info->catch_env->state == SsaEnv::kUnreachable) {
+        auto [true_cont, false_cont] =
+            builder_->BranchExpectTrue(builder_->Int32Constant(1));
+        builder_->SetControl(false_cont);
+        Goto(decoder, try_info->catch_env);
+        try_info->exception = false_cont;
+        builder_->SetControl(true_cont);
+      }
+    }
     return true;
   }
 

src/wasm/turboshaft-graph-interface.cc

@@ -3482,6 +3482,8 @@ class TurboshaftGraphBuildingInterface : public WasmGraphBuilderBase {
   // TODO(jkummerow): This check would be more elegant if we made
   // {ArrayNewSegment} a high-level node that's lowered later.
   bool IsArrayNewSegment(V<Object> array) {
+    DCHECK_IMPLIES(!array.valid(), __ generating_unreachable_operations());
+    if (__ generating_unreachable_operations()) return false;
     const CallOp* call = __ output_graph().Get(array).TryCast<CallOp>();
     if (call == nullptr) return false;
     int64_t stub_id{};

test/mjsunit/regress/wasm/regress-324690505.js

@@ -0,0 +1,79 @@
+// Copyright 2024 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: --experimental-wasm-imported-strings
+
+d8.file.execute('test/mjsunit/wasm/wasm-module-builder.js');
+
+let builder = new WasmModuleBuilder();
+let tag0 = builder.addTag(kSig_v_v);
+
+builder.startRecGroup();
+let kArrayI8 = builder.addArray(kWasmI8, true, kNoSuperType, true);
+builder.endRecGroup();
+let kArray8Ref = wasmRefNullType(kArrayI8);
+let kRefExtern = wasmRefType(kWasmExternRef);
+let kSig_e_i = makeSig([kWasmI32], [kRefExtern]);
+let kStringFromCharCode =
+    builder.addImport('wasm:js-string', 'fromCharCode', kSig_e_i);
+let kStringFromUtf8Array = builder.addImport(
+    'wasm:text-decoder', 'decodeStringFromUTF8Array',
+    makeSig([kArray8Ref, kWasmI32, kWasmI32], [kRefExtern]));
+let imp_nop = builder.addImport("m", "f", kSig_v_v);
+
+let id = builder.addFunction('id', kSig_i_i).addBody([
+    kExprLocalGet, 0,
+]);
+let nop = builder.addFunction('nop', kSig_v_v).addBody([]);
+
+let main = builder.addFunction('main', kSig_e_i).exportFunc().addBody([
+  kExprTry, kWasmVoid,
+    kExprI32Const, 97,
+    // The FunctionBodyDecoder assumes that any call can throw, so it marks
+    // the {catch_all} block as reachable.
+    // The graph builder interface recognizes the well-known import and knows
+    // that it won't throw, so it considers the {catch_all} block (and
+    // everything after it) unreachable.
+    kExprCallFunction, kStringFromCharCode,
+    kExprReturn,
+    //kExprDrop,
+  kExprCatchAll,
+  kExprEnd,
+  kExprRefNull, kArrayI8,
+  kExprI32Const, 0,
+  kExprI32Const, 0,
+  // The point of this test: what happens for this instruction when the
+  // compiler knows it's emitting unreachable code.
+  kExprCallFunction, kStringFromUtf8Array,
+]);
+
+builder.addFunction('inlined', kSig_e_i).exportFunc().addBody([
+  kExprLocalGet, 0,
+  kExprCallFunction, main.index,
+]);
+
+let callee_no_catch = builder.addFunction('', kSig_e_i).addBody([
+  kExprI32Const, 98,
+  kExprCallFunction, kStringFromCharCode,
+  kExprReturn,
+]);
+
+builder.addFunction('inlined_catch', kSig_e_i).exportFunc().addBody([
+  kExprTry, kWasmVoid,
+    kExprLocalGet, 0,
+    kExprCallFunction, callee_no_catch.index,
+    kExprReturn,
+  kExprCatchAll,
+  kExprEnd,
+  kExprRefNull, kArrayI8,
+  kExprI32Const, 0,
+  kExprI32Const, 0,
+  kExprCallFunction, kStringFromUtf8Array,
+]);
+
+let kBuiltins = {builtins: ['js-string', 'text-decoder']};
+let instance = builder.instantiate({m: {f: () => {}}}, kBuiltins);
+instance.exports.main(1);
+instance.exports.inlined(1);
+instance.exports.inlined_catch(1);