Skip to content

Commit

Permalink
8321974: Crash in ciKlass::is_subtype_of because TypeAryPtr::_klass i…
Browse files Browse the repository at this point in the history
…s not initialized

Reviewed-by: roland, kvn
  • Loading branch information
TobiHartmann committed Dec 14, 2023
1 parent cf94854 commit c8ad7b7
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 35 deletions.
46 changes: 12 additions & 34 deletions src/hotspot/share/opto/type.cpp
Expand Up @@ -4664,7 +4664,7 @@ template <class T1, class T2> bool TypePtr::is_meet_subtype_of_helper_for_array
}

if (other_elem == nullptr && this_elem == nullptr) {
return this_one->_klass->is_subtype_of(other->_klass);
return this_one->klass()->is_subtype_of(other->klass());
}

return false;
Expand Down Expand Up @@ -5993,7 +5993,7 @@ template <class T1, class T2> bool TypePtr::is_java_subtype_of_helper_for_instan
return true;
}

return this_one->_klass->is_subtype_of(other->_klass) && this_one->_interfaces->contains(other->_interfaces);
return this_one->klass()->is_subtype_of(other->klass()) && this_one->_interfaces->contains(other->_interfaces);
}

bool TypeInstKlassPtr::is_java_subtype_of_helper(const TypeKlassPtr* other, bool this_exact, bool other_exact) const {
Expand All @@ -6008,7 +6008,7 @@ template <class T1, class T2> bool TypePtr::is_same_java_type_as_helper_for_inst
if (!this_one->is_instance_type(other)) {
return false;
}
return this_one->_klass->equals(other->_klass) && this_one->_interfaces->eq(other->_interfaces);
return this_one->klass()->equals(other->klass()) && this_one->_interfaces->eq(other->_interfaces);
}

bool TypeInstKlassPtr::is_same_java_type_as_helper(const TypeKlassPtr* other) const {
Expand All @@ -6022,7 +6022,7 @@ template <class T1, class T2> bool TypePtr::maybe_java_subtype_of_helper_for_ins
}

if (this_one->is_array_type(other)) {
return !this_exact && this_one->_klass->equals(ciEnv::current()->Object_klass()) && other->_interfaces->contains(this_one->_interfaces);
return !this_exact && this_one->klass()->equals(ciEnv::current()->Object_klass()) && other->_interfaces->contains(this_one->_interfaces);
}

assert(this_one->is_instance_type(other), "unsupported");
Expand All @@ -6031,12 +6031,12 @@ template <class T1, class T2> bool TypePtr::maybe_java_subtype_of_helper_for_ins
return this_one->is_java_subtype_of(other);
}

