Skip to content

Commit cee4bd3

Browse files
fiskxmas92stefank
committed
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
1 parent 7fae3a1 commit cee4bd3

File tree

5 files changed

+18
-18
lines changed

5 files changed

+18
-18
lines changed

src/hotspot/share/code/relocInfo.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1014,7 +1014,7 @@ class oop_Relocation : public DataRelocation {
10141014

10151015
void verify_oop_relocation();
10161016

1017-
address value() override { return cast_from_oop<address>(*oop_addr()); }
1017+
address value() override { return *reinterpret_cast<address*>(oop_addr()); }
10181018

10191019
bool oop_is_immediate() { return oop_index() == 0; }
10201020

src/hotspot/share/compiler/oopMap.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,8 @@ static inline intptr_t derived_pointer_value(derived_pointer p) {
5858
return static_cast<intptr_t>(p);
5959
}
6060

61-
static inline derived_pointer to_derived_pointer(oop obj) {
62-
return static_cast<derived_pointer>(cast_from_oop<intptr_t>(obj));
61+
static inline derived_pointer to_derived_pointer(intptr_t obj) {
62+
return static_cast<derived_pointer>(obj);
6363
}
6464

6565
static inline intptr_t operator-(derived_pointer p, derived_pointer p1) {
@@ -414,7 +414,7 @@ class ProcessDerivedOop : public DerivedOopClosure {
414414
// All derived pointers must be processed before the base pointer of any derived pointer is processed.
415415
// Otherwise, if two derived pointers use the same base, the second derived pointer will get an obscured
416416
// offset, if the base pointer is processed in the first derived pointer.
417-
derived_pointer derived_base = to_derived_pointer(*base);
417+
derived_pointer derived_base = to_derived_pointer(*reinterpret_cast<intptr_t*>(base));
418418
intptr_t offset = *derived - derived_base;
419419
*derived = derived_base;
420420
_oop_cl->do_oop((oop*)derived);
@@ -923,7 +923,7 @@ void DerivedPointerTable::add(derived_pointer* derived_loc, oop *base_loc) {
923923
assert(*derived_loc != base_loc_as_derived_pointer, "location already added");
924924
assert(Entry::_list != nullptr, "list must exist");
925925
assert(is_active(), "table must be active here");
926-
intptr_t offset = *derived_loc - to_derived_pointer(*base_loc);
926+
intptr_t offset = *derived_loc - to_derived_pointer(*reinterpret_cast<intptr_t*>(base_loc));
927927
// This assert is invalid because derived pointers can be
928928
// arbitrarily far away from their base.
929929
// assert(offset >= -1000000, "wrong derived pointer info");
@@ -954,7 +954,7 @@ void DerivedPointerTable::update_pointers() {
954954
oop base = **reinterpret_cast<oop**>(derived_loc);
955955
assert(Universe::heap()->is_in_or_null(base), "must be an oop");
956956

957-
derived_pointer derived_base = to_derived_pointer(base);
957+
derived_pointer derived_base = to_derived_pointer(cast_from_oop<intptr_t>(base));
958958
*derived_loc = derived_base + offset;
959959
assert(*derived_loc - derived_base == offset, "sanity check");
960960

src/hotspot/share/compiler/oopMap.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -449,12 +449,12 @@ class ImmutableOopMapBuilder {
449449

450450
class SkipNullValue {
451451
public:
452-
static inline bool should_skip(oop val);
452+
static inline bool should_skip(void* val);
453453
};
454454

455455
class IncludeAllValues {
456456
public:
457-
static bool should_skip(oop value) { return false; }
457+
static bool should_skip(void* value) { return false; }
458458
};
459459

460460
template <typename OopFnT, typename DerivedOopFnT, typename ValueFilterT>

src/hotspot/share/compiler/oopMap.inline.hpp

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@ inline const ImmutableOopMap* ImmutableOopMapPair::get_from(const ImmutableOopMa
4747
return set->oopmap_at_offset(_oopmap_offset);
4848
}
4949

50-
inline bool SkipNullValue::should_skip(oop val) {
51-
return val == (oop)nullptr || CompressedOops::is_base(val);
50+
inline bool SkipNullValue::should_skip(void* val) {
51+
return val == nullptr || (UseCompressedOops && CompressedOops::is_base(val));
5252
}
5353

5454
template <typename OopFnT, typename DerivedOopFnT, typename ValueFilterT>
@@ -84,14 +84,15 @@ void OopMapDo<OopFnT, DerivedOopFnT, ValueFilterT>::iterate_oops_do(const frame
8484
}
8585
guarantee(loc != nullptr, "missing saved register");
8686
derived_pointer* derived_loc = (derived_pointer*)loc;
87-
oop* base_loc = fr->oopmapreg_to_oop_location(omv.content_reg(), reg_map);
88-
// Ignore nullptr oops and decoded nullptr narrow oops which
87+
void** base_loc = (void**) fr->oopmapreg_to_location(omv.content_reg(), reg_map);
88+
89+
// Ignore nullptr oops and decoded null narrow oops which
8990
// equal to CompressedOops::base() when a narrow oop
9091
// implicit null check is used in compiled code.
9192
// The narrow_oop_base could be nullptr or be the address
9293
// of the page below heap depending on compressed oops mode.
93-
if (base_loc != nullptr && *base_loc != (oop)nullptr && !CompressedOops::is_base(*base_loc)) {
94-
_derived_oop_fn->do_derived_oop(base_loc, derived_loc);
94+
if (base_loc != nullptr && !SkipNullValue::should_skip(*base_loc)) {
95+
_derived_oop_fn->do_derived_oop((oop*)base_loc, derived_loc);
9596
}
9697
}
9798
}
@@ -102,7 +103,7 @@ void OopMapDo<OopFnT, DerivedOopFnT, ValueFilterT>::iterate_oops_do(const frame
102103
OopMapValue omv = oms.current();
103104
if (omv.type() != OopMapValue::oop_value && omv.type() != OopMapValue::narrowoop_value)
104105
continue;
105-
oop* loc = fr->oopmapreg_to_oop_location(omv.reg(),reg_map);
106+
void** loc = (void**) fr->oopmapreg_to_location(omv.reg(),reg_map);
106107
// It should be an error if no location can be found for a
107108
// register mentioned as contained an oop of some kind. Maybe
108109
// this was allowed previously because value_value items might
@@ -122,7 +123,7 @@ void OopMapDo<OopFnT, DerivedOopFnT, ValueFilterT>::iterate_oops_do(const frame
122123
}
123124
guarantee(loc != nullptr, "missing saved register");
124125
if ( omv.type() == OopMapValue::oop_value ) {
125-
oop val = *loc;
126+
void* val = *loc;
126127
if (ValueFilterT::should_skip(val)) { // TODO: UGLY (basically used to decide if we're freezing/thawing continuation)
127128
// Ignore nullptr oops and decoded nullptr narrow oops which
128129
// equal to CompressedOops::base() when a narrow oop
@@ -131,7 +132,7 @@ void OopMapDo<OopFnT, DerivedOopFnT, ValueFilterT>::iterate_oops_do(const frame
131132
// of the page below heap depending on compressed oops mode.
132133
continue;
133134
}
134-
_oop_fn->do_oop(loc);
135+
_oop_fn->do_oop((oop*)loc);
135136
} else if ( omv.type() == OopMapValue::narrowoop_value ) {
136137
narrowOop *nl = (narrowOop*)loc;
137138
#ifndef VM_LITTLE_ENDIAN

src/hotspot/share/utilities/globalDefinitions.hpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1063,7 +1063,6 @@ const int badCodeHeapFreeVal = 0xDD; // value used to zap
10631063
// (These must be implemented as #defines because C++ compilers are
10641064
// not obligated to inline non-integral constants!)
10651065
#define badAddress ((address)::badAddressVal)
1066-
#define badOop (cast_to_oop(::badOopVal))
10671066
#define badHeapWord (::badHeapWordVal)
10681067

10691068
// Default TaskQueue size is 16K (32-bit) or 128K (64-bit)

0 commit comments

Comments
 (0)