8335708: C2: Compile::verify_graph_edges must start at root and safepoints, just like CCP traversal

Reviewed-by: chagedorn, epeter

src/hotspot/share/opto/compile.cpp

@@ -421,7 +421,7 @@ void Compile::remove_useless_node(Node* dead) {
 }
 
 // Disconnect all useless nodes by disconnecting those at the boundary.
-void Compile::disconnect_useless_nodes(Unique_Node_List& useful, Unique_Node_List& worklist) {
+void Compile::disconnect_useless_nodes(Unique_Node_List& useful, Unique_Node_List& worklist, const Unique_Node_List* root_and_safepoints) {
   uint next = 0;
   while (next < useful.size()) {
     Node *n = useful.at(next++);
@@ -474,7 +474,7 @@ void Compile::disconnect_useless_nodes(Unique_Node_List& useful, Unique_Node_Lis
   remove_useless_late_inlines(         &_string_late_inlines, useful);
   remove_useless_late_inlines(         &_boxing_late_inlines, useful);
   remove_useless_late_inlines(&_vector_reboxing_late_inlines, useful);
-  debug_only(verify_graph_edges(true/*check for no_dead_code*/);)
+  debug_only(verify_graph_edges(true /*check for no_dead_code*/, root_and_safepoints);)
 }
 
 // ============================================================================
@@ -4253,11 +4253,25 @@ bool Compile::needs_clinit_barrier(ciInstanceKlass* holder, ciMethod* accessing_
 //------------------------------verify_bidirectional_edges---------------------
 // For each input edge to a node (ie - for each Use-Def edge), verify that
 // there is a corresponding Def-Use edge.
-void Compile::verify_bidirectional_edges(Unique_Node_List &visited) {
+void Compile::verify_bidirectional_edges(Unique_Node_List& visited, const Unique_Node_List* root_and_safepoints) const {
   // Allocate stack of size C->live_nodes()/16 to avoid frequent realloc
   uint stack_size = live_nodes() >> 4;
-  Node_List nstack(MAX2(stack_size, (uint)OptoNodeListSize));
-  nstack.push(_root);
+  Node_List nstack(MAX2(stack_size, (uint) OptoNodeListSize));
+  if (root_and_safepoints != nullptr) {
+    assert(root_and_safepoints->member(_root), "root is not in root_and_safepoints");
+    for (uint i = 0, limit = root_and_safepoints->size(); i < limit; i++) {
+      Node* root_or_safepoint = root_and_safepoints->at(i);
+      // If the node is a safepoint, let's check if it still has a control input
+      // Lack of control input signifies that this node was killed by CCP or
+      // recursively by remove_globally_dead_node and it shouldn't be a starting
+      // point.
+      if (!root_or_safepoint->is_SafePoint() || root_or_safepoint->in(0) != nullptr) {
+        nstack.push(root_or_safepoint);
+      }
+    }
+  } else {
+    nstack.push(_root);
+  }
 
   while (nstack.size() > 0) {
     Node* n = nstack.pop();
@@ -4307,12 +4321,12 @@ void Compile::verify_bidirectional_edges(Unique_Node_List &visited) {
 //------------------------------verify_graph_edges---------------------------
 // Walk the Graph and verify that there is a one-to-one correspondence
 // between Use-Def edges and Def-Use edges in the graph.
-void Compile::verify_graph_edges(bool no_dead_code) {
+void Compile::verify_graph_edges(bool no_dead_code, const Unique_Node_List* root_and_safepoints) const {
   if (VerifyGraphEdges) {
     Unique_Node_List visited;
 
     // Call graph walk to check edges
-    verify_bidirectional_edges(visited);
+    verify_bidirectional_edges(visited, root_and_safepoints);
     if (no_dead_code) {
       // Now make sure that no visited node is used by an unvisited node.
       bool dead_nodes = false;

src/hotspot/share/opto/compile.hpp

@@ -1035,7 +1035,7 @@ public:
 
   void              identify_useful_nodes(Unique_Node_List &useful);
   void              update_dead_node_list(Unique_Node_List &useful);
-  void              disconnect_useless_nodes(Unique_Node_List& useful, Unique_Node_List& worklist);
+  void disconnect_useless_nodes(Unique_Node_List& useful, Unique_Node_List& worklist, const Unique_Node_List* root_and_safepoints = nullptr);
 
   void              remove_useless_node(Node* dead);
 
@@ -1242,14 +1242,28 @@ public:
   static void print_intrinsic_statistics() PRODUCT_RETURN;
 
   // Graph verification code
-  // Walk the node list, verifying that there is a one-to-one
-  // correspondence between Use-Def edges and Def-Use edges
-  // The option no_dead_code enables stronger checks that the
-  // graph is strongly connected from root in both directions.
-  void verify_graph_edges(bool no_dead_code = false) PRODUCT_RETURN;
+  // Walk the node list, verifying that there is a one-to-one correspondence
+  // between Use-Def edges and Def-Use edges. The option no_dead_code enables
+  // stronger checks that the graph is strongly connected from starting points
+  // in both directions.
+  // root_and_safepoints is used to give the starting points for the traversal.
+  // If not supplied, only root is used. When this check is called after CCP,
+  // we need to start traversal from Root and safepoints, just like CCP does its
+  // own traversal (see PhaseCCP::transform for reasons).
+  //
+  // To call this function, there are 2 ways to go:
+  // - give root_and_safepoints to start traversal everywhere needed (like after CCP)
+  // - if the whole graph is assumed to be reachable from Root's input,
+  //   root_and_safepoints is not needed (like in PhaseRemoveUseless).
+  //
+  // Failure to specify root_and_safepoints in case the graph is not fully
+  // reachable from Root's input make this check unsound (can miss inconsistencies)
+  // and even incomplete (can make up non-existing problems) if no_dead_code is
+  // true.
+  void verify_graph_edges(bool no_dead_code = false, const Unique_Node_List* root_and_safepoints = nullptr) const PRODUCT_RETURN;
 
   // Verify bi-directional correspondence of edges
-  void verify_bidirectional_edges(Unique_Node_List &visited);
+  void verify_bidirectional_edges(Unique_Node_List& visited, const Unique_Node_List* root_and_safepoints = nullptr) const;
 
   // End-of-run dumps.
   static void print_statistics() PRODUCT_RETURN;

src/hotspot/share/opto/node.hpp

@@ -1745,7 +1745,7 @@ public:
   Unique_Node_List(Unique_Node_List&&) = default;
 
   void remove( Node *n );
-  bool member( Node *n ) { return _in_worklist.test(n->_idx) != 0; }
+  bool member(const Node* n) const { return _in_worklist.test(n->_idx) != 0; }
   VectorSet& member_set(){ return _in_worklist; }
 
   void push(Node* b) {

src/hotspot/share/opto/phaseX.cpp

@@ -2110,7 +2110,7 @@ Node *PhaseCCP::transform( Node *n ) {
   C->update_dead_node_list(useful);
   remove_useless_nodes(useful.member_set());
   _worklist.remove_useless_nodes(useful.member_set());
-  C->disconnect_useless_nodes(useful, _worklist);
+  C->disconnect_useless_nodes(useful, _worklist, &_root_and_safepoints);
 
   Node* new_root = node_map[n->_idx];
   assert(new_root->is_Root(), "transformed root node must be a root node");

test/hotspot/jtreg/compiler/loopopts/VerifyGraphEdgesWithDeadCodeCheckFromSafepoints.java

@@ -0,0 +1,70 @@
+/*
+ * Copyright (c) 2025, 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 8335708
+ * @summary Crash Compile::verify_graph_edges with dead code check when safepoints are reachable but not connected back to Root's inputs
+ * @library /test/lib
+ *
+ * @run driver compiler.loopopts.VerifyGraphEdgesWithDeadCodeCheckFromSafepoints
+ * @run main/othervm
+ *       -XX:+IgnoreUnrecognizedVMOptions
+ *       -XX:-TieredCompilation -XX:+VerifyGraphEdges
+ *       -XX:+StressIGVN -Xcomp
+ *       -XX:CompileCommand=compileonly,compiler.loopopts.VerifyGraphEdgesWithDeadCodeCheckFromSafepoints::mainTest
+ *       compiler.loopopts.VerifyGraphEdgesWithDeadCodeCheckFromSafepoints
+ *
+ */
+
+package compiler.loopopts;
+
+import jdk.test.lib.Utils;
+
+public class VerifyGraphEdgesWithDeadCodeCheckFromSafepoints {
+    public static void main(String[] args) throws Exception {
+        Thread thread = new Thread() {
+            public void run() {
+                VerifyGraphEdgesWithDeadCodeCheckFromSafepoints instance = new VerifyGraphEdgesWithDeadCodeCheckFromSafepoints();
+                byte[] a = new byte[997];
+                for (int i = 0; i < 100; ++i) {
+                    instance.mainTest(a, a);
+                }
+            }
+        };
+        // Give thread some time to trigger compilation
+        thread.setDaemon(true);
+        thread.start();
+        Thread.sleep(Utils.adjustTimeout(500));
+    }
+
+    public void mainTest(byte[] a, byte[] b) {
+        int i = 0;
+        while (i < (a.length - 4)) {
+            a[i] = b[i];
+        }
+        while (true) {
+            a[i] = b[i];
+        }
+    }
+}