Null check object parameter of unsafe access even if it's known to be non null

[Roman Kennke]

Couldn't we have barriers that don't ever generate a null check? E.g.
shenandoah_read_barrier_not_null()? Or would that crash too?

Other than that, I have no objections.

Roman

Am Donnerstag, den 16.02.2017, 14:45 +0100 schrieb Roland Westrelin:
> This is the bug that Aleksey spotted on 8u but also applies to 9.
> 
> A little bit of background: An unsafe access at an offset that's
> small
> is guaranteed to be an access to an object field (or array element)
> and
> so implicitly the object is not null. Currently the C2 IR contains a
> cast in that case (that is C2 knows the object is not null but no
> null
> check is emitted). There are 2 reasons for the cast:
> 
> 1) we don't need the following barrier to check for null
> 
> 2) without it we have:
> 
> if (o != null) {
>   o = read/write barrier(o)
> } else {
>   o = null;
> }
> access to o
> 
> which c2 could transform to:
> 
> if (o != null) {
>   o = read/write barrier(o)
>   access to o
> } else {
>   o = null;
>   access to o
> }
> 
> and in the else branch c2 sees an oop access to a null object,
> something
> impossible and it crashes.
> 
> Now the current problem is if we have a check for null with both
> branches that the compiler suppose can be taken followed by an unsafe
> access to something the compiler knows is not null:
> 
> if (o != null) {
>  // do something
> } else {
>   o = null;
> }
> o = cast_non_null(o);
> o = read_barrier(o);
> i = o.f;
> 
> the compiler can push the cast in both branches:
> 
> if (o != null) {
>  // do something
>   o = cast_non_null(o);
> } else {
>   o = cast_non_null(null);
> }
> 
> o = read_barrier(o);
> i = o.f;
> 
> In the else branch, o is dead but the control flow is not. That
> inconsistency causes the compiler to crash. A fix is to change the
> cast
> to a null check. Then in the case above, the else control flow also
> becomes dead and there's no inconsistency. It's unfortunate to add a
> check for null for something we know is not but the null check should
> become an implicit null check and so be virtually free.
> 
> http://cr.openjdk.java.net/~roland/shenandoah/nullcheck_unsafeaccess/
> webrev.00/
> 
> Roland.