Hi Severin,
I had looked at this review when it came out, but was hesitant to ok it
because I really don't know this code at all. If you can get another
reviewer who does know the code, then I'll approve it. This only impacts
32-bit, right? If so, keep in mind that it won't get tested by Oracle
testing, including the submit repo, so make sure you do thorough testing.
Also, why is there any code being executed that was not compiled with
-fno-omit-frame-pointer? The description in the CR just shows a simple
java program reproducing this, so all the mixed stack traces belong to
the JVM and libs, and I thought we made sure to compile all of them with
-fno-omit-frame-pointer.
thanks,
Chris
On 7/26/18 10:11 AM, Severin Gehwolf wrote:
On Thu, 2018-07-26 at 10:04 -0700, Sharath Ballal wrote:
Changes looks good Severin.
Thanks for the review, Sharath!
I am not a reviewer though, so you still need a Reviewer to review.
Anyone?
Thanks,
Severin
-----Original Message-----
From: Severin Gehwolf [mailto:sgehw...@redhat.com]
Sent: Thursday, July 26, 2018 1:04 PM
To: serviceability-dev
Subject: [PING] RFR(XS): 8208091: SA: jhsdb jstack --mixed throws
UnmappedAddressException on i686
On Mon, 2018-07-23 at 18:27 +0200, Severin Gehwolf wrote:
Hi,
Could I please get a review of this one-liner change related to jhsdb
--mixed when attaching to a running Java process? The issue arises
when threads are in native code and that native code has frame
pointers not properly preserved. In such a case the SA performs a
simple frame pointer valididy check: ebp >= esp
However, the code of retrieving the value for esp is incorrect in as
much as it's not in sync with native code in regards to the register
index:
native code => X86ThreadContext.SP
Java code => X86ThreadContext.ESP
X86ThreadContext.ESP is never being set by the native code. Since
X86ThreadContext.getRegisterAsAddress(X86ThreadContext.ESP) then
returns null, ebp.lessThan(esp) wrongly returns false causing the
issue. This webrev fixes it by using SP as index on the Java side.
Thoughts?
webrev:
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8208091/webrev.01/
bug: https://bugs.openjdk.java.net/browse/JDK-8208091
Anyone willing to review this one-liner?
Thanks,
Severin
Thanks,
Severin
