RFR(S) 8243506 SharedBaseAddress is ignored by -Xshare:dump

[Ioi Lam]

Thanks Thomas & Calvin for the review!

- Ioi

On 6/1/20 11:05 AM, Calvin Cheung wrote:
> Hi Ioi,
>
> The updated webrev (including .v03-delta) looks good to me.
>
> thanks,
>
> Calvin
>
> On 5/28/20 5:14 PM, Ioi Lam wrote:
>> Hi Calvin,
>>
>> Thanks for the review. I've fixed the problems you pointed out below.
>>
>> I also rebased the patch on top of Thomas's JDK-8243392: Remodel 
>> CDS/Metaspace storage reservation. I reorganized the validation code 
>> of SharedBaseAddress to check for platform validity and address space 
>> wrap-around.
>>
>> SharedBaseAddressOption.java is updated to enable WhiteBox which is 
>> required since JDK-8243947.
>>
>>
>> http://cr.openjdk.java.net/~iklam/jdk15/8243506-SharedBaseAddress-ignored.v02/ 
>>
>> http://cr.openjdk.java.net/~iklam/jdk15/8243506-SharedBaseAddress-ignored.v02-delta/ 
>>
>>
>> Thanks
>> - Ioi
>>
>> On 5/11/20 8:56 PM, Calvin Cheung wrote:
>>> Hi Ioi,
>>>
>>> Just couple of minor comments:
>>>
>>> metaspaceShared.cpp:
>>>
>>> 336   // We don't want any valid object to be at the very of the 
>>> archive.
>>>
>>> " ... the very of ..." -> "... the very bottom of..." ?
>>>
>>> SharedBaseAddressOption.java:
>>>
>>>   77         // a top archive specified in the base archive position
>>>   78         run2(topArchiveName, baseArchiveName,
>>>
>>> It looks like the order of the top and base archives are not reversed.
>>>
>>> Because the topArchiveName is used for dumpBaseArchive in line 65 
>>> and the order of the archives specified for dump2 (line 70) are the 
>>> same as above (line 78). So the topArchiveName actually refers to 
>>> the base archive?
>>>
>>> thanks,
>>>
>>> Calvin
>>>
>>> On 5/5/20 11:40 AM, Ioi Lam wrote:
>>>> https://bugs.openjdk.java.net/browse/JDK-8243506
>>>> http://cr.openjdk.java.net/~iklam/jdk15/8243506-SharedBaseAddress-ignored.v01/ 
>>>>
>>>>
>>>> -XX:SharedBaseAddress was essentially ignored due to a bug in
>>>> JDK-8231610 [1].
>>>>
>>>> This fix will enhance both the flexibility and security of CDS.
>>>>
>>>>
>>>> After this fix, -XX:SharedBaseAddress=N will have the following 
>>>> effect:
>>>>
>>>> N != 0
>>>>
>>>>   CDS archive written with a base address of N. If 
>>>> -XX:SharedBaseAddress
>>>>   is not specified, we will use a default address (0x8_00000000 on
>>>>   64-bit OS).
>>>>
>>>>   At run time, we will first attempt to map the CDS archive at N. 
>>>> If this
>>>>   fails (the OS doesn't have enough contiguous free space at N), we 
>>>> will
>>>>   map the CDS archive at a suitable location picked by the OS.
>>>>
>>>>   Specifying a non-default address could be useful:
>>>>   - for diagnosing purposes
>>>>   - if the default address is already occupied for some reason on your
>>>>     environment, but you have another location that's always free.
>>>>
>>>> N == 0
>>>>
>>>>   At run time, the CDS archive will always be mapped to an address
>>>>   selected by the OS.
>>>>
>>>>   This is useful for improved security -- on OSes that support ASLR 
>>>> (Address
>>>>   Space Layout Randomization), the address of the archived 
>>>> metaspace objects
>>>>   will be randomized on every run to make attacks more difficult.
>>>>
>>>>
>>>> I have added checks to make the code more robust in handling 
>>>> arbitrary values
>>>> of N. See new tests in 
>>>> cest/hotspot/jtreg/runtime/cds/SharedBaseAddress.java,
>>>> and MetaspaceShared::initialize_dumptime_shared_and_meta_spaces().
>>>>
>>>>
>>>> Thanks
>>>> - Ioi
>>>>
>>>> ------
>>>>
>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8231610
>>>>     Relocate the CDS archive if it cannot be mapped to the 
>>>> requested address
>>