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

[Lindenmaier, Goetz]

Hi,

incorporating comments by Coleen, I prepared a new webrev:
http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-NPE/08/ 
incremental webrev:
http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-NPE/08-incremental/

I renamed the following new classes: 
  TrackingStackCreator  -> ExcpetionMessageBuilder
  SimulatedOperandStack -> SimulatedOperandStack
  StackSlotAnalysisData -> StackSlotAnalysisData
and moved their declaration to the .cpp file.

A new wrapper class BytecodeUtils contains the 
method that is called from jvm.cpp. Some code
is moved from jvm.cpp to bytecodeUtils.cpp
so the internal data structures are no more exposed.

StackSlotAnalysisData now has two fields getting
rid of / and %. This also makes debugging more simple.

Lots of comments were added or improved.

I adapted test langtools/jdk/jshell/ToolSimpleTest.java
to the new message.

Coleen, thanks for your help!

Best regards,
  Goetz.



> -----Original Message-----
> From: Lindenmaier, Goetz
> Sent: Freitag, 12. April 2019 12:33
> To: 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,
> 
> while waiting for progress on corresponding the JEP, I improved
> the implementation of generating the NPE message.  It now uses
> a single outputStream. This removes several allocations of temporary
> data. I also removed TrackingStackSource. The analysis code originally
> addressed several use cases, for NullPointerExceptions this is
> not needed.  I cleaned up bytecodeUtils from some code not (really) needed.
> 
> I split get_null_pointer_slot() into two methods: get_NPE_null_slot()
> and print_NPE_failed_action(). This simplifies the
> implementation, and streamlines it more with the text in the JEP.
> 
> I print methods using the code added in
> "8221470: Print methods in exception messages in java-like Syntax.",
> so it now prints 'void m(int)' instead of 'm(I)V'.
> 
> I implemented a row of new test cases, and rearranged the test
> to test the message part of print_NPE_failed_action() and
> print_NPE_cause() separated. I made sure all bytecodes
> handled in these methods are covered.
> Further I arranged the tests in methods according to the
> functional properties as discussed in the JEP.
> 
> http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-NPE/07
> 
> Best regards,
>   Goetz.
> 
> 
> 
> 
> 
> > -----Original Message-----
> > From: Lindenmaier, Goetz
> > Sent: Donnerstag, 14. März 2019 21:56
> > To: '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' <hotspot-runtime-dev at openjdk.java.net>
> > Subject: RE: RFR(L): 8218628: Add detailed message to NullPointerException
> > describing what is null.
> >
> > Hi,
> >
> > I had promised to work on a better wording of the messages.
> >
> > This I deliver with this webrev:
> > http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-NPE/05-
> > otherMessages/
> >
> > The test in the webrev is modified to just print messages along with the
> > code that raised the messages.
> >
> > Please have a look at these files with test output contained in the webrev:
> > Messages with debug information:
> > http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-NPE/05-
> > otherMessages/output_with_debug_info.txt
> > Messages without debug information:
> > http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-NPE/05-
> > otherMessages/output_no_debug_info.txt
> >
> > Especially look at the first few messages, they point out the usefulness
> > of this change.  They precisely say what was null in a chain of dereferences.
> >
> > Best regards,
> >   Goetz.
> >
> >
> > > -----Original Message-----
> > > From: Lindenmaier, Goetz
> > > Sent: Wednesday, February 13, 2019 10:09 AM
> > > To: '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 Mandy,
> > >
> > > Thanks for supporting my intend of adding the message as such!
> > > I'll start implementing this in Java and come back with a webrev
> > > in a while.
> > >
> > > In parallel, I would like to continue discussing the other
> > > topics, e.g., the wording of the message. I will probably come up
> > > with a separate webrev for that.
> > >
> > > Best regards,
> > >   Goetz.
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: core-libs-dev <core-libs-dev-bounces at openjdk.java.net> On Behalf
> > > > Of Mandy Chung
> > > > Sent: Tuesday, February 12, 2019 7:32 PM
> > > > To: 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.
> > > >
> > > > On 2/8/19 11:46 AM, Roger Riggs wrote:
> > > > > Hi,
> > > > >
> > > > > A few higher level issues should be considered, though the details
> > > > > of the webrev captured my immediate attention.
> > > > >
> > > > > Is this the right feature and is this the right level of implementation
> > > > > (C++/native)?
> > > > > :
> > > > > How much of this can be done in Java code with StackWalker and other
> > > > > java APIs?
> > > > > It would be a shame to add this much native code if there was a more
> > > > robust
> > > > > way to implement it using APIs with more leverage.
> > > >
> > > > Improving the NPE message for better diagnosability is helpful while
> > > > I share the same concern Roger raised.
> > > >
> > > > Implementing this feature in Java and the library would be a better
> > > > choice as this isn't absolutely required to be done in VM in native.
> > > >
> > > > NPE keeps a backtrace capturing the method id and bci of each stack
> > > > frame.  One option to explore is to have StackWalker to accept a
> > > > Throwable object that returns a stream of StackFrame which allows
> > > > you to get the method and BCI and also code source (I started a
> > > > prototype for JDK-8189752 some time ago).  It can use the bytecode
> > > > library e.g. ASM to read the bytecode.  For NPE message, you can
> > > > implement a specialized StackFrameTraverser just for building
> > > > an exception message purpose.
> > > >
> > > > Mandy