llvm dev - Aug 2016 - XRay: Demo on x86_64/Linux almost done; some questions.

> On 30 Jul 2016, at 05:07, Serge Rogatch via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> 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.
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.

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.

We should make that happen so we can easier support ARM.
> 
> 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.
> 
> 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).
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.

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.


> 
> Cheers.
> 
> On 29 July 2016 at 21:00, Tim Northover <t.p.northover at gmail.com>
wrote:
> On 28 July 2016 at 16:14, Serge Rogatch via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
> > Can I ask you why you chose to patch both function entrances and
exits,
> > rather than just patching the entrances and (in the patches) pushing
on the
> > stack the address of __xray_FunctionExit , so that the user function
returns
> > normally (with RETQ or POP RIP or whatever else instruction) rather
than
> > jumping into __xray_FunctionExit?
> 
> > This approach should also be faster because smaller code better fits
in CPU
> > cache, and patching itself should run faster (because there is less
code to
> > modify).
> 
> It may well be slower. Larger CPUs tend to track the call stack in
> hardware and returning to an address pushed manually is an inevitable
> branch mispredict in those cases.
> 
> Cheers.
> 
> Tim.
> 
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

> On 4 Aug 2016, at 06:27, Serge Rogatch <serge.rogatch at gmail.com>
wrote:
> 
> Hi Dean,
> 
> I have a question about the following piece of code in
compiler-rt/trunk/lib/xray/xray_trampoline_x86.S :
>   movq  _ZN6__xray19XRayPatchedFunctionE(%rip), %rax
>   testq  %rax, %rax
>   je  .Ltmp0
> 
>   // assume that %r10d has the function id.
>   movl  %r10d, %edi
>   xor  %esi,%esi
>   callq  *%rax
> 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?
That would be unfortunate. :)

Yes, you're right, it will be called despite having the handler be un-set
from the global atomic. Though I'd suggest that log handler implementations
really shouldn't be crashing, and should assume that it will be called at
any given time (from multiple threads, so it should also be thread-safe).
Consider also that we're taking a function pointer by design -- assuming
that it will be a pointer to a function that will only deal with globals and the
explicitly passed in arguments.

An implementer of the log handler should really be responsible for
"internal signalling" -- i.e. implementing it so that any required
synchronisation happens internally.

Does this help?

PS. I think more definitive documentation for this is due, let me think about
putting something some documentation on how to use XRay as it is currently
implemented once we have the naive in-memory logging implementation land
(https://reviews.llvm.org/D21982).

Cheers

-- Dean