Skip to content

8375441: [lworld] C2: assert(is_instance()) failed: bad cast #1923

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
marc-chevalier wants to merge 11 commits into
base: lworld
Choose a base branch
from
+244 −112
Open

8375441: [lworld] C2: assert(is_instance()) failed: bad cast #1923

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
f051e94
Shield against null
marc-chevalier Jan 16, 2026
4f1b58e
expand_constant
marc-chevalier Jan 23, 2026
dcea06c
Clean up
marc-chevalier Jan 26, 2026
02c1df6
explicit casts
marc-chevalier Jan 26, 2026
cb72b4f
ShouldNotReachHere
marc-chevalier Jan 26, 2026
a887dc0
Remove debug
marc-chevalier Jan 26, 2026
2c2b4bf
fix comment
marc-chevalier Jan 26, 2026
cc039f1
Also cache correctly whole stable fields
marc-chevalier Jan 27, 2026
a5e7079
oops
marc-chevalier Jan 27, 2026
e4f7ace
exception handling
marc-chevalier Jan 27, 2026
99c32a3
Details
marc-chevalier Jan 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions src/hotspot/share/ci/ciConstant.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,26 @@
//
// This class represents a constant value.

ciConstant ciConstant::make_zero_or_null(BasicType bt) {
switch (bt) {
case T_FLOAT: return ciConstant((jfloat).0f);
case T_DOUBLE: return ciConstant((jdouble).0);
case T_LONG: return ciConstant((jlong)0L);
case T_BOOLEAN:
case T_CHAR:
case T_BYTE:
case T_SHORT:
case T_INT:
return ciConstant(bt, 0);
case T_OBJECT:
case T_ARRAY:
return ciConstant(bt, CURRENT_ENV->get_object(nullptr));
default:
ShouldNotReachHere();
return ciConstant();
}
}

// ------------------------------------------------------------------
// ciConstant::is_null_or_zero
// This assumes `this->is_valid()`, otherwise, `as_object` will assert.
Expand Down
1 change: 1 addition & 0 deletions src/hotspot/share/ci/ciConstant.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ class ciConstant {
return _value._object;
}

static ciConstant make_zero_or_null(BasicType);
bool is_null_or_zero() const;

