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.cimadam...@oracle.com>
Sent: Freitag, 15. März 2019 12:33
To: Lindenmaier, Goetz <goetz.lindenma...@sap.com>; Mandy Chung
<mandy.ch...@oracle.com>; Roger Riggs <roger.ri...@oracle.com>
Cc: Java Core Libs <core-libs-dev@openjdk.java.net>; hotspot-runtime-
d...@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.

Reply via email to