RFR(L): 8218628: Add detailed message to NullPointerException describing what is null.

[Lindenmaier, Goetz]

Hi Mandy,

> ... This brings up one
> issue that when constructing the enhanced NPE message, a method
> might become obsolete due to redefintion, it should handle
> gracefully.
The C++ solution has access to the proper version of redefined codes,
so this is no problem.  It is attached to the backtrace.
And yes, in the Java variant we could just skip the message if we don't 
have the proper variant of the bytecode at hand. But we need 
a way to find out that this is the case.  It's unclear to me how this 
should work ... it also depends on the way we retrieve the bytecodes
to analyze.

Best regards,
  Goetz.



> Mandy
> 
> On 3/15/19 10:34 AM, Maurizio Cimadamore wrote:
> > You touch an important point here - who is this feature for? Is it for
> > the casual developer in need of some hand holding? Or is it for
> > diagnosing complex instrumented stacks? I think the two use cases are
> > not necessarily similar and might be served by different sets of
> > enhancements. In the latter case it might be ok, for instance, to say
> > "and, btw, the NPE came from local[1]". But for users in the former
> > bucket (and most users, really), this level of detail will simply be
> > unacceptable (since now they have to understand the JVMS to parse the
> > message :-)).
> >
> > I suggest we separate the use cases, at least for the purpose of the
> > design discussion.
> >
> > Maurizio
> >
> > On 15/03/2019 13:11, Lindenmaier, Goetz wrote:
> >> Hi Maurizio,
> >>
> >> thanks for sharing your patch!  This is about what I thought about
> >> so far, just that it's working already :)
> >>
> >> It prints the information of the failing bytecode.  Adding the dataflow
> >> analysis, the other information could be printed as well.
> >>
> >> The major problem I see is here:
> >> +            File f = new File(location.getFile(),
> >> +                    clazz.getCanonicalName().replaceAll("\\.", "/") +
> >> ".class");
> >> +            ClassReader cr = new ClassReader(new FileInputStream(f));
> >> +            ClassNode classNode = new ClassNode();
> >> +            cr.accept(classNode, 0);
> >>
> >> As I understand, this reads the Bytecode from the classfile.
> >> The bytecode in the classfile must not match that executed
> >> by the VM, i.e, the pc you get from the stackWalker will not
> >> reference the bytecode that caused the NPE.
> >>
> >> Other issues have been discussed in the mail thread before.
> >> Good that you point this out, I'll add it to the JEP.
> >>
> >> Best regards,
> >>    Goetz
> >>
> >> ... Sorry, I missed the "reply all" on my first reply.
> >>
> >>
> >>
> >>
> >>> -----Original Message-----
> >>> From: Maurizio Cimadamore <maurizio.cimadamore at oracle.com>
> >>> Sent: Freitag, 15. März 2019 12:33
> >>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Mandy Chung
> >>> <mandy.chung at oracle.com>; Roger Riggs <Roger.Riggs at oracle.com>
> >>> Cc: Java Core Libs <core-libs-dev at openjdk.java.net>; hotspot-runtime-
> >>> dev at openjdk.java.net
> >>> Subject: Re: RFR(L): 8218628: Add detailed message to
> >>> NullPointerException
> >>> describing what is null.
> >>>
> >>> Hi Goetz,
> >>> please find the attached ASM-based patch. It is just a PoC, as such it
> >>> does not provide as fine-grained messages as the one discussed in the
> >>> RFE/JEP, but can be enhanced to cover custom debugging attribute, I
> >>> believe.
> >>>
> >>> When running this:
> >>>
> >>> Object o = null;
> >>> o.toString();
> >>>
> >>> you get:
> >>>
> >>> Exception in thread "main" java.lang.NullPointerException: attempt to
> >>> dereference 'null' when calling method 'toString'
> >>>       at org.oracle.npe.NPEHandler.main(NPEHandler.java:103)
> >>>
> >>> While when running this:
> >>>
> >>> Foo foo = null;
> >>> int y = foo.x;
> >>>
> >>> You get this:
> >>>
> >>> Exception in thread "main" java.lang.NullPointerException: attempt to
> >>> dereference 'null' when accessing field 'x'
> >>>       at org.oracle.npe.NPEHandler.main(NPEHandler.java:105)
> >>>
> >>> One problem I had is that ASM provides no way to get the instruction
> >>> given a program counter - which means we have to scan all the bytecodes
> >>> and update the sizes as we go along, and, ASM unfortunately doesn’t
> >>> expose opcode sizes either. A more robust solution would be to have a
> >>> big switch which returned the opcode size of any given opcode. Also,
> >>> accessing to StackWalker API on exception creation might not be
> >>> desirable in terms of performances, so this might be one of these area
> >>> where some VM help could be beneficial. Another problem is that we
> >>> cannot distinguish between user-generated exceptions (e.g. `throw new
> >>> NullPointerException`) and genuine NPE issued by the VM.
> >>>
> >>> But I guess the upshot is that it works to leave all the gory detail of
> >>> bytecode grovelling to a bytecode API - if the logic is applied lazily,
> >>> then the impact on performances should be minimal, and the solution
> more
> >>> maintainable longer term.
> >>>
> >>> Cheers
> >>> Maurizio
> >>>
> >>> On 15/03/2019 07:59, Lindenmaier, Goetz wrote:
> >>>> Yes, it would be nice if you shared that.