From 1c0f0f4211cf564c46753d2cb187c1ef485751cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20=C3=96sterlund?= Date: Thu, 11 Aug 2022 11:45:11 +0000 Subject: [PATCH] 8292077: G1 nmethod entry barriers don't keep oops alive Reviewed-by: tschatzl, stefank, vlivanov --- .../share/gc/shared/barrierSetNMethod.cpp | 32 ++++++++++++------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/src/hotspot/share/gc/shared/barrierSetNMethod.cpp b/src/hotspot/share/gc/shared/barrierSetNMethod.cpp index 40a26a592c6..ac29f93f772 100644 --- a/src/hotspot/share/gc/shared/barrierSetNMethod.cpp +++ b/src/hotspot/share/gc/shared/barrierSetNMethod.cpp @@ -28,8 +28,10 @@ #include "gc/shared/barrierSet.hpp" #include "gc/shared/barrierSetAssembler.hpp" #include "gc/shared/barrierSetNMethod.hpp" +#include "gc/shared/collectedHeap.hpp" #include "logging/log.hpp" #include "memory/iterator.hpp" +#include "memory/universe.hpp" #include "oops/access.inline.hpp" #include "oops/method.inline.hpp" #include "runtime/frame.inline.hpp" @@ -38,14 +40,6 @@ #include "runtime/threads.hpp" #include "utilities/debug.hpp" -class LoadPhantomOopClosure : public OopClosure { -public: - virtual void do_oop(oop* p) { - NativeAccess::oop_load(p); - } - virtual void do_oop(narrowOop* p) { ShouldNotReachHere(); } -}; - int BarrierSetNMethod::disarmed_value() const { return *disarmed_value_address(); } @@ -67,10 +61,26 @@ bool BarrierSetNMethod::supports_entry_barrier(nmethod* nm) { } bool BarrierSetNMethod::nmethod_entry_barrier(nmethod* nm) { + class OopKeepAliveClosure : public OopClosure { + public: + virtual void do_oop(oop* p) { + // Loads on nmethod oops are phantom strength. + // + // Note that we could have used NativeAccess::oop_load(p), + // but that would have *required* us to convert the returned LoadOopProxy to an oop, + // or else keep alive load barrier will never be called. It's the LoadOopProxy-to-oop + // conversion that performs the load barriers. This is too subtle, so we instead + // perform an explicit keep alive call. + oop obj = NativeAccess::oop_load(p); + Universe::heap()->keep_alive(obj); + } + + virtual void do_oop(narrowOop* p) { ShouldNotReachHere(); } + }; + // If the nmethod is the only thing pointing to the oops, and we are using a - // SATB GC, then it is important that this code marks them live. This is done - // by the phantom load. - LoadPhantomOopClosure cl; + // SATB GC, then it is important that this code marks them live. + OopKeepAliveClosure cl; nm->oops_do(&cl); // CodeCache sweeper support