Yep. > On Jul 24, 2014, at 5:28 PM, Todd Fiala <tfi...@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 <jmole...@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 <endlessr...@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@cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits > > > > > -- > Todd Fiala | Software Engineer | tfi...@google.com | 650-943-3180 >
_______________________________________________ lldb-commits mailing list lldb-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
