RFR(S): 8196401: PPC64+s390: get_frame_at_stack_banging_point uses wrong PC

[Doerr, Martin]

Hi Thomas,

thanks for clarification. Yes, I had thought in terms of storage duration. I agree.
Thanks again for reviewing.

Best regards,
Martin


From: Thomas Stüfe [mailto:thomas.stuefe at gmail.com]
Sent: Mittwoch, 31. Januar 2018 08:59
To: Doerr, Martin <martin.doerr at sap.com>
Cc: hotspot-compiler-dev at openjdk.java.net; Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Schmidt, Lutz <lutz.schmidt at sap.com>
Subject: Re: RFR(S): 8196401: PPC64+s390: get_frame_at_stack_banging_point uses wrong PC

Hi Martin,

On Tue, Jan 30, 2018 at 4:45 PM, Doerr, Martin <martin.doerr at sap.com<mailto:martin.doerr at sap.com>> wrote:
Hi Thomas,

the storage class „static” is used by default, so the 2 versions are equivalent.

We misunderstood each other. Default storage class for global functions is "external". static is needed to avoid symbol export.

But I’m ok with using the keyword to make it explicitly static.

I also need to update copyright information.

New webrev:
http://cr.openjdk.java.net/~mdoerr/8196401_ppc64_s390_stack_bang_pc/webrev.01/


Looks fine to me. Thanks for fixing.

..Thomas

Best regards,
Martin


From: Thomas Stüfe [mailto:thomas.stuefe at gmail.com<mailto:thomas.stuefe at gmail.com>]
Sent: Dienstag, 30. Januar 2018 16:04

To: Doerr, Martin <martin.doerr at sap.com<mailto:martin.doerr at sap.com>>
Cc: hotspot-compiler-dev at openjdk.java.net<mailto:hotspot-compiler-dev at openjdk.java.net>; Lindenmaier, Goetz <goetz.lindenmaier at sap.com<mailto:goetz.lindenmaier at sap.com>>; Schmidt, Lutz <lutz.schmidt at sap.com<mailto:lutz.schmidt at sap.com>>
Subject: Re: RFR(S): 8196401: PPC64+s390: get_frame_at_stack_banging_point uses wrong PC

Hi Martin,

On Tue, Jan 30, 2018 at 3:57 PM, Doerr, Martin <martin.doerr at sap.com<mailto:martin.doerr at sap.com>> wrote:
Hi Thomas,

thanks for reviewing.

> Could you please move ucontext_get_lr() to the Aix resp Linux namespaces, like the other ucontext_xxx functions, or at least make them static file scope ?

This could be done on AIX, but I don’t see how it could be done on Linux without touching shared linux code. I don’t want to change non-ppc/s390 files because the PC retrieval is specific to these platforms.

I see what you mean. I am fine with keeping it file static.

The new functions are already static file scope. Or did you mean something else?


I mean:

-address ucontext_get_lr(const ucontext_t * uc) {
+static address ucontext_get_lr(const ucontext_t * uc) {

Best Regards, Thomas

Best regards,
Martin


From: Thomas Stüfe [mailto:thomas.stuefe at gmail.com<mailto:thomas.stuefe at gmail.com>]
Sent: Dienstag, 30. Januar 2018 15:42
To: Doerr, Martin <martin.doerr at sap.com<mailto:martin.doerr at sap.com>>
Cc: hotspot-compiler-dev at openjdk.java.net<mailto:hotspot-compiler-dev at openjdk.java.net>; Lindenmaier, Goetz <goetz.lindenmaier at sap.com<mailto:goetz.lindenmaier at sap.com>>; Schmidt, Lutz <lutz.schmidt at sap.com<mailto:lutz.schmidt at sap.com>>
Subject: Re: RFR(S): 8196401: PPC64+s390: get_frame_at_stack_banging_point uses wrong PC

Hi Martin,

Could you please move ucontext_get_lr() to the Aix resp Linux namespaces, like the other ucontext_xxx functions, or at least make them static file scope ?

Otherwise, this looks good.

Regards, Thomas

On Tue, Jan 30, 2018 at 2:47 PM, Doerr, Martin <martin.doerr at sap.com<mailto:martin.doerr at sap.com>> wrote:
Hi,

we have found a bug in get_frame_at_stack_banging_point on PPC64 and s390 by reading code.

C1 and C2 JIT compilers generate stack banging code before the code for saving the PC on PPC64 and s390.
Hence, the PC needs to be picked from the signal context (LR on PPC64 and R14 on s390) to create a correct frame object.

Currently, the top frame can not be checked for reserved stack annotation because the frame doesn't get a PC which points into the nmethod.

Proposed fix:
http://cr.openjdk.java.net/~mdoerr/8196401_ppc64_s390_stack_bang_pc/webrev.00/

Please review.

Best regards,
Martin




-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20180131/94390354/attachment-0001.html>