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

Andrew Dinn adinn at redhat.com
Tue Nov 6 17:45:33 UTC 2018

On 06/11/18 17:20, Erik Österlund wrote:

> On 2018-11-06 17:46, Andrew Dinn wrote:
   . . .
>> Well, you say that but -- as I said before -- I am not sure why *both*
>> are relevant to the non-Shenandoah case. And, given Roman's correction
>> (doh, yes of course! the space for the Brooks pointer field had better
>> lie in RW-mapped memory :-) I don't see why it is relevant to the
>> Shenandoah case either.
> The reason it is needed is because there may or may not be null checks
> in both the decode() step to get the base pointer of an access, and the
> access itself, given some base pointer. This results in two ranges for
> those two cases correspondingly. If the decoding uses decode_not_null,
> with an implicit null check in the decoding itself that blindly adds the
> heap base without checking for null, we will crash into the first page
> of the heap which is protected when performing accesses into that. But
> sometimes we have a decode()ed base pointer (with null check), that has
> already been fixed up to NULL, and then perform an access at an offset
> into that with an implicit null check. This results in a trap at the
> first page. So in summary, both ranges are needed because the
> implicitness of the null check may be both in the access and the
> narrowOop decoding.

Hmm, that doesn't sound quite kosher. If decode_not_null is being called
in places where we may potentially load a null value (or rather a zero
store value stored in a field) then I would have said that we really
ought to crash and then fix the code to use a decode that tests for
null. If this is recommended a way to optimize loads of potentially null
object fields then it doesn't sound like a reliably performant way to do it.

Could you point me at some place in the compiler where narrow oop loads
are done using a decode_not_null on the assumption that a load of a null
value that gets translated to a low heap_base address will be caught and
corrected by a check initiated from the signal handler?

>> I'd be very glad to hear from someone who knows about the history of the
>> original function as to why the extra test for addresses above heap_base
>> is included [hint: John Rose? David Holmes? Coleen? anyone else who
>> might have been around when it was added?]
>> If that check is not actually needed then getting rid of it would make
>> the whole thing a lot simpler.
> It would indeed. But unfortunately we need it.

I'd still like to hear the history from whoever wrote this code.

>> Well, without the answer as to whether the check against heap_base is
>> needed I am not sure. Assuming it is then then ...
>> Yes, it is possible, in theory, that an oop offset could turn out to
>> have value (heap_base + D) where 0 <= D < os_page_size(). In that case
>> the offset might wrongly be classified as *not* needing a null check
>> even though it really does. I suspect it is unlikely in practice given
>> how mmap hands out addresses but you never know.
>> As for readibility, I think the two functions are probably clearer --
>> expecially if they stay adjacent in the source code.
> Okay, sounds like we are on the same page. I agree it is unlikely that
> this will crash the VM (as evidenced by the lack of crashes observed).
> But I don't like relying on luck for correctness.
Absolutely. Luck is not enough.


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

More information about the hotspot-dev mailing list