RFR JDK-8194642: Improve error reporting in hs_error file for JDK8

[Fairoz Matte]

Thanks David and Kevin for the reviews.

> -----Original Message-----
> From: Kevin Walls
> Sent: Monday, March 05, 2018 4:06 PM
> To: David Holmes <david.holmes at oracle.com>; Fairoz Matte
> <fairoz.matte at oracle.com>; hotspot-compiler-dev at openjdk.java.net;
> hotspot-runtime-dev at openjdk.java.net
> Subject: Re: RFR JDK-8194642: Improve error reporting in hs_error file for
> JDK8
> 
> Hi Fairoz,
> 
> Yes, looks good and useful.  And thanks David.
> 
> In webrev.01 you want a blank line after the function end at 346 and the start
> of the comment for VMError::report().
Added an extra line

> 
> If you can add a link to
> https://bugs.openjdk.java.net/browse/JDK-8064814 to this change in jbs
> that would be good, as you're kind of backporting that plus some part of
> 8026324 / 8026333 / 8026336 where print_oom_reasons() moves to its own
> function.
Links have been added.

Thanks,
Fairoz
> 
> Thanks
> Kevin
> 
> 
> On 05/03/2018 06:23, David Holmes wrote:
> > On 5/03/2018 3:10 PM, Fairoz Matte wrote:
> >> Hi David,
> >>
> >>> -----Original Message-----
> >>> From: David Holmes
> >>> Sent: Monday, March 05, 2018 10:35 AM
> >>> To: Fairoz Matte <fairoz.matte at oracle.com>; hotspot-compiler-
> >>> dev at openjdk.java.net; hotspot-runtime-dev at openjdk.java.net
> >>> Subject: Re: RFR JDK-8194642: Improve error reporting in hs_error
> >>> file for
> >>> JDK8
> >>>
> >>> Hi Fairoz,
> >>>
> >>> This seems fine.
> >>>
> >>> Not sure why you need the extra blank line output here:
> >>>
> >>>          if (_verbose && _siginfo) {
> >>> +        st->cr();
> >>>            os::print_siginfo(st, _siginfo);
> >>>
> >>
> >> It is just to get on next line, I will revert this change.
> >> I hope, next webrev for this change is not required?
> >
> > No, no need for a new webrev. All the preceding sections end with
> > st->cr(), which seems to be the basic pattern in this code.
> >
> > Thanks,
> > David
> >
> >> Thanks,
> >> Fairoz
> >>
> >>> Thanks,
> >>> David
> >>>
> >>> On 5/03/2018 2:29 PM, Fairoz Matte wrote:
> >>>> Hi David,
> >>>>
> >>>> Thanks for the review.
> >>>> Restricting this issue only to improve OOM related error messaging.
> >>>> Fatal
> >>> error reporting can be taken separately as there is already couple
> >>> of other fatal errors need to be handled in similar way.
> >>>> Changed the description and scope of the work.
> >>>>
> >>>> Kindly review the webrev.01 having OOM related changes.
> >>>> http://cr.openjdk.java.net/~fmatte/8194642/webrev.01/
> >>>>
> >>>> JBS - https://bugs.openjdk.java.net/browse/JDK-8194642
> >>>>
> >>>> Thanks,
> >>>> Fairoz
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: David Holmes
> >>>>> Sent: Monday, February 26, 2018 11:00 AM
> >>>>> To: Fairoz Matte <fairoz.matte at oracle.com>; hotspot-compiler-
> >>>>> dev at openjdk.java.net; hotspot-runtime-dev at openjdk.java.net
> >>>>> Subject: Re: RFR JDK-8194642: Improve error reporting in hs_error
> >>>>> file for
> >>>>> JDK8
> >>>>>
> >>>>> Hi Fairoz,
> >>>>>
> >>>>> On 26/02/2018 2:10 PM, Fairoz Matte wrote:
> >>>>>> Hi All,
> >>>>>>
> >>>>>> Kindly review the small enhancement for 8u-dev, which is a mini
> >>>>>> backport
> >>>>> of JDK-8136421, only things related to hs_error file improvements
> >>>>> were considered.
> >>>>>> JBS - https://bugs.openjdk.java.net/browse/JDK-8194642
> >>>>>> Webrev - http://cr.openjdk.java.net/~fmatte/8194642/webrev.00/
> >>>>>>
> >>>>>> Reference
> >>>>>> JDK9 bug - https://bugs.openjdk.java.net/browse/JDK-8136421
> >>>>>> JDK9 changeset -
> >>>>>>
> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/a41fe5ffa839#l38
> >>>>>> 1.1
> >>>>>> and
> >>>>>>
> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/a41fe5ffa839#l40
> >>>>>> 1.1
> >>>>>
> >>>>> src/share/vm/utilities/vmError.cpp
> >>>>>
> >>>>> The backport of the OOM reason changes seems quite reasonable.
> >>>>>
> >>>>> src/share/vm/runtime/sharedRuntime.cpp
> >>>>>
> >>>>> It is not at all clear to me that simply doing "return NULL" is
> >>>>> sufficient to achieve the desired goal here. Given all the other
> >>>>> changes that were done in
> >>>>> 8136421 I can't tell if something else may be needed for this part
> >>>>> - which seems to be the key change you are after. I have to wonder
> >>>>> why we did not already just "return NULL" if regular error
> >>>>> reporting can already
> >>> handle it?
> >>>>>
> >>>>>> Testing: JPRT no issues found
> >>>>>
> >>>>> What crash testing have you done to verify that the new error
> >>>>> reports are as expected?
> >>>>>
> >>>>> Thanks,
> >>>>> David
> >>>>>
> >>>>>> Thanks,
> >>>>>> Fairoz
> >>>>>>
>