[XRay] Change ARM/AArch64/powerpc64le to use version 2 sled (PC-relative address)

[MaskRay]

Summary

Follow-up of D78082 (x86-64).

This change avoids dynamic relocations in xray_instr_map for ARM/AArch64/powerpc64le.

MIPS64 cannot use 64-bit PC-relative addresses because R_MIPS_PC64 is not defined.
Because MIPS32 shares the same code, for simplicity, we don't use PC-relative addresses for MIPS32 as well.

Tested on AArch64 Linux and ppc64le Linux.

[MaskRay]

@dberris xray_fn_idx is still using absolute relocations and thus must have the SHF_WRITE flag. If we are also going to make it read-only, how can we address compatibility issues in compiler-rt/lib/xray ?

[MaskRay]

Fix ARM (I can't test, though)

[ianlevesque]

@MaskRay for what it's worth i'm currently trying to make xray_fn_idx optional - we tend to patch an entire DSO at a time so we don't need it to find individual sleds (I think we could support the entire compiler-rt interface, just slower, without it). Perhaps if someone cares about avoiding the relocations in xray_fn_idx they could just use the option to omit it as well.

[dberris]

In D78590#1995525, @MaskRay wrote:

@dberris xray_fn_idx is still using absolute relocations and thus must have the SHF_WRITE flag. If we are also going to make it read-only, how can we address compatibility issues in compiler-rt/lib/xray ?

Okay, that one's a little tougher, because the index entries don't have a version. :(

We have options:

1. Leave this as-is.
2. Check at runtime whether the index start is a relative address (i.e. whether the higher order bits in x86 are non-zero) or if index start is zeros because the relative offsets (32-bits) are packed into the index end quad, and handle accordingly. It might introduce an overhead (probably not measurable either). Warn once if we ever encounter the absolute addresses (can be done in xray_init.cpp).
3. Do an ABI break and expand the index entries to include index versions.

If we can do 2 in a portable manner, that would be nice to start with.

If we need to go to 3, then we might consider a transition plan:

• First, create a different index for the read-only section, in parallel (flag-controlled). This would make it possible to check at runtime whether you have two different index sections and warn. Use the read-write version first, but flag-control the runtime to attempt to use the read-only section.
• When enough time has passed (maybe one release) we can make the runtime default to using the read-only section.
• Update the compiler to not emit the read-write section anymore, and remove the code in the runtime referring to the read-write section.

I understand that 3 is cumbersome but it's probably the safest way to transition off the read-write sections.

[MaskRay]

Does this patch itself look good?

[johnislarry]

Thank you for doing this @MaskRay!

[smeenai]

@ianlevesque if you just accept without a message, a mail doesn't get sent to the appropriate *-commits list, which can be confusing for people browsing that list. It's a good idea to always accept with a "LGTM" or similar message to fix that.

compiler-rt/lib/xray/xray_AArch64.cpp

inline static bool patchSled(const bool Enable, const uint32_t FuncId,
   // Replacement of the first 4-byte instruction should be the last and atomic
   // operation, so that the user code which reaches the sled concurrently
   // either jumps over the whole sled, or executes the whole sled when the
   // latter is ready.
   //
   // When |Enable|==false, we set back the first instruction in the sled to be
   //   B #32
 
-  uint32_t *FirstAddress = reinterpret_cast<uint32_t *>(Sled.Address);
+  uint32_t *FirstAddress = reinterpret_cast<uint32_t *>(Sled.address());
   uint32_t *CurAddress = FirstAddress + 1;
   if (Enable) {
     *CurAddress = uint32_t(PatchOpcodes::PO_LdrW0_12);
     CurAddress++;
     *CurAddress = uint32_t(PatchOpcodes::PO_LdrX16_12);
     CurAddress++;
     *CurAddress = uint32_t(PatchOpcodes::PO_BlrX16);
     CurAddress++;

compiler-rt/lib/xray/xray_arm.cpp

inline static bool patchSled(const bool Enable, const uint32_t FuncId,
   // Replacement of the first 4-byte instruction should be the last and atomic
   // operation, so that the user code which reaches the sled concurrently
   // either jumps over the whole sled, or executes the whole sled when the
   // latter is ready.
   //
   // When |Enable|==false, we set back the first instruction in the sled to be
   //   B #20
 
-  uint32_t *FirstAddress = reinterpret_cast<uint32_t *>(Sled.Address);
+  uint32_t *FirstAddress = reinterpret_cast<uint32_t *>(Sled.address());
   uint32_t *CurAddress = FirstAddress + 1;
   if (Enable) {
     CurAddress =
         write32bitLoadR0(CurAddress, reinterpret_cast<uint32_t>(FuncId));
     CurAddress =
         write32bitLoadIP(CurAddress, reinterpret_cast<uint32_t>(TracingHook));
     *CurAddress = uint32_t(PatchOpcodes::PO_BlxIp);
     CurAddress++;

compiler-rt/lib/xray/xray_interface_internal.h

 #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; }
+  uint32_t address() const {
+    if (Version < 2)
+      return Address;
+    // The target address is relative to the location of the Address variable.
+    return reinterpret_cast<uint32_t>(&Address) + Address;
+  }
 #else
 #error "Unsupported word size."
 #endif
 };
 
 struct XRayFunctionSledIndex {
   const XRaySledEntry *Begin;
   const XRaySledEntry *End;

compiler-rt/lib/xray/xray_powerpc64.cpp

 
 extern "C" void __clear_cache(void *start, void *end);
 
 namespace __xray {
 
 bool patchFunctionEntry(const bool Enable, uint32_t FuncId,
                         const XRaySledEntry &Sled,
                         void (*Trampoline)()) XRAY_NEVER_INSTRUMENT {
-  const uint64_t Address = Sled.Address;
+  const uint64_t Address = Sled.address();
   if (Enable) {
     // lis 0, FuncId[16..32]
     // li 0, FuncId[0..15]
     *reinterpret_cast<uint64_t *>(Address) =
         (0x3c000000ull + (FuncId >> 16)) +
         ((0x60000000ull + (FuncId & 0xffff)) << 32);
   } else {
     // b +JumpOverInstNum instructions.
     *reinterpret_cast<uint32_t *>(Address) =
         0x48000000ull + (JumpOverInstNum << 2);
   }
   clearCache(reinterpret_cast<void *>(Address), 8);
   return true;
 }
 
 bool patchFunctionExit(const bool Enable, uint32_t FuncId,
                        const XRaySledEntry &Sled) XRAY_NEVER_INSTRUMENT {
-  const uint64_t Address = Sled.Address;
+  const uint64_t Address = Sled.address();
   if (Enable) {
     // lis 0, FuncId[16..32]
     // li 0, FuncId[0..15]
     *reinterpret_cast<uint64_t *>(Address) =
         (0x3c000000ull + (FuncId >> 16)) +
         ((0x60000000ull + (FuncId & 0xffff)) << 32);
   } else {
     // Copy the blr/b instruction after JumpOverInstNum instructions.

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

void AsmPrinter::emitXRayTable() {
   if (Sleds.empty())
     return;
 
   auto PrevSection = OutStreamer->getCurrentSectionOnly();
   const Function &F = MF->getFunction();
   MCSection *InstMap = nullptr;
   MCSection *FnSledIndex = nullptr;
   const Triple &TT = TM.getTargetTriple();
-  // Version 2 uses a PC-relative address on all supported targets.
-  bool PCRel = TT.isX86();
+  // Use PC-relative addresses on all targets except MIPS (MIPS64 cannot use
+  // PC-relative addresses because R_MIPS_PC64 does not exist).
+  bool PCRel = !TT.isMIPS();
   if (TT.isOSBinFormatELF()) {
     auto LinkedToSym = cast<MCSymbolELF>(CurrentFnSym);
     auto Flags = ELF::SHF_ALLOC | ELF::SHF_LINK_ORDER;
     if (!PCRel)
       Flags |= ELF::SHF_WRITE;
     StringRef GroupName;
     if (F.hasComdat()) {
       Flags |= ELF::SHF_GROUP;

llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp

void AArch64AsmPrinter::EmitSled(const MachineInstr &MI, SledKind Kind)
   // The operand has to be the number of 4-byte instructions to jump over,
   // including the current instruction.
   EmitToStreamer(*OutStreamer, MCInstBuilder(AArch64::B).addImm(8));
 
   for (int8_t I = 0; I < NoopsInSledCount; I++)
     EmitToStreamer(*OutStreamer, MCInstBuilder(AArch64::HINT).addImm(0));
 
   OutStreamer->emitLabel(Target);
-  recordSled(CurSled, MI, Kind);
+  recordSled(CurSled, MI, Kind, 2);
 }
 
 void AArch64AsmPrinter::LowerHWASAN_CHECK_MEMACCESS(const MachineInstr &MI) {
   Register Reg = MI.getOperand(0).getReg();
   bool IsShort =
       MI.getOpcode() == AArch64::HWASAN_CHECK_MEMACCESS_SHORTGRANULES;
   uint32_t AccessInfo = MI.getOperand(1).getImm();
   MCSymbol *&Sym =

llvm/lib/Target/ARM/ARMMCInstLower.cpp

void ARMAsmPrinter::EmitSled(const MachineInstr &MI, SledKind Kind)
   // By analogy to ARMAsmPrinter::emitPseudoExpansionLowering() |case ARM::B|.
   // It is not clear why |addReg(0)| is needed (the last operand).
   EmitToStreamer(*OutStreamer, MCInstBuilder(ARM::Bcc).addImm(20)
     .addImm(ARMCC::AL).addReg(0));
 
   emitNops(NoopsInSledCount);
 
   OutStreamer->emitLabel(Target);
-  recordSled(CurSled, MI, Kind);
+  recordSled(CurSled, MI, Kind, 2);
 }
 
 void ARMAsmPrinter::LowerPATCHABLE_FUNCTION_ENTER(const MachineInstr &MI)
 {
   EmitSled(MI, SledKind::FUNCTION_ENTER);
 }
 
 void ARMAsmPrinter::LowerPATCHABLE_FUNCTION_EXIT(const MachineInstr &MI)
 {
   EmitSled(MI, SledKind::FUNCTION_EXIT);
 }
 
 void ARMAsmPrinter::LowerPATCHABLE_TAIL_CALL(const MachineInstr &MI)
 {
   EmitSled(MI, SledKind::TAIL_CALL);
 }

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

case TargetOpcode::PATCHABLE_FUNCTION_ENTER: {
     EmitToStreamer(*OutStreamer, MCInstBuilder(PPC::MFLR8).addReg(PPC::X0));
     EmitToStreamer(*OutStreamer,
                    MCInstBuilder(PPC::BL8_NOP)
                        .addExpr(MCSymbolRefExpr::create(
                            OutContext.getOrCreateSymbol("__xray_FunctionEntry"),
                            OutContext)));
     EmitToStreamer(*OutStreamer, MCInstBuilder(PPC::MTLR8).addReg(PPC::X0));
     OutStreamer->emitLabel(EndOfSled);
-    recordSled(BeginOfSled, *MI, SledKind::FUNCTION_ENTER);
+    recordSled(BeginOfSled, *MI, SledKind::FUNCTION_ENTER, 2);
     break;
   }
   case TargetOpcode::PATCHABLE_RET: {
     unsigned RetOpcode = MI->getOperand(0).getImm();
     MCInst RetInst;
     RetInst.setOpcode(RetOpcode);
     for (const auto &MO :
          make_range(std::next(MI->operands_begin()), MI->operands_end())) {
EmitToStreamer(*OutStreamer,
                    MCInstBuilder(PPC::BL8_NOP)
                        .addExpr(MCSymbolRefExpr::create(
                            OutContext.getOrCreateSymbol("__xray_FunctionExit"),
                            OutContext)));
     EmitToStreamer(*OutStreamer, MCInstBuilder(PPC::MTLR8).addReg(PPC::X0));
     EmitToStreamer(*OutStreamer, RetInst);
     if (IsConditional)
       OutStreamer->emitLabel(FallthroughLabel);
-    recordSled(BeginOfSled, *MI, SledKind::FUNCTION_EXIT);
+    recordSled(BeginOfSled, *MI, SledKind::FUNCTION_EXIT, 2);
     break;
   }
   case TargetOpcode::PATCHABLE_FUNCTION_EXIT:
     llvm_unreachable("PATCHABLE_FUNCTION_EXIT should never be emitted");
   case TargetOpcode::PATCHABLE_TAIL_CALL:
     // TODO: Define a trampoline `__xray_FunctionTailExit` and differentiate a
     // normal function exit from a tail exit.
     llvm_unreachable("Tail call is handled in the normal case. See comments "

llvm/test/CodeGen/AArch64/xray-tail-call-sled.ll

 ; CHECK-NEXT:  nop
 ; CHECK-NEXT:  nop
 ; CHECK-NEXT:  nop
 ; CHECK-NEXT:  nop
 ; CHECK-LABEL: .Ltmp1:
 ; CHECK-NEXT:  ret
 }
 ; CHECK-LABEL: xray_instr_map
-; CHECK-LABEL: Lxray_sleds_start0:
-; CHECK:       .xword .Lxray_sled_0
-; CHECK:       .xword .Lxray_sled_1
+; CHECK-LABEL: .Lxray_sleds_start0:
+; CHECK-NEXT:  .Ltmp2:
+; CHECK:       .xword .Lxray_sled_0-.Ltmp2
+; CHECK:       .Ltmp3:
+; CHECK-NEXT:  .xword .Lxray_sled_1-.Ltmp3
 ; CHECK-LABEL: Lxray_sleds_end0:
 ; CHECK-LABEL: xray_fn_idx
 ; CHECK:       .xword .Lxray_sleds_start0
 ; CHECK-NEXT:  .xword .Lxray_sleds_end0
 
 define i32 @caller() nounwind noinline uwtable "function-instrument"="xray-always" {
 ; CHECK:       .p2align 2
 ; CHECK-LABEL: Lxray_sled_2:
 ; CHECK-NEXT:  b  #32
 ; CHECK-NEXT:  nop
 ; CHECK-NEXT:  nop
 ; CHECK-NEXT:  nop
 ; CHECK-NEXT:  nop
 ; CHECK-NEXT:  nop
 ; CHECK-NEXT:  nop
 ; CHECK-NEXT:  nop
-; CHECK-LABEL: .Ltmp2:
+; CHECK-LABEL: .Ltmp4:
 ; CHECK:       .p2align 2
 ; CHECK-LABEL: Lxray_sled_3:
 ; CHECK-NEXT:  b  #32
 ; CHECK-NEXT:  nop
 ; CHECK-NEXT:  nop
 ; CHECK-NEXT:  nop
 ; CHECK-NEXT:  nop
 ; CHECK-NEXT:  nop
 ; CHECK-NEXT:  nop
 ; CHECK-NEXT:  nop
-; CHECK-LABEL: .Ltmp3:
+; CHECK-LABEL: .Ltmp5:
   %retval = tail call i32 @callee()
 ; CHECK:       b  callee
   ret i32 %retval
 }
 ; CHECK-LABEL: xray_instr_map
 ; CHECK-LABEL: Lxray_sleds_start1:
 ; CHECK:       .xword .Lxray_sled_2
 ; CHECK:       .xword .Lxray_sled_3
 ; CHECK-LABEL: Lxray_sleds_end1:
 ; CHECK:       .section xray_fn_idx,{{.*}}
 ; CHECK:       .xword .Lxray_sleds_start1
 ; CHECK-NEXT:  .xword .Lxray_sleds_end1

llvm/test/CodeGen/ARM/xray-tail-call-sled.ll

 ; CHECK-LABEL: Lxray_sled_2:
 ; CHECK-NEXT:  b  #20
 ; CHECK-NEXT:  nop
 ; CHECK-NEXT:  nop
 ; CHECK-NEXT:  nop
 ; CHECK-NEXT:  nop
 ; CHECK-NEXT:  nop
 ; CHECK-NEXT:  nop
-; CHECK-LABEL: Ltmp2:
+; CHECK-LABEL: Ltmp4:
 ; CHECK:       .p2align 2
 ; CHECK-LABEL: Lxray_sled_3:
 ; CHECK-NEXT:  b  #20
 ; CHECK-NEXT:  nop
 ; CHECK-NEXT:  nop
 ; CHECK-NEXT:  nop
 ; CHECK-NEXT:  nop
 ; CHECK-NEXT:  nop
 ; CHECK-NEXT:  nop
-; CHECK-LABEL: Ltmp3:
+; CHECK-LABEL: Ltmp5:
   %retval = tail call i32 @callee()
 ; CHECK:       b  {{.*}}callee
   ret i32 %retval
 }

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

 ; CHECK:              blr
 ; CHECK-NEXT:         nop
 ; CHECK-NEXT:         std 0, -8(1)
 ; CHECK-NEXT:         mflr 0
 ; CHECK-NEXT:         bl __xray_FunctionExit
 ; CHECK-NEXT:         nop
 ; CHECK-NEXT:         mtlr 0
 }
-; CHECK-LABEL: xray_instr_map,"awo",@progbits,foo{{$}}
+; CHECK-LABEL: xray_instr_map,"ao",@progbits,foo{{$}}
 ; CHECK:      .Lxray_sleds_start0:
-; CHECK-NEXT:         .quad   .Ltmp0
+; CHECK-NEXT: .Ltmp3:
+; CHECK-NEXT:         .quad   .Ltmp0-.Ltmp3
 ; CHECK-NEXT:         .quad   foo
 ; CHECK-NEXT:         .byte   0x00
 ; CHECK-NEXT:         .byte   0x01
-; CHECK-NEXT:         .byte   0x00
+; CHECK-NEXT:         .byte   0x02
 ; CHECK-NEXT:         .space  13
-; CHECK-NEXT:         .quad   .Ltmp2
+; CHECK-NEXT: .Ltmp4:
+; CHECK-NEXT:         .quad   .Ltmp2-.Ltmp4
 ; CHECK-NEXT:         .quad   foo
 ; CHECK-NEXT:         .byte   0x01
 ; CHECK-NEXT:         .byte   0x01
-; CHECK-NEXT:         .byte   0x00
+; CHECK-NEXT:         .byte   0x02
 ; CHECK-NEXT:         .space  13
 ; CHECK-NEXT: .Lxray_sleds_end0:
 ; CHECK-LABEL: xray_fn_idx,"awo",@progbits,foo{{$}}
 ; CHECK:              .p2align        4
 ; CHECK-NEXT:         .quad   .Lxray_sleds_start0
 ; CHECK-NEXT:         .quad   .Lxray_sleds_end0
 ; CHECK-NEXT:         .text

llvm/test/CodeGen/PowerPC/xray-tail-call-sled.ll

 ; CHECK-NEXT:         std 0, -8(1)
 ; CHECK-NEXT:         mflr 0
 ; CHECK-NEXT:         bl __xray_FunctionExit
 ; CHECK-NEXT:         nop
 ; CHECK-NEXT:         mtlr 0
 }
 
 define i32 @caller() nounwind noinline uwtable "function-instrument"="xray-always" {
-; CHECK-LABEL: .Ltmp3:
-; CHECK:              b .Ltmp4
+; CHECK-LABEL: .Ltmp5:
+; CHECK-NEXT:         b .Ltmp6
 ; CHECK-NEXT:         nop
 ; CHECK-NEXT:         std 0, -8(1)
 ; CHECK-NEXT:         mflr 0
 ; CHECK-NEXT:         bl __xray_FunctionEntry
 ; CHECK-NEXT:         nop
 ; CHECK-NEXT:         mtlr 0
-; CHECK-LABEL: .Ltmp4:
+; CHECK-LABEL: .Ltmp6:
   %retval = tail call i32 @callee()
   ret i32 %retval
-; CHECK-LABEL: .Ltmp5:
+; CHECK-LABEL: .Ltmp7:
 ; CHECK:              blr
 ; CHECK-NEXT:         nop
 ; CHECK-NEXT:         std 0, -8(1)
 ; CHECK-NEXT:         mflr 0
 ; CHECK-NEXT:         bl __xray_FunctionExit
 ; CHECK-NEXT:         nop
 ; CHECK-NEXT:         mtlr 0
 }