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

[Tong Shen]

Hi Jason,

I don't have any strong preference here, so let's do it the way you see fit
:-)
Your patch works fine, though there's a right bracket missing at the end of
line:
 regloc.SetAtCFAPlusOffset (-(stack_offset + row->GetCFAOffset());

Thank you for looking into this!


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


-- 
Best Regards, Tong Shen
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20140724/f1fc5018/attachment.html>