[Lldb-commits] Possible UnwindAssembly-x86 Problem & Fix

[Jason Molenda]

Yep.

> On Jul 24, 2014, at 5:28 PM, Todd Fiala <tfiala at google.com> wrote:
> 
> Hey Jason - if Tong verifies this is all still good, is that essentially a LGTM from you?  (i.e. is that something I can get checked in for him?)
> 
> 
> On Thu, Jul 24, 2014 at 5:16 PM, Jason Molenda <jmolenda at apple.com> wrote:
> Good catch Tong.  I don't think clang generates this instruction pattern (it normally pushq's all the registers in the prologue that it wants to preserve) so we've never hit this bug.
> 
> I don't have a strong preference about mov_reg_to_local_stack_frame_p() returning a positive value in rbp_offset except that we have other sections returning an offset from the CFA e.g. UnwindPlan::Row::GetCFAOffset() which return a positive offset value that must be subtracted from the CFA to get the correct address.
> 
> If you would prefer to change mov_reg_to_local_stack_frame_p() to return a negative value, I'm not going to argue against it.  But maybe a slight change to your patch like this?  It's complicated enough code that I added a couple of comments while I was at it.
> 
> 
> 
> 
> 
> 
> 
> > On Jul 24, 2014, at 3:33 PM, Tong Shen <endlessroad at google.com> wrote:
> >
> > Hi jmolenda, lldb-commits,
> >
> > While hacking around x86 assembly profiler, I found a problem about non-volatile register information.
> >
> > At UnwindAssembly-x86.cpp line 630 (ViewVC link), stack_offset is calculated but not used.
> > I think it should be used in line 636:
> >         regloc.SetAtCFAPlusOffset (-row->GetCFAOffset() + stack_offset);
> > instead of what's there now:
> >         regloc.SetAtCFAPlusOffset (-row->GetCFAOffset());
> >
> > Also, in line 417 of the same file, when calculating stack_offset, why is rbp_offset set to abs(offset)?
> >
> > For testing, I wrote an assembly (test.S in attachment) to test if lldb can recover non-volatile registers.
> > You can put a breakpoint after where asm_frame() overrides %rbx, then do "f 1" & "register read" to see if %rbx is correctly restored.
> > In my test, unmodified lldb gives wrong %rbx.
> > Attached patch solved this problem, and make lldb recover those registers correctly.
> >
> > Am I missing anything here? Is the original behavior intentional? Please correct me if I'm wrong :-)
> >
> > Thank you.
> >
> > --
> > Best Regards, Tong Shen
> > <test.S><1.patch>
> 
> 
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
> 
> 
> 
> 
> -- 
> Todd Fiala |	 Software Engineer |	 tfiala at google.com |	 650-943-3180
>