From b42d19ed116545e6ec0cd4b7358520746ea5c97c Mon Sep 17 00:00:00 2001
From: Toon Verwaest <verwaest@chromium.org>
Date: Fri, 23 Dec 2022 14:52:22 +0100
Subject: [PATCH] [maglev] Also drop existing merges in a liveness hole

It's possible that various branches merged already with a value that's
in a liveness hole, but we only figure out later. If so, drop the merge
as well.

Bug: v8:7700, chromium:1403399

Change-Id: Ifd97e0c1959ffe51017e400fb028041047885a9c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4111932
Auto-Submit: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Victor Gomes <victorgomes@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/main@{#85013}
---
 src/maglev/maglev-regalloc.cc                 | 40 +++++++++----------
 .../maglev/regress/regress-crbug-1403399.js   | 36 +++++++++++++++++
 2 files changed, 56 insertions(+), 20 deletions(-)
 create mode 100644 test/mjsunit/maglev/regress/regress-crbug-1403399.js

diff --git a/src/maglev/maglev-regalloc.cc b/src/maglev/maglev-regalloc.cc
index 8e4ccc06e5..19f0cf5b68 100644
--- a/src/maglev/maglev-regalloc.cc
+++ b/src/maglev/maglev-regalloc.cc
@@ -1894,6 +1894,25 @@ void StraightForwardRegisterAllocator::MergeRegisterValues(ControlNode* control,
       return;
     }
 
+    if (node != nullptr && !node->is_loadable() && !node->has_register()) {
+      // If we have a node already, but can't load it here, we must be in a
+      // liveness hole for it, so nuke the merge state.
+      // This can only happen for conversion nodes, as they can split and take
+      // over the liveness of the node they are converting.
+      // TODO(v8:7700): Overeager DCHECK.
+      // DCHECK(node->properties().is_conversion());
+      if (v8_flags.trace_maglev_regalloc) {
+        printing_visitor_->os() << "  " << reg << " - can't load "
+                                << PrintNodeLabel(graph_labeller(), node)
+                                << ", dropping the merge\n";
+      }
+      // We always need to be able to restore values on JumpLoop since the value
+      // is definitely live at the loop header.
+      CHECK(!control->Is<JumpLoop>());
+      state = {nullptr, initialized_node};
+      return;
+    }
+
     if (merge) {
       // The register is already occupied with a different node. Figure out
       // where that node is allocated on the incoming branch.
@@ -1926,30 +1945,11 @@ void StraightForwardRegisterAllocator::MergeRegisterValues(ControlNode* control,
       if (v8_flags.trace_maglev_regalloc) {
         printing_visitor_->os()
             << "  " << reg << " - can't load incoming "
-            << PrintNodeLabel(graph_labeller(), node) << ", bailing out\n";
+            << PrintNodeLabel(graph_labeller(), incoming) << ", bailing out\n";
       }
       return;
     }
 
-    if (node != nullptr && !node->is_loadable() && !node->has_register()) {
-      // If we have a node already, but can't load it here, we must be in a
-      // liveness hole for it, so nuke the merge state.
-      // This can only happen for conversion nodes, as they can split and take
-      // over the liveness of the node they are converting.
-      // TODO(v8:7700): Overeager DCHECK.
-      // DCHECK(node->properties().is_conversion());
-      if (v8_flags.trace_maglev_regalloc) {
-        printing_visitor_->os() << "  " << reg << " - can't load "
-                                << PrintNodeLabel(graph_labeller(), node)
-                                << ", dropping the merge\n";
-      }
-      // We always need to be able to restore values on JumpLoop since the value
-      // is definitely live at the loop header.
-      CHECK(!control->Is<JumpLoop>());
-      state = {nullptr, initialized_node};
-      return;
-    }
-
     const size_t size = sizeof(RegisterMerge) +
                         predecessor_count * sizeof(compiler::AllocatedOperand);
     void* buffer = compilation_info_->zone()->Allocate<void*>(size);
diff --git a/test/mjsunit/maglev/regress/regress-crbug-1403399.js b/test/mjsunit/maglev/regress/regress-crbug-1403399.js
new file mode 100644
index 0000000000..b13387771c
--- /dev/null
+++ b/test/mjsunit/maglev/regress/regress-crbug-1403399.js
@@ -0,0 +1,36 @@
+// Copyright 2022 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: --maglev --allow-natives-syntax
+
+function __f_0() {
+    for (let __v_3 = 0; __v_3 < 52; ++__v_3) {
+      let __v_4 = __v_3 | 0;
+      switch (__v_4) {
+        case 28:
+            if (__v_3 != null && typeof __v_3 == "object") {
+              try {
+                 Object.defineProperty(  {
+                   get: function () {
+                           ({get: function () {
+                             return __v_4;
+                           }})
+                   }
+                 });
+              } catch (e) {}
+            }
+        case 29:
+        case 31:
+        case 32:
+        case 33:
+            __v_4 += 1;
+
+        case 34:
+      }
+    }
+}
+%PrepareFunctionForOptimization(__f_0);
+__f_0();
+%OptimizeMaglevOnNextCall(__f_0);
+__f_0();