<div dir="ltr">> <span style="font-family:arial,sans-serif;font-size:13px"> If you were to make them only run on one OS it would be pretty easy to do it with hand written assembly.</span><div class="" style="font-family:arial,sans-serif;font-size:13px">
</div><div class="" style="font-family:arial,sans-serif;font-size:13px"><br></div><div class="" style="font-family:arial,sans-serif;font-size:13px">Yeah I was planning on having that get added as a follow-up pass. We'll just have it qualified to only run locally and only when the host architecture is what we need (which I think is x86_64 in this case). I think with the assembly it will be OS agnostic but architecture specific as long as we get the build rules right?</div>
<div class="" style="font-family:arial,sans-serif;font-size:13px"><br></div><div class="" style="font-family:arial,sans-serif;font-size:13px">-Todd</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Jul 25, 2014 at 3:03 AM, Jason Molenda <span dir="ltr"><<a href="mailto:jason@molenda.com" target="_blank">jason@molenda.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">It's a little tricky to write unwinder tests - they have to be written in assembly to set up conditions correctly and that makes them OS/ABI-specific. A combination of a C driver program (with a main() function) and a .s file that main() calls might be possible to do for Mac/Linux/*BSD but I'm not sure about that. If you were to make them only run on one OS it would be pretty easy to do it with hand written assembly.<br>
<div class="HOEnZb"><div class="h5"><br>
On Jul 24, 2014, at 10:26 PM, Zachary Turner <<a href="mailto:zturner@google.com">zturner@google.com</a>> wrote:<br>
<br>
> Can we add Tong's test case as an LLDB test?<br>
><br>
><br>
> On Thu, Jul 24, 2014 at 6:24 PM, Todd Fiala <<a href="mailto:tfiala@google.com">tfiala@google.com</a>> wrote:<br>
> Tested and committed:<br>
><br>
> svn commit<br>
> Sending source/Plugins/UnwindAssembly/x86/UnwindAssembly-x86.cpp<br>
> Transmitting file data .<br>
> Committed revision 213914.<br>
><br>
><br>
><br>
> On Thu, Jul 24, 2014 at 5:35 PM, Todd Fiala <<a href="mailto:tfiala@google.com">tfiala@google.com</a>> wrote:<br>
> :-)<br>
><br>
><br>
> On Thu, Jul 24, 2014 at 5:34 PM, Tong Shen <<a href="mailto:endlessroad@google.com">endlessroad@google.com</a>> wrote:<br>
> Thanks Todd!<br>
><br>
><br>
> On Thu, Jul 24, 2014 at 5:33 PM, Todd Fiala <<a href="mailto:tfiala@google.com">tfiala@google.com</a>> wrote:<br>
> Hey Tong - I'll add that and do all the test runs. I'll get it checked in unless something breaks.<br>
><br>
> -Todd<br>
><br>
><br>
> On Thu, Jul 24, 2014 at 5:32 PM, Tong Shen <<a href="mailto:endlessroad@google.com">endlessroad@google.com</a>> wrote:<br>
> Hi Jason,<br>
><br>
> I don't have any strong preference here, so let's do it the way you see fit :-)<br>
> Your patch works fine, though there's a right bracket missing at the end of line:<br>
> regloc.SetAtCFAPlusOffset (-(stack_offset + row->GetCFAOffset());<br>
><br>
> Thank you for looking into this!<br>
><br>
><br>
> On Thu, Jul 24, 2014 at 5:16 PM, Jason Molenda <<a href="mailto:jmolenda@apple.com">jmolenda@apple.com</a>> wrote:<br>
> 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>
> > On Jul 24, 2014, at 3:33 PM, Tong Shen <<a href="mailto:endlessroad@google.com">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>
> > <test.S><1.patch><br>
><br>
><br>
><br>
><br>
><br>
> --<br>
> Best Regards, Tong Shen<br>
><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>
><br>
><br>
><br>
> --<br>
> Todd Fiala | Software Engineer | <a href="mailto:tfiala@google.com">tfiala@google.com</a> | <a href="tel:650-943-3180" value="+16509433180">650-943-3180</a><br>
><br>
><br>
><br>
><br>
> --<br>
> Best Regards, Tong Shen<br>
><br>
><br>
><br>
> --<br>
> Todd Fiala | Software Engineer | <a href="mailto:tfiala@google.com">tfiala@google.com</a> | <a href="tel:650-943-3180" value="+16509433180">650-943-3180</a><br>
><br>
><br>
><br>
><br>
> --<br>
> Todd Fiala | Software Engineer | <a href="mailto:tfiala@google.com">tfiala@google.com</a> | <a href="tel:650-943-3180" value="+16509433180">650-943-3180</a><br>
><br>
><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>
><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>
</div></div></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>