[PING] RFR(XS): 8208091: SA: jhsdb jstack --mixed throws UnmappedAddressException on i686

[Chris Plummer]

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:sgehwolf at 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
>>