From cee4bd3ee610492aaa96cb0c5fcf2c32a1c12e2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20=C3=96sterlund?= Date: Mon, 30 Jan 2023 10:28:40 +0000 Subject: [PATCH] 8301047: Clean up type unsafe uses of oop from compiler code Co-authored-by: Axel Boldt-Christmas Co-authored-by: Stefan Karlsson Reviewed-by: kvn, stefank --- src/hotspot/share/code/relocInfo.hpp | 2 +- src/hotspot/share/compiler/oopMap.cpp | 10 +++++----- src/hotspot/share/compiler/oopMap.hpp | 4 ++-- src/hotspot/share/compiler/oopMap.inline.hpp | 19 ++++++++++--------- .../share/utilities/globalDefinitions.hpp | 1 - 5 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/hotspot/share/code/relocInfo.hpp b/src/hotspot/share/code/relocInfo.hpp index f4041c4415a..ef8027760d7 100644 --- a/src/hotspot/share/code/relocInfo.hpp +++ b/src/hotspot/share/code/relocInfo.hpp @@ -1014,7 +1014,7 @@ class oop_Relocation : public DataRelocation { void verify_oop_relocation(); - address value() override { return cast_from_oop
(*oop_addr()); } + address value() override { return *reinterpret_cast(oop_addr()); } bool oop_is_immediate() { return oop_index() == 0; } diff --git a/src/hotspot/share/compiler/oopMap.cpp b/src/hotspot/share/compiler/oopMap.cpp index 7567d2374c9..a6b8a156450 100644 --- a/src/hotspot/share/compiler/oopMap.cpp +++ b/src/hotspot/share/compiler/oopMap.cpp @@ -58,8 +58,8 @@ static inline intptr_t derived_pointer_value(derived_pointer p) { return static_cast(p); } -static inline derived_pointer to_derived_pointer(oop obj) { - return static_cast(cast_from_oop(obj)); +static inline derived_pointer to_derived_pointer(intptr_t obj) { + return static_cast(obj); } 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. // 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. - derived_pointer derived_base = to_derived_pointer(*base); + derived_pointer derived_base = to_derived_pointer(*reinterpret_cast(base)); intptr_t offset = *derived - derived_base; *derived = derived_base; _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(Entry::_list != nullptr, "list must exist"); 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(base_loc)); // This assert is invalid because derived pointers can be // arbitrarily far away from their base. // assert(offset >= -1000000, "wrong derived pointer info"); @@ -954,7 +954,7 @@ void DerivedPointerTable::update_pointers() { oop base = **reinterpret_cast(derived_loc); 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(base)); *derived_loc = derived_base + offset; assert(*derived_loc - derived_base == offset, "sanity check"); diff --git a/src/hotspot/share/compiler/oopMap.hpp b/src/hotspot/share/compiler/oopMap.hpp index b4fbda7abd9..3c23943e097 100644 --- a/src/hotspot/share/compiler/oopMap.hpp +++ b/src/hotspot/share/compiler/oopMap.hpp @@ -449,12 +449,12 @@ private: class SkipNullValue { public: - static inline bool should_skip(oop val); + static inline bool should_skip(void* val); }; class IncludeAllValues { public: - static bool should_skip(oop value) { return false; } + static bool should_skip(void* value) { return false; } }; template diff --git a/src/hotspot/share/compiler/oopMap.inline.hpp b/src/hotspot/share/compiler/oopMap.inline.hpp index d2d1066e761..3f8a81d773b 100644 --- a/src/hotspot/share/compiler/oopMap.inline.hpp +++ b/src/hotspot/share/compiler/oopMap.inline.hpp @@ -47,8 +47,8 @@ inline const ImmutableOopMap* ImmutableOopMapPair::get_from(const ImmutableOopMa return set->oopmap_at_offset(_oopmap_offset); } -inline bool SkipNullValue::should_skip(oop val) { - return val == (oop)nullptr || CompressedOops::is_base(val); +inline bool SkipNullValue::should_skip(void* val) { + return val == nullptr || (UseCompressedOops && CompressedOops::is_base(val)); } template @@ -84,14 +84,15 @@ void OopMapDo::iterate_oops_do(const frame } guarantee(loc != nullptr, "missing saved register"); derived_pointer* derived_loc = (derived_pointer*)loc; - oop* base_loc = fr->oopmapreg_to_oop_location(omv.content_reg(), reg_map); - // Ignore nullptr oops and decoded nullptr narrow oops which + void** base_loc = (void**) fr->oopmapreg_to_location(omv.content_reg(), reg_map); + + // Ignore nullptr oops and decoded null narrow oops which // equal to CompressedOops::base() when a narrow oop // implicit null check is used in compiled code. // The narrow_oop_base could be nullptr or be the address // of the page below heap depending on compressed oops mode. - if (base_loc != nullptr && *base_loc != (oop)nullptr && !CompressedOops::is_base(*base_loc)) { - _derived_oop_fn->do_derived_oop(base_loc, derived_loc); + if (base_loc != nullptr && !SkipNullValue::should_skip(*base_loc)) { + _derived_oop_fn->do_derived_oop((oop*)base_loc, derived_loc); } } } @@ -102,7 +103,7 @@ void OopMapDo::iterate_oops_do(const frame OopMapValue omv = oms.current(); if (omv.type() != OopMapValue::oop_value && omv.type() != OopMapValue::narrowoop_value) 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 // register mentioned as contained an oop of some kind. Maybe // this was allowed previously because value_value items might @@ -122,7 +123,7 @@ void OopMapDo::iterate_oops_do(const frame } guarantee(loc != nullptr, "missing saved register"); 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) // Ignore nullptr oops and decoded nullptr narrow oops which // equal to CompressedOops::base() when a narrow oop @@ -131,7 +132,7 @@ void OopMapDo::iterate_oops_do(const frame // of the page below heap depending on compressed oops mode. continue; } - _oop_fn->do_oop(loc); + _oop_fn->do_oop((oop*)loc); } else if ( omv.type() == OopMapValue::narrowoop_value ) { narrowOop *nl = (narrowOop*)loc; #ifndef VM_LITTLE_ENDIAN diff --git a/src/hotspot/share/utilities/globalDefinitions.hpp b/src/hotspot/share/utilities/globalDefinitions.hpp index 899aa45aea1..4f6e8bb191f 100644 --- a/src/hotspot/share/utilities/globalDefinitions.hpp +++ b/src/hotspot/share/utilities/globalDefinitions.hpp @@ -1063,7 +1063,6 @@ const int badCodeHeapFreeVal = 0xDD; // value used to zap // (These must be implemented as #defines because C++ compilers are // not obligated to inline non-integral constants!) #define badAddress ((address)::badAddressVal) -#define badOop (cast_to_oop(::badOopVal)) #define badHeapWord (::badHeapWordVal) // Default TaskQueue size is 16K (32-bit) or 128K (64-bit)