RFR (L, final): 8218626: Add detailed message to NullPointerException describing what is null.

[forax at univ-mlv.fr]

----- Mail original -----
> De: "Goetz Lindenmaier" <goetz.lindenmaier at sap.com>
> À: "Remi Forax" <forax at univ-mlv.fr>
> Cc: "hotspot-runtime-dev" <hotspot-runtime-dev at openjdk.java.net>, "core-libs-dev" <core-libs-dev at openjdk.java.net>
> Envoyé: Mercredi 18 Septembre 2019 09:37:36
> Objet: RE: RFR (L, final): 8218626: Add detailed message to NullPointerException describing what is null.

> Hi Remi,

Hi Goetz,

> 
>> in bytecodeUtils.cpp, in print_local_var(),
>> i believe that the code
>>   if (!method->is_static() && (slot == 0)) {
>>       os->print("this");
>>   }
>>   ...
>> is only true if the bytecode is generated by javac and ecj, tools like proguard
>> that tries to obfuscate the code will reuse the slot 0 once "this" is not needed
>> anymore.
>> This is obviously also true for any other parameters, so in my opinion, you
>> should not try to be too heroic here and always display "local%d".
> Yes, you are right. I assume the bytecode local slots are mapped 1:1 to
> the parameters and are not reused for other values.
> 
>> The other solution is to propagate "this" and "parameter%d" during the static
>> analysis, so "this" will become "local0" once a store_0 is seen.
> It would be possible to spot the place where "this" is first overwritten.
> For other parameters, this is not feasible, they are not read-only, so
> stores don't indicate obfuscation.

Java is not the only language to run on the Java plateform so try to detect "obfuscation" is again akind of shortcut. 

> 
> I could mark the whole method as 'obfuscated' once I see a store_0,
> and then print "local" instead of "parameter" in all places.
> This does not work for static methods, though. Nor for methods
> where "this" is live to the end, but the parameter slots are
> reused.

and you can have a path with a store_0 and one without (the code is not linear).

> 
> For obfuscated methods I would claim that printing "this" etc
> is fine even if the slot is reused for another value.  It just
> adds to the obfuscation!  But there might be code
> that is just optimized and not meant to be obfuscated.

I've used obfuscation as an example, but there are also valid case to reuse slots likeit will consume less stack memory if you are using a very small device.

> 
> Best regards,
>  Goetz.

regards,
Rémi

> 
> 
> 
> 
> 
> 
>> 
>> Rémi
>> 
>> ----- Mail original -----
>> > De: "Goetz Lindenmaier" <goetz.lindenmaier at sap.com>
>> > À: "hotspot-runtime-dev" <hotspot-runtime-dev at openjdk.java.net>,
>> "core-libs-dev" <core-libs-dev at openjdk.java.net>
>> > Envoyé: Mardi 17 Septembre 2019 16:18:03
>> > Objet: RE: RFR (L, final): 8218626: Add detailed message to
>> NullPointerException describing what is null.
>> 
>> > @core-libs experts, I would appreciate comments on the changes
>> > to NullPointerException.java, especially wrt. the Javadoc comment.
>> > The change there is S.
>> >
>> > Best regards,
>> >  Goetz.
>> >
>> >> -----Original Message-----
>> >> From: Lindenmaier, Goetz
>> >> Sent: Dienstag, 10. September 2019 11:48
>> >> To: 'Hotspot dev runtime' <hotspot-runtime-dev at openjdk.java.net>;
>> Java Core
>> >> Libs <core-libs-dev at openjdk.java.net>
>> >> Subject: RFR (L, final): 8218626: Add detailed message to
>> NullPointerException
>> >> describing what is null.
>> >>
>> >> Hi,
>> >>
>> >>
>> >>
>> >> this is the implementation of JEP 358: Helpful NullPointerExceptions.
>> >>
>> >>
>> >>
>> >> The JEP is in status "Candidate". Coleen (many, many thanks!) ran
>> >>
>> >> it through the Oracle-internal processes.  Now I please need final reviews
>> >>
>> >> for this change so that I can propose it to target jdk 14.
>> >>
>> >>
>> >>
>> >> JEP: https://bugs.openjdk.java.net/browse/JDK-8220715
>> >>
>> >> Enhancement: https://bugs.openjdk.java.net/browse/JDK-8218628
>> >>
>> >> webrev: http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-
>> NPE/16/
>> >>
>> >>
>> >>
>> >> The change ran through a lot of testing, all jtreg and jck tests to name
>> >>
>> >> only some. The webrev points to patch
>> >>
>> >> http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-
>> >> NPE/16/enable_NPE_message.patch
>> >>
>> >> that enables the change by default,  which was useful for testing to
>> >>
>> >> assure the code is used in the tests.
>> >>
>> >> I just pushed the change to jdk/submit once more.
>> >>
>> >>
>> >>
>> >> Please review.
>> >>
>> >>
>> >>
>> >> Thanks!
>> >>
> > > >   Goetz.