<div dir="ltr">Hi Dean,<div><br></div><div>I have a question for 32-bit platforms. I see in the code that you used the following in compiler-rt/trunk/lib/xray/xray_interface_internal.h :</div><div><div>struct XRaySledEntry {</div><div> uint64_t Address;</div><div> uint64_t Function;</div><div> unsigned char Kind;</div><div> unsigned char AlwaysInstrument;</div><div> unsigned char Padding[14]; // Need 32 bytes</div><div>};</div></div><div><br></div><div>And the peer code in llvm/trunk/lib/Target/X86/X86MCInstLower.cpp :<br></div><div><div><br></div><div>void X86AsmPrinter::EmitXRayTable() {</div><div> if (Sleds.empty())</div><div> return;</div><div> if (Subtarget->isTargetELF()) {</div><div> auto *Section = OutContext.getELFSection(</div><div> "xray_instr_map", ELF::SHT_PROGBITS,</div><div> ELF::SHF_ALLOC | ELF::SHF_GROUP | ELF::SHF_MERGE, 0,</div><div> CurrentFnSym->getName());</div><div> auto PrevSection = OutStreamer->getCurrentSectionOnly();</div><div> OutStreamer->SwitchSection(Section);</div><div> for (const auto &Sled : Sleds) {</div><div> OutStreamer->EmitSymbolValue(Sled.Sled, 8);</div><div> OutStreamer->EmitSymbolValue(CurrentFnSym, 8);</div><div> auto Kind = static_cast<uint8_t>(Sled.Kind);</div><div> OutStreamer->EmitBytes(</div><div> StringRef(reinterpret_cast<const char *>(&Kind), 1));</div><div> OutStreamer->EmitBytes(</div><div> StringRef(reinterpret_cast<const char *>(&Sled.AlwaysInstrument), 1));</div><div> OutStreamer->EmitZeros(14);</div><div> }</div><div> OutStreamer->SwitchSection(PrevSection);</div><div> }</div><div> Sleds.clear();</div><div>}</div></div><div><br></div><div>So useful part of your entry is 18 bytes, and you add 14 bytes of padding to achieve 32-byte alignment. But for 32-bit CPUs I understood that XRaySledEntry::Address and XRaySledEntry::Function can be 32-bit too, right? So the entry can fit 16 bytes, with 10 useful bytes and 6 bytes of padding. Can I use 16-byte entries or is there some external (OS? ELF? Linker?) requirement that one entry must be 32-bytes, or aligned at 32 bytes, etc.?</div><div><br></div><div>Thanks,</div><div>Serge</div></div><div class="gmail_extra"><br><div class="gmail_quote">On 4 August 2016 at 19:29, Serge Rogatch <span dir="ltr"><<a href="mailto:serge.rogatch@gmail.com" target="_blank">serge.rogatch@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">Yes, double-checking on the handler side whether its resources are still operational, e.g. by calling weak_ptr::lock() may be a better idea than elimination of spurious handler calls in XRay at a cost of heavy synchronization objects. Ok, let's live with spurious handler calls.<div><br></div><div>Cheers,</div><div>Serge</div><div><div class="h5"><div><div class="gmail_extra"><br><div class="gmail_quote">On 4 August 2016 at 10:57, 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><br>
> On 4 Aug 2016, at 06:27, Serge Rogatch <<a href="mailto:serge.rogatch@gmail.com" target="_blank">serge.rogatch@gmail.com</a>> wrote:<br>
><br>
> Hi Dean,<br>
><br>
> I have a question about the following piece of code in compiler-rt/trunk/lib/xray/xra<wbr>y_trampoline_x86.S :<br>
> movq _ZN6__xray19XRayPatchedFunctio<wbr>nE(%rip), %rax<br>
> testq %rax, %rax<br>
> je .Ltmp0<br>
><br>
> // assume that %r10d has the function id.<br>
> movl %r10d, %edi<br>
> xor %esi,%esi<br>
> callq *%rax<br>
> 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__xray19XRayPatchedFunctio<wbr>nE(%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?<br>
<br>
</span>That would be unfortunate. :)<br>
<br>
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.<br>
<br>
An implementer of the log handler should really be responsible for "internal signalling" -- i.e. implementing it so that any required synchronisation happens internally.<br>
<br>
Does this help?<br>
<br>
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 (<a href="https://reviews.llvm.org/D21982" rel="noreferrer" target="_blank">https://reviews.llvm.org/D219<wbr>82</a>).<br>
<br>
Cheers<br>
<span><font color="#888888"><br>
-- Dean<br>
<br>
</font></span></blockquote></div><br></div></div></div></div></div>
</blockquote></div><br></div>