<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Aug 8, 2016 at 2:46 PM, David Majnemer <span dir="ltr"><<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.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"><div><div class="h5"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Aug 8, 2016 at 12:51 PM, David Majnemer <span dir="ltr"><<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.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"><div><div><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Aug 8, 2016 at 10:37 AM, Reid Kleckner <span dir="ltr"><<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span>On Fri, Aug 5, 2016 at 6:53 PM, David Majnemer <span dir="ltr"><<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span>On Fri, Aug 5, 2016 at 2:47 PM, Reid Kleckner via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Author: rnk<br>
Date: Fri Aug 5 16:47:46 2016<br>
New Revision: 277874<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=277874&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=277874&view=rev</a><br>
Log:<br>
Fix two tests in Win64 ASan<br>
<br>
Go back to intercepting kernel32!RaiseException, and only go for<br>
ntdll!RtlRaiseException if that fails. Fixes throw_and_catch.cc test.<br>
<br>
Work around an issue in LLVM's win64 epilogues. We end up with an<br>
epilogue that looks like this, and it drives the Win64 unwinder crazy<br>
until stack overflow:<br>
call ill_cc!__asan_handle_no_return<br>
xor eax,eax<br>
add rsp,40h // epilogue starts </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
pop rbp // CSR<br>
ud2 // Trap here<br>
ret // Ret?<br>
nop word ptr [rax+rax]<br>
sub rsp,28h // Next function<br></blockquote><div><br></div><div><br></div></span><div>Interesting. So we started the epilogue but didn't finish it. Happen to have some IR or C lying around? I'm curious exactly when we decided that a ud2 between the pop and the ret was a smart move...</div></div></div></div></blockquote><div><br></div></span><div>It's pretty easy to convince LLVM to put the ud2 there, but it's hard to build a test case that will cause the Win64 unwinder to fail in the same way that I observed in this test without the extra conditional control flow that I added. </div><div><br></div><div><div>$ cat t.cpp</div><div>void foo();</div><div>int main() { foo(); __builtin_trap(); }</div></div><div><br></div><div>$ clang -S --target=x86_64-windows t.cpp -fno-omit-frame-pointer -o -<br></div><div>...</div><div><div> callq "?foo@@YAXXZ"</div><div> xorl %eax, %eax</div><div> addq $32, %rsp</div><div> popq %rbp</div><div> ud2</div><div> retq</div></div><div>...</div><div><br></div><div>I think the issue is that the faulting PC does not appear to be within a standard epilogue, so the unwinder re-emulates the add 32 and pop rbp.</div></div></div></div>
</blockquote></div><br></div></div></div><div class="gmail_extra">Hrm, the CoreCLR copy of the x64 unwinder *should* handle this case: <a href="https://github.com/dotnet/coreclr/blob/master/src/unwinder/amd64/unwinder_amd64.cpp#L1304" target="_blank">https://github.com/dotne<wbr>t/coreclr/blob/master/src/<wbr>unwinder/amd64/unwinder_amd64.<wbr>cpp#L1304</a></div><div class="gmail_extra"><br></div><div class="gmail_extra">Our retq is encoded as 0xc3 which is their RET_OP.</div><div class="gmail_extra"><br></div><div class="gmail_extra">Using the same IR but forcing us out of FastISel leaves us with:</div><div class="gmail_extra"><div class="gmail_extra"><span style="white-space:pre-wrap"> </span>callq<span style="white-space:pre-wrap"> </span>"?foo@@YAXXZ"</div><div class="gmail_extra"><span style="white-space:pre-wrap"> </span>ud2</div><span><div class="gmail_extra"><span style="white-space:pre-wrap"> </span>xorl<span style="white-space:pre-wrap"> </span>%eax, %eax</div><div class="gmail_extra"><span style="white-space:pre-wrap"> </span>addq<span style="white-space:pre-wrap"> </span>$32, %rsp</div><div class="gmail_extra"><span style="white-space:pre-wrap"> </span>popq<span style="white-space:pre-wrap"> </span>%rbp</div></span><div class="gmail_extra"><span style="white-space:pre-wrap"> </span>retq</div><div><br></div><div>The difference is caused by a different selection of terminator in emitEpligoue (FastISel uses the TRAP while SDAG uses the RETQ).</div><div><br></div><div>From reading the x64 ABI docs, I cannot find where we strongly violated a constraint. My only guess is that it doesn't like us partially unwinding a frame function before performing the trap (something along the lines of the unwinder _expecting_ to see an SP adjustment but seeing nothing but the ret...).</div></div></div>
</blockquote></div><br></div><div class="gmail_extra"><br></div></div></div><div class="gmail_extra">After talking with Gor, the problem has been made more clear. We are in a frame function but we, not the unwinder, restored the FP. This would be OK if we were in an epilogue sequence but we are not because the faulting instruction is a ud2 which makes the unwinder believe that we are in the body of the function.</div><div class="gmail_extra"><br></div><div class="gmail_extra">All of this started because we lowered the __builtin_trap to a TRAP which is marked isTerminator. emitEpilogue believes that this is our return opcode because it is the first terminator in the MBB. The easiest way of fixing this, IMO, is to insert a NOP after the TRAP. This will stop it from looking like a terminator in the epilogue code...</div></div>
</blockquote></div><br></div><div class="gmail_extra">On second thought, it would be even better to introduce another trap instruction which is not a terminator. This way we can preserve the invariant that terminators are grouped at the end of MBBs.</div></div>