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

[Kevin Walls]

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().

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.

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#l381.1
>>>>>> and
>>>>>> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/a41fe5ffa839#l401.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
>>>>>>