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

Tue May 5 21:19:58 UTC 2020

On 5/5/20 12:45 PM, Thomas Stüfe wrote:
> Hi Ioi,
>
> thanks for fixing this!
>
> Not a full review yet, just a remark inline.
>
>
> On Tue, May 5, 2020 at 8:42 PM Ioi Lam <ioi.lam at oracle.com 
> <mailto:ioi.lam at oracle.com>> 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).
>
>
> Good!
>
>
>        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.
>
>
> Why would someone need this at runtime?
>
>        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.
>
>
> But if the default address is taken the OS (or the platform layer in 
> the VM, see the aarch64 code) will choose a proper placement for the 
> CDS+class space. What could be gained from manually interfering with 
> that process? We already have to pay the relocation costs, so we gain 
> nothing performance wise.
>

Hi Thomas,

Actually, the placement chosen by the OS under this circumstance may not 
be "proper".

Let me first start explain why currently we always write the archive at 
0x800000000.

(The bug description should be more clear -- it's not a bug to always 
write to 0x800000000. That was intentional. The bug is now there's no 
way to specify an address other than 0x800000000 -- unless you edit 
globals.hpp and rebuild the JDK).

During JDK-8231610 (Relocate the CDS archive if it cannot be mapped to 
the requested), my biggest concern was this:

When we build an official release build on Linux, perhaps once in a blue 
moon 0x800000000 is unavailable. But do we want to ship an official JDK  
with a default CDS archive whose base address happened to be picked 
randomly on the specific host that built the JDK? We have never tested 
against such an address. It's not a "proper" address as we don't know 
whether it will even work on the huge variety of hosts used by our 
users. So this is a black swan event -- it mostly likely won't happen, 
but it might. If it does happen, it has unknown negative impact (e.g., 
the default CDS archive may not work in half of the machines in the 
world). How do we handle it?

We have tested with 0x800000000 millions of times, and it has worked for 
many JDK releases, so it seems safer to always release a JDK that uses 
this base address.

We could detect the black swan event during the JDK build, and 
regenerate the archive until we get 0x800000000. But if the chance of 
this happening is very low, how do we test our Makefile logic, and what 
if the build keeps spinning and never gives us 0x800000000?

So, the decision in JDK-8231610 was to always relocate the archive to 
0x800000000 before writing it. (We test this functionality with 
"-Xshare:dump -XX:ArchiveRelocationMode=1" in our regression tests). 
This ensures that the black swan event never happens during the JDK build.

===============

I agree that your concerns below are valid. The question is -- do we 
need to provide a method to specify an address different than 0x800000000?

If the answer is unclear, I think we can shelf this fix until someone 
actually complains :-)

Also, in light of Project Leyden, we may ditch the proprietary JSA file 
format and store the CDS data into a platform-specific DLL file (along 
with the AOT code). We will probably need a different approach.

> The problem I have with honoring SharedBaseAddress at runtime is twofold:
> - it increases the complexity of the reservation. One more 
> configuration option to test.
> - it gives the user ample opportunity to shoot himself in the foot. 
> Examples:
>   -> User specifies an address which puts CDS/class space in the way 
> of the sbrk, on platforms where that matters. You may get malloc OOMs 
> out of the blue. The platform code is aware of this and usually deals 
> with it.
>   -> You bypass the optimal cds placement done by the platform and 
> either specify a base address which does not work at all or which does 
> not allow the best CompressedKlassPointer encoding possible on the 
> platform. See also the discussions about better CompressedKlassPointer 
> encoding on aarch64-dev, and we may do something similar on ppc.
>   -> More subtle: a user who misinterprets the option may specify it 
> always. I bet you this option finds its way as "optimization 
> technique" into any number of blogs :) but by specifying it at runtime 
> you always pay for the relocation cost. Even if the primary address 
> baked into the archive were free and would work just fine.
>
> I think if the user wants a different placement for cds+class space, 
> he should regenerate the archives anew.
>
>     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.
>
>
> is this any different from -XX:ArchiveRelocationMode=1 ?      [edited]

In practice there's not much difference, except that 
ArchiveRelocationMode is a diagnostic flag.

Thanks
- Ioi

>
>     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
>
>
> Thank you!
>
> ..Thomas