8264268: Don't use oop types for derived pointers

Reviewed-by: jrose, kbarrett
This commit is contained in:
Stefan Karlsson 2021-03-29 13:27:47 +00:00
parent 3516c2650c
commit 019080e470
5 changed files with 72 additions and 51 deletions

View File

@ -50,6 +50,24 @@
#include "jvmci/jvmci_globals.hpp"
#endif
static_assert(sizeof(oop) == sizeof(intptr_t), "Derived pointer sanity check");
static inline intptr_t derived_pointer_value(derived_pointer p) {
return static_cast<intptr_t>(p);
}
static inline derived_pointer to_derived_pointer(oop obj) {
return static_cast<derived_pointer>(cast_from_oop<intptr_t>(obj));
}
static inline intptr_t operator-(derived_pointer p, derived_pointer p1) {
return derived_pointer_value(p) - derived_pointer_value(p1);
}
static inline derived_pointer operator+(derived_pointer p, intptr_t offset) {
return static_cast<derived_pointer>(derived_pointer_value(p) + offset);
}
// OopMapStream
OopMapStream::OopMapStream(OopMap* oop_map) {
@ -193,23 +211,24 @@ void OopMapSet::add_gc_map(int pc_offset, OopMap *map ) {
add(map);
}
static void add_derived_oop(oop* base, oop* derived, OopClosure* oop_fn) {
static void add_derived_oop(oop* base, derived_pointer* derived, OopClosure* oop_fn) {
#if COMPILER2_OR_JVMCI
DerivedPointerTable::add(derived, base);
#endif // COMPILER2_OR_JVMCI
}
static void ignore_derived_oop(oop* base, oop* derived, OopClosure* oop_fn) {
static void ignore_derived_oop(oop* base, derived_pointer* derived, OopClosure* oop_fn) {
}
static void process_derived_oop(oop* base, oop* derived, OopClosure* oop_fn) {
static void process_derived_oop(oop* base, derived_pointer* derived, OopClosure* oop_fn) {
// 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.
uintptr_t offset = cast_from_oop<uintptr_t>(*derived) - cast_from_oop<uintptr_t>(*base);
*derived = *base;
oop_fn->do_oop(derived);
*derived = cast_to_oop(cast_from_oop<uintptr_t>(*derived) + offset);
derived_pointer derived_base = to_derived_pointer(*base);
intptr_t offset = *derived - derived_base;
*derived = derived_base;
oop_fn->do_oop((oop*)derived);
*derived = *derived + offset;
}
@ -244,21 +263,20 @@ static void trace_codeblob_maps(const frame *fr, const RegisterMap *reg_map) {
void OopMapSet::oops_do(const frame *fr, const RegisterMap* reg_map, OopClosure* f, DerivedPointerIterationMode mode) {
switch (mode) {
case DerivedPointerIterationMode::_directly:
all_do(fr, reg_map, f, process_derived_oop, &do_nothing_cl);
all_do(fr, reg_map, f, process_derived_oop);
break;
case DerivedPointerIterationMode::_with_table:
all_do(fr, reg_map, f, add_derived_oop, &do_nothing_cl);
all_do(fr, reg_map, f, add_derived_oop);
break;
case DerivedPointerIterationMode::_ignore:
all_do(fr, reg_map, f, ignore_derived_oop, &do_nothing_cl);
all_do(fr, reg_map, f, ignore_derived_oop);
break;
}
}
void OopMapSet::all_do(const frame *fr, const RegisterMap *reg_map,
OopClosure* oop_fn, void derived_oop_fn(oop*, oop*, OopClosure*),
OopClosure* value_fn) {
OopClosure* oop_fn, void derived_oop_fn(oop*, derived_pointer*, OopClosure*)) {
CodeBlob* cb = fr->cb();
assert(cb != NULL, "no codeblob");
@ -284,10 +302,9 @@ void OopMapSet::all_do(const frame *fr, const RegisterMap *reg_map,
}
#endif
#endif // !COMPILER2
oop* loc = fr->oopmapreg_to_location(omv.reg(),reg_map);
guarantee(loc != NULL, "missing saved register");
oop *derived_loc = loc;
oop *base_loc = fr->oopmapreg_to_location(omv.content_reg(), reg_map);
derived_pointer* derived_loc = (derived_pointer*)fr->oopmapreg_to_location(omv.reg(),reg_map);
guarantee(derived_loc != NULL, "missing saved register");
oop* base_loc = fr->oopmapreg_to_oop_location(omv.content_reg(), reg_map);
// Ignore NULL oops and decoded NULL narrow oops which
// equal to CompressedOops::base() when a narrow oop
// implicit null check is used in compiled code.
@ -303,7 +320,7 @@ void OopMapSet::all_do(const frame *fr, const RegisterMap *reg_map,
// We want coop and oop oop_types
for (OopMapStream oms(map); !oms.is_done(); oms.next()) {
OopMapValue omv = oms.current();
oop* loc = fr->oopmapreg_to_location(omv.reg(),reg_map);
oop* loc = fr->oopmapreg_to_oop_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
@ -366,7 +383,7 @@ void OopMapSet::update_register_map(const frame *fr, RegisterMap *reg_map) {
OopMapValue omv = oms.current();
if (omv.type() == OopMapValue::callee_saved_value) {
VMReg reg = omv.content_reg();
oop* loc = fr->oopmapreg_to_location(omv.reg(), reg_map);
oop* loc = fr->oopmapreg_to_oop_location(omv.reg(), reg_map);
reg_map->set_location(reg, (address) loc);
DEBUG_ONLY(nof_callee++;)
}
@ -657,17 +674,17 @@ ImmutableOopMapSet* ImmutableOopMapSet::build_from(const OopMapSet* oopmap_set)
#if COMPILER2_OR_JVMCI
class DerivedPointerTable::Entry : public CHeapObj<mtCompiler> {
oop* _location; // Location of derived pointer, also pointing to base
intptr_t _offset; // Offset from base pointer
Entry* volatile _next;
derived_pointer* _location; // Location of derived pointer, also pointing to base
intptr_t _offset; // Offset from base pointer
Entry* volatile _next;
static Entry* volatile* next_ptr(Entry& entry) { return &entry._next; }
public:
Entry(oop* location, intptr_t offset) :
Entry(derived_pointer* location, intptr_t offset) :
_location(location), _offset(offset), _next(NULL) {}
oop* location() const { return _location; }
derived_pointer* location() const { return _location; }
intptr_t offset() const { return _offset; }
Entry* next() const { return _next; }
@ -695,18 +712,15 @@ void DerivedPointerTable::clear() {
_active = true;
}
// Returns value of location as an int
inline intptr_t value_of_loc(oop *pointer) {
return cast_from_oop<intptr_t>((*pointer));
}
void DerivedPointerTable::add(oop *derived_loc, oop *base_loc) {
void DerivedPointerTable::add(derived_pointer* derived_loc, oop *base_loc) {
assert(Universe::heap()->is_in_or_null(*base_loc), "not an oop");
assert(derived_loc != base_loc, "Base and derived in same location");
assert(*derived_loc != (void*)base_loc, "location already added");
assert(derived_loc != (void*)base_loc, "Base and derived in same location");
derived_pointer base_loc_as_derived_pointer =
static_cast<derived_pointer>(reinterpret_cast<intptr_t>(base_loc));
assert(*derived_loc != base_loc_as_derived_pointer, "location already added");
assert(Entry::_list != NULL, "list must exist");
assert(is_active(), "table must be active here");
intptr_t offset = value_of_loc(derived_loc) - value_of_loc(base_loc);
intptr_t offset = *derived_loc - to_derived_pointer(*base_loc);
// This assert is invalid because derived pointers can be
// arbitrarily far away from their base.
// assert(offset >= -1000000, "wrong derived pointer info");
@ -716,11 +730,11 @@ void DerivedPointerTable::add(oop *derived_loc, oop *base_loc) {
"Add derived pointer@" INTPTR_FORMAT
" - Derived: " INTPTR_FORMAT
" Base: " INTPTR_FORMAT " (@" INTPTR_FORMAT ") (Offset: " INTX_FORMAT ")",
p2i(derived_loc), p2i(*derived_loc), p2i(*base_loc), p2i(base_loc), offset
p2i(derived_loc), derived_pointer_value(*derived_loc), p2i(*base_loc), p2i(base_loc), offset
);
}
// Set derived oop location to point to base.
*derived_loc = cast_to_oop(base_loc);
*derived_loc = base_loc_as_derived_pointer;
Entry* entry = new Entry(derived_loc, offset);
Entry::_list->push(*entry);
}
@ -731,19 +745,20 @@ void DerivedPointerTable::update_pointers() {
while (entries != NULL) {
Entry* entry = entries;
entries = entry->next();
oop* derived_loc = entry->location();
derived_pointer* derived_loc = entry->location();
intptr_t offset = entry->offset();
// The derived oop was setup to point to location of base
oop base = **(oop**)derived_loc;
oop base = **reinterpret_cast<oop**>(derived_loc);
assert(Universe::heap()->is_in_or_null(base), "must be an oop");
*derived_loc = cast_to_oop(cast_from_oop<address>(base) + offset);
assert(value_of_loc(derived_loc) - value_of_loc(&base) == offset, "sanity check");
derived_pointer derived_base = to_derived_pointer(base);
*derived_loc = derived_base + offset;
assert(*derived_loc - derived_base == offset, "sanity check");
if (TraceDerivedPointers) {
tty->print_cr("Updating derived pointer@" INTPTR_FORMAT
" - Derived: " INTPTR_FORMAT " Base: " INTPTR_FORMAT " (Offset: " INTX_FORMAT ")",
p2i(derived_loc), p2i(*derived_loc), p2i(base), offset);
p2i(derived_loc), derived_pointer_value(*derived_loc), p2i(base), offset);
}
// Delete entry

View File

@ -45,6 +45,8 @@ class frame;
class RegisterMap;
class OopClosure;
enum class derived_pointer : intptr_t {};
class OopMapValue: public StackObj {
friend class VMStructs;
private:
@ -229,8 +231,7 @@ class OopMapSet : public ResourceObj {
// Iterates through frame for a compiled method for dead ones and values, too
static void all_do(const frame* fr, const RegisterMap* reg_map,
OopClosure* oop_fn,
void derived_oop_fn(oop* base, oop* derived, OopClosure* oop_fn),
OopClosure* value_fn);
void derived_oop_fn(oop* base, derived_pointer* derived, OopClosure* oop_fn));
// Printing
void print_on(outputStream* st) const;
@ -413,12 +414,12 @@ class DerivedPointerTable : public AllStatic {
friend class VMStructs;
private:
class Entry;
static bool _active; // do not record pointers for verify pass etc.
static bool _active; // do not record pointers for verify pass etc.
public:
static void clear(); // Called before scavenge/GC
static void add(oop *derived, oop *base); // Called during scavenge/GC
static void update_pointers(); // Called after scavenge/GC
static void clear(); // Called before scavenge/GC
static void add(derived_pointer* derived, oop *base); // Called during scavenge/GC
static void update_pointers(); // Called after scavenge/GC
static bool is_empty();
static bool is_active() { return _active; }
static void set_active(bool value) { _active = value; }

View File

@ -942,7 +942,7 @@ class CompiledArgumentOopFinder: public SignatureIterator {
// Extract low order register number from register array.
// In LP64-land, the high-order bits are valid but unhelpful.
VMReg reg = _regs[_offset].first();
oop *loc = _fr.oopmapreg_to_location(reg, _reg_map);
oop *loc = _fr.oopmapreg_to_oop_location(reg, _reg_map);
assert(loc != NULL, "missing register map entry");
_f->do_oop(loc);
}
@ -997,7 +997,7 @@ oop frame::retrieve_receiver(RegisterMap* reg_map) {
// First consult the ADLC on where it puts parameter 0 for this signature.
VMReg reg = SharedRuntime::name_for_receiver();
oop* oop_adr = caller.oopmapreg_to_location(reg, reg_map);
oop* oop_adr = caller.oopmapreg_to_oop_location(reg, reg_map);
if (oop_adr == NULL) {
guarantee(oop_adr != NULL, "bad register save location");
return NULL;

View File

@ -364,7 +364,8 @@ class frame {
void describe(FrameValues& values, int frame_no);
// Conversion from a VMReg to physical stack location
oop* oopmapreg_to_location(VMReg reg, const RegisterMap* reg_map) const;
address oopmapreg_to_location(VMReg reg, const RegisterMap* reg_map) const;
oop* oopmapreg_to_oop_location(VMReg reg, const RegisterMap* reg_map) const;
// Oops-do's
void oops_compiled_arguments_do(Symbol* signature, bool has_receiver, bool has_appendix, const RegisterMap* reg_map, OopClosure* f) const;

View File

@ -51,16 +51,20 @@ inline bool frame::is_first_frame() const {
return is_entry_frame() && entry_frame_is_first();
}
inline oop* frame::oopmapreg_to_location(VMReg reg, const RegisterMap* reg_map) const {
inline address frame::oopmapreg_to_location(VMReg reg, const RegisterMap* reg_map) const {
if(reg->is_reg()) {
// If it is passed in a register, it got spilled in the stub frame.
return (oop *)reg_map->location(reg);
return reg_map->location(reg);
} else {
int sp_offset_in_bytes = reg->reg2stack() * VMRegImpl::stack_slot_size;
return (oop*)(((address)unextended_sp()) + sp_offset_in_bytes);
return ((address)unextended_sp()) + sp_offset_in_bytes;
}
}
inline oop* frame::oopmapreg_to_oop_location(VMReg reg, const RegisterMap* reg_map) const {
return (oop*)oopmapreg_to_location(reg, reg_map);
}
inline bool StackFrameStream::is_done() {
return (_is_done) ? true : (_is_done = _fr.is_first_frame(), false);
}