[compiler][wasm] Fix endless loop in WasmTyper

Liedtke · V8 LUCI CQ · commit 18e6776184b7 · 2024-08-30T14:51:18.000Z
The graph builder was applying a TypeGuard into the wrong control chain. In this case the br_on_non_null implementation added a TypeGuard for the non-null type before performing the actual branch. The br_on_non_null target is anyways going to merge controls with some other branch and Turboshaft is hopefully making all this code oboslete soon, so the easiest and safest fix is to drop the TypeGuard completely. Fixed: 361862737 Change-Id: Ifc1c34ab726576b861d3d5dc6f6a9d20e93d4af0 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5826664 Auto-Submit: Matthias Liedtke <mliedtke@chromium.org> Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> Commit-Queue: Jakob Kummerow <jkummerow@chromium.org> Cr-Commit-Position: refs/heads/main@{#95897}
diff --git a/src/wasm/graph-builder-interface.cc b/src/wasm/graph-builder-interface.cc
@@ -1206,8 +1206,7 @@ class WasmGraphBuildingInterface {
 
   void BrOnNonNull(FullDecoder* decoder, const Value& ref_object, Value* result,
                    uint32_t depth, bool /* drop_null_on_fallthrough */) {
-    result->node =
-        builder_->TypeGuard(ref_object.node, ref_object.type.AsNonNull());
+    result->node = ref_object.node;
     SsaEnv* false_env = ssa_env_;
     SsaEnv* true_env = Split(decoder->zone(), false_env);
     false_env->SetNotMerged();
diff --git a/test/mjsunit/regress/wasm/regress-361862737.js b/test/mjsunit/regress/wasm/regress-361862737.js
@@ -0,0 +1,31 @@
+// 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.
+
+d8.file.execute('test/mjsunit/wasm/wasm-module-builder.js');
+
+// This test case reproduces a case in Turbofan where the WasmTyper was
+// flip-flopping between two different states never reaching a fix point.
+const builder = new WasmModuleBuilder();
+let $array1 = builder.addArray(kWasmI16, true, kNoSuperType, true);
+let loopSig = builder.addType(makeSig([kWasmFuncRef], []));
+let funcEndless = builder.addFunction('funcEndless', kSig_v_v).exportFunc()
+  .addLocals(kWasmFuncRef, 1)  // $var0
+  .addBody([
+    kExprRefNull, kFuncRefCode,
+    kExprLoop, loopSig,
+      kExprLocalTee, 0,  // $var0
+      kExprBrOnNonNull, 0,
+      kExprLocalGet, 0,  // $var0
+      kExprRefAsNonNull,
+      kExprBrOnNonNull, 0,
+    kExprEnd,
+
+    // Use something with a gc prefix to enable the WasmTyper.
+    kExprRefNull, $array1,
+    kGCPrefix, kExprArrayLen,
+    kExprDrop,
+  ]);
+
+const instance = builder.instantiate();
+assertTraps(kTrapNullDereference, () => instance.exports.funcEndless(null));
