Request for review: 6310967: SA: jstack -m produce failures in output

[Staffan Larsen]

Great - where are those tests? Trying to learn my way around...

/Staffan

On 14 jun 2012, at 11:03, Poonam Bajaj wrote:

> Thanks Staffan. 
> 
> We do have regression tests covering jstack -m testing.
> 
> regards,
> Poonam
> 
> On 6/14/2012 2:02 PM, Staffan Larsen wrote:
>> 
>> Poonam,
>> 
>> The fix looks good. 
>> 
>> It's not clear to me if there is a good regression test for this already - if not, I'd really like to see one along with the fix.
>> 
>> Thanks,
>> /Staffan
>> 
>> On 14 jun 2012, at 06:10, Poonam Bajaj wrote:
>> 
>>> Could I get one more review for this fix, please.
>>> 
>>> Thanks,
>>> Poonam
>>> 
>>> On 6/13/2012 7:57 AM, David Holmes wrote:
>>>> 
>>>> On 13/06/2012 12:25 PM, Poonam Bajaj wrote: 
>>>>> Hi David, 
>>>>> 
>>>>> On 6/13/2012 7:35 AM, David Holmes wrote: 
>>>>>> Hi Poonam, 
>>>>>> 
>>>>>> It seems to me that rather than passing the ThreadProxy through to 
>>>>>> f.sender the frame, which has to be a frame of some thread, should 
>>>>>> already know what that thread is and so be able to access it directly. 
>>>>>> 
>>>>>> In other words: shouldn't each CFrame maintain a reference to the 
>>>>>> thread it corresponds to? 
>>>>> 
>>>>> Yes, it would have made more sense to have a reference of ThreadProxy in 
>>>>> the Frame classes. But that requires restructuring of lot more code. All 
>>>>> the Debugger classes (e.g. 
>>>>> agent/src/share/classes/sun/jvm/hotspot/debugger/linux/LinuxCDebugger.java) 
>>>>> also need to be touched that have the topFrameForThread() where we 
>>>>> create the first frame for the thread. 
>>>> 
>>>> Oh I see. I was hoping it would actually lead to fewer code changes. It seems risky to pass in a thread reference when there is only ever one valid thread reference that can be passed - but no way to verify that you actually pass the correct one. 
>>>> 
>>>>> This fix was mainly for 6uxx and it didn't seem reasonable to me to 
>>>>> restructure and touch too many classes for this simple fix. 
>>>> 
>>>> Ok. 
>>>> 
>>>> David 
>>>> ----- 
>>>> 
>>>>> Thanks, 
>>>>> Poonam 
>>>>> 
>>>>>> 
>>>>>> David 
>>>>>> 
>>>>>> On 12/06/2012 11:05 PM, Poonam Bajaj wrote: 
>>>>>>> Hi, 
>>>>>>> 
>>>>>>> Please review this fix for bug 6310967 
>>>>>>> <http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6310967>. 
>>>>>>> 
>>>>>>> 6310967 <http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6310967>: 
>>>>>>> SA: jstack -m produce failures in output 
>>>>>>> 
>>>>>>> http://cr.openjdk.java.net/~poonam/6310967/webrev.00/ 
>>>>>>> 
>>>>>>> Problem: jstack -m fails with UnalignedAddressException.The problem is 
>>>>>>> that while finding the caller frame of a frame in sender() method, we 
>>>>>>> don't check the validity of the frame pointer (rbp / ebp). 
>>>>>>> 
>>>>>>> These changes add a simple check that the frame pointer(rbp) should be a 
>>>>>>> valid pointer on the stack by making sure that it is not less than the 
>>>>>>> stack 
>>>>>>> pointer(rsp). 
>>>>>>> 
>>>>>>> 
>>>>>>> Thanks, 
>>>>>>> Poonam
>>>>>>>  
>>>>>>> 
>> 
>  
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20120614/aa328083/attachment.html