[Lldb-commits] Possible UnwindAssembly-x86 Problem & Fix
Jason Molenda
jmolenda at apple.com
Thu Jul 24 17:29:11 PDT 2014
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
>
More information about the lldb-commits
mailing list