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

[Lindenmaier, Goetz]

Hi Remi, 

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.

Best regards,
  Goetz.



> -----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.