8299125: UnifiedOopRef in JFR leakprofiler should treat thread local oops correctly

Reviewed-by: eosterlund, mgronlun

Author: xmas92

src/hotspot/share/jfr/leakprofiler/chains/rootSetClosure.cpp

@@ -48,7 +48,7 @@ template <typename Delegate>
 void RootSetClosure<Delegate>::do_oop(oop* ref) {
   assert(ref != NULL, "invariant");
   assert(is_aligned(ref, HeapWordSize), "invariant");
-  if (*ref != NULL) {
+  if (NativeAccess<>::oop_load(ref) != nullptr) {
     _delegate->do_root(UnifiedOopRef::encode_in_native(ref));
   }
 }
@@ -64,14 +64,42 @@ void RootSetClosure<Delegate>::do_oop(narrowOop* ref) {
 
 class RootSetClosureMarkScope : public MarkScope {};
 
+template <typename Delegate>
+class RawRootClosure : public OopClosure {
+  Delegate* _delegate;
+
+public:
+  RawRootClosure(Delegate* delegate) : _delegate(delegate) {}
+
+  void do_oop(oop* ref) {
+    assert(ref != NULL, "invariant");
+    assert(is_aligned(ref, HeapWordSize), "invariant");
+    if (*ref != nullptr) {
+      _delegate->do_root(UnifiedOopRef::encode_as_raw(ref));
+    }
+  }
+
+  void do_oop(narrowOop* ref) {
+    assert(ref != NULL, "invariant");
+    assert(is_aligned(ref, HeapWordSize), "invariant");
+    if (!CompressedOops::is_null(*ref)) {
+      _delegate->do_root(UnifiedOopRef::encode_as_raw(ref));
+    }
+  }
+};
+
 template <typename Delegate>
 void RootSetClosure<Delegate>::process() {
   RootSetClosureMarkScope mark_scope;
+
   CLDToOopClosure cldt_closure(this, ClassLoaderData::_claim_none);
   ClassLoaderDataGraph::always_strong_cld_do(&cldt_closure);
-  // We don't follow code blob oops, because they have misaligned oops.
-  Threads::oops_do(this, NULL);
+
   OopStorageSet::strong_oops_do(this);
+
+  // We don't follow code blob oops, because they have misaligned oops.
+  RawRootClosure<Delegate> rrc(_delegate);
+  Threads::oops_do(&rrc, NULL);
 }
 
 template class RootSetClosure<BFSClosure>;

src/hotspot/share/jfr/leakprofiler/utilities/unifiedOopRef.hpp

@@ -29,21 +29,33 @@
 #include "utilities/globalDefinitions.hpp"
 
 struct UnifiedOopRef {
+  static const uintptr_t tag_mask   = LP64_ONLY(0b111) NOT_LP64(0b011);
+  static const uintptr_t native_tag = 0b001;
+  static const uintptr_t raw_tag    = 0b010;
+  static const uintptr_t narrow_tag = LP64_ONLY(0b100) NOT_LP64(0);
+  STATIC_ASSERT((native_tag & raw_tag) == 0);
+  STATIC_ASSERT((native_tag & narrow_tag) == 0);
+  STATIC_ASSERT((raw_tag & narrow_tag) == 0);
+  STATIC_ASSERT((native_tag | raw_tag | narrow_tag) == tag_mask);
+
   uintptr_t _value;
 
   template <typename T>
   T addr() const;
 
   bool is_narrow() const;
   bool is_native() const;
+  bool is_raw() const;
   bool is_null() const;
 
   oop dereference() const;
 
   static UnifiedOopRef encode_in_native(const narrowOop* ref);
   static UnifiedOopRef encode_in_native(const oop* ref);
-  static UnifiedOopRef encode_in_heap(const oop* ref);
+  static UnifiedOopRef encode_as_raw(const narrowOop* ref);
+  static UnifiedOopRef encode_as_raw(const oop* ref);
   static UnifiedOopRef encode_in_heap(const narrowOop* ref);
+  static UnifiedOopRef encode_in_heap(const oop* ref);
   static UnifiedOopRef encode_null();
 };
 

src/hotspot/share/jfr/leakprofiler/utilities/unifiedOopRef.inline.hpp

@@ -32,7 +32,7 @@
 
 template <typename T>
 inline T UnifiedOopRef::addr() const {
-  return reinterpret_cast<T>(_value & ~uintptr_t(3));
+  return reinterpret_cast<T>(UnifiedOopRef::addr<uintptr_t>());
 }
 
 // Visual Studio 2019 and earlier have a problem with reinterpret_cast
@@ -42,47 +42,70 @@ inline T UnifiedOopRef::addr() const {
 // this specialization provides a workaround.
 template<>
 inline uintptr_t UnifiedOopRef::addr<uintptr_t>() const {
-  return _value & ~uintptr_t(3);
+  return (_value & ~tag_mask) LP64_ONLY(>> 1);
 }
 
 inline bool UnifiedOopRef::is_narrow() const {
-  return _value & 1;
+  return (_value & narrow_tag) != 0;
 }
 
 inline bool UnifiedOopRef::is_native() const {
-  return _value & 2;
+  return (_value & native_tag) != 0;
+}
+
+inline bool UnifiedOopRef::is_raw() const {
+  return (_value & raw_tag) != 0;
 }
 
 inline bool UnifiedOopRef::is_null() const {
   return _value == 0;
 }
 
-inline UnifiedOopRef UnifiedOopRef::encode_in_native(const narrowOop* ref) {
+template <typename T>
+inline UnifiedOopRef create_with_tag(T ref, uintptr_t tag) {
   assert(ref != NULL, "invariant");
-  UnifiedOopRef result = { reinterpret_cast<uintptr_t>(ref) | 3 };
-  assert(result.addr<narrowOop*>() == ref, "sanity");
+
+  uintptr_t value = reinterpret_cast<uintptr_t>(ref);
+
+#ifdef _LP64
+  // tag_mask is 3 bits. When ref is a narrowOop* we only have 2 alignment
+  // bits, because of the 4 byte alignment of compressed oops addresses.
+  // Shift up to make way for one more bit.
+  assert((value & (1ull << 63)) == 0, "Unexpected high-order bit");
+  value <<= 1;
+#endif
+  assert((value & UnifiedOopRef::tag_mask) == 0, "Unexpected low-order bits");
+
+  UnifiedOopRef result = { value | tag };
+  assert(result.addr<T>() == ref, "sanity");
   return result;
 }
 
+inline UnifiedOopRef UnifiedOopRef::encode_in_native(const narrowOop* ref) {
+  NOT_LP64(ShouldNotReachHere());
+  return create_with_tag(ref, native_tag | narrow_tag);
+}
+
 inline UnifiedOopRef UnifiedOopRef::encode_in_native(const oop* ref) {
-  assert(ref != NULL, "invariant");
-  UnifiedOopRef result = { reinterpret_cast<uintptr_t>(ref) | 2 };
-  assert(result.addr<oop*>() == ref, "sanity");
-  return result;
+  return create_with_tag(ref, native_tag);
+}
+
+inline UnifiedOopRef UnifiedOopRef::encode_as_raw(const narrowOop* ref) {
+  NOT_LP64(ShouldNotReachHere());
+  return create_with_tag(ref, raw_tag | narrow_tag);
+}
+
+inline UnifiedOopRef UnifiedOopRef::encode_as_raw(const oop* ref) {
+  return create_with_tag(ref, raw_tag);
 }
 
 inline UnifiedOopRef UnifiedOopRef::encode_in_heap(const narrowOop* ref) {
-  assert(ref != NULL, "invariant");
-  UnifiedOopRef result = { reinterpret_cast<uintptr_t>(ref) | 1 };
-  assert(result.addr<narrowOop*>() == ref, "sanity");
-  return result;
+  NOT_LP64(ShouldNotReachHere());
+  return create_with_tag(ref, narrow_tag);
 }
 
 inline UnifiedOopRef UnifiedOopRef::encode_in_heap(const oop* ref) {
-  assert(ref != NULL, "invariant");
-  UnifiedOopRef result = { reinterpret_cast<uintptr_t>(ref) | 0 };
-  assert(result.addr<oop*>() == ref, "sanity");
-  return result;
+  return create_with_tag(ref, 0);
 }
 
 inline UnifiedOopRef UnifiedOopRef::encode_null() {
@@ -91,14 +114,23 @@ inline UnifiedOopRef UnifiedOopRef::encode_null() {
 }
 
 inline oop UnifiedOopRef::dereference() const {
-  if (is_native()) {
+  if (is_raw()) {
+    if (is_narrow()) {
+      NOT_LP64(ShouldNotReachHere());
+      return RawAccess<>::oop_load(addr<narrowOop*>());
+    } else {
+      return *addr<oop*>();
+    }
+  } else if (is_native()) {
     if (is_narrow()) {
+      NOT_LP64(ShouldNotReachHere());
       return NativeAccess<AS_NO_KEEPALIVE>::oop_load(addr<narrowOop*>());
     } else {
       return NativeAccess<AS_NO_KEEPALIVE>::oop_load(addr<oop*>());
     }
   } else {
     if (is_narrow()) {
+      NOT_LP64(ShouldNotReachHere());
       return HeapAccess<AS_NO_KEEPALIVE>::oop_load(addr<narrowOop*>());
     } else {
       return HeapAccess<AS_NO_KEEPALIVE>::oop_load(addr<oop*>());

src/hotspot/share/oops/accessBackend.hpp

@@ -1253,6 +1253,14 @@ namespace AccessInternal {
     inline bool operator !=(const T& other) const {
       return load<decorators | INTERNAL_VALUE_IS_OOP, P, T>(_addr) != other;
     }
+
+    inline bool operator ==(std::nullptr_t) const {
+      return load<decorators | INTERNAL_VALUE_IS_OOP, P, oop>(_addr) == nullptr;
+    }
+
+    inline bool operator !=(std::nullptr_t) const {
+      return load<decorators | INTERNAL_VALUE_IS_OOP, P, oop>(_addr) != nullptr;
+    }
   };
 
   // Infer the type that should be returned from an Access::load_at.