RFR[S] 8241158 SA TestHeapDumpForInvokeDynamic.java fails when CDS archive is relocated

[Calvin Cheung]

Hi Ioi,

Regarding the changes in javaClasses.cpp, I saw that in your new code in 
systemDictionaryShared.cpp, the 
java_lang_Class::update_archived_mirror_native_pointers(m) won't be 
called if MetaspaceShared::relocation_delta() is 0. But I noticed 
there's another code path to 
java_lang_Class::update_archived_mirror_native_pointers() after 
HeapShared::fixup_mapped_heap_regions() is called in 
SystemDictionary::resolve_well_known_classes():

java_lang_Class::update_archived_mirror_native_pointers(oop) : void
     java_lang_Class::restore_archived_mirror(Klass *, Handle, Handle, 
Handle, Thread *) : bool
         java_lang_Class::fixup_mirror(Klass *, Thread *) : void
             Universe::fixup_mirrors(Thread *) : void
SystemDictionary::resolve_well_known_classes(Thread *) : void
                     SystemDictionary::initialize(Thread *) : void
                         Universe::genesis(Thread *) : void
                             universe2_init() : void
                                 init_globals() : ?

Will MetaspaceShared::relocation_delta() always be non-zero in the above 
case?

thanks,

Calvin

On 4/14/20 10:07 PM, Ioi Lam wrote:
> Hi Chris,
>
> Thanks for the info. I've synced with the latest repo and updated the 
> webrev in-place:
>
> http://cr.openjdk.java.net/~iklam/jdk15/8241158-sa-heap-dump-fails.v01/
>
> I'll rerun tiers 1-4 as two of the affected tests 
> (ClhsdbDumpheap.java, TestHeapDumpForInvokeDynamic.java) will now be 
> executed on macos by mach5.
>
> Thanks
> - Ioi
>
> On 4/14/20 3:34 PM, Chris Plummer wrote:
>> [Not a review]
>>
>> Hi Ioi,
>>
>> Your ProblemList.txt is out of date. All those Solaris entries for 
>> 8193639 are now gone.
>>
>> thanks,
>>
>> Chris
>>
>> On 4/14/20 3:01 PM, Ioi Lam wrote:
>>> https://bugs.openjdk.java.net/browse/JDK-8241158
>>> http://cr.openjdk.java.net/~iklam/jdk15/8241158-sa-heap-dump-fails.v01/
>>>
>>> This is a bug in the CDS relocation code. When 
>>> -XX:ArchiveRelocationMode=1 is
>>> specified, the CDS archive is mapped to an address picked by the OS
>>> (instead of the default address 0x800000000). As a result, we need 
>>> to patch
>>> the native Klass* pointers inside java.lang.Class instances that are
>>> stored in the CDS archived heap region.
>>>
>>> Before this fix, the patching is done incrementally, when a class is 
>>> resolved.
>>> However, when SA tries to do a heap dump, it may see a 
>>> java.lang.Class instance
>>> that is not yet resolved, whose Klass* pointer has not been patched.
>>> Dereferencing the unpatched pointer causes SA to fail.
>>>
>>> (The failure happens on macos only, probably because the invalid 
>>> dereference
>>> causes different exceptions on other platforms due to specific 
>>> memory layout,
>>> and SA is able to handle such exceptions and limp on).
>>>
>>> The fix is to unconditionally patch all the Klass* pointers during VM
>>> initialization.
>>>
>>> We already patch a lot of stuff at start-up when the CDS archive is 
>>> relocated,
>>> so doing a little patching more doesn't hurt.
>>>
>>> ----
>>>
>>> Passed mach5 tiers 1-4. Ran the failed test manually on macos and it 
>>> passed.
>>>
>>> Thanks
>>> - Ioi
>>
>