bool is_valid() const {
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/ci/ciField.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ ciConstant ciField::constant_value() {
if (_constant_value.basic_type() == T_ILLEGAL) {
// Static fields are placed in mirror objects.
ciInstance* mirror = _holder->java_mirror();
_constant_value = mirror->field_value_impl(type()->basic_type(), offset_in_bytes());
_constant_value = mirror->field_value_impl(type()->basic_type(), offset_in_bytes(), false);
}
if (FoldStableValues && is_stable() && _constant_value.is_null_or_zero()) {
return ciConstant();
Expand Down
4 changes: 1 addition & 3 deletions src/hotspot/share/ci/ciField.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ class ciField : public ArenaObj {
// In that case the declared holder of f would be B and
// the canonical holder of f would be A.
ciInstanceKlass* holder() const { return _holder; }
ciInstanceKlass* original_holder() const { return _original_holder; }

// Name of this field?
ciSymbol* name() const { return _name; }
Expand All @@ -110,9 +111,6 @@ class ciField : public ArenaObj {
// How is this field actually stored in memory?
BasicType layout_type() { return type2field[(_type == nullptr) ? T_OBJECT : _type->basic_type()]; }

// How big is this field in memory?
int size_in_bytes() { return type2aelembytes(layout_type()); }

// What is the offset of this field? (Fields are aligned to the byte level.)
int offset_in_bytes() const {
assert(_offset >= 1, "illegal call to offset()");
Expand Down
78 changes: 25 additions & 53 deletions src/hotspot/share/ci/ciFlatArray.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,53 +30,12 @@
#include "ci/ciUtilities.inline.hpp"
#include "oops/oop.inline.hpp"

ciConstant ciFlatArray::null_marker_of_element_by_offset_impl(arrayOop ary, int index) {
if (ary == nullptr) {
return ciConstant();
}
assert(ary->is_array(), "");
if (index < 0 || index >= ary->length()) {
return ciConstant();
}
assert(ary->is_objArray(), "");
flatArrayOop objary = (flatArrayOop) ary;
jboolean elem = objary->null_marker_of_obj_at(index);
return ciConstant(T_BOOLEAN, elem);
}

ciConstant ciFlatArray::check_constant_null_marker_cache(int off) {
if (_constant_null_markers != nullptr) {
for (int i = 0; i < _constant_null_markers->length(); ++i) {
ConstantValue cached_val = _constant_null_markers->at(i);
if (cached_val.off() == off) {
return cached_val.value();
}
}
}
return ciConstant();
}

void ciFlatArray::add_to_constant_null_marker_cache(int off, ciConstant val) {
assert(val.is_valid(), "value must be valid");
assert(!check_constant_value_cache(off, val.basic_type()).is_valid(), "duplicate");
if (_constant_null_markers == nullptr) {
Arena* arena = CURRENT_ENV->arena();
_constant_null_markers = new (arena) GrowableArray<ConstantValue>(arena, 1, 0, ConstantValue());
}
_constant_null_markers->append(ConstantValue(off, val));
}

// Current value of an element.
// Returns T_ILLEGAL if there is no element at the given index.
ciConstant ciFlatArray::null_marker_of_element_by_index(int index) {
ciConstant value = check_constant_null_marker_cache(index);
if (value.is_valid()) {
return value;
}
GUARDED_VM_ENTRY(
value = null_marker_of_element_by_offset_impl(get_arrayOop(), index);)
add_to_constant_null_marker_cache(index, value);
return value;
ciConstant nm = field_value(index, nullptr);
postcond(!nm.is_valid() || nm.basic_type() == T_BOOLEAN);
return nm;
}

ciConstant ciFlatArray::null_marker_of_element_by_offset(intptr_t element_offset) {
Expand Down Expand Up @@ -133,23 +92,36 @@ ciConstant ciFlatArray::field_value_by_offset(intptr_t field_offset) {
}

ciConstant ciFlatArray::field_value(int index, ciField* field) {
auto get_field_from_object_constant = [field](const ciConstant& v) -> ciConstant {
Copy link
Member

@merykitty merykitty Jan 20, 2026

Choose a reason for hiding this comment

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

I don't really agree with this fix, ciFlatArray::field_value should be dumber, it is the caller who knows that we do not fold the load if the element is null, the callee should just return the field as it is.

Copy link
Member Author

@marc-chevalier marc-chevalier Jan 20, 2026

Choose a reason for hiding this comment

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

I fear I don't understand. Let's say, I have a flat array MyValue[] arr where MyValue is a value class with a single field f. Let's also assume arr[0] == null, arr.field_value(0, f) (assuming the ci... versions of it with matching names) tries to get the constant value of the field f of arr[0], and arr[0].f is not null, it's rather undefined. It's not about stability and folding. On the other hand, if arr[0] is not null, but arr[0].f is null, arr.field_value(0, f) already returns null (the ciConstant that means that).

Am I missing something?

Copy link
Member

@merykitty merykitty Jan 20, 2026

Choose a reason for hiding this comment

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

That is only true from the Java perspective. From the VM perspective, a flat array would be something like (C++ pseudocode):

class MyValuePayload {
  oop* f;
  bool null_marker;
};

MyValuePayload* arr = new MyValuePayload[n];

Then, it is clear that even if arr[0].null_marker == false, arr[0].f is still defined and has a value (which should be nullptr).

Copy link
Member Author

@marc-chevalier marc-chevalier Jan 20, 2026

Choose a reason for hiding this comment

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

As discussed offline, I'm trying change that.

ciObject* obj = v.as_object();
if (obj->is_null_object()) {
if (field == nullptr) {
return ciConstant::make_zero_or_null(T_BOOLEAN);
}
return ciConstant::make_zero_or_null(field->type()->basic_type());
}
// obj cannot be an ciArray since it is an element of a flat array, so it must be a value class, which arrays are not.
ciInstance* inst = obj->as_instance();
if (field == nullptr) {
return ciConstant(T_BOOLEAN, 1);
}
return inst->field_value(field);
};

BasicType elembt = element_basic_type();
ciConstant value = check_constant_value_cache(index, elembt);
if (value.is_valid()) {
if (field == nullptr) {
return value.as_object()->as_instance()->null_marker_value();
}
return value.as_object()->as_instance()->field_value(field);
return get_field_from_object_constant(value);
}
GUARDED_VM_ENTRY(
value = element_value_impl(T_OBJECT, get_arrayOop(), index);
)

add_to_constant_value_cache(index, value);

if (field == nullptr) {
return value.as_object()->as_instance()->null_marker_value();
if (!value.is_valid()) {
return ciConstant();
}
return value.as_object()->as_instance()->field_value(field);

add_to_constant_value_cache(index, value);
return get_field_from_object_constant(value);
}

7 changes: 0 additions & 7 deletions src/hotspot/share/ci/ciFlatArray.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,6 @@ class ciFlatArray : public ciArray {
ciConstant field_value(int index, ciField* field);
ciConstant null_marker_of_element_by_offset(intptr_t element_offset);
ciConstant null_marker_of_element_by_index(int index);

private:
ciConstant null_marker_of_element_by_offset_impl(arrayOop ary, int index);
ciConstant check_constant_null_marker_cache(int off);
void add_to_constant_null_marker_cache(int off, ciConstant val);

GrowableArray<ConstantValue>* _constant_null_markers = nullptr;
};

#endif // SHARE_VM_CI_CIFLATARRAY_HPP
138 changes: 115 additions & 23 deletions src/hotspot/share/ci/ciInstance.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
#include "ci/ciUtilities.inline.hpp"
#include "classfile/javaClasses.inline.hpp"
#include "classfile/vmClasses.hpp"
#include "oops/fieldStreams.hpp"
#include "oops/fieldStreams.inline.hpp"
#include "oops/oop.inline.hpp"

// ciInstance
Expand Down Expand Up @@ -59,7 +61,7 @@ ciType* ciInstance::java_mirror_type() {

// ------------------------------------------------------------------
// ciInstance::field_value_impl
ciConstant ciInstance::field_value_impl(BasicType field_btype, int offset) {
ciConstant ciInstance::field_value_impl(BasicType field_btype, int offset, bool field_is_flat) {
ciConstant value = check_constant_value_cache(offset, field_btype);
if (value.is_valid()) {
return value;
Expand All @@ -78,19 +80,39 @@ ciConstant ciInstance::field_value_impl(BasicType field_btype, int offset) {
case T_LONG: value = ciConstant(obj->long_field(offset)); break;
case T_OBJECT: // fall through
case T_ARRAY: {
oop o = obj->obj_field(offset);
if (field_is_flat) {
InstanceKlass* holder = InstanceKlass::cast(obj->klass());
int index = -1;
for (JavaFieldStream fs(holder); !fs.done(); fs.next()) {
if (!fs.access_flags().is_static() && fs.offset() == offset) {
index = fs.index();
break;
}
}
assert(index != -1, "no field at given offset");
InlineLayoutInfo* layout_info = holder->inline_layout_info_adr(index);
InlineKlass* vk = layout_info->klass();
oop res = vk->read_payload_from_addr(obj, offset, layout_info->kind(), THREAD);
if (HAS_PENDING_EXCEPTION) {
CLEAR_PENDING_EXCEPTION;
return ciConstant();
}
value = ciConstant(field_btype, CURRENT_ENV->get_object(res));
} else {
oop o = obj->obj_field(offset);

// A field will be "constant" if it is known always to be
// a non-null reference to an instance of a particular class,
// or to a particular array. This can happen even if the instance
// or array is not perm. In such a case, an "unloaded" ciArray
// or ciInstance is created. The compiler may be able to use
// information about the object's class (which is exact) or length.
// A field will be "constant" if it is known always to be
// a non-null reference to an instance of a particular class,
// or to a particular array. This can happen even if the instance
// or array is not perm. In such a case, an "unloaded" ciArray
// or ciInstance is created. The compiler may be able to use
// information about the object's class (which is exact) or length.

if (o == nullptr) {
value = ciConstant(field_btype, ciNullObject::make());
} else {
value = ciConstant(field_btype, CURRENT_ENV->get_object(o));
if (o == nullptr) {
value = ciConstant(field_btype, ciNullObject::make());
} else {
value = ciConstant(field_btype, CURRENT_ENV->get_object(o));
}
}
break;
}
Expand All @@ -101,26 +123,96 @@ ciConstant ciInstance::field_value_impl(BasicType field_btype, int offset) {
return value;
}

// Constant value of the null marker.
ciConstant ciInstance::null_marker_value() {
if (!klass()->is_inlinetype()) {
return ciConstant();
}
ciInlineKlass* ik = klass()->as_inline_klass();
return field_value_impl(T_BOOLEAN, ik->null_marker_offset_in_payload() + ik->payload_offset());
}

// ------------------------------------------------------------------
// ciInstance::field_value
//
// Constant value of a field.
// Constant value of a field. The field can be flat, in which case a cached copy
// of the value object is returned. Since stable fields are "constant" but not
// really, we need to cache the value of fields so that the compiler will observe
// only one value. We also need to ensure that sub-fields (flattened fields) from
// a single stable field of value class type will be observed to be consistent with
// each other. To do so, we need to always fetch the declared field containing the
// desired field. If we want a sub-field of a flat field, we then extract the field
// out of the cached copy.
ciConstant ciInstance::field_value(ciField* field) {
assert(is_loaded(), "invalid access - must be loaded");
assert(field->holder()->is_loaded(), "invalid access - holder must be loaded");
assert(field->is_static() || field->holder()->is_inlinetype() || klass()->is_subclass_of(field->holder()),
"invalid access - must be subclass");
ciInstanceKlass* klass = this->klass()->as_instance_klass();
ciField* containing_field;
if (field->original_holder() == nullptr) {
containing_field = field;
} else {
int containing_field_idx = klass->field_index_by_offset(field->offset_in_bytes());
containing_field = klass->declared_nonstatic_field_at(containing_field_idx);
}
ciConstant containing_field_value = field_value_impl(containing_field->type()->basic_type(), containing_field->offset_in_bytes(), containing_field->is_flat());
if (!containing_field_value.is_valid()) {
return ciConstant();
}
if (field->original_holder() == nullptr) {
return containing_field_value;
} else {
ciObject* obj = containing_field_value.as_object();
if (obj->is_instance()) {
ciInstance* inst = obj->as_instance();
// inst->klass() must be an inline klass since it is the value of a flat field.
ciInlineKlass* inst_klass = inst->klass()->as_inline_klass();
ciField* field_in_value_klass = inst_klass->get_field_by_offset(inst_klass->payload_offset() + field->offset_in_bytes() - containing_field->offset_in_bytes(), false);
return inst->non_flat_field_value(field_in_value_klass);
} else if (obj->is_null_object()) {
return ciConstant::make_zero_or_null(field->type()->basic_type());
}
// obj should not be an array since we are trying to get a field inside it
ShouldNotReachHere();
return ciConstant();
}
}

return field_value_impl(field->type()->basic_type(), field->offset_in_bytes());
// Extract a field from a value object.
// This won't cache. Must be used only on cached values.
ciConstant ciInstance::non_flat_field_value(ciField* field) {
precond(klass()->is_inlinetype());
precond(!field->is_flat());
int offset = field->offset_in_bytes();
BasicType field_btype = field->type()->basic_type();

ciConstant value;
VM_ENTRY_MARK;
oop obj = get_oop();
assert(obj != nullptr, "bad oop");
switch(field_btype) {
case T_BYTE: value = ciConstant(field_btype, obj->byte_field(offset)); break;
case T_CHAR: value = ciConstant(field_btype, obj->char_field(offset)); break;
case T_SHORT: value = ciConstant(field_btype, obj->short_field(offset)); break;
case T_BOOLEAN: value = ciConstant(field_btype, obj->bool_field(offset)); break;
case T_INT: value = ciConstant(field_btype, obj->int_field(offset)); break;
case T_FLOAT: value = ciConstant(obj->float_field(offset)); break;
case T_DOUBLE: value = ciConstant(obj->double_field(offset)); break;
case T_LONG: value = ciConstant(obj->long_field(offset)); break;
case T_OBJECT: // fall through
case T_ARRAY: {
oop o = obj->obj_field(offset);

// A field will be "constant" if it is known always to be
// a non-null reference to an instance of a particular class,
// or to a particular array. This can happen even if the instance
// or array is not perm. In such a case, an "unloaded" ciArray
// or ciInstance is created. The compiler may be able to use
// information about the object's class (which is exact) or length.

if (o == nullptr) {
value = ciConstant(field_btype, ciNullObject::make());
} else {
value = ciConstant(field_btype, CURRENT_ENV->get_object(o));
}
break;
}
default:
fatal("no field value: %s", type2name(field_btype));
}
return value;
}

// ------------------------------------------------------------------
Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/share/ci/ciInstance.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ class ciInstance : public ciObject {

void print_impl(outputStream* st);

ciConstant field_value_impl(BasicType field_btype, int offset);
ciConstant field_value_impl(BasicType field_btype, int offset, bool field_is_flat);
ciConstant non_flat_field_value(ciField* field);

public:
// If this object is a java mirror, return the corresponding type.
Expand All @@ -65,7 +66,6 @@ class ciInstance : public ciObject {

// Constant value of a field at the specified offset.
ciConstant field_value_by_offset(int field_offset);
ciConstant null_marker_value();

ciKlass* java_lang_Class_klass();
char* java_lang_String_str(char* buf, size_t buflen);
Expand Down
1 change: 0 additions & 1 deletion src/hotspot/share/ci/ciInstanceKlass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,6 @@ ciField* ciInstanceKlass::get_non_flat_field_by_offset(int field_offset) {
}

int ciInstanceKlass::field_index_by_offset(int offset) {
assert(contains_field_offset(offset), "invalid field offset");
int best_offset = 0;
int best_index = -1;
// Search the field with the given offset
Expand Down
Loading