hzongaro
commented
1 year ago Annabelle @a7ehuo, may I ask you to investigate this issue?
Closed dylanjtuttle closed 1 year ago
hzongaro
commented
1 year ago Annabelle @a7ehuo, may I ask you to investigate this issue?
a7ehuo
commented
1 year ago I reproduced this assert. Attach the full backtrace at the end [1].
Assertion failed at /root/home/ahuo/src/openj9-openjdk-jdk11/openj9/runtime/compiler/runtime/ClassUnloadAssumption.cpp:501: kind == RuntimeAssumptionOnClassRedefinitionPIC || kind == RuntimeAssumptionOnClassRedefinitionUPIC || kind == RuntimeAssumptionOnClassRedefinitionNOP
non redefinition assumption (RA=0x7e255093e0b0 kind=0 key=0x241200) left after metadata reclamationThe assert happens in TR_RuntimeAssumptionTable::markAssumptionsAndDetach during GC. It checks if the runtime assumption that's not being removed is one of the redefinition assumptions. In this case, the left assumption is TR_UnloadedClassPicSite with getAssumptionKind() that returns RuntimeAssumptionOnClassUnload (0), which is not one of the three redefinition assumptions allowed in the assert check.
(gdb) p *cursor
$5 = {_vptr.RuntimeAssumption = 0x7e25a758bb50 <vtable for TR_UnloadedClassPicSite+16>, _next = 0x0,
_nextAssumptionForSameJittedBody = 0x7e255093e070, _key = 2363904}
(gdb) p *(TR_UnloadedClassPicSite *)(cursor)
$13 = {<OMR::ValueModifyRuntimeAssumption> = {<OMR::RuntimeAssumption> = {
_vptr.RuntimeAssumption = 0x7e25a758bb50 <vtable for TR_UnloadedClassPicSite+16>, _next = 0x0,
_nextAssumptionForSameJittedBody = 0x7e255093e070, _key = 2363904}, <No data fields>},
_picLocation = 0x7e258acb935c "P\026$", _size = 4}Interestingly, TR_UnloadedClassPicSite *createClassUnloadPicSite actually passes the kind as RuntimeAssumptionOnClassRedefinitionPIC to create TR_UnloadedClassPicSite. But RuntimeAssumptionOnClassRedefinitionPIC is not used by TR_UnloadedClassPicSite::make. TR_UnloadedClassPicSite::make uses RuntimeAssumptionOnClassUnload to add to RAT.
I wonder
(1) If TR_UnloadedClassPicSite should have a kind type as RuntimeAssumptionOnClassRedefinitionPIC instead of RuntimeAssumptionOnClassUnload.
(2) If not, if RuntimeAssumptionOnClassUnload should be allowed in this assert check on kind.
@mpirvu @klangman I'm wondering if you could help shed some light on these two questions. Thank you in advance!
[1]
#13 <signal handler called>
#14 0x00007e25ac61d168 in raise () from /lib/powerpc64le-linux-gnu/libc.so.6
#15 0x00007e25a6ce9070 in TR::trap () at /root/home/ahuo/src/openj9-openjdk-jdk11/omr/compiler/infra/Assert.cpp:67
#16 0x00007e25a6ce922c in TR::va_fatal_assertion (ap=0x7e25505d9ee0 "\260\340\223P%~",
format=0x7e25a73395c8 "non redefinition assumption (RA=%p kind=%d key=%p) left after metadata reclamation\n",
condition=0x7e25a7339620 "kind == RuntimeAssumptionOnClassRedefinitionPIC || kind == RuntimeAssumptionOnClassRedefinitionUPIC || kind == RuntimeAssumptionOnClassRedefinitionNOP", line=501,
file=0x7e25a7339440 "/root/home/ahuo/src/openj9-openjdk-jdk11/openj9/runtime/compiler/runtime/ClassUnloadAssumption.cpp")
at /root/home/ahuo/src/openj9-openjdk-jdk11/omr/compiler/infra/Assert.cpp:139
#17 TR::assertion (
file=0x7e25a7339440 "/root/home/ahuo/src/openj9-openjdk-jdk11/openj9/runtime/compiler/runtime/ClassUnloadAssumption.cpp",
line=<optimized out>,
condition=0x7e25a7339620 "kind == RuntimeAssumptionOnClassRedefinitionPIC || kind == RuntimeAssumptionOnClassRedefinitionUPIC || kind == RuntimeAssumptionOnClassRedefinitionNOP",
format=0x7e25a73395c8 "non redefinition assumption (RA=%p kind=%d key=%p) left after metadata reclamation\n")
at /root/home/ahuo/src/openj9-openjdk-jdk11/omr/compiler/infra/Assert.cpp:156
#18 0x00007e25a6ad4af8 in TR_RuntimeAssumptionTable::markAssumptionsAndDetach (this=0x7e25a80d9d10, md=0x7e25886c50f8,
reclaimPrePrologueAssumptions=<optimized out>)
at /root/home/ahuo/src/openj9-openjdk-jdk11/openj9/runtime/compiler/runtime/ClassUnloadAssumption.cpp:501
--Type <RET> for more, q to quit, c to continue without paging--
#19 0x00007e25a6aded40 in (anonymous namespace)::markAssumptionsAndDetach (reclaimPrePrologueAssumptions=false,
metaData=0x7e25886c50f8, jitConfig=0x7e25a80d6700)
at /root/home/ahuo/src/openj9-openjdk-jdk11/openj9/runtime/compiler/runtime/HookHelpers.cpp:122
#20 jitReleaseCodeCollectMetaData (jitConfig=0x7e25a80d6700, vmThread=0x48c200, metaData=0x7e25886c50f8,
faintCacheBlock=0x7e25a82fafd0) at /root/home/ahuo/src/openj9-openjdk-jdk11/openj9/runtime/compiler/runtime/HookHelpers.cpp:299
#21 0x00007e25a669a420 in jitReleaseCodeStackWalk (omrVMThread=0x48ccb0, condYield=condYield@entry=0x0)
at /root/home/ahuo/src/openj9-openjdk-jdk11/openj9/runtime/compiler/control/HookedByTheJit.cpp:6802
#22 0x00007e25a669a6b0 in jitHookReleaseCodeLocalGCEnd (hook=<optimized out>, eventNum=<optimized out>, eventData=<optimized out>,
userData=<optimized out>) at /root/home/ahuo/src/openj9-openjdk-jdk11/openj9/runtime/compiler/control/HookedByTheJit.cpp:6863
#23 0x00007e25ac0e19c0 in J9HookDispatch (hookInterface=0x7e25a8040328, taggedEventNum=<optimized out>, eventData=0x7e25505da570)
at /root/home/ahuo/src/openj9-openjdk-jdk11/omr/util/hookable/hookable.cpp:235
#24 0x00007e25a6366bec in MM_Scavenger::reportGCEnd (this=0x7e25a8048730, env=<optimized out>)
at /root/home/ahuo/src/openj9-openjdk-jdk11/omr/gc/base/standard/Scavenger.cpp:727
#25 0x00007e25a63771d8 in MM_Scavenger::mainThreadGarbageCollect (this=0x7e25a8048730, envBase=0x7e25a8838e18,
allocDescription=<optimized out>, initMarkMap=<optimized out>, rebuildMarkBits=<optimized out>)
at /root/home/ahuo/src/openj9-openjdk-jdk11/omr/gc/base/standard/Scavenger.cpp:4364
#26 0x00007e25a6378260 in MM_Scavenger::internalGarbageCollect (this=0x7e25a8048730, envBase=0x7e25a8838e18,
subSpace=0x7e25a809e320, allocDescription=0x7e25505daeb0)
at /root/home/ahuo/src/openj9-openjdk-jdk11/omr/gc/base/standard/Scavenger.cpp:4794
#27 0x00007e25a62ed6d8 in MM_Collector::garbageCollect (this=0x7e25a8048730, env=0x7e25a8838e18, callingSubSpace=0x7e25a809e320,
allocateDescription=0x7e25505daeb0, gcCode=<optimized out>, objectAllocationInterface=0x7e25a881def0,
baseSubSpace=0x7e25a809e320, context=0x0) at /root/home/ahuo/src/openj9-openjdk-jdk11/omr/gc/base/Collector.cpp:500
#28 0x00007e25a63f26a4 in MM_MemorySubSpaceSemiSpace::allocationRequestFailed (this=0x7e25a809e320, env=0x7e25a8838e18,
allocateDescription=0x7e25505daeb0, allocationType=<optimized out>, objectAllocationInterface=0x7e25a881def0,
baseSubSpace=<optimized out>, previousSubSpace=<optimized out>)
at /root/home/ahuo/src/openj9-openjdk-jdk11/omr/gc/base/MemorySubSpaceSemiSpace.cpp:137
#29 0x00007e25a63cfa2c in MM_MemorySubSpaceGeneric::allocateTLH (this=0x7e25a809df80, env=0x7e25a8838e18,
allocDescription=0x7e25505daeb0, objectAllocationInterface=<optimized out>, baseSubSpace=0x0,
previousSubSpace=<optimized out>, shouldCollectOnFailure=<optimized out>)
at /root/home/ahuo/src/openj9-openjdk-jdk11/omr/gc/base/MemorySubSpaceGeneric.cpp:377
#30 0x00007e25a63239d8 in MM_TLHAllocationSupport::refresh (this=0x7e25a881dfa0, env=0x7e25a8838e18,
allocDescription=0x7e25505daeb0, shouldCollectOnFailure=<optimized out>)
at /root/home/ahuo/src/openj9-openjdk-jdk11/omr/gc/base/TLHAllocationSupport.cpp:210
#31 0x00007e25a6323c64 in MM_TLHAllocationSupport::allocateFromTLH (this=0x7e25a881dfa0, env=<optimized out>,
allocDescription=0x7e25505daeb0, shouldCollectOnFailure=<optimized out>)
at /root/home/ahuo/src/openj9-openjdk-jdk11/omr/gc/base/TLHAllocationSupport.cpp:279
#32 0x00007e25a63218e8 in MM_TLHAllocationInterface::allocateFromTLH (this=<error reading variable: value has been optimized out>,
env=<optimized out>, allocDescription=<optimized out>, shouldCollectOnFailure=<optimized out>)
at /root/home/ahuo/src/openj9-openjdk-jdk11/omr/gc/base/TLHAllocationInterface.cpp:148
#33 0x00007e25a6321ef4 in MM_TLHAllocationInterface::allocateObject (shouldCollectOnFailure=true, memorySpace=0x7e25a80a3c40,
allocDescription=0x7e25505daeb0, env=0x7e25a8838e18, this=0x7e25a881def0)
at /root/home/ahuo/src/openj9-openjdk-jdk11/omr/gc/base/TLHAllocationInterface.cpp:183
#34 MM_TLHAllocationInterface::allocateArray (this=0x7e25a881def0, env=0x7e25a8838e18, allocateDescription=0x7e25505daeb0,
memorySpace=0x7e25a80a3c40, shouldCollectOnFailure=<optimized out>)
at /root/home/ahuo/src/openj9-openjdk-jdk11/omr/gc/base/TLHAllocationInterface.cpp:215
#35 0x00007e25a632a7b0 in MM_AllocateInitialization::allocateAndInitializeObject (omrVMThread=<optimized out>, this=0x7e25505dae98)
at /root/home/ahuo/src/openj9-openjdk-jdk11/omr/gc/base/AllocateInitialization.hpp:198
#36 OMR_GC_AllocateObject (omrVMThread=<optimized out>, allocator=0x7e25505dae98)
at /root/home/ahuo/src/openj9-openjdk-jdk11/omr/gc/startup/omrgcalloc.cpp:39
--Type <RET> for more, q to quit, c to continue without paging--
#37 0x00007e25a61ff14c in J9AllocateIndexableObject (vmThread=0x48c200, clazz=0xba300, numberOfIndexedFields=<optimized out>,
allocateFlags=1) at /root/home/ahuo/src/openj9-openjdk-jdk11/openj9/runtime/gc_modron_startup/mgcalloc.cpp:578
#38 0x00007e25ac1cd2f0 in VM_BytecodeInterpreterCompressed::newarray (_pc=<synthetic pointer>: 0x7e2550685ba0 "\274\bL״\017",
_sp=<synthetic pointer>: 0x748dd0, this=0x7e25505db5f0)
at /root/home/ahuo/src/openj9-openjdk-jdk11/openj9/runtime/vm/BytecodeInterpreter.hpp:8251
#39 VM_BytecodeInterpreterCompressed::run (vmThread=<optimized out>, this=0x7e25505db5f0)
at /root/home/ahuo/src/openj9-openjdk-jdk11/openj9/runtime/vm/BytecodeInterpreter.hpp:11599
#40 bytecodeLoopCompressed (currentThread=<optimized out>)
at /root/home/ahuo/src/openj9-openjdk-jdk11/openj9/runtime/vm/BytecodeInterpreter.inc:112
#41 0x00007e25ac276418 in c_cInterpreter ()
at /root/home/ahuo/src/openj9-openjdk-jdk11/build/linux-ppc64le-normal-server-release/vm/runtime/vm/pcinterp.s:329
#42 0x00007e25ac126430 in runJavaThread (currentThread=0x48c200)
at /root/home/ahuo/src/openj9-openjdk-jdk11/openj9/runtime/vm/callin.cpp:682
#43 0x00007e25ac1bbae8 in javaProtectedThreadProc (portLibrary=<optimized out>, entryarg=0x48c200)
at /root/home/ahuo/src/openj9-openjdk-jdk11/openj9/runtime/vm/vmthread.cpp:2101
#44 0x00007e25a7e4934c in omrsig_protect (portLibrary=0x7e25ac431f00 <j9portLibrary>,
fn=0x7e25ac1bb9b0 <javaProtectedThreadProc(J9PortLibrary*, void*)>, fn_arg=0x48c200,
handler=0x7e25ac14dfd0 <structuredSignalHandler>, handler_arg=0x48c200, flags=<optimized out>, result=0x7e25505dc498)
at /root/home/ahuo/src/openj9-openjdk-jdk11/omr/port/unix/omrsignal.c:425
#45 0x00007e25ac1b73b0 in javaThreadProc (entryarg=0x7e25a800f900)
at /root/home/ahuo/src/openj9-openjdk-jdk11/openj9/runtime/vm/vmthread.cpp:380
#46 0x00007e25ac0ad8bc in thread_wrapper (arg=0x7e25a83513a8)
at /root/home/ahuo/src/openj9-openjdk-jdk11/omr/thread/common/omrthread.c:1724
#47 0x00007e25ac508838 in start_thread () from /lib/powerpc64le-linux-gnu/libpthread.so.0
#48 0x00007e25ac71b884 in clone () from /lib/powerpc64le-linux-gnu/libc.so.6@klangman and I looked at the core. Here is his analysis:
The unloaded class is java/util/StringTokenizer (key=0x241200). TR_UnloadedClassPicSite::_picLocation is 0x7e258acb935c [1], pointing to the J9Method in the pre-prolog of {java/util/StringTokenizer.setMaxDelimCodePoint} -11 [2]. The J9Method in the pre-prolog is for the helper to know what method to execute for a revert to the interpreter. This address needs to be patched on a redefinition. This might explain why the assumption remained after freeing the method while leaving a stub behind for handling JIT->JIT direct calls.
The likely fix for this assert would be to allow RuntimeAssumptionOnClassUnload since TR_UnloadedClassPicSite has the type of RuntimeAssumptionOnClassUnload.
TR_ASSERT(kind == RuntimeAssumptionOnClassUnload || //<---- allow
kind == RuntimeAssumptionOnClassRedefinitionPIC ||
kind == RuntimeAssumptionOnClassRedefinitionUPIC ||
kind == RuntimeAssumptionOnClassRedefinitionNOP,[1]
$5 = {<OMR::ValueModifyRuntimeAssumption> = {<OMR::RuntimeAssumption> = {
_vptr.RuntimeAssumption = 0x7e25a758bb50 <vtable for TR_UnloadedClassPicSite+16>, _next = 0x0,
_nextAssumptionForSameJittedBody = 0x7e255093e070, _key = 0x241200}, <No data fields>}, _picLocation = 0x7e258acb935c,
_size = 0x4}[2]
(kca) whatis 0x7e258acb935c
{java/util/StringTokenizer.setMaxDelimCodePoint} -11kca) method verbose {*StringTokenizer.setMaxDelimCodePoint*}
J9Class J9Method Start Len {ClassPath/Name.MethodName}
--------------------------------------------------------------------------------------------
(0x0000000000241200 0x0000000000241650) 0x00007e258b2d9144 550 {java/util/StringTokenizer.setMaxDelimCodePoint()V}
(0x0000000000241200 0x0000000000241650) 0x00007e258acb9344 299 {java/util/StringTokenizer.setMaxDelimCodePoint()V}[3]
0x7e258acb9344 {java/.../StringTokenizer.setMaxDelimCodePoint} -17 4a495457 rlwinm r20, r26, 9, 5, 5
0x7e258acb9348 {java/.../StringTokenizer.setMaxDelimCodePoint} -16 f8506c88 lbz r3, 0x50f8(r12)
0x7e258acb934c {java/.../StringTokenizer.setMaxDelimCodePoint} -15 257e0000 invalid instruction
0x7e258acb9350 {java/.../StringTokenizer.setMaxDelimCodePoint} -14 a602087c mflr r0
0x7e258acb9354 {java/.../StringTokenizer.setMaxDelimCodePoint} -13 00006ef8 std r3, 0(r14)
0x7e258acb9358 {java/.../StringTokenizer.setMaxDelimCodePoint} -12 c9671f48 bl 0x7e258aeafb20
0x7e258acb935c {java/.../StringTokenizer.setMaxDelimCodePoint} -11 50162400 invalid instruction -> J9Method - {java/util/StringTokenizer.setMaxDelimCodePoint}
0x7e258acb9360 {java/.../StringTokenizer.setMaxDelimCodePoint} -10 00000000 invalid instruction
0x7e258acb9364 {java/.../StringTokenizer.setMaxDelimCodePoint} -9 18cb27a7 lhzu r25, -0x34e8(r7)(gdb) disassemble 0x7e258acb935c,+32
Dump of assembler code from 0x7e258acb935c to 0x7e258acb937c:
0x00007e258acb935c: .long 0x241650
0x00007e258acb9360: .long 0x0
0x00007e258acb9364: lhzu r25,-13544(r7)
0x00007e258acb9368: .long 0x7e25
0x00007e258acb936c: .long 0x0
0x00007e258acb9370: mflr r0
0x00007e258acb9374: bl 0x7e258aeaf520
0x00007e258acb9378: lbz r3,21240(r12)
End of assembler dump.
mpirvu
commented
1 year ago What I don't understand is why do we attempt to leave a stub behind for a compiled body of java/util/StringTokenizer.setMaxDelimCodePoint()V if we are in the process of unloading its class java/util/StringTokenizer ?
mpirvu
commented
1 year ago To refine my question:
If A.foo() refers to a class B in its body, we add a class unload assumption for class B.
If A.foo() refers to the same class A (or to a jmethod from A) in its body, we don't need a class loading assumption for A because the picLocation that is supposed to be patched goes away when A is unloaded. From the analysis above we seem to be in this case.
Thus I still think that the assume we eliminated was valid. Maybe the correct solution here is to not generate that assumption in the first place.
mpirvu
commented
1 year ago FYI @klangman since you helped with the analysis.
hzongaro
commented
1 year ago Sorry, Marius @mpirvu. I didn't see your questions before I merged the change. Should I revert pull request #17998?
mpirvu
commented
1 year ago @hzongaro In the long run I think we should revert the assume change. It's up to you if you want to do it now, or later once we change the code to prevent the assert from firing
hzongaro
commented
1 year ago I think it's best if we revert for now, as the goal is to ensure we fix any problems exposed by the TR_ASSERTs - either mistakes in the asserts themselves or bugs in the compiler that the asserts have exposed.
I've opened pull request #18010.
mpirvu
commented
1 year ago I think the assumption is added either in this line https://github.com/eclipse/omr/blob/master/compiler/p/codegen/PPCBinaryEncoding.cpp#L1642 or in this line https://github.com/eclipse/omr/blob/master/compiler/p/codegen/PPCBinaryEncoding.cpp#L1732
cg()->jitAddPicToPatchOnClassUnload((void *) (cg()->fe()->createResolvedMethod(cg()->trMemory(), (TR_OpaqueMethodBlock *) (comp->target().is64Bit()?node->getLongInt():node->getInt()), comp->getCurrentMethod())->classOfMethod()), (void *)cursor);From the node we get an integer which (I think) represents the j9method, we create a resolved method out of it and then find its class which is used in jitAddPicToPatchOnClassUnload(classPointer, addressToBePatched). First, I wonder why can't we get the class of the method directly from the j9method, without creating a resolved method, which is expensive. Second, we should be able to add a test: if the class we attempt to protect against is the same as the class of the method being compiled, we can skip the classUnloadAssumption. Similar reasoning applies for the other instance.
klangman
commented
1 year ago I am not sure the StringTokenizer class was unloaded, I think it's setMaxDelimCodePoint()V method was just replaced and the old body was partially reclaimed, so all the assumptions targeting the pre-prologue will remain.
mpirvu
commented
1 year ago I had a private discussion with @klangman and we agreed that the classUnloadAssumption in the pre-prologue should not have been generated.
a7ehuo
commented
1 year ago Thank you very much @mpirvu for pointing out the issue here! I'll take a look at the code mentioned in https://github.com/eclipse-openj9/openj9/issues/17734#issuecomment-1691724320
a7ehuo
commented
1 year ago (1)
The unloaded class is
java/util/StringTokenizer(key=0x241200).
I realized my above statement is not too accurate. The class is not being unloaded when the assert happens. The class java/util/StringTokenizer is the key for TR_UnloadedClassPicSite. The assert happens at the end of GC cycle when jitReleaseCodeStackWalk is invoked. It didn't happen during class unloading when jitHookClassLoaderUnload hook is invoked. [1]
(2)
After looking into the issue further, I found the TR_UnloadedClassPicSite assumption that triggers the assert is added during AOT load not added through addMetaDataForCodeAddress.
TR_RelocationRecordSymbolFromManager::activatePointer
TR_RelocationTarget::addPICtoPatchPtrOnClassUnload
TR_PPC64RelocationTarget::platformAddPICtoPatchPtrOnClassUnload
createClassUnloadPicSiteIn the latest reproduced case [2], the _key for TR_UnloadedClassPicSite is class sun/nio/cs/UTF_8$Encoder (0x13db00) and _picLocation is {sun/nio/cs/UTF_8$Encoder.encodeArrayLoop} -11 (0x7eb50361fb24) [3]. The J9Method pointer (0x13e0f8) stored at _picLocation is also sun/nio/cs/UTF_8$Encoder.encodeArrayLoop.
Method sun/nio/cs/UTF_8$Encoder.encodeArrayLoop is first loaded from AOT, which is when the TR_UnloadedClassPicSite is created for an instruction in the pre-prologue [4]. Later the method is JIT recompiled at warm. The code memory for AOT load can be reclaimed at this point, but my understanding of relcaimation is that the pre-prologue for the AOT load code remains so that other code that has a call to the stale AOT loaded body can continue execution.
@mpirvu If I understand it correctly, if the pre-prologue for the AOT load remains, should TR_UnloadedClassPicSite still be created for the instruction in the pre-prologue in this case? In this particular case, it's not exactly A.foo() calls A.foo(). With my limited understanding of relcaimation, the J9Method pointer in the pre-prologue of the compiled body has its own purpose.
[1]
#17 TR::fatal_assertion (file=<optimized out>, line=<optimized out>, condition=<optimized out>, format=<optimized out>)
at /root/home/ahuo/src/openj9-openjdk-jdk11/omr/compiler/infra/Assert.cpp:164
#18 0x00007eb51f084500 in TR_RuntimeAssumptionTable::markAssumptionsAndDetach (this=0x7eb5200d9d90, md=0x7eb500db0ff8,
reclaimPrePrologueAssumptions=<optimized out>)
at /root/home/ahuo/src/openj9-openjdk-jdk11/openj9/runtime/compiler/runtime/ClassUnloadAssumption.cpp:500
#19 0x00007eb51f08d480 in (anonymous namespace)::markAssumptionsAndDetach (reclaimPrePrologueAssumptions=false,
metaData=0x7eb500db0ff8, jitConfig=0x7eb5200d67a0)
at /root/home/ahuo/src/openj9-openjdk-jdk11/openj9/runtime/compiler/runtime/HookHelpers.cpp:122
#20 jitReleaseCodeCollectMetaData (jitConfig=0x7eb5200d67a0, vmThread=0x99200, metaData=0x7eb500db0ff8,
faintCacheBlock=0x7eb4f4017d70) at /root/home/ahuo/src/openj9-openjdk-jdk11/openj9/runtime/compiler/runtime/HookHelpers.cpp:299
#21 0x00007eb51ecadda0 in jitReleaseCodeStackWalk (omrVMThread=omrVMThread@entry=0x99cb0, condYield=<optimized out>)
at /root/home/ahuo/src/openj9-openjdk-jdk11/openj9/runtime/compiler/control/HookedByTheJit.cpp:6820
#22 0x00007eb51ecae05c in jitHookReleaseCodeGCCycleEnd (hook=<optimized out>, eventNum=<optimized out>, eventData=0x7eb524aabbc0,
...[2]
Assertion failed at /root/home/ahuo/src/openj9-openjdk-jdk11/openj9/runtime/compiler/runtime/ClassUnloadAssumption.cpp:500: kind == RuntimeAssumptionOnClassRedefinitionPIC || kind == RuntimeAssumptionOnClassRedefinitionUPIC || kind == RuntimeAssumptionOnClassRedefinitionNOP
non redefinition assumption (RA=0x7eb4c922fec0 kind=0 key=0x13db00) left after metadata reclamation[3]
(gdb) fr 18
#18 0x00007eb51f084500 in TR_RuntimeAssumptionTable::markAssumptionsAndDetach (this=0x7eb5200d9d90, md=0x7eb500db0ff8,
reclaimPrePrologueAssumptions=<optimized out>)
at /root/home/ahuo/src/openj9-openjdk-jdk11/openj9/runtime/compiler/runtime/ClassUnloadAssumption.cpp:500
500 TR_ASSERT_FATAL(kind == RuntimeAssumptionOnClassRedefinitionPIC ||
(gdb) p cursor
$1 = (OMR::RuntimeAssumption *) 0x7eb4c922fec0
(gdb) p /x *(TR_UnloadedClassPicSite *)(cursor)
$2 = {<OMR::ValueModifyRuntimeAssumption> = {<OMR::RuntimeAssumption> = {
_vptr.RuntimeAssumption = 0x7eb51fa2bbb8 <vtable for TR_UnloadedClassPicSite+16>, _next = 0x7eb4c922ba80,
_nextAssumptionForSameJittedBody = 0x7eb4c922fe80, _key = 0x13db00}, <No data fields>}, _picLocation = 0x7eb50361fb24,
_size = 0x4}(kca) what 0x7eb50361fb24
{sun/nio/cs/UTF_8$Encoder.encodeArrayLoop} -11
(kca) what (0x7eb50361fb24)
0x7eb50361fb24: 0x000000000013e0f8 J9Method - {sun/nio/cs/UTF_8$Encoder.encodeArrayLoop}
(kca) j9class 0x13DB00
Class Path/Name: {sun/nio/cs/UTF_8$Encoder} J9Class 0x000000000013db00
ClassObject: 0x0000000080dd7360
Access: Final Synchronized (30)
ClassLoader: 0x00007eb5200aa158 Object: 0x0000000080e05978 {jdk/internal/loader/ClassLoaders$BootClassLoader}T
SubClassLink: 0x000000000013b000 {java/io/Writer}
Instance Size: 44 (with header: 56) bytes
Hierarchy: (depth 2)
{java/nio/charset/CharsetEncoder} J9Class 0x000000000013d700
{java/lang/Object} J9Class 0x00000000000b1100
Interfaces: None!
J9Methods: 0x000000000013e058 (8 methods)
(kca) m {sun/nio/cs/UTF_8$Encoder.encodeArrayLoop*}
J9Class J9Method Start Len {ClassPath/Name.MethodName}
--------------------------------------------------------------------------------------------
(0x000000000013db00 0x000000000013e0f8) 0x00007eb503f53084 739 {sun/nio/cs/UTF_8$Encoder.encodeArrayLoop(Ljava/nio/CharBuffer;Ljava/nio/ByteBuffer;)Ljava/nio/charset/CoderResult;}
(0x000000000013db00 0x000000000013e0f8) 0x00007eb50361fb04 679 {sun/nio/cs/UTF_8$Encoder.encodeArrayLoop(Ljava/nio/CharBuffer;Ljava/nio/ByteBuffer;)Ljava/nio/charset/CoderResult;}[4]
#INFO: TR_RelocationRecordSymbolFromManager::activatePointer: DEBUG clazz 000000000013DB00 reloLocation 00007EB50361FB24 sun/nio/cs/UTF_8$Encoder.encodeArrayLoop(Ljava/nio/CharBuffer;Ljava/nio/ByteBuffer;)Ljava/nio/charset/CoderResult;
#INFO: TR_PPC64RelocationTarget::platformAddPICtoPatchPtrOnClassUnload: DEBUG classKey 000000000013DB00 sitePointer 00007EB50361FB24 sun/nio/cs/UTF_8$Encoder.encodeArrayLoop(Ljava/nio/CharBuffer;Ljava/nio/ByteBuffer;)Ljava/nio/charset/CoderResult;
#INFO: createClassUnloadPicSite: DEBUG classPointer 000000000013DB00 addressToBePatched 00007EB50361FB24 sentinel 00007EB493001558 sun/nio/cs/UTF_8$Encoder.encodeArrayLoop(Ljava/nio/CharBuffer;Ljava/nio/ByteBuffer;)Ljava/nio/charset/CoderResult;
...
+ (AOT load) sun/nio/cs/UTF_8$Encoder.encodeArrayLoop(Ljava/nio/CharBuffer;Ljava/nio/ByteBuffer;)Ljava/nio/charset/CoderResult; @ 00007EB50361FB50-00007EB5036205A0 Q_SZ=308 Q_SZI=304 QW=479 j9m=000000000013E0F8 bcsz=489 compThreadID=2
...
+ (warm) sun/nio/cs/UTF_8$Encoder.encodeArrayLoop(Ljava/nio/CharBuffer;Ljava/nio/ByteBuffer;)Ljava/nio/charset/CoderResult; @ 00007EB503F530F4-00007EB503F53C10 OrdinaryMethod G Q_SZ=23 Q_SZI=4 QW=194 j9m=000000000013E0F8 bcsz=489 OSR compThreadID=1 CpuLoad=0%(0%avg) JvmCpu=54%...
relocateAOTCodeAndData jitConfig=00007EB5200D67A0 aotDataCache=0000000000000000 aotMccCodeCache=00007EB4F800A190 method=000000000013E0F8 tempDataStart=00007EB4DA4C5AA0 exceptionTable=00007EB500DB0FF8 oldDataStart=00007B1591100030 codeStart=00007EB50361FB00 oldCodeStart=00007B15922FE8C0 classReloAmount=0000000000000001 cacheEntry=00007EB4DA4C5AA0
relocateAOTCodeAndData: method sun/nio/cs/UTF_8$Encoder.encodeArrayLoop(Ljava/nio/CharBuffer;Ljava/nio/ByteBuffer;)Ljava/nio/charset/CoderResult;
<relocatableDataMetaDataRT>
sun/nio/cs/UTF_8$Encoder.encodeArrayLoop(Ljava/nio/CharBuffer;Ljava/nio/ByteBuffer;)Ljava/nio/charset/CoderResult;
startPC endPC size gcStackAtlas bodyInfo
0x7b15922fe9100x7b15922ff36029a 0x7b15911006b00x7b1591100880
inlinedCalls
(nil)
</relocatableDataMetaDataRT>
...
TR_HCR 00007EB4DA4C5FAA
size 12 type 41 flags 0 reloFlags 0
offset 13e0f8
action: hcrEnabled 1
reloLocation: from 00007EB4DA4C5FBA at 00007EB50361FB24 (offset 24)
TR_RelocationRecordSymbolFromManager 00007EB4DA4C5FBC
size c type 98 flags 0 reloFlags 0
symbolID 3
symbolType typeMethod
reloLocation: from 00007EB4DA4C5FC6 at 00007EB50361FB24 (offset 24)I believe Marius's argument is that the assumption is there to patch the J9Method in the pre-prolog when that methods class was unloaded, but that J9Class for the for the method body is the same as the J9Class for the J9Method in the pre-prolog. Therefore it is not necessary to worry about the pre-prolog J9Method since the body it's self would be removed on the class unload making it impossible to ever reach the code making use of the J9Method.
mpirvu
commented
1 year ago if the pre-prologue for the AOT load remains, should TR_UnloadedClassPicSite still be created for the instruction in the pre-prologue in this case? In this particular case, it's not exactly A.foo() calls A.foo(). With my limited understanding of relcaimation, the J9Method pointer in the pre-prologue of the compiled body has its own purpose.
Yes, it's not A.foo() calls A.foo(), but if A.foo() refers to anything that has to do with class A (i.e. j9class of class A or any j9method that belongs to class A) then we don't need a classUnloadAssumption because when class A get unloaded, all those points that need to be patched in A.foo (either in the method body or pre-prologue) go away.
mpirvu
commented
1 year ago Since the JVM unloads class loaders and every class that that was loaded by that class loader, we can make the test even more generic: when we think we need to have a classUnloadAssumption, we can check whether the class loader for the method being compiled is the same as the classLoader for the class we try protect against with the classUnloadAssumption. If they are the same, we can skip generating the classUnloadAssumption.
a7ehuo
commented
1 year ago @mpirvu @klangman Thank you for the clarification! Now I see there is no need to create the assumption: If the class for the J9Method in the pre-prologue is the same as the class of the compiled method, no code will ever reach the J9Method in the pre-prologue once the class is unloaded.
we can check whether the class loader for the method being compiled is the same as the classLoader for the class we try protect against with the classUnloadAssumption
This particular case in AOT load should have checked TR_J9VM::isUnloadAssumptionRequired before creating the assumption. TR_J9VM::isUnloadAssumptionRequired takes care of the comparison of the classloaders as Marius suggests here
mpirvu
commented
1 year ago TR_J9VM::isUnloadAssumptionRequired takes care of the comparison of the classloaders
That's exactly what we need.
klangman
commented
1 year ago I think it will be the case that for this pre-prolog J9Method the isUnloadAssumptionRequired() test will always be false. So I believe the code to generate the assumption can simply be deleted.
a7ehuo
commented
1 year ago for this pre-prolog J9Method the isUnloadAssumptionRequired() test will always be false. So I believe the code to generate the assumption can simply be deleted
It looks to me TR_RelocationRecordSymbolFromManager::activatePointer is not limited to just for J9Method in pre-prologues. I think isUnloadAssumptionRequired should be called instead of removing the code completely. I'm running sanity tests on the fix now before creating a PR.
mpirvu
commented
1 year ago comp->fej9()->isUnloadAssumptionRequired(clazz, comp->getCurrentMethod()))
I wonder whether comp->getCurrentMethod() should be comp->getMethodBeingCompiled().
a7ehuo
commented
1 year ago In other places where isUnloadAssumptionRequired is called. comp->getCurrentMethod() is used
https://github.com/eclipse-openj9/openj9/blob/022a2a444e57b27f234a3c84aff8fd97d106ebfc/runtime/compiler/runtime/RelocationRecord.cpp#L2696 https://github.com/eclipse-openj9/openj9/blob/022a2a444e57b27f234a3c84aff8fd97d106ebfc/runtime/compiler/runtime/MetaData.cpp#L1255 https://github.com/eclipse-openj9/openj9/blob/022a2a444e57b27f234a3c84aff8fd97d106ebfc/runtime/compiler/x/codegen/X86PrivateLinkage.cpp#L1811
mpirvu
commented
1 year ago In other places where isUnloadAssumptionRequired is called. comp->getCurrentMethod() is used
ok. The two are equivalent while doing code generation.
The assertion at
/home/jenkins/workspace/Build_JDK11_ppc64le_linux_Personal/openj9/runtime/compiler/runtime/ClassUnloadAssumption.cpp:505: kind == RuntimeAssumptionOnClassRedefinitionPIC || kind == RuntimeAssumptionOnClassRedefinitionUPIC || kind == RuntimeAssumptionOnClassRedefinitionNOPfires while compiling the
sanity.functionaltest suite onppc64le_linuxfor Java 11 with the-DPROD_WITH_ASSUMESflag enabled.Link to the Jenkins job.
Link to the compilation.log.
Stack trace (from the compilation log):