[PATCH] D26413: [XRay] Support AArch64 in compiler-rt

[Serge Rogatch via llvm-commits]

rSerge marked an inline comment as done.
rSerge added inline comments.


================
Comment at: lib/xray/xray_interface.cc:29
 
+static const int16_t cSledLength =
 #if defined(__x86_64__)
----------------
rengolin wrote:
> I'd rather have the repetition here. This is looking odd... :)
Changing.


================
Comment at: lib/xray/xray_trampoline_AArch64.S:32
+    /* Handler address is nullptr if handler is not set */
+    CMP X2, #0
+    BEQ FunctionEntry_restore
----------------
rengolin wrote:
> can't you compare the address before saving and restoring all those registers?
This situation is rare. Usually if the handler is set to `nullptr`, then the user code is unpatched, so the trampolines are not called. So there is no performance issue here.
However, there is a race condition issue. It is important to check for `nullptr` handler as late as possible before its call. If we push the registers after checking for `nullptr` handler, we increase the chances that handler is not `nullptr` when it's checked, but `nullptr` when it's called.
Though anyway we currently require that the handler function works correctly even if called after its removal via XRay API.


https://reviews.llvm.org/D26413