RFR: JDK-8213199: GC abstraction for Assembler::needs_explicit_null_check()

[Andrew Dinn]

Hi Erik,

On 06/11/18 14:33, Erik Österlund wrote:

> For offsets, we are interested in the following range:
> [-cell_header_size, os::vm_page_size)
> 
> For addresses, we are interested in the following ranges:
> 1) with heap base: [heap_base - cell_header_size, heap_base +
> os::vm_page_size)
> 2) without heap base: [-cell_header_size, os::vm_page_size)

I don't think that is correct. Even when we are using a heap_base null
oops are still represented by address 0. So, the address range of
interest in case 1 is [-cell_header_size, os::vm_page_size).

That's the value used when a 'null' value is instantiated in memory. It
is also the value tested for by encode_heap_oop when it decides to write
0 into an object field to represent a null oop and is also the value
returned by decode_heap_oop when it detects 0 in an object field.

In that case, I hear you ask, why does the old code check for values in
the range [heap_base - cell_header_size, heap_base + os::vm_page_size)?
Good question. I don't know the answer. I think that test is bogus with
no Shenandoah GC present.

So, anyway, this means your code for uses_implicit_null_check in
assembler.cpp is borked.

> Just expressing these trivial ranges separately would have been very
> easy to understand.

Except it's not that easy. Also, with Shenandoah there is a new wrinkle.
It is possible for an object to be allocated at the first non-guarded
address in the heap area i.e. heap_base + os::vm_page_size. I that case
a dereference of its Brooks pointer will lie in the protected page.

> Thoughts?

I am agnostic as to whether this is done using one or two functions.
However, besides the above issue I believe there is a small error in
your rewrite of os_linux_aarch64.cpp. At line 259 you correct si_addr to
or back in the top byte if bit 55 is non-zero.

359     address addr = (address) info->si_addr;
360
361     // Make sure the high order byte is sign extended, as it may be
masked away by the hardware.
362     if ((uintptr_t(addr) & (uintptr_t(1) << 55)) != 0) {
363       addr = address(uintptr_t(addr) | (uintptr_t(0xFF) << 56));
364     }
365

However, at line 463 you fail to pass the corrected address to
uses_implicit_null_check

462       } else if (sig == SIGSEGV &&
463
MacroAssembler::uses_implicit_null_check(info->si_addr)) {

regards,


Andrew Dinn
-----------
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander