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

[Jason Molenda]

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. 



-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: x86-unwind.txt
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20140724/1c40965e/attachment.txt>
-------------- next part --------------



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