8335977: Deoptimization fails with assert "object should be reallocated already"

Co-authored-by: Christian Hagedorn <chagedorn@openjdk.org>
Reviewed-by: thartmann, kvn, vlivanov

Author: Cesar Soares Lucas

src/hotspot/share/opto/output.cpp

@@ -792,7 +792,14 @@ void PhaseOutput::FillLocArray( int idx, MachSafePointNode* sfpt, Node *local,
 
       for (uint i = 1; i < smerge->req(); i++) {
         Node* obj_node = smerge->in(i);
-        (void)FillLocArray(mv->possible_objects()->length(), sfpt, obj_node, mv->possible_objects(), objs);
+        int idx = mv->possible_objects()->length();
+        (void)FillLocArray(idx, sfpt, obj_node, mv->possible_objects(), objs);
+
+        // By default ObjectValues that are in 'possible_objects' are not root objects.
+        // They will be marked as root later if they are directly referenced in a JVMS.
+        assert(mv->possible_objects()->length() > idx, "Didn't add entry to possible_objects?!");
+        assert(mv->possible_objects()->at(idx)->is_object(), "Entries in possible_objects should be ObjectValue.");
+        mv->possible_objects()->at(idx)->as_ObjectValue()->set_root(false);
       }
     }
     array->append(mv);
@@ -1127,7 +1134,14 @@ void PhaseOutput::Process_OopMap_Node(MachNode *mach, int current_offset) {
 
           for (uint i = 1; i < smerge->req(); i++) {
             Node* obj_node = smerge->in(i);
-            FillLocArray(mv->possible_objects()->length(), sfn, obj_node, mv->possible_objects(), objs);
+            int idx = mv->possible_objects()->length();
+            (void)FillLocArray(idx, sfn, obj_node, mv->possible_objects(), objs);
+
+            // By default ObjectValues that are in 'possible_objects' are not root objects.
+            // They will be marked as root later if they are directly referenced in a JVMS.
+            assert(mv->possible_objects()->length() > idx, "Didn't add entry to possible_objects?!");
+            assert(mv->possible_objects()->at(idx)->is_object(), "Entries in possible_objects should be ObjectValue.");
+            mv->possible_objects()->at(idx)->as_ObjectValue()->set_root(false);
           }
         }
         scval = mv;
@@ -1158,11 +1172,17 @@ void PhaseOutput::Process_OopMap_Node(MachNode *mach, int current_offset) {
 
         for (int j = 0; j< merge->possible_objects()->length(); j++) {
           ObjectValue* ov = merge->possible_objects()->at(j)->as_ObjectValue();
-          bool is_root = locarray->contains(ov) ||
-                         exparray->contains(ov) ||
-                         contains_as_owner(monarray, ov) ||
-                         contains_as_scalarized_obj(jvms, sfn, objs, ov);
-          ov->set_root(is_root);
+          if (ov->is_root()) {
+            // Already flagged as 'root' by something else. We shouldn't change it
+            // to non-root in a younger JVMS because it may need to be alive in
+            // a younger JVMS.
+          } else {
+            bool is_root = locarray->contains(ov) ||
+                           exparray->contains(ov) ||
+                           contains_as_owner(monarray, ov) ||
+                           contains_as_scalarized_obj(jvms, sfn, objs, ov);
+            ov->set_root(is_root);
+          }
         }
       }
     }

test/hotspot/jtreg/compiler/escapeAnalysis/TestReduceAllocationAndJVMStates.java

@@ -0,0 +1,79 @@
+/*
+ * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8335977
+ * @summary Check that Reduce Allocation Merges doesn't crash when there
+ *          is an uncommon_trap with a chain of JVMS and, the reduced phi
+ *          input(s) are local(s) in an old JVMS but not in a younger JVMS.
+ *          I.e., check that we don't lose track of "_is_root" when traversing
+ *          a JVMS chain.
+ * @run main/othervm -Xbatch
+ *                   -XX:CompileOnly=compiler.escapeAnalysis.TestReduceAllocationAndJVMStates::test*
+ *                   compiler.escapeAnalysis.TestReduceAllocationAndJVMStates
+ * @run main compiler.escapeAnalysis.TestReduceAllocationAndJVMStates
+ */
+package compiler.escapeAnalysis;
+
+public class TestReduceAllocationAndJVMStates {
+    static boolean bFld;
+    static int iFld;
+
+    public static void main(String[] args) {
+        bFld = false;
+
+        for (int i = 0; i < 10000; i++) {
+            test(i % 2 == 0);
+        }
+        bFld = true;
+
+        // This will trigger a deoptimization which
+        // will make the issue manifest to the user
+        test(true);
+    }
+
+    static int test(boolean flag) {
+        Super a = new A();
+        Super b = new B();
+        Super s = (flag ? a : b);
+
+        // This needs to be inlined by C2
+        check();
+
+        return a.i + b.i + s.i;
+    }
+
+    // This shouldn't be manually inlined
+    static void check() {
+        if (bFld) {
+            iFld = 34;
+        }
+    }
+}
+
+class Super {
+    int i;
+}
+class A extends Super {}
+class B extends Super {}