<div dir="ltr">Hey Tong - I'll add that and do all the test runs. I'll get it checked in unless something breaks.<div><br></div><div>-Todd</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Jul 24, 2014 at 5:32 PM, Tong Shen <span dir="ltr"><<a href="mailto:endlessroad@google.com" target="_blank">endlessroad@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi Jason,<div><br></div><div>I don't have any strong preference here, so let's do it the way you see fit :-)</div>
<div>Your patch works fine, though there's a right bracket missing at the end of line:</div>
<div><span style="color:rgb(0,0,0);font-family:'Courier New',Courier,monospace;font-size:14px"> regloc.SetAtCFAPlusOffset (-(stack_offset + row->GetCFAOffset());</span><br></div><div><span style="color:rgb(0,0,0);font-family:'Courier New',Courier,monospace;font-size:14px"><br>
</span></div><div>Thank you for looking into this!<br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote"><div><div class="h5">On Thu, Jul 24, 2014 at 5:16 PM, Jason Molenda <span dir="ltr"><<a href="mailto:jmolenda@apple.com" target="_blank">jmolenda@apple.com</a>></span> wrote:<br>
</div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5">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.<br>
<br>
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.<br>
<br>
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.<br>
<br>
<br>
<br>
<br><br>
<br>
<br></div></div><div><div class="h5">
> On Jul 24, 2014, at 3:33 PM, Tong Shen <<a href="mailto:endlessroad@google.com" target="_blank">endlessroad@google.com</a>> wrote:<br>
><br>
> Hi jmolenda, lldb-commits,<br>
><br>
> While hacking around x86 assembly profiler, I found a problem about non-volatile register information.<br>
><br>
> At UnwindAssembly-x86.cpp line 630 (ViewVC link), stack_offset is calculated but not used.<br>
> I think it should be used in line 636:<br>
> regloc.SetAtCFAPlusOffset (-row->GetCFAOffset() + stack_offset);<br>
> instead of what's there now:<br>
> regloc.SetAtCFAPlusOffset (-row->GetCFAOffset());<br>
><br>
> Also, in line 417 of the same file, when calculating stack_offset, why is rbp_offset set to abs(offset)?<br>
><br>
> For testing, I wrote an assembly (test.S in attachment) to test if lldb can recover non-volatile registers.<br>
> You can put a breakpoint after where asm_frame() overrides %rbx, then do "f 1" & "register read" to see if %rbx is correctly restored.<br>
> In my test, unmodified lldb gives wrong %rbx.<br>
> Attached patch solved this problem, and make lldb recover those registers correctly.<br>
><br>
> Am I missing anything here? Is the original behavior intentional? Please correct me if I'm wrong :-)<br>
><br>
> Thank you.<br>
><br>
> --<br>
> Best Regards, Tong Shen<br></div></div>
> <test.S><1.patch><br>
<br>
<br></blockquote></div><div class=""><br><br clear="all"><div><br></div>-- <br><div dir="ltr">Best Regards, Tong Shen</div>
</div></div>
<br>_______________________________________________<br>
lldb-commits mailing list<br>
<a href="mailto:lldb-commits@cs.uiuc.edu">lldb-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits</a><br>
<br></blockquote></div><br><br clear="all"><div><br></div>-- <br><div dir="ltr"><table cellspacing="0" cellpadding="0" style="color:rgb(136,136,136);font-family:'Times New Roman'"><tbody><tr style="color:rgb(85,85,85);font-family:sans-serif;font-size:small">
<td nowrap style="border-top-style:solid;border-top-color:rgb(213,15,37);border-top-width:2px">Todd Fiala |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(51,105,232);border-top-width:2px"> Software Engineer |</td>
<td nowrap style="border-top-style:solid;border-top-color:rgb(0,153,57);border-top-width:2px"> <a href="mailto:tfiala@google.com" style="color:rgb(17,85,204)" target="_blank"><span style="background-color:rgb(255,255,204);color:rgb(34,34,34);background-repeat:initial initial">tfiala@google.com</span></a> |</td>
<td nowrap style="border-top-style:solid;border-top-color:rgb(238,178,17);border-top-width:2px"><font color="#1155cc"> <a>650-943-3180</a></font></td></tr></tbody></table><br></div>
</div>