RFR(m): 8145184: [aix] Implement os::platform_print_native_stack on AIX

[Thomas Stüfe]

Hi Goetz,

thanks a lot for reviewing!

See here the new webrev:

http://cr.openjdk.java.net/~stuefe/webrevs/8145184-aix-implement-os_platform_print_native_stack/webrev.01/webrev/

Comments inline.

On Wed, Jan 13, 2016 at 12:48 PM, Lindenmaier, Goetz <
goetz.lindenmaier at sap.com> wrote:

> Hi Thomas,
>
> I had a look at your change.
>
> The construction with #ifndef PLATFORM_PRINT_NATIVE_STACK is quite
> unconventional ... but it was there before, so it's not an issue of your
> change.
>
>
Yes.. :-/


> It's good to have this now, I've experienced missing stack traces before.
>
> Is it necessary to use macro ERRBYE?  It doesn't really help reading the
> code, as the 'return' is hidden in it.  I would remove it with this change.
>
>
You are right, I removed than macro.


> Don't you have to remove the '== 0' here?  You changed the return value
> from '0' to true.
> -      if (getFuncName((codeptr_t) p, funcname, sizeof(funcname),
> &displacement,
> -                      NULL, NULL, 0, false) == 0) {
> +      if (AixSymbols::get_function_name(p, funcname, sizeof(funcname),
> +                      &displacement, NULL, true) == 0) {
>
>
Good catch! Found two places where I did not revert the logic, fixe and
tested.


> Get_module_name has wrong indentation of parameters.  Probably they fit
> into
> one line.
>
Besides that looks good.


Thanks!

Kind Regards, Thomas


>
>
Best regards,
>   Goetz.
>
>
>
>
> > -----Original Message-----
> > From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-
> > bounces at openjdk.java.net] On Behalf Of Thomas Stüfe
> > Sent: Montag, 11. Januar 2016 14:40
> > To: hotspot-runtime-dev at openjdk.java.net; ppc-aix-port-
> > dev at openjdk.java.net
> > Subject: Re: RFR(m): 8145184: [aix] Implement
> > os::platform_print_native_stack on AIX
> >
> > Hi all,
> >
> > could I please have a review?
> >
> > Thank you!
> >
> > ..Thomas
> >
> > On Wed, Dec 16, 2015 at 3:18 PM, Thomas Stüfe
> > <thomas.stuefe at gmail.com>
> > wrote:
> >
> > > Hi all,
> > >
> > > please review this AIX-only patch.
> > >
> > > This ports a more sophisticated native callstack printer for AIX to the
> > > OpenJDK. It prints out more information and is more robust in cases the
> > > stack is corrupted.
> > >
> > > bug: https://bugs.openjdk.java.net/browse/JDK-8145184
> > > webrev:
> > > http://cr.openjdk.java.net/~stuefe/webrevs/8145184-aix-implement-
> > os_platform_print_native_stack/webrev.00/webrev/
> > >
> > > Note that also some of functions were moved and made more coding-
> > conform
> > > to the OpenJDK: the old getFuncName() and getModuleName() functions
> > are
> > > moved to AixSymbols::get_function_name() and
> > AixSymbols::get_module_name().
> > >
> > > Thanks and Kind Regards, Thomas
> > >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/attachments/20160114/b49b538b/attachment-0001.html>