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 >>>>> >>>>>
