stan/jdk-24
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

8305880: Loom: Avoid putting stale object pointers in oops

Browse Source 
Reviewed-by: eosterlund, aboldtch
This commit is contained in:
Stefan Karlsson 2023-04-20 09:18:28 +00:00
parent 310aa93478
commit 6a7dff30ed
10 changed files with 46 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
src/hotspot/share/compiler/oopMap.cpp
View File

@ -392,7 +392,7 @@ class AddDerivedOop : public DerivedOopClosure {
SkipNull = true, NeedsLock = true
};
virtual void do_derived_oop(oop* base, derived_pointer* derived) {
virtual void do_derived_oop(derived_base* base, derived_pointer* derived) {
#if COMPILER2_OR_JVMCI
DerivedPointerTable::add(derived, base);
#endif // COMPILER2_OR_JVMCI
@ -410,7 +410,7 @@ public:
SkipNull = true, NeedsLock = true
};
virtual void do_derived_oop(oop* base, derived_pointer* derived) {
virtual void do_derived_oop(derived_base* base, derived_pointer* derived) {
// 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.
@ -430,7 +430,7 @@ public:
SkipNull = true, NeedsLock = true
};
virtual void do_derived_oop(oop* base, derived_pointer* derived) {}
virtual void do_derived_oop(derived_base* base, derived_pointer* derived) {}
};
void OopMapSet::oops_do(const frame* fr, const RegisterMap* reg_map, OopClosure* f, DerivedPointerIterationMode mode) {
@ -915,8 +915,8 @@ void DerivedPointerTable::clear() {
_active = true;
}
void DerivedPointerTable::add(derived_pointer* derived_loc, oop *base_loc) {
assert(Universe::heap()->is_in_or_null(*base_loc), "not an oop");
void DerivedPointerTable::add(derived_pointer* derived_loc, derived_base* base_loc) {
assert(Universe::heap()->is_in_or_null((void*)*base_loc), "not an oop");
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));
@ -933,7 +933,7 @@ void DerivedPointerTable::add(derived_pointer* derived_loc, oop *base_loc) {
"Add derived pointer@" INTPTR_FORMAT
" - Derived: " INTPTR_FORMAT
" Base: " INTPTR_FORMAT " (@" INTPTR_FORMAT ") (Offset: " INTX_FORMAT ")",
p2i(derived_loc), derived_pointer_value(*derived_loc), p2i(*base_loc), p2i(base_loc), offset
p2i(derived_loc), derived_pointer_value(*derived_loc), intptr_t(*base_loc), p2i(base_loc), offset
);
}
// Set derived oop location to point to base.

9
src/hotspot/share/compiler/oopMap.hpp
View File

@ -48,6 +48,7 @@ class OopClosure;
class CodeBlob;
class ImmutableOopMap;
enum class derived_base : intptr_t {};
enum class derived_pointer : intptr_t {};
class OopMapValue: public StackObj {
@ -481,12 +482,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(derived_pointer* 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, derived_base* 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; }

6
src/hotspot/share/compiler/oopMap.inline.hpp
View File

@ -84,15 +84,15 @@ void OopMapDo<OopFnT, DerivedOopFnT, ValueFilterT>::iterate_oops_do(const frame
}
guarantee(loc != nullptr, "missing saved register");
derived_pointer* derived_loc = (derived_pointer*)loc;
void** base_loc = (void**) fr->oopmapreg_to_location(omv.content_reg(), reg_map);
derived_base* base_loc = (derived_base*) 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 && !SkipNullValue::should_skip(*base_loc)) {
_derived_oop_fn->do_derived_oop((oop*)base_loc, derived_loc);
if (base_loc != nullptr && !SkipNullValue::should_skip((void*)*base_loc)) {
_derived_oop_fn->do_derived_oop(base_loc, derived_loc);
}
}
}

3
src/hotspot/share/memory/iterator.hpp
View File

