[XRay] Change xray_instr_map sled addresses from absolute to PC relative for x86-64

[MaskRay]

Summary

xray_instr_map contains absolute addresses of sleds, which are relocated
by R_*_RELATIVE when linked in -pie or -shared mode.

By making these addresses relative to PC, we can avoid the dynamic
relocations and remove the SHF_WRITE flag from xray_instr_map. We can
thus save VM pages containg xray_instr_map (because they are not
modified).

This patch changes x86-64. Subsequent changes will change powerpc64le
and AArch64.

[MaskRay]

llvm/test/CodeGen/X86/xray-section-group.ll

1

The test can actually be merged into the main test.

I may change the main test to be similar to patchable-function-entry.ll

[ianlevesque]

Is "llvm-xray extract" still able to symbolize these correctly? I don't know offhand what test coverage is there for that. The change looks great to me otherwise.

[MaskRay]

Fix llvm-xray

[MaskRay]

Fix DebugInfo/X86/xray-split-dwarf-interaction.ll

[MaskRay]

In D78082#1981500, @ianlevesque wrote:

Is "llvm-xray extract" still able to symbolize these correctly? I don't know offhand what test coverage is there for that. The change looks great to me otherwise.

Thanks. I should have added a note that the binaries used by llvm-xray tests are large (du -sh => 2M) and I really dislike the prebuilt binaries. They permanently increased the repository size by 2M.

The address: fields may look weird because they haven't been fixed. I'll defer that to a future change.

[dberris]

LGTM, but I'm concerned about the backwards compatibility story.

compiler-rt/lib/xray/xray_interface_internal.h

35

How do you handle backwards-compatibility here? i.e. what happens when you link in a new compiler-rt implementation with a previous-versioned XRay instrumented .o file?

My suspicion is you'll need to increment the version to determine whether you're using pc-relative addressing or whether it's the old style absolute addressing. Otherwise there'd be potential for random memory addresses and potential corruption going on.

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

3217

const bool perhaps?

[MaskRay]

In D78082#1982695, @dberris wrote:

LGTM, but I'm concerned about the backwards compatibility story.

We could pick a different section name. Do you have a suggestion? Doing that can change a lot of tests and I need some ack before moving toward that goal.

I want to avoid something like xray_instr_maps_ro because really the read-only section is commonplace, and I don't want to penalize the good readonly section name...

[dberris]

In D78082#1982713, @MaskRay wrote:

In D78082#1982695, @dberris wrote:

LGTM, but I'm concerned about the backwards compatibility story.

We could pick a different section name. Do you have a suggestion? Doing that can change a lot of tests and I need some ack before moving toward that goal.

I want to avoid something like xray_instr_maps_ro because really the read-only section is commonplace, and I don't want to penalize the good readonly section name...

As I mentioned in the other inline comment, you can use the 'Version' field for each entry. If you increment the version number in the compiler, then you can change the runtime code to support some older versions. Doing that allows you to avoid renaming the section and maintaining some backwards compatibility.

[MaskRay]

In D78082#1982766, @dberris wrote:

In D78082#1982713, @MaskRay wrote:

In D78082#1982695, @dberris wrote:

LGTM, but I'm concerned about the backwards compatibility story.

We could pick a different section name. Do you have a suggestion? Doing that can change a lot of tests and I need some ack before moving toward that goal.

I want to avoid something like xray_instr_maps_ro because really the read-only section is commonplace, and I don't want to penalize the good readonly section name...

As I mentioned in the other inline comment, you can use the 'Version' field for each entry. If you increment the version number in the compiler, then you can change the runtime code to support some older versions. Doing that allows you to avoid renaming the section and maintaining some backwards compatibility.

Supporting two versions of Sled.Address can make the runtime overly complex I think?