if (!this_one->_klass->is_subtype_of(other->_klass) && !other->_klass->is_subtype_of(this_one->_klass)) {
if (!this_one->klass()->is_subtype_of(other->klass()) && !other->klass()->is_subtype_of(this_one->klass())) {
return false;
}

if (this_exact) {
return this_one->_klass->is_subtype_of(other->_klass) && this_one->_interfaces->contains(other->_interfaces);
return this_one->klass()->is_subtype_of(other->klass()) && this_one->_interfaces->contains(other->_interfaces);
}

return true;
Expand Down Expand Up @@ -6116,7 +6116,7 @@ uint TypeAryKlassPtr::hash(void) const {

//----------------------compute_klass------------------------------------------
// Compute the defining klass for this class
ciKlass* TypeAryPtr::compute_klass(DEBUG_ONLY(bool verify)) const {
ciKlass* TypeAryPtr::compute_klass() const {
// Compute _klass based on element type.
ciKlass* k_ary = nullptr;
const TypeInstPtr *tinst;
Expand All @@ -6137,28 +6137,7 @@ ciKlass* TypeAryPtr::compute_klass(DEBUG_ONLY(bool verify)) const {
// and object; Top occurs when doing join on Bottom.
// Leave k_ary at null.
} else {
// Cannot compute array klass directly from basic type,
// since subtypes of TypeInt all have basic type T_INT.
#ifdef ASSERT
if (verify && el->isa_int()) {
// Check simple cases when verifying klass.
BasicType bt = T_ILLEGAL;
if (el == TypeInt::BYTE) {
bt = T_BYTE;
} else if (el == TypeInt::SHORT) {
bt = T_SHORT;
} else if (el == TypeInt::CHAR) {
bt = T_CHAR;
} else if (el == TypeInt::INT) {
bt = T_INT;
} else {
return _klass; // just return specified klass
}
return ciTypeArrayKlass::make(bt);
}
#endif
assert(!el->isa_int(),
"integral arrays must be pre-equipped with a class");
assert(!el->isa_int(), "integral arrays must be pre-equipped with a class");
// Compute array klass directly from basic type
k_ary = ciTypeArrayKlass::make(el->basic_type());
}
Expand Down Expand Up @@ -6434,7 +6413,7 @@ template <class T1, class T2> bool TypePtr::is_java_subtype_of_helper_for_array(
return this_one->is_reference_type(this_elem)->is_java_subtype_of_helper(this_one->is_reference_type(other_elem), this_exact, other_exact);
}
if (this_elem == nullptr && other_elem == nullptr) {
return this_one->_klass->is_subtype_of(other->_klass);
return this_one->klass()->is_subtype_of(other->klass());
}
return false;
}
Expand Down Expand Up @@ -6466,8 +6445,7 @@ template <class T1, class T2> bool TypePtr::is_same_java_type_as_helper_for_arra
return this_one->is_reference_type(this_elem)->is_same_java_type_as(this_one->is_reference_type(other_elem));
}
if (other_elem == nullptr && this_elem == nullptr) {
assert(this_one->_klass != nullptr && other->_klass != nullptr, "");
return this_one->_klass->equals(other->_klass);
return this_one->klass()->equals(other->klass());
}
return false;
}
Expand All @@ -6487,7 +6465,7 @@ template <class T1, class T2> bool TypePtr::maybe_java_subtype_of_helper_for_arr
return true;
}
if (this_one->is_instance_type(other)) {
return other->_klass->equals(ciEnv::current()->Object_klass()) && other->_interfaces->intersection_with(this_one->_interfaces)->eq(other->_interfaces);
return other->klass()->equals(ciEnv::current()->Object_klass()) && other->_interfaces->intersection_with(this_one->_interfaces)->eq(other->_interfaces);
}
assert(this_one->is_array_type(other), "");

Expand All @@ -6506,7 +6484,7 @@ template <class T1, class T2> bool TypePtr::maybe_java_subtype_of_helper_for_arr
return this_one->is_reference_type(this_elem)->maybe_java_subtype_of_helper(this_one->is_reference_type(other_elem), this_exact, other_exact);
}
if (other_elem == nullptr && this_elem == nullptr) {
return this_one->_klass->is_subtype_of(other->_klass);
return this_one->klass()->is_subtype_of(other->klass());
}
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/opto/type.hpp
Expand Up @@ -1413,7 +1413,7 @@ class TypeAryPtr : public TypeOopPtr {
const TypeAry *_ary; // Array we point into
const bool _is_autobox_cache;

ciKlass* compute_klass(DEBUG_ONLY(bool verify = false)) const;
ciKlass* compute_klass() const;

// A pointer to delay allocation to Type::Initialize_shared()

Expand Down
54 changes: 54 additions & 0 deletions test/hotspot/jtreg/compiler/c2/TestUninitializedKlassField.java
@@ -0,0 +1,54 @@
/*
* Copyright (c) 2023, 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. Oracle designates this
* particular file as subject to the "Classpath" exception as provided
* by Oracle in the LICENSE file that accompanied this code.
*
* 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 8321974
* @summary Test that the TypeAryPtr::_klass field is properly initialized on use.
* @comment This test only reproduces the issue with release builds of the JVM because
* verification code in debug builds leads to eager initialization of the field.
* @run main/othervm -Xbatch -XX:CompileCommand=compileonly,TestUninitializedKlassField::test*
* -XX:-TieredCompilation TestUninitializedKlassField
*/

public class TestUninitializedKlassField {
static void test(long array2[]) {
long array1[] = new long[1];
// Loop is needed to create a backedge phi that is processed only during IGVN.
for (int i = 0; i < 1; i++) {
// CmpPNode::sub will check if classes of array1 and array2 are unrelated
// and the subtype checks will crash if the _klass field is not initialized.
if (array2 != array1) {
array1 = new long[2];
}
}
}

public static void main(String[] args) {
for (int i = 0; i < 50_000; ++i) {
test(null);
}
}
}

1 comment on commit c8ad7b7

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.