[aarch64-port-dev ] RFR(M): 8233743: AArch64: Make r27 conditionally allocatable

[Andrew Haley]

On 11/28/19 7:50 AM, Nick Gasson wrote:
> Hi Andrew,
> 
>>>
>>> CompressedKlassPointers::base() => 0xffff0b4b5000
>>> CompressedKlassPointers::shift() => 3
>>
>> This is bad. Can you have a look at the allocation code to see why the search
>> for an appropriate address range fails?
> 
> We have a loop in Metaspace::allocate_metaspace_compressed_klass_ptrs 
> that searches for a 4G aligned location for the compressed class space 
> on AArch64, but this search is not done if CDS is in use and the archive 
> was loaded successfully, because in that case the class space has 
> already been mapped (i.e. `metaspace_rs.is_reserved()' is true).

Right. At the time I wrote that code, CDS was not much used by anything, so
I thought of it as a mariganl use case.

> Previously it was only possible to map the CDS archive at 0x800000000. 
> The compressed class base is set to the start of this region which 
> happens to be 4G aligned so our MacroAssembler::load_klass optimisation 
> applies and we emit the short code sequence.
> 
> With the recent change in 8231610, if the CDS archive cannot be mapped 
> at that address (e.g. because of ASLR or because the heap is mapped 
> there) then the CDS archive will be relocated to an arbitrary address 
> decided by mmap. That's where the oddly-aligned compressed klass base 
> above comes from. This causes MacroAssembler::load_klass to emit the 
> inefficient sequence which then overflows the buffer for the itable stub 
> (the worst-case size estimate there is wrong, which needs to be fixed 
> separately).

Correcting the stub size is a minor tidy-up which does not really need
its own Bug ID.

> A minimal way to reproduce this is:
> 
> $ java -XX:HeapBaseMinAddress=33G -Xshare:on -Xlog:cds=debug -version
> ...
> [0.050s][info ][cds] CDS archive was created with max heap size = 128M, 
> and the following configuration:
> [0.050s][info ][cds]     narrow_klass_base = 0x0000fffec7507000, 
> narrow_klass_shift = 3
> ...
> #  guarantee(masm->pc() <= s->code_end()) failed: itable #2: overflowed 
> buffer, estimated len: 180, actual len: 184, overrun: 4
> 
> 
> I suggest we move the 4G-aligned search from 
> allocate_metaspace_compressed_klass_ptrs into its own function that can 
> then be called from MetaspaceShared::reserve_shared_space when 
> requested_address==NULL (i.e. the fallback path when mmap at 0x800000000 
> fails). If you're happy with this I'll make a patch for review?

Yes, that sounds excellent. We really need it to avoid compressed
class pointers becoming an expensive option.

-- 
Andrew Haley  (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671