8303279: C2: crash in SubTypeCheckNode::sub() at IGVN split if

Reviewed-by: kvn, vlivanov, thartmann, simonis

src/hotspot/share/opto/callGenerator.cpp

@@ -706,6 +706,10 @@ void CallGenerator::do_late_inline_helper() {
       result = (result_size == 1) ? kit.pop() : kit.pop_pair();
     }
 
+    if (call->is_CallStaticJava() && call->as_CallStaticJava()->is_boxing_method()) {
+      result = kit.must_be_not_null(result, false);
+    }
+
     if (inline_cg()->is_inline()) {
       C->set_has_loops(C->has_loops() || inline_cg()->method()->has_loops());
       C->env()->notice_inlined_method(inline_cg()->method());

src/hotspot/share/opto/doCall.cpp

@@ -824,7 +824,9 @@ void Parse::catch_call_exceptions(ciExceptionHandlerStream& handlers) {
 
   if (!default_handler) {
     bcis->append(-1);
-    extypes->append(TypeOopPtr::make_from_klass(env()->Throwable_klass())->is_instptr());
+    const Type* extype = TypeOopPtr::make_from_klass(env()->Throwable_klass())->is_instptr();
+    extype = extype->join(TypeInstPtr::NOTNULL);
+    extypes->append(extype);
   }
 
   int len = bcis->length();

src/hotspot/share/opto/ifnode.cpp

@@ -91,9 +91,14 @@ static Node* split_if(IfNode *iff, PhaseIterGVN *igvn) {
   // See that the merge point contains some constants
   Node *con1=nullptr;
   uint i4;
-  for( i4 = 1; i4 < phi->req(); i4++ ) {
+  RegionNode* phi_region = phi->region();
+  for (i4 = 1; i4 < phi->req(); i4++ ) {
     con1 = phi->in(i4);
-    if( !con1 ) return nullptr;    // Do not optimize partially collapsed merges
+    // Do not optimize partially collapsed merges
+    if (con1 == nullptr || phi_region->in(i4) == nullptr || igvn->type(phi_region->in(i4)) == Type::TOP) {
+      igvn->_worklist.push(iff);
+      return nullptr;
+    }
     if( con1->is_Con() ) break; // Found a constant
     // Also allow null-vs-not-null checks
     const TypePtr *tp = igvn->type(con1)->isa_ptr();
@@ -115,7 +120,7 @@ static Node* split_if(IfNode *iff, PhaseIterGVN *igvn) {
 
   // No intervening control, like a simple Call
   Node* r = iff->in(0);
-  if (!r->is_Region() || r->is_Loop() || phi->region() != r || r->as_Region()->is_copy()) {
+  if (!r->is_Region() || r->is_Loop() || phi_region != r || r->as_Region()->is_copy()) {
     return nullptr;
   }
 

src/hotspot/share/opto/library_call.cpp

@@ -4017,9 +4017,9 @@ bool LibraryCallKit::inline_unsafe_newArray(bool uninitialized) {
     CallJavaNode* slow_call = nullptr;
     if (uninitialized) {
       // Generate optimized virtual call (holder class 'Unsafe' is final)
-      slow_call = generate_method_call(vmIntrinsics::_allocateUninitializedArray, false, false);
+      slow_call = generate_method_call(vmIntrinsics::_allocateUninitializedArray, false, false, true);
     } else {
-      slow_call = generate_method_call_static(vmIntrinsics::_newArray);
+      slow_call = generate_method_call_static(vmIntrinsics::_newArray, true);
     }
     Node* slow_result = set_results_for_java_call(slow_call);
     // this->control() comes from set_results_for_java_call
@@ -4264,7 +4264,7 @@ Node* LibraryCallKit::generate_virtual_guard(Node* obj_klass,
 // not another intrinsic.  (E.g., don't use this for making an
 // arraycopy call inside of the copyOf intrinsic.)
 CallJavaNode*
-LibraryCallKit::generate_method_call(vmIntrinsics::ID method_id, bool is_virtual, bool is_static) {
+LibraryCallKit::generate_method_call(vmIntrinsicID method_id, bool is_virtual, bool is_static, bool res_not_null) {
   // When compiling the intrinsic method itself, do not use this technique.
   guarantee(callee() != C->method(), "cannot make slow-call to self");
 
@@ -4273,6 +4273,14 @@ LibraryCallKit::generate_method_call(vmIntrinsics::ID method_id, bool is_virtual
   guarantee(method_id == method->intrinsic_id(), "must match");
 
   const TypeFunc* tf = TypeFunc::make(method);
+  if (res_not_null) {
+    assert(tf->return_type() == T_OBJECT, "");
+    const TypeTuple* range = tf->range();
+    const Type** fields = TypeTuple::fields(range->cnt());
+    fields[TypeFunc::Parms] = range->field_at(TypeFunc::Parms)->filter_speculative(TypePtr::NOTNULL);
+    const TypeTuple* new_range = TypeTuple::make(range->cnt(), fields);
+    tf = TypeFunc::make(tf->domain(), new_range);
+  }
   CallJavaNode* slow_call;
   if (is_static) {
     assert(!is_virtual, "");
@@ -4422,7 +4430,7 @@ bool LibraryCallKit::inline_native_hashcode(bool is_virtual, bool is_static) {
     // No need for PreserveJVMState, because we're using up the present state.
     set_all_memory(init_mem);
     vmIntrinsics::ID hashCode_id = is_static ? vmIntrinsics::_identityHashCode : vmIntrinsics::_hashCode;
-    CallJavaNode* slow_call = generate_method_call(hashCode_id, is_virtual, is_static);
+    CallJavaNode* slow_call = generate_method_call(hashCode_id, is_virtual, is_static, false);
     Node* slow_result = set_results_for_java_call(slow_call);
     // this->control() comes from set_results_for_java_call
     result_reg->init_req(_slow_path, control());
@@ -4948,7 +4956,7 @@ bool LibraryCallKit::inline_native_clone(bool is_virtual) {
     set_control(_gvn.transform(slow_region));
     if (!stopped()) {
       PreserveJVMState pjvms(this);
-      CallJavaNode* slow_call = generate_method_call(vmIntrinsics::_clone, is_virtual);
+      CallJavaNode* slow_call = generate_method_call(vmIntrinsics::_clone, is_virtual, false, true);
       // We need to deoptimize on exception (see comment above)
       Node* slow_result = set_results_for_java_call(slow_call, false, /* deoptimize */ true);
       // this->control() comes from set_results_for_java_call

src/hotspot/share/opto/library_call.hpp

@@ -176,13 +176,9 @@ class LibraryCallKit : public GraphKit {
   Node* generate_array_guard_common(Node* kls, RegionNode* region,
                                     bool obj_array, bool not_array);
   Node* generate_virtual_guard(Node* obj_klass, RegionNode* slow_region);
-  CallJavaNode* generate_method_call(vmIntrinsics::ID method_id,
-                                     bool is_virtual = false, bool is_static = false);
-  CallJavaNode* generate_method_call_static(vmIntrinsics::ID method_id) {
-    return generate_method_call(method_id, false, true);
-  }
-  CallJavaNode* generate_method_call_virtual(vmIntrinsics::ID method_id) {
-    return generate_method_call(method_id, true, false);
+  CallJavaNode* generate_method_call(vmIntrinsicID method_id, bool is_virtual, bool is_static, bool res_not_null);
+  CallJavaNode* generate_method_call_static(vmIntrinsicID method_id, bool res_not_null) {
+    return generate_method_call(method_id, false, true, res_not_null);
   }
   Node* load_field_from_object(Node* fromObj, const char* fieldName, const char* fieldTypeString, DecoratorSet decorators = IN_HEAP, bool is_static = false, ciInstanceKlass* fromKls = nullptr);
   Node* field_address_from_object(Node* fromObj, const char* fieldName, const char* fieldTypeString, bool is_exact = true, bool is_static = false, ciInstanceKlass* fromKls = nullptr);

src/hotspot/share/opto/subtypenode.cpp

@@ -34,6 +34,7 @@
 
 const Type* SubTypeCheckNode::sub(const Type* sub_t, const Type* super_t) const {
   const TypeKlassPtr* superk = super_t->isa_klassptr();
+  assert(sub_t != Type::TOP && !TypePtr::NULL_PTR->higher_equal(sub_t), "should be not null");
   const TypeKlassPtr* subk = sub_t->isa_klassptr() ? sub_t->is_klassptr() : sub_t->is_oopptr()->as_klass_type();
 
   // Oop can't be a subtype of abstract type that has no subclass.

test/hotspot/jtreg/compiler/splitif/TestCrashAtIGVNSplitIfSubType.java

@@ -0,0 +1,74 @@
+/*
+ * Copyright (c) 2023, Red Hat, Inc. 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 8303279
+ * @summary C2: crash in SubTypeCheckNode::sub() at IGVN split if
+ * @run main/othervm -XX:-TieredCompilation -XX:-BackgroundCompilation -XX:+UnlockDiagnosticVMOptions -XX:+StressIGVN -XX:StressSeed=598200189 TestCrashAtIGVNSplitIfSubType
+ * @run main/othervm -XX:-TieredCompilation -XX:-BackgroundCompilation -XX:+UnlockDiagnosticVMOptions -XX:+StressIGVN TestCrashAtIGVNSplitIfSubType
+ */
+
+public class TestCrashAtIGVNSplitIfSubType {
+    private static volatile int barrier;
+
+    public static void main(String[] args) {
+        A a = new A();
+        B b = new B();
+        for (int i = 0; i < 20_000; i++) {
+            test(a);
+            test(b);
+            testHelper1(null, 0);
+        }
+    }
+
+    private static void test(Object o) {
+        int i = 2;
+        for (; i < 4; i *= 2) {
+
+        }
+        o = testHelper1(o, i);
+        if (o instanceof A) {
+            barrier = 0x42;
+        }
+    }
+
+    private static Object testHelper1(Object o, int i) {
+        if (i < 3) {
+            o = null;
+        } else {
+            if (o == null) {
+            }
+        }
+        if (i < 2) {
+            barrier = 42;
+        }
+        return o;
+    }
+
+    private static class A {
+    }
+
+    private static class B {
+    }
+}