8301047: Clean up type unsafe uses of oop from compiler code

Co-authored-by: Axel Boldt-Christmas <aboldtch@openjdk.org>
Co-authored-by: Stefan Karlsson <stefank@openjdk.org>
Reviewed-by: kvn, stefank
This commit is contained in:
Erik Österlund 2023-01-30 10:28:40 +00:00
parent 7fae3a1d8d
commit cee4bd3ee6
5 changed files with 18 additions and 18 deletions

View File

@ -1014,7 +1014,7 @@ class oop_Relocation : public DataRelocation {
void verify_oop_relocation(); void verify_oop_relocation();
address value() override { return cast_from_oop<address>(*oop_addr()); } address value() override { return *reinterpret_cast<address*>(oop_addr()); }
bool oop_is_immediate() { return oop_index() == 0; } bool oop_is_immediate() { return oop_index() == 0; }

View File

@ -58,8 +58,8 @@ static inline intptr_t derived_pointer_value(derived_pointer p) {
return static_cast<intptr_t>(p); return static_cast<intptr_t>(p);
} }
static inline derived_pointer to_derived_pointer(oop obj) { static inline derived_pointer to_derived_pointer(intptr_t obj) {
return static_cast<derived_pointer>(cast_from_oop<intptr_t>(obj)); return static_cast<derived_pointer>(obj);
} }
static inline intptr_t operator-(derived_pointer p, derived_pointer p1) { static inline intptr_t operator-(derived_pointer p, derived_pointer p1) {
@ -414,7 +414,7 @@ public:
// All derived pointers must be processed before the base pointer of any derived pointer is processed. // All derived pointers must be processed before the base pointer of any derived pointer is processed.
// Otherwise, if two derived pointers use the same base, the second derived pointer will get an obscured // Otherwise, if two derived pointers use the same base, the second derived pointer will get an obscured
// offset, if the base pointer is processed in the first derived pointer. // offset, if the base pointer is processed in the first derived pointer.
derived_pointer derived_base = to_derived_pointer(*base); derived_pointer derived_base = to_derived_pointer(*reinterpret_cast<intptr_t*>(base));
intptr_t offset = *derived - derived_base; intptr_t offset = *derived - derived_base;
*derived = derived_base; *derived = derived_base;
_oop_cl->do_oop((oop*)derived); _oop_cl->do_oop((oop*)derived);
@ -923,7 +923,7 @@ void DerivedPointerTable::add(derived_pointer* derived_loc, oop *base_loc) {
assert(*derived_loc != base_loc_as_derived_pointer, "location already added"); assert(*derived_loc != base_loc_as_derived_pointer, "location already added");
assert(Entry::_list != nullptr, "list must exist"); assert(Entry::_list != nullptr, "list must exist");
assert(is_active(), "table must be active here"); assert(is_active(), "table must be active here");
intptr_t offset = *derived_loc - to_derived_pointer(*base_loc); intptr_t offset = *derived_loc - to_derived_pointer(*reinterpret_cast<intptr_t*>(base_loc));
// This assert is invalid because derived pointers can be // This assert is invalid because derived pointers can be
// arbitrarily far away from their base. // arbitrarily far away from their base.
// assert(offset >= -1000000, "wrong derived pointer info"); // assert(offset >= -1000000, "wrong derived pointer info");
@ -954,7 +954,7 @@ void DerivedPointerTable::update_pointers() {
oop base = **reinterpret_cast<oop**>(derived_loc); oop base = **reinterpret_cast<oop**>(derived_loc);
assert(Universe::heap()->is_in_or_null(base), "must be an oop"); assert(Universe::heap()->is_in_or_null(base), "must be an oop");
derived_pointer derived_base = to_derived_pointer(base); derived_pointer derived_base = to_derived_pointer(cast_from_oop<intptr_t>(base));
*derived_loc = derived_base + offset; *derived_loc = derived_base + offset;
assert(*derived_loc - derived_base == offset, "sanity check"); assert(*derived_loc - derived_base == offset, "sanity check");

View File

@ -449,12 +449,12 @@ private:
class SkipNullValue { class SkipNullValue {
public: public:
static inline bool should_skip(oop val); static inline bool should_skip(void* val);
}; };
class IncludeAllValues { class IncludeAllValues {
public: public:
static bool should_skip(oop value) { return false; } static bool should_skip(void* value) { return false; }
}; };
template <typename OopFnT, typename DerivedOopFnT, typename ValueFilterT> template <typename OopFnT, typename DerivedOopFnT, typename ValueFilterT>

View File

@ -47,8 +47,8 @@ inline const ImmutableOopMap* ImmutableOopMapPair::get_from(const ImmutableOopMa
return set->oopmap_at_offset(_oopmap_offset); return set->oopmap_at_offset(_oopmap_offset);
} }
inline bool SkipNullValue::should_skip(oop val) { inline bool SkipNullValue::should_skip(void* val) {
return val == (oop)nullptr || CompressedOops::is_base(val); return val == nullptr || (UseCompressedOops && CompressedOops::is_base(val));
} }
template <typename OopFnT, typename DerivedOopFnT, typename ValueFilterT> template <typename OopFnT, typename DerivedOopFnT, typename ValueFilterT>
@ -84,14 +84,15 @@ void OopMapDo<OopFnT, DerivedOopFnT, ValueFilterT>::iterate_oops_do(const frame
} }
guarantee(loc != nullptr, "missing saved register"); guarantee(loc != nullptr, "missing saved register");
derived_pointer* derived_loc = (derived_pointer*)loc; derived_pointer* derived_loc = (derived_pointer*)loc;
oop* base_loc = fr->oopmapreg_to_oop_location(omv.content_reg(), reg_map); void** base_loc = (void**) fr->oopmapreg_to_location(omv.content_reg(), reg_map);
// Ignore nullptr oops and decoded nullptr narrow oops which
// Ignore nullptr oops and decoded null narrow oops which
// equal to CompressedOops::base() when a narrow oop // equal to CompressedOops::base() when a narrow oop
// implicit null check is used in compiled code. // implicit null check is used in compiled code.
// The narrow_oop_base could be nullptr or be the address // The narrow_oop_base could be nullptr or be the address
// of the page below heap depending on compressed oops mode. // of the page below heap depending on compressed oops mode.
if (base_loc != nullptr && *base_loc != (oop)nullptr && !CompressedOops::is_base(*base_loc)) { if (base_loc != nullptr && !SkipNullValue::should_skip(*base_loc)) {
_derived_oop_fn->do_derived_oop(base_loc, derived_loc); _derived_oop_fn->do_derived_oop((oop*)base_loc, derived_loc);
} }
} }
} }
@ -102,7 +103,7 @@ void OopMapDo<OopFnT, DerivedOopFnT, ValueFilterT>::iterate_oops_do(const frame
OopMapValue omv = oms.current(); OopMapValue omv = oms.current();
if (omv.type() != OopMapValue::oop_value && omv.type() != OopMapValue::narrowoop_value) if (omv.type() != OopMapValue::oop_value && omv.type() != OopMapValue::narrowoop_value)
continue; continue;
oop* loc = fr->oopmapreg_to_oop_location(omv.reg(),reg_map); void** loc = (void**) fr->oopmapreg_to_location(omv.reg(),reg_map);
// It should be an error if no location can be found for a // It should be an error if no location can be found for a
// register mentioned as contained an oop of some kind. Maybe // register mentioned as contained an oop of some kind. Maybe
// this was allowed previously because value_value items might // this was allowed previously because value_value items might
@ -122,7 +123,7 @@ void OopMapDo<OopFnT, DerivedOopFnT, ValueFilterT>::iterate_oops_do(const frame
} }
guarantee(loc != nullptr, "missing saved register"); guarantee(loc != nullptr, "missing saved register");
if ( omv.type() == OopMapValue::oop_value ) { if ( omv.type() == OopMapValue::oop_value ) {
oop val = *loc; void* val = *loc;
if (ValueFilterT::should_skip(val)) { // TODO: UGLY (basically used to decide if we're freezing/thawing continuation) if (ValueFilterT::should_skip(val)) { // TODO: UGLY (basically used to decide if we're freezing/thawing continuation)
// Ignore nullptr oops and decoded nullptr narrow oops which // Ignore nullptr oops and decoded nullptr narrow oops which
// equal to CompressedOops::base() when a narrow oop // equal to CompressedOops::base() when a narrow oop
@ -131,7 +132,7 @@ void OopMapDo<OopFnT, DerivedOopFnT, ValueFilterT>::iterate_oops_do(const frame
// of the page below heap depending on compressed oops mode. // of the page below heap depending on compressed oops mode.
continue; continue;
} }
_oop_fn->do_oop(loc); _oop_fn->do_oop((oop*)loc);
} else if ( omv.type() == OopMapValue::narrowoop_value ) { } else if ( omv.type() == OopMapValue::narrowoop_value ) {
narrowOop *nl = (narrowOop*)loc; narrowOop *nl = (narrowOop*)loc;
#ifndef VM_LITTLE_ENDIAN #ifndef VM_LITTLE_ENDIAN

View File

@ -1063,7 +1063,6 @@ const int badCodeHeapFreeVal = 0xDD; // value used to zap
// (These must be implemented as #defines because C++ compilers are // (These must be implemented as #defines because C++ compilers are
// not obligated to inline non-integral constants!) // not obligated to inline non-integral constants!)
#define badAddress ((address)::badAddressVal) #define badAddress ((address)::badAddressVal)
#define badOop (cast_to_oop(::badOopVal))
#define badHeapWord (::badHeapWordVal) #define badHeapWord (::badHeapWordVal)
// Default TaskQueue size is 16K (32-bit) or 128K (64-bit) // Default TaskQueue size is 16K (32-bit) or 128K (64-bit)