@ -129,11 +129,12 @@ public:
virtual void oops_do(OopClosure* cl) = 0;
};
enum class derived_base : intptr_t;
enum class derived_pointer : intptr_t;
class DerivedOopClosure : public Closure {
public:
enum { SkipNull = true };
virtual void do_derived_oop(oop* base, derived_pointer* derived) = 0;
virtual void do_derived_oop(derived_base* base, derived_pointer* derived) = 0;
};
class KlassClosure : public Closure {

31
src/hotspot/share/oops/stackChunkOop.cpp
View File

@ -38,6 +38,11 @@
#include "runtime/smallRegisterMap.inline.hpp"
#include "runtime/stackChunkFrameStream.inline.hpp"
// Note: Some functions in this file work with stale object pointers, e.g.
// DerivedPointerSupport. Be extra careful to not put those pointers into
// variables of the 'oop' type. There's extra GC verification around oops
// that may fail when stale oops are being used.
template <typename RegisterMapT>
class FrameOopIterator : public OopIterator {
private:
@ -153,43 +158,45 @@ template void stackChunkOopDesc::do_barriers<stackChunkOopDesc::BarrierType::Sto
class DerivedPointersSupport {
public:
static void relativize(oop* base_loc, derived_pointer* derived_loc) {
oop base = *base_loc;
if (base == nullptr) {
static void relativize(derived_base* base_loc, derived_pointer* derived_loc) {
// The base oop could be stale from the GC's point-of-view. Treat it as an
// uintptr_t to stay clear of the oop verification code in oopsHierarcy.hpp.
uintptr_t base = *(uintptr_t*)base_loc;
if (base == 0) {
return;
}
assert(!UseCompressedOops || !CompressedOops::is_base(base), "");
assert(!UseCompressedOops || !CompressedOops::is_base((void*)base), "");
// This is always a full derived pointer
uintptr_t derived_int_val = *(uintptr_t*)derived_loc;
// Make the pointer an offset (relativize) and store it at the same location
uintptr_t offset = derived_int_val - cast_from_oop<uintptr_t>(base);
uintptr_t offset = derived_int_val - base;
*(uintptr_t*)derived_loc = offset;
}
static void derelativize(oop* base_loc, derived_pointer* derived_loc) {
oop base = *base_loc;
if (base == nullptr) {
static void derelativize(derived_base* base_loc, derived_pointer* derived_loc) {
uintptr_t base = *(uintptr_t*)base_loc;
if (base == 0) {
return;
}
assert(!UseCompressedOops || !CompressedOops::is_base(base), "");
assert(!UseCompressedOops || !CompressedOops::is_base((void*)base), "");
// All derived pointers should have been relativized into offsets
uintptr_t offset = *(uintptr_t*)derived_loc;
// Restore the original derived pointer
*(uintptr_t*)derived_loc = cast_from_oop<uintptr_t>(base) + offset;
*(uintptr_t*)derived_loc = base + offset;
}
struct RelativizeClosure : public DerivedOopClosure {
virtual void do_derived_oop(oop* base_loc, derived_pointer* derived_loc) override {
virtual void do_derived_oop(derived_base* base_loc, derived_pointer* derived_loc) override {
DerivedPointersSupport::relativize(base_loc, derived_loc);
}
};
struct DerelativizeClosure : public DerivedOopClosure {
virtual void do_derived_oop(oop* base_loc, derived_pointer* derived_loc) override {
virtual void do_derived_oop(derived_base* base_loc, derived_pointer* derived_loc) override {
DerivedPointersSupport::derelativize(base_loc, derived_loc);
}
};

6
src/hotspot/share/runtime/continuationWrapper.inline.hpp
View File

@ -92,10 +92,8 @@ public:
}
inline ~SafepointOp() { // reload oops
_cont._continuation = _conth();
if (_cont._tail != nullptr) {
_cont._tail = jdk_internal_vm_Continuation::tail(_cont._continuation);
}
_cont.disallow_safepoint();
_cont._tail = jdk_internal_vm_Continuation::tail(_cont._continuation);
_cont.disallow_safepoint();
}
};

8
src/hotspot/share/runtime/frame.cpp
View File

@ -1241,7 +1241,7 @@ class FrameValuesOopClosure: public OopClosure, public DerivedOopClosure {
private:
GrowableArray<oop*>* _oops;
GrowableArray<narrowOop*>* _narrow_oops;
GrowableArray<oop*>* _base;
GrowableArray<derived_base*>* _base;
GrowableArray<derived_pointer*>* _derived;
NoSafepointVerifier nsv;
@ -1249,7 +1249,7 @@ public:
FrameValuesOopClosure() {
_oops = new (mtThread) GrowableArray<oop*>(100, mtThread);
_narrow_oops = new (mtThread) GrowableArray<narrowOop*>(100, mtThread);
_base = new (mtThread) GrowableArray<oop*>(100, mtThread);
_base = new (mtThread) GrowableArray<derived_base*>(100, mtThread);
_derived = new (mtThread) GrowableArray<derived_pointer*>(100, mtThread);
}
~FrameValuesOopClosure() {
@ -1261,7 +1261,7 @@ public:
virtual void do_oop(oop* p) override { _oops->push(p); }
virtual void do_oop(narrowOop* p) override { _narrow_oops->push(p); }
virtual void do_derived_oop(oop* base_loc, derived_pointer* derived_loc) override {
virtual void do_derived_oop(derived_base* base_loc, derived_pointer* derived_loc) override {
_base->push(base_loc);
_derived->push(derived_loc);
}
@ -1281,7 +1281,7 @@ public:
}
assert(_base->length() == _derived->length(), "should be the same");
for (int i = 0; i < _base->length(); i++) {
oop* base = _base->at(i);
derived_base* base = _base->at(i);
derived_pointer* derived = _derived->at(i);
values.describe(frame_no, (intptr_t*)derived, err_msg("derived pointer (base: " INTPTR_FORMAT ") for #%d", p2i(base), frame_no));
}

2
src/hotspot/share/runtime/stackChunkFrameStream.inline.hpp
View File

@ -410,7 +410,7 @@ inline void StackChunkFrameStream<frame_kind>::iterate_derived_pointers(DerivedO
assert(is_in_oops(base_loc, map), "not found: " INTPTR_FORMAT, p2i(base_loc));
assert(!is_in_oops(derived_loc, map), "found: " INTPTR_FORMAT, p2i(derived_loc));
Devirtualizer::do_derived_oop(closure, (oop*)base_loc, (derived_pointer*)derived_loc);
Devirtualizer::do_derived_oop(closure, (derived_base*)base_loc, (derived_pointer*)derived_loc);
}
}

2
src/hotspot/share/utilities/devirtualizer.hpp
View File

@ -38,7 +38,7 @@ class Devirtualizer {
template <typename OopClosureType> static void do_klass(OopClosureType* closure, Klass* k);
template <typename OopClosureType> static void do_cld(OopClosureType* closure, ClassLoaderData* cld);
template <typename OopClosureType> static bool do_metadata(OopClosureType* closure);
template <typename DerivedOopClosureType> static void do_derived_oop(DerivedOopClosureType* closure, oop* base, derived_pointer* derived);
template <typename DerivedOopClosureType> static void do_derived_oop(DerivedOopClosureType* closure, derived_base* base, derived_pointer* derived);
template <typename BitMapClosureType> static bool do_bit(BitMapClosureType* closure, BitMap::idx_t index);
};

6
src/hotspot/share/utilities/devirtualizer.inline.hpp
View File

@ -154,18 +154,18 @@ void Devirtualizer::do_cld(OopClosureType* closure, ClassLoaderData* cld) {
template <typename Receiver, typename Base, typename DerivedOopClosureType>
static typename EnableIf<std::is_same<Receiver, Base>::value, void>::type
call_do_derived_oop(void (Receiver::*)(oop*, derived_pointer*), void (Base::*)(oop*, derived_pointer*), DerivedOopClosureType* closure, oop* base, derived_pointer* derived) {
call_do_derived_oop(void (Receiver::*)(derived_base*, derived_pointer*), void (Base::*)(derived_base*, derived_pointer*), DerivedOopClosureType* closure, derived_base* base, derived_pointer* derived) {
closure->do_derived_oop(base, derived);
}
template <typename Receiver, typename Base, typename DerivedOopClosureType>
static typename EnableIf<!std::is_same<Receiver, Base>::value, void>::type
call_do_derived_oop(void (Receiver::*)(oop*, derived_pointer*), void (Base::*)(oop*, derived_pointer*), DerivedOopClosureType* closure, oop* base, derived_pointer* derived) {
call_do_derived_oop(void (Receiver::*)(derived_base*, derived_pointer*), void (Base::*)(derived_base*, derived_pointer*), DerivedOopClosureType* closure, derived_base* base, derived_pointer* derived) {
closure->DerivedOopClosureType::do_derived_oop(base, derived);
}
template <typename DerivedOopClosureType>
inline void Devirtualizer::do_derived_oop(DerivedOopClosureType* closure, oop* base, derived_pointer* derived) {
inline void Devirtualizer::do_derived_oop(DerivedOopClosureType* closure, derived_base* base, derived_pointer* derived) {
call_do_derived_oop(&DerivedOopClosureType::do_derived_oop, &DerivedOopClosure::do_derived_oop, closure, base, derived);
}