<div dir="ltr">Hi Dean,<div><br></div><div>I have a question about the following piece of code in compiler-rt/trunk/lib/xray/xray_trampoline_x86.S :</div><div><div>  movq  _ZN6__xray19XRayPatchedFunctionE(%rip), %rax</div><div>  testq  %rax, %rax</div><div>  je  .Ltmp0</div><div><br></div><div>  // assume that %r10d has the function id.</div><div>  movl  %r10d, %edi</div><div>  xor  %esi,%esi</div><div>  callq  *%rax</div></div><div>What happens if someone unsets the handler function (i.e. calls __xray_remove_handler() ) or changes the handler (i.e. calls __xray_set_handler() with a different pointer to function) between "movq  _ZN6__xray19XRayPatchedFunctionE(%rip), %rax" and "callq  *%rax" ? I understood that the old handler will still be called, but isn't it a problem? What if the code removing the handler starts destructing some objects after that, so that a handler call would result in a crash or other bad things?</div><div><br></div><div>The same concern for analogous code in __xray_FunctionExit .</div><div><br></div><div>Cheers,</div><div>Serge</div></div><div class="gmail_extra"><br><div class="gmail_quote">On 30 July 2016 at 09:45, Dean Michael Berris <span dir="ltr"><<a href="mailto:dean.berris@gmail.com" target="_blank">dean.berris@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
> On 30 Jul 2016, at 05:07, Serge Rogatch via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<br>
><br>
> Thanks for pointing this out, Tim. Then maybe this approach is not the best choice for x86, though ideally measuring is needed, it is just that on ARM the current x86 approach is not applicable because ARM doesn't have a single return instruction (such as RETQ on x86_64), furthermore, the return instructions on ARM can be conditional.<br>
<br>
</span>I think this is a flaw in the current implementation. I long suspected that other platforms didn't have a single return instruction like x86, and I think it's worth changing this.<br>
<br>
In an early version of the implementation that made it into LLVM, I made the determination whether an instruction was a return instruction, and make that determination per-platform.<br>
<br>
We should make that happen so we can easier support ARM.<br>
<span class=""><br>
><br>
> I have another question: what happens if the instrumented function (or its callees) throws an exception and doesn't catch? I understood that currently XRay will not report an exit from this function in such case because the function doesn't return with RETQ, but rather the stack unwinder jumps through functions calling the destructors of local variable objects.<br>
><br>
> If so, why not to instrument the functions by placing a tracing object as the first local variable, with its constructor calling __xray_FunctionEntry and destructor calling __xray_FunctionExit ? Perhaps this approach requires changes in the front-end (C++ compiler, before emitting IR).<br>
<br>
</span>That's right -- the approach I've been thinking about (but haven't gotten to implementing yet, because of other priorities) has been to have special instrumentation at the throw and catch points. Similarly understanding things like tail call exits and sibling call optimisations and instrumenting those exit points is another thing that is yet to be implemented.<br>
<br>
Unfortunately changing the front-end to add an RAII or guard object will cause a whole lot of problems too. For instance, optimisations on the IR can move instructions before/after the guard object initialisation and then we end up with less accurate instrumentation. This also introduces potentially unwanted semantics and changes the stack layout and affecting the performance of an application with XRay sleds but instrumentation not enabled.<br>
<span class=""><br>
<br>
<br>
><br>
> Cheers.<br>
><br>
> On 29 July 2016 at 21:00, Tim Northover <<a href="mailto:t.p.northover@gmail.com">t.p.northover@gmail.com</a>> wrote:<br>
> On 28 July 2016 at 16:14, Serge Rogatch via llvm-dev<br>
> <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<br>
> > Can I ask you why you chose to patch both function entrances and exits,<br>
> > rather than just patching the entrances and (in the patches) pushing on the<br>
> > stack the address of __xray_FunctionExit , so that the user function returns<br>
> > normally (with RETQ or POP RIP or whatever else instruction) rather than<br>
> > jumping into __xray_FunctionExit?<br>
><br>
> > This approach should also be faster because smaller code better fits in CPU<br>
> > cache, and patching itself should run faster (because there is less code to<br>
> > modify).<br>
><br>
> It may well be slower. Larger CPUs tend to track the call stack in<br>
> hardware and returning to an address pushed manually is an inevitable<br>
> branch mispredict in those cases.<br>
><br>
> Cheers.<br>
><br>
> Tim.<br>
><br>
</span>> _______________________________________________<br>
> LLVM Developers mailing list<br>
> <a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
<br>
</blockquote></div><br></div>