--- a/compiler-rt/lib/xray/xray_interface.cpp
+++ b/compiler-rt/lib/xray/xray_interface.cpp
@@ -264,14 +264,14 @@ XRayPatchingStatus controlPatching(bool Enable) XRAY_NEVER_INSTRUMENT {
   // now we're assuming we can mprotect the whole section of text between the
   // minimum sled address and the maximum sled address (+ the largest sled
   // size).
-  auto MinSled = InstrMap.Sleds[0];
-  auto MaxSled = InstrMap.Sleds[InstrMap.Entries - 1];
+  auto *MinSled = &InstrMap.Sleds[0];
+  auto *MaxSled = &InstrMap.Sleds[InstrMap.Entries - 1];
   for (std::size_t I = 0; I < InstrMap.Entries; I++) {
     const auto &Sled = InstrMap.Sleds[I];
-    if (Sled.Address < MinSled.Address)
-      MinSled = Sled;
-    if (Sled.Address > MaxSled.Address)
-      MaxSled = Sled;
+    if (Sled.address() < MinSled->address())
+      MinSled = &Sled;
+    if (Sled.address() > MaxSled->address())
+      MaxSled = &Sled;
   }

One approach is to teach the member function about return Version < 1 ? Address : reinterpret_cast<uint64_t>(&Address) + Address; but this will impose overhead on every operation.

x86 CUSTOM_EVENT has used version 1. Bump versions of all SledKind's to 2?

--- a/llvm/lib/Target/X86/X86MCInstLower.cpp
+++ b/llvm/lib/Target/X86/X86MCInstLower.cpp
@@ -1510,7 +1510,7 @@ void X86AsmPrinter::LowerPATCHABLE_EVENT_CALL(const MachineInstr &MI,
   // Record the sled version. Older versions of this sled were spelled
   // differently, so we let the runtime handle the different offsets we're
   // using.
-  recordSled(CurSled, MI, SledKind::CUSTOM_EVENT, 1);
+  recordSled(CurSled, MI, SledKind::CUSTOM_EVENT, 2);
 }

It'd be nice to reject old versions.

[dberris]

In D78082#1987960, @MaskRay wrote:

In D78082#1982766, @dberris wrote:

In D78082#1982713, @MaskRay wrote:

In D78082#1982695, @dberris wrote:

LGTM, but I'm concerned about the backwards compatibility story.

We could pick a different section name. Do you have a suggestion? Doing that can change a lot of tests and I need some ack before moving toward that goal.

I want to avoid something like xray_instr_maps_ro because really the read-only section is commonplace, and I don't want to penalize the good readonly section name...

As I mentioned in the other inline comment, you can use the 'Version' field for each entry. If you increment the version number in the compiler, then you can change the runtime code to support some older versions. Doing that allows you to avoid renaming the section and maintaining some backwards compatibility.

Supporting two versions of Sled.Address can make the runtime overly complex I think?

--- a/compiler-rt/lib/xray/xray_interface.cpp
+++ b/compiler-rt/lib/xray/xray_interface.cpp
@@ -264,14 +264,14 @@ XRayPatchingStatus controlPatching(bool Enable) XRAY_NEVER_INSTRUMENT {
   // now we're assuming we can mprotect the whole section of text between the
   // minimum sled address and the maximum sled address (+ the largest sled
   // size).
-  auto MinSled = InstrMap.Sleds[0];
-  auto MaxSled = InstrMap.Sleds[InstrMap.Entries - 1];
+  auto *MinSled = &InstrMap.Sleds[0];
+  auto *MaxSled = &InstrMap.Sleds[InstrMap.Entries - 1];
   for (std::size_t I = 0; I < InstrMap.Entries; I++) {
     const auto &Sled = InstrMap.Sleds[I];
-    if (Sled.Address < MinSled.Address)
-      MinSled = Sled;
-    if (Sled.Address > MaxSled.Address)
-      MaxSled = Sled;
+    if (Sled.address() < MinSled->address())
+      MinSled = &Sled;
+    if (Sled.address() > MaxSled->address())
+      MaxSled = &Sled;
   }

One approach is to teach the member function about return Version < 1 ? Address : reinterpret_cast<uint64_t>(&Address) + Address; but this will impose overhead on every operation.

x86 CUSTOM_EVENT has used version 1. Bump versions of all SledKind's to 2?

--- a/llvm/lib/Target/X86/X86MCInstLower.cpp
+++ b/llvm/lib/Target/X86/X86MCInstLower.cpp
@@ -1510,7 +1510,7 @@ void X86AsmPrinter::LowerPATCHABLE_EVENT_CALL(const MachineInstr &MI,
   // Record the sled version. Older versions of this sled were spelled
   // differently, so we let the runtime handle the different offsets we're
   // using.
-  recordSled(CurSled, MI, SledKind::CUSTOM_EVENT, 1);
+  recordSled(CurSled, MI, SledKind::CUSTOM_EVENT, 2);
 }

This is my preferred solution -- and it won't just be the custom events, it will be for all the instrumentation points (entry, exit, etc.) since they're all going to be affected. For versioning I don't expect that we stick with the monotonically increasing numbers either, we might be able to get away with dates (say 2005 to represent 2020-05) and start communicating/coordinating a policy to drop support for older versions.

The cost of doing that comparison is not really that high and I'd be surprised if it's significant enough to show up as a significant detectable overhead.

It'd be nice to reject old versions.

Someday, yes, if we can teach the linker (or use symbol collision tricks) so that we can't link together object files with different XRay instrumentation versions. That decision might even need to be done by the runtime.

[MaskRay]

Implement version 2

[dberris]

LGTM -- thanks!

llvm/lib/Target/X86/X86MCInstLower.cpp

1511–1514

One suggestion: consider documenting what happened with version 1, and why we're going to version 2 here (for the future reader).

[MaskRay]

Add comments about version 2

compiler-rt/lib/xray/xray_interface.cpp

XRayPatchingStatus controlPatching(bool Enable) XRAY_NEVER_INSTRUMENT {
   // First we want to find the bounds for which we have instrumentation points,
   // and try to get as few calls to mprotect(...) as possible. We're assuming
   // that all the sleds for the instrumentation map are contiguous as a single
   // set of pages. When we do support dynamic shared object instrumentation,
   // we'll need to do this for each set of page load offsets per DSO loaded. For
   // now we're assuming we can mprotect the whole section of text between the
   // minimum sled address and the maximum sled address (+ the largest sled
   // size).
-  auto MinSled = InstrMap.Sleds[0];
-  auto MaxSled = InstrMap.Sleds[InstrMap.Entries - 1];
+  auto *MinSled = &InstrMap.Sleds[0];
+  auto *MaxSled = &InstrMap.Sleds[InstrMap.Entries - 1];
   for (std::size_t I = 0; I < InstrMap.Entries; I++) {
     const auto &Sled = InstrMap.Sleds[I];
-    if (Sled.Address < MinSled.Address)
-      MinSled = Sled;
-    if (Sled.Address > MaxSled.Address)
-      MaxSled = Sled;
+    if (Sled.address() < MinSled->address())
+      MinSled = &Sled;
+    if (Sled.address() > MaxSled->address())
+      MaxSled = &Sled;
   }
 
   const size_t PageSize = flags()->xray_page_size_override > 0
                               ? flags()->xray_page_size_override
                               : GetPageSizeCached();
   if ((PageSize == 0) || ((PageSize & (PageSize - 1)) != 0)) {
     Report("System page size is not a power of two: %lld\n", PageSize);
     return XRayPatchingStatus::FAILED;
   }
 
   void *PageAlignedAddr =
-      reinterpret_cast<void *>(MinSled.Address & ~(PageSize - 1));
+      reinterpret_cast<void *>(MinSled->address() & ~(PageSize - 1));
   size_t MProtectLen =
-      (MaxSled.Address - reinterpret_cast<uptr>(PageAlignedAddr)) + cSledLength;
+      (MaxSled->address() - reinterpret_cast<uptr>(PageAlignedAddr)) +
+      cSledLength;
   MProtectHelper Protector(PageAlignedAddr, MProtectLen, PageSize);
   if (Protector.MakeWriteable() == -1) {
     Report("Failed mprotect: %d\n", errno);
     return XRayPatchingStatus::FAILED;
   }
 
   for (std::size_t I = 0; I < InstrMap.Entries; ++I) {
     auto &Sled = InstrMap.Sleds[I];
if ((PageSize == 0) || ((PageSize & (PageSize - 1)) != 0)) {
     return XRayPatchingStatus::FAILED;
   }
 
   // Here we compute the minumum sled and maximum sled associated with a
   // particular function ID.
   auto SledRange = InstrMap.SledsIndex[FuncId - 1];
   auto *f = SledRange.Begin;
   auto *e = SledRange.End;
-  auto MinSled = *f;
-  auto MaxSled = *(SledRange.End - 1);
+  auto *MinSled = f;
+  auto *MaxSled = (SledRange.End - 1);
   while (f != e) {
-    if (f->Address < MinSled.Address)
-      MinSled = *f;
-    if (f->Address > MaxSled.Address)
-      MaxSled = *f;
+    if (f->address() < MinSled->address())
+      MinSled = f;
+    if (f->address() > MaxSled->address())
+      MaxSled = f;
     ++f;
   }
 
   void *PageAlignedAddr =
-      reinterpret_cast<void *>(MinSled.Address & ~(PageSize - 1));
+      reinterpret_cast<void *>(MinSled->address() & ~(PageSize - 1));
   size_t MProtectLen =
-      (MaxSled.Address - reinterpret_cast<uptr>(PageAlignedAddr)) + cSledLength;
+      (MaxSled->address() - reinterpret_cast<uptr>(PageAlignedAddr)) +
+      cSledLength;
   MProtectHelper Protector(PageAlignedAddr, MProtectLen, PageSize);
   if (Protector.MakeWriteable() == -1) {
     Report("Failed mprotect: %d\n", errno);
     return XRayPatchingStatus::FAILED;
   }
   return patchFunction(FuncId, Enable);
 }
 

compiler-rt/lib/xray/xray_interface_internal.h

 struct XRaySledEntry {
 #if SANITIZER_WORDSIZE == 64
   uint64_t Address;
   uint64_t Function;
   unsigned char Kind;
   unsigned char AlwaysInstrument;
   unsigned char Version;
   unsigned char Padding[13]; // Need 32 bytes
+  uint64_t address() const {
+#ifndef __x86_64__
+    // R_MIPS_PC64 does not exist. Use absolute address even for version 2.
+    return Address;
+#else
+    // TODO Eventually all targets but MIPS64 should take this branch.
+    if (Version < 2)
+      return Address;
+    // The target address is relative to the location of the Address variable.
+    return reinterpret_cast<uint64_t>(&Address) + Address;
+#endif
+  }
 #elif SANITIZER_WORDSIZE == 32
   uint32_t Address;
   uint32_t Function;
   unsigned char Kind;
   unsigned char AlwaysInstrument;
   unsigned char Version;
   unsigned char Padding[5]; // Need 16 bytes
+  uint32_t address() const { return Address; }
 #else
 #error "Unsupported word size."
 #endif
 };
 
 struct XRayFunctionSledIndex {
   const XRaySledEntry *Begin;
   const XRaySledEntry *End;

compiler-rt/lib/xray/xray_x86_64.cpp

bool patchFunctionEntry(const bool Enable, const uint32_t FuncId,
   // 1. Put the function id first, 2 bytes from the start of the sled (just
   // after the 2-byte jmp instruction).
   // 2. Put the call opcode 6 bytes from the start of the sled.
   // 3. Put the relative offset 7 bytes from the start of the sled.
   // 4. Do an atomic write over the jmp instruction for the "mov r10d"
   // opcode and first operand.
   //
   // Prerequisite is to compute the relative offset to the trampoline's address.
-  const uint64_t Address = Sled.Address;
+  const uint64_t Address = Sled.address();
   int64_t TrampolineOffset = reinterpret_cast<int64_t>(Trampoline) -
                              (static_cast<int64_t>(Address) + 11);
   if (TrampolineOffset < MinOffset || TrampolineOffset > MaxOffset) {
     Report("XRay Entry trampoline (%p) too far from sled (%p)\n", Trampoline,
            reinterpret_cast<void *>(Address));
     return false;
   }
   if (Enable) {
bool patchFunctionExit(const bool Enable, const uint32_t FuncId,
   // after the 1-byte ret instruction).
   // 2. Put the jmp opcode 6 bytes from the start of the sled.
   // 3. Put the relative offset 7 bytes from the start of the sled.
   // 4. Do an atomic write over the jmp instruction for the "mov r10d"
   // opcode and first operand.
   //
   // Prerequisite is to compute the relative offset fo the
   // __xray_FunctionExit function's address.
-  const uint64_t Address = Sled.Address;
+  const uint64_t Address = Sled.address();
   int64_t TrampolineOffset = reinterpret_cast<int64_t>(__xray_FunctionExit) -
                              (static_cast<int64_t>(Address) + 11);
   if (TrampolineOffset < MinOffset || TrampolineOffset > MaxOffset) {
     Report("XRay Exit trampoline (%p) too far from sled (%p)\n",
            __xray_FunctionExit, reinterpret_cast<void *>(Address));
     return false;
   }
   if (Enable) {
bool patchFunctionExit(const bool Enable, const uint32_t FuncId,
   }
   return true;
 }
 
 bool patchFunctionTailExit(const bool Enable, const uint32_t FuncId,
                            const XRaySledEntry &Sled) XRAY_NEVER_INSTRUMENT {
   // Here we do the dance of replacing the tail call sled with a similar
   // sequence as the entry sled, but calls the tail exit sled instead.
-  const uint64_t Address = Sled.Address;
+  const uint64_t Address = Sled.address();
   int64_t TrampolineOffset =
       reinterpret_cast<int64_t>(__xray_FunctionTailExit) -
       (static_cast<int64_t>(Address) + 11);
   if (TrampolineOffset < MinOffset || TrampolineOffset > MaxOffset) {
     Report("XRay Tail Exit trampoline (%p) too far from sled (%p)\n",
            __xray_FunctionTailExit, reinterpret_cast<void *>(Address));
     return false;
   }
bool patchCustomEvent(const bool Enable, const uint32_t FuncId,
   //   nopw             // 2 bytes*
   //   ...
   //
   //
   // The "unpatch" should just turn the 'nopw' back to a 'jmp +20'.
   //
   // ---
   //
-  // In Version 1:
+  // In Version 1 or 2:
   //
   //   The jump offset is now 15 bytes (0x0f), so when restoring the nopw back
   //   to a jmp, use 15 bytes instead.
   //
-  const uint64_t Address = Sled.Address;
+  const uint64_t Address = Sled.address();
   if (Enable) {
     std::atomic_store_explicit(
         reinterpret_cast<std::atomic<uint16_t> *>(Address), NopwSeq,
         std::memory_order_release);
   } else {
     switch (Sled.Version) {
     case 1:
+    case 2:
       std::atomic_store_explicit(
           reinterpret_cast<std::atomic<uint16_t> *>(Address), Jmp15Seq,
           std::memory_order_release);
       break;
     case 0:
     default:
       std::atomic_store_explicit(
           reinterpret_cast<std::atomic<uint16_t> *>(Address), Jmp20Seq,
bool patchTypedEvent(const bool Enable, const uint32_t FuncId,
   //   ...
   //
   //
   // The "unpatch" should just turn the 'nopw' back to a 'jmp +20'.
   // The 20 byte sled stashes three argument registers, calls the trampoline,
   // unstashes the registers and returns. If the arguments are already in
   // the correct registers, the stashing and unstashing become equivalently
   // sized nops.
-  const uint64_t Address = Sled.Address;
+  const uint64_t Address = Sled.address();
   if (Enable) {
     std::atomic_store_explicit(
         reinterpret_cast<std::atomic<uint16_t> *>(Address), NopwSeq,
         std::memory_order_release);
   } else {
     std::atomic_store_explicit(
         reinterpret_cast<std::atomic<uint16_t> *>(Address), Jmp20Seq,
         std::memory_order_release);

llvm/include/llvm/CodeGen/AsmPrinter.h

public:
   struct XRayFunctionEntry {
     const MCSymbol *Sled;
     const MCSymbol *Function;
     SledKind Kind;
     bool AlwaysInstrument;
     const class Function *Fn;
     uint8_t Version;
 
-    void emit(int, MCStreamer *, const MCSymbol *) const;
+    void emit(int, MCStreamer *, const MCExpr *, const MCSymbol *) const;
   };
 
   // All the sleds to be emitted.
   SmallVector<XRayFunctionEntry, 4> Sleds;
 
   // Helper function to record a given XRay sled.
   void recordSled(MCSymbol *Sled, const MachineInstr &MI, SledKind Kind,
                   uint8_t Version = 0);

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

 AsmPrinterHandler::~AsmPrinterHandler() = default;
 
 void AsmPrinterHandler::markFunctionEnd() {}
 
 // In the binary's "xray_instr_map" section, an array of these function entries
 // describes each instrumentation point.  When XRay patches your code, the index
 // into this table will be given to your handler as a patch point identifier.
 void AsmPrinter::XRayFunctionEntry::emit(int Bytes, MCStreamer *Out,
+                                         const MCExpr *Location,
                                          const MCSymbol *CurrentFnSym) const {
+  if (Location)
+    Out->emitValueImpl(Location, Bytes);
+  else
     Out->emitSymbolValue(Sled, Bytes);
   Out->emitSymbolValue(CurrentFnSym, Bytes);
   auto Kind8 = static_cast<uint8_t>(Kind);
   Out->emitBinaryData(StringRef(reinterpret_cast<const char *>(&Kind8), 1));
   Out->emitBinaryData(
       StringRef(reinterpret_cast<const char *>(&AlwaysInstrument), 1));
   Out->emitBinaryData(StringRef(reinterpret_cast<const char *>(&Version), 1));
   auto Padding = (4 * Bytes) - ((2 * Bytes) + 3);
   assert(Padding >= 0 && "Instrumentation map entry > 4 * Word Size");
   Out->emitZeros(Padding);
 }
 
 void AsmPrinter::emitXRayTable() {
   if (Sleds.empty())
     return;
 
   auto PrevSection = OutStreamer->getCurrentSectionOnly();
   const Function &F = MF->getFunction();
   MCSection *InstMap = nullptr;
   MCSection *FnSledIndex = nullptr;
-  if (MF->getSubtarget().getTargetTriple().isOSBinFormatELF()) {
+  const Triple &TT = TM.getTargetTriple();
+  // Version 2 uses a PC-relative address on all supported targets.
+  bool PCRel = TT.isX86();
+  if (TT.isOSBinFormatELF()) {
     auto LinkedToSym = cast<MCSymbolELF>(CurrentFnSym);
-    auto Flags = ELF::SHF_WRITE | ELF::SHF_ALLOC | ELF::SHF_LINK_ORDER;
+    auto Flags = ELF::SHF_ALLOC | ELF::SHF_LINK_ORDER;
+    if (!PCRel)
+      Flags |= ELF::SHF_WRITE;
     StringRef GroupName;
     if (F.hasComdat()) {
       Flags |= ELF::SHF_GROUP;
       GroupName = F.getComdat()->getName();
     }
     InstMap = OutContext.getELFSection("xray_instr_map", ELF::SHT_PROGBITS,
                                        Flags, 0, GroupName,
                                        MCSection::NonUniqueID, LinkedToSym);
void AsmPrinter::emitXRayTable() {
   auto WordSizeBytes = MAI->getCodePointerSize();
 
   // Now we switch to the instrumentation map section. Because this is done
   // per-function, we are able to create an index entry that will represent the
   // range of sleds associated with a function.
   MCSymbol *SledsStart = OutContext.createTempSymbol("xray_sleds_start", true);
   OutStreamer->SwitchSection(InstMap);
   OutStreamer->emitLabel(SledsStart);
-  for (const auto &Sled : Sleds)
-    Sled.emit(WordSizeBytes, OutStreamer.get(), CurrentFnSym);
+  for (const auto &Sled : Sleds) {
+    const MCExpr *Location = nullptr;
+    if (PCRel) {
+      MCSymbol *Dot = OutContext.createTempSymbol();
+      OutStreamer->emitLabel(Dot);
+      Location = MCBinaryExpr::createSub(
+          MCSymbolRefExpr::create(Sled.Sled, OutContext),
+          MCSymbolRefExpr::create(Dot, OutContext), OutContext);
+    }
+    Sled.emit(WordSizeBytes, OutStreamer.get(), Location, CurrentFnSym);
+  }
   MCSymbol *SledsEnd = OutContext.createTempSymbol("xray_sleds_end", true);
   OutStreamer->emitLabel(SledsEnd);
 
   // We then emit a single entry in the index per function. We use the symbols
   // that bound the instrumentation map as the range for a specific function.
   // Each entry here will be 2 * word size aligned, as we're writing down two
   // pointers. This should work for both 32-bit and 64-bit platforms.
   OutStreamer->SwitchSection(FnSledIndex);

llvm/lib/Target/X86/X86MCInstLower.cpp

void X86AsmPrinter::LowerPATCHABLE_EVENT_CALL(const MachineInstr &MI,
   for (unsigned I = sizeof UsedMask; I-- > 0;)
     if (UsedMask[I])
       EmitAndCountInstruction(MCInstBuilder(X86::POP64r).addReg(DestRegs[I]));
     else
       EmitNops(*OutStreamer, 1, Subtarget->is64Bit(), getSubtargetInfo());
 
   OutStreamer->AddComment("xray custom event end.");
 
-  // Record the sled version. Older versions of this sled were spelled
-  // differently, so we let the runtime handle the different offsets we're
-  // using.
-  recordSled(CurSled, MI, SledKind::CUSTOM_EVENT, 1);
+  // Record the sled version. Version 0 of this sled was spelled differently, so
+  // we let the runtime handle the different offsets we're using. Version 2
+  // changed the absolute address to a PC-relative address.
+  recordSled(CurSled, MI, SledKind::CUSTOM_EVENT, 2);
 }
 
 void X86AsmPrinter::LowerPATCHABLE_TYPED_EVENT_CALL(const MachineInstr &MI,
                                                     X86MCInstLower &MCIL) {
   assert(Subtarget->is64Bit() && "XRay typed events only supports X86-64");
 
   NoAutoPaddingScope NoPadScope(*OutStreamer);
 
for (unsigned I = sizeof UsedMask; I-- > 0;)
     if (UsedMask[I])
       EmitAndCountInstruction(MCInstBuilder(X86::POP64r).addReg(DestRegs[I]));
     else
       EmitNops(*OutStreamer, 1, Subtarget->is64Bit(), getSubtargetInfo());
 
   OutStreamer->AddComment("xray typed event end.");
 
   // Record the sled version.
-  recordSled(CurSled, MI, SledKind::TYPED_EVENT, 0);
+  recordSled(CurSled, MI, SledKind::TYPED_EVENT, 2);
 }
 
 void X86AsmPrinter::LowerPATCHABLE_FUNCTION_ENTER(const MachineInstr &MI,
                                                   X86MCInstLower &MCIL) {
 
   NoAutoPaddingScope NoPadScope(*OutStreamer);
 
   const Function &F = MF->getFunction();
void X86AsmPrinter::LowerPATCHABLE_FUNCTION_ENTER(const MachineInstr &MI,
   OutStreamer->emitCodeAlignment(2);
   OutStreamer->emitLabel(CurSled);
 
   // Use a two-byte `jmp`. This version of JMP takes an 8-bit relative offset as
   // an operand (computed as an offset from the jmp instruction).
   // FIXME: Find another less hacky way do force the relative jump.
   OutStreamer->emitBytes("\xeb\x09");
   EmitNops(*OutStreamer, 9, Subtarget->is64Bit(), getSubtargetInfo());
-  recordSled(CurSled, MI, SledKind::FUNCTION_ENTER);
+  recordSled(CurSled, MI, SledKind::FUNCTION_ENTER, 2);
 }
 
 void X86AsmPrinter::LowerPATCHABLE_RET(const MachineInstr &MI,
                                        X86MCInstLower &MCIL) {
   NoAutoPaddingScope NoPadScope(*OutStreamer);
 
   // Since PATCHABLE_RET takes the opcode of the return statement as an
   // argument, we use that to emit the correct form of the RET that we want.
void X86AsmPrinter::LowerPATCHABLE_RET(const MachineInstr &MI,
   unsigned OpCode = MI.getOperand(0).getImm();
   MCInst Ret;
   Ret.setOpcode(OpCode);
   for (auto &MO : make_range(MI.operands_begin() + 1, MI.operands_end()))
     if (auto MaybeOperand = MCIL.LowerMachineOperand(&MI, MO))
       Ret.addOperand(MaybeOperand.getValue());
   OutStreamer->emitInstruction(Ret, getSubtargetInfo());
   EmitNops(*OutStreamer, 10, Subtarget->is64Bit(), getSubtargetInfo());
-  recordSled(CurSled, MI, SledKind::FUNCTION_EXIT);
+  recordSled(CurSled, MI, SledKind::FUNCTION_EXIT, 2);
 }
 
 void X86AsmPrinter::LowerPATCHABLE_TAIL_CALL(const MachineInstr &MI,
                                              X86MCInstLower &MCIL) {
   NoAutoPaddingScope NoPadScope(*OutStreamer);
 
   // Like PATCHABLE_RET, we have the actual instruction in the operands to this
   // instruction so we lower that particular instruction and its operands.
   // Unlike PATCHABLE_RET though, we put the sled before the JMP, much like how
   // we do it for PATCHABLE_FUNCTION_ENTER. The sled should be very similar to
   // the PATCHABLE_FUNCTION_ENTER case, followed by the lowering of the actual
   // tail call much like how we have it in PATCHABLE_RET.
   auto CurSled = OutContext.createTempSymbol("xray_sled_", true);
   OutStreamer->emitCodeAlignment(2);
   OutStreamer->emitLabel(CurSled);
   auto Target = OutContext.createTempSymbol();
 
   // Use a two-byte `jmp`. This version of JMP takes an 8-bit relative offset as
   // an operand (computed as an offset from the jmp instruction).
   // FIXME: Find another less hacky way do force the relative jump.
   OutStreamer->emitBytes("\xeb\x09");
   EmitNops(*OutStreamer, 9, Subtarget->is64Bit(), getSubtargetInfo());
   OutStreamer->emitLabel(Target);
-  recordSled(CurSled, MI, SledKind::TAIL_CALL);
+  recordSled(CurSled, MI, SledKind::TAIL_CALL, 2);
 
   unsigned OpCode = MI.getOperand(0).getImm();
   OpCode = convertTailJumpOpcode(OpCode);
   MCInst TC;
   TC.setOpcode(OpCode);
 
   // Before emitting the instruction, add a comment to indicate that this is
   // indeed a tail call.

llvm/test/CodeGen/X86/xray-attribute-instrumentation.ll

NotEqual:
   ret i32 1
 ; CHECK:       .p2align 1, 0x90
 ; CHECK-LABEL: Lxray_sled_4:
 ; CHECK:       retq
 ; CHECK-NEXT:  nopw %cs:512(%rax,%rax)
 }
 ; CHECK-LABEL: xray_instr_map
 ; CHECK-LABEL: Lxray_sleds_start1:
-; CHECK:       .quad {{.*}}xray_sled_2
-; CHECK:       .quad {{.*}}xray_sled_3
-; CHECK:       .quad {{.*}}xray_sled_4
+; CHECK:       Ltmp2:
+; CHECK-NEXT:   .quad {{.*}}xray_sled_2-{{\.?}}Ltmp2
+; CHECK:       Ltmp3:
+; CHECK-NEXT:   .quad {{.*}}xray_sled_3-{{\.?}}Ltmp3
+; CHECK:       Ltmp4:
+; CHECK-NEXT:   .quad {{.*}}xray_sled_4-{{\.?}}Ltmp4
 ; CHECK-LABEL: Lxray_sleds_end1:
 ; CHECK-LABEL: xray_fn_idx
 ; CHECK:       .quad {{.*}}xray_sleds_start1
 ; CHECK-NEXT:  .quad {{.*}}xray_sleds_end1

llvm/test/CodeGen/X86/xray-log-args.ll

 ; When logging arguments is specified, emit the entry sled accordingly.
 
 ; RUN: llc -verify-machineinstrs -filetype=asm -o - -mtriple=x86_64-unknown-linux-gnu < %s | FileCheck %s
 ; RUN: llc -verify-machineinstrs -filetype=asm -o - -mtriple=x86_64-darwin-unknown < %s | FileCheck %s
 
 define i32 @callee(i32 %arg) nounwind noinline uwtable "function-instrument"="xray-always" "xray-log-args"="1" {
   ret i32 %arg
 }
 ; CHECK-LABEL: Lxray_sleds_start0:
-; CHECK:  .quad {{\.?}}Lxray_sled_0
-; CHECK:  .quad {{_?}}callee
-; CHECK:  .byte 0x03
-; CHECK:  .byte 0x01
-; CHECK:  .byte 0x00
-; CHECK:  .{{(zero|space)}}  13
-; CHECK:  .quad {{\.?}}Lxray_sled_1
-; CHECK:  .quad {{_?}}callee
-; CHECK:  .byte 0x01
-; CHECK:  .byte 0x01
-; CHECK:  .byte 0x00
+; CHECK-NEXT:  Ltmp0:
+; CHECK-NEXT:   .quad {{\.?}}Lxray_sled_0-{{\.?}}Ltmp0
+; CHECK-NEXT:   .quad {{_?}}callee
+; CHECK-NEXT:   .byte 0x03
+; CHECK-NEXT:   .byte 0x01
+; CHECK-NEXT:   .byte 0x02
+; CHECK:        .{{(zero|space)}}  13
+; CHECK:       Ltmp1:
+; CHECK-NEXT:   .quad {{\.?}}Lxray_sled_1-{{\.?}}Ltmp1
+; CHECK-NEXT:   .quad {{_?}}callee
+; CHECK-NEXT:   .byte 0x01
+; CHECK-NEXT:   .byte 0x01
+; CHECK-NEXT:   .byte 0x02
 ; CHECK:  .{{(zero|space)}}  13
 
 define i32 @caller(i32 %arg) nounwind noinline uwtable "function-instrument"="xray-always" "xray-log-args"="1" {
   %retval = tail call i32 @callee(i32 %arg)
   ret i32 %retval
 }
 ; CHECK-LABEL: Lxray_sleds_start1:
-; CHECK:  .quad {{\.?}}Lxray_sled_2
-; CHECK:  .quad {{_?}}caller
-; CHECK:  .byte 0x03
-; CHECK:  .byte 0x01
-; CHECK:  .byte 0x00
+; CHECK-NEXT:  Ltmp3:
+; CHECK-NEXT:   .quad {{\.?}}Lxray_sled_2-{{\.?}}Ltmp3
+; CHECK-NEXT:   .quad {{_?}}caller
+; CHECK-NEXT:   .byte 0x03
+; CHECK-NEXT:   .byte 0x01
+; CHECK-NEXT:   .byte 0x02
 ; CHECK:  .{{(zero|space)}}  13
-; CHECK:  .quad {{\.?}}Lxray_sled_3
-; CHECK:  .quad {{_?}}caller
-; CHECK:  .byte 0x02
-; CHECK:  .byte 0x01
-; CHECK:  .byte 0x00
+; CHECK:       Ltmp4:
+; CHECK-NEXT:   .quad {{\.?}}Lxray_sled_3-{{\.?}}Ltmp4
+; CHECK-NEXT:   .quad {{_?}}caller
+; CHECK-NEXT:   .byte 0x02
+; CHECK-NEXT:   .byte 0x01
+; CHECK-NEXT:   .byte 0x02
 ; CHECK:  .{{(zero|space)}}  13

llvm/test/CodeGen/X86/xray-section-group.ll

 ; RUN: llc -verify-machineinstrs -filetype=asm -o - -mtriple=x86_64-unknown-linux-gnu -function-sections < %s | FileCheck %s
 ; RUN: llc -verify-machineinstrs -filetype=obj -o %t -mtriple=x86_64-unknown-linux-gnu -function-sections < %s
 ; RUN: llvm-objdump --triple=x86_64-unknown-linux-gnu --disassemble-all %t | FileCheck %s --check-prefix=CHECK-OBJ
 
 define i32 @foo() nounwind noinline uwtable "function-instrument"="xray-always" {
 ; CHECK: .section .text.foo,"ax",@progbits
   ret i32 0
-; CHECK: .section xray_instr_map,"awo",@progbits,foo{{$}}
+; CHECK: .section xray_instr_map,"ao",@progbits,foo{{$}}
 }
 
 $bar = comdat any
 define i32 @bar() nounwind noinline uwtable "function-instrument"="xray-always" comdat($bar) {
 ; CHECK: .section .text.bar,"axG",@progbits,bar,comdat
   ret i32 1
-; CHECK: .section xray_instr_map,"aGwo",@progbits,bar,comdat,bar{{$}}
+; CHECK: .section xray_instr_map,"aGo",@progbits,bar,comdat,bar{{$}}
 }
 
 ; CHECK-OBJ:      section xray_instr_map:

llvm/test/DebugInfo/X86/xray-split-dwarf-interaction.ll

 ;   for (;;)
 ;     ;
 ; }
 ;
 ; In this test we want to make sure that the xray_instr_map section for
 ; `a::b()` is actually associated with the function's symbol instead of the
 ; .debug_types.dwo section.
 ;
-; CHECK-ASM: xray_fn_idx,"awo",@progbits,_ZN1a1bEv{{$}}
+; CHECK-ASM: xray_fn_idx,"ao",@progbits,_ZN1a1bEv{{$}}
 ;
 ; CHECK-ELF-DAG: [[FSECT:[0-9]+]]] .text._ZN1a1bEv PROGBITS
 ; CHECK-ELF-DAG: [{{.*}}] .debug_types.dwo PROGBITS
-; CHECK-ELF-DAG: [{{.*}}] xray_instr_map PROGBITS {{.*}} {{.*}} {{.*}} {{.*}} WAL [[FSECT]]
+; CHECK-ELF-DAG: [{{.*}}] xray_instr_map PROGBITS {{.*}} {{.*}} {{.*}} {{.*}} AL [[FSECT]]
 target triple = "x86_64-pc-linux"
 
 %class.a = type { i8 }
 
 ; Function Attrs: nounwind readnone uwtable
 define i32 @_ZN1a1bEv(%class.a* nocapture readnone) local_unnamed_addr #0 align 2 !dbg !8 {
   tail call void @llvm.dbg.value(metadata %class.a* %0, metadata !17, metadata !DIExpression()), !dbg !19
   br label %2, !dbg !20