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

[Lindenmaier, Goetz]

Hi,

> 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.
Do you propose to have two different messages, to be selected by some 
flag or the like?
To my knowledge exceptions have only one message, and it should
be as helpful as possible I think.

>  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 :-)).
Can you please elaborate why they have to understand the JVMS?

And why would information you don't need be unacceptable?
You can just ignore it ... 

Best regards,
  Goetz.
 
> 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.