RFR(s): 8191101: Show register content in hs-err file on assert (second version)

[Thomas Stüfe]

Thank you Christoph!

Here the new webrev:

http://cr.openjdk.java.net/~stuefe/webrevs/8191101-show-registers-on-assert/webrev.03/webrev/

- copyright years fixed
- spaces removed as you noted
- Removed one blank line at the end of the jtreg test file.

Best Regards, Thomas



On Wed, Mar 21, 2018 at 9:51 AM, Langer, Christoph <christoph.langer at sap.com
> wrote:

> Hi Thomas,
>
> overall this looks good to me. I went through the changes and can't see
> nothing obviously wrong. But I didn't play with it nor do I have much
> experience myself in the area of error reporting.
>
> I found a few formatting nits:
> In several files you forgot to update the copyright year. But I guess
> you'd check this anyway before pushing.
> Then, in your new test 'test/hotspot/jtreg/runtime/ErrorHandling/
> ShowRegistersOnAssertTest.java' you have two blank lines in the end. I
> guess jcheck would find it, though...
> The if condition in all the os* files 'if ( (sig == SIGSEGV || sig ==
> SIGBUS) && info != NULL && info->si_addr == g_assert_poison)' contains a
> blank after the first '(' which I wouldn't have used there - but that's
> really personal taste ��
>
> Best regards
> Christoph
>
> > -----Original Message-----
> > From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-
> > bounces at openjdk.java.net] On Behalf Of Thomas Stüfe
> > Sent: Dienstag, 20. März 2018 15:12
> > To: Hotspot dev runtime <hotspot-runtime-dev at openjdk.java.net>
> > Subject: RFR(s): 8191101: Show register content in hs-err file on assert
> > (second version)
> >
> > Hi all,
> >
> > May I please have reviews for this new version of this RFE.
> >
> > Bug id:  https://bugs.openjdk.java.net/browse/JDK-8191101
> > webrev:
> > http://cr.openjdk.java.net/~stuefe/webrevs/8191101-show-registers-on-
> > assert/webrev.02/webrev/
> >
> > Please find old discussion thread here:
> > http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2017-
> > October/025018.html
> >
> > And the first review thread from last november:
> > http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2017-
> > November/025371.html
> >
> > Since then, the issue was sleeping in a pile of other issues, but I'd
> like
> > to finish this now.
> >
> > For those who looked at the first version of this fix: this version is
> > substantially smaller than the last one. I removed all the
> > os::safe_memcpy() stuff and unified all the ucontext_t coding locally in
> > debug.cpp. That, at the costs of some very small #ifdefs, greatly reduces
> > the chaff this patch causes (all the copy_ucontext.. functions we had in
> > the os_cpu files before).
> >
> > Thanks, Thomas
>