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

[Lindenmaier, Goetz]

Hi Remi,

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

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.

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.

Best regards,
  Goetz.






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