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é: Lundi 23 Septembre 2019 12:03:30
> Objet: RE: RFR (L, final): 8218626: Add detailed message to NullPointerException describing what is null.

> Hi Remi,

Hi Goetz,

> 
> what do you think about dealing with the problem like this:
> http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-NPE/18-obfuscation/
> It's at the cost of one 64-bit field per bytecode in the analysis data.
> Also, if there is a real assignment to a parameter it's not named 'parameteri'
> after that any more.  See the example in the test.
> 

yes, it's a nice approximation, having a method with more than 63/64 parameters is rare.

patch looks good, thumbs up !

> Best regards,
>  Goetz.
> 

regards,
Rémi

> 
> 
>> -----Original Message-----
>> From: forax at univ-mlv.fr <forax at univ-mlv.fr>
>> Sent: Freitag, 20. September 2019 00:01
>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
>> Cc: hotspot-runtime-dev <hotspot-runtime-dev at openjdk.java.net>; core-libs-
>> dev <core-libs-dev at openjdk.java.net>
>> Subject: Re: RFR (L, final): 8218626: Add detailed message to
>> NullPointerException describing what is null.
>> 
>> ----- 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.