hg: valhalla/valhalla: 8198748: [lworld] Javac should insert null checks at casts to value types.

[Srikanth]

On Monday 12 March 2018 12:29 PM, Remi Forax wrote:
> Hi Srikanth,
> thanks for doing that.
>
> There is already a code path in the compiler to generate nullchecks using getClass() + pop,
> i think you should reuse this code instead of adding another way to do a nullcheck.
Hello Remi,

Javac *already* employs two distinct mechanisms to implement null checks:

Please see: Gen#genNullCheck

/** Generate a null check from the object value at stack top. */ private 
void genNullCheck(JCTree tree) {
     code.statBegin(tree.pos); if (allowBetterNullChecks) {
         callMethod(tree.pos(), syms.objectsType, names.requireNonNull, List.of(syms.objectType), true); }else {
         callMethod(tree.pos(), syms.objectType, names.getClass, List.nil(), false); }
     code.emitop0(pop); }


where the controlling boolean is initialized as:

allowBetterNullChecks =target.hasObjects();


and target.hasObjects() is really:

/** Does the target JDK contains the java.util.Objects class? */ public 
boolean hasObjects() {
     return compareTo(JDK1_7) >=0; }


In short, I am not expressly selecting the method of implementing the 
null check, only inserting a null-check AST node.

So this pattern is already emitted by javac and any/all analyzers should 
anyway be upgraded (should have been upgraded already)

Thanks!
Srikanth

> And with my hat of backporter/bytecode analyzer writer, adding a new runtime dependency, here, to Objects.requireNonNull, makes things more complex because some analyzers may have to be enhanced to recognize this new pattern.
>
> cheers,
> Rémi
>
> ----- Mail original -----
>> De: "Srikanth" <srikanth.adayapalam at oracle.com>
>> À: "valhalla-dev" <valhalla-dev at openjdk.java.net>
>> Envoyé: Lundi 12 Mars 2018 07:32:13
>> Objet: Re: hg: valhalla/valhalla: 8198748: [lworld] Javac should insert null checks at casts to value types.
>> On Monday 12 March 2018 11:51 AM, srikanth.adayapalam at oracle.com wrote:
>>> Changeset: 7c1c51c340e6
>>> Author:    sadayapalam
>>> Date:      2018-03-12 11:46 +0530
>>> URL:       http://hg.openjdk.java.net/valhalla/valhalla/rev/7c1c51c340e6
>>>
>>> 8198748: [lworld] Javac should insert null checks at casts to value types.
>> Notes:
>>
>> Where Point is a value type, code generated for casting an object to
>> Point as in:
>>
>> Object o = ...
>>
>> Point p = (Point) o;
>>
>> is:
>>
>>           2: aload_1
>>           3: dup
>>           4: invokestatic  #2                  // Method
>> java/util/Objects.requireNonNull:(Ljava/lang/Object;)Ljava/lang/Object;
>>           7: pop
>>           8: checkcast     #3                  // class Point
>>          11: astore_2
>>
>> If the cast occurs in source code compiled at a level where value types
>> are not recognized as such, then there is no insertion of null check.
>>
>> (FWIW, Javac is not in the business of nullness tracking, so these
>> checks will be inserted even where a cursory glance of source code
>> context could inform a reader that no null check would be needed)
>>
>> Thanks
>> Srikanth
>>
>>> ! src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java
>>> ! src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java
>>> + test/langtools/tools/javac/valhalla/lworld-values/CastNoNullCheckTest.java
>>> + test/langtools/tools/javac/valhalla/lworld-values/CastNullCheckTest.java