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

[Mandy Chung]

That's a good suggestion.  The JDK out-of-the-box case and the
complex environment case with instrumentation agents transforming
the bytecodes are two different use cases.  This brings up one
issue that when constructing the enhanced NPE message, a method
might become obsolete due to redefintion, it should handle
gracefully.

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.