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

[Calvin Cheung]

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
>