[XRay] Synthesize a reference to the xray_instr_map

[dberris]

Summary

Without the synthesized reference to a symbol in the xray_instr_map,
linker section garbage collection will helpfully remove the whole
xray_instr_map section from the final executable (or archive). This will
cause the runtime to not be able to identify the sleds and hot-patch the
calls/jumps into the runtime trampolines.

This change adds a reference from the text section at the end of the
function to keep around the associated xray_instr_map section as well.

We also make sure that we catch this reference in the test.

[mehdi_amini]

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

2 ↗

(On Diff #67667)

I don't understand this line, this is the same run as the first line, but for the check-prefix. The intent is not clear.
Note that "CHECK" is the default prefix, used by the first run. But "-LABEL" is a "magic" suffix for FileCheck. So "CHECK-LABEL" is already used by the first check!
See The “CHECK-LABEL:” directive section here: http://llvm.org/docs/CommandGuide/FileCheck.html

[dberris]

• Fix use of CHECK-LABEL

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

2 ↗

(On Diff #67667)

Whoops -- that was me not understanding what this meant. :/

Fixed.

[dberris]

Rebase

[echristo]

Not happy with this solution, but it'll work for now :)

-eric

[rnk]

How does gold know to avoid stripping .init_array sections? If it's not a special case, I wonder if we could try to do what's done there.

[dberris]

In D23398#542014, @rnk wrote:

How does gold know to avoid stripping .init_array sections? If it's not a special case, I wonder if we could try to do what's done there.

Apparently according to @majnemer (please correct me if I'm wrong here) that it's a special case in gold -- so unless we teach gold to treat xray_instr_map sections as special, or find a way to make this an option you can provide as a flag then this seems to be the thing that works for now. I agree we could do better by changing the linkers to do the right thing, but that will take longer.

[rnk]

My thought was that gold and other linkers could look out for use of the __+section+_start symbol in the final executable, and use that to figure out when to power down --gc-sections. If it's a data (not code) section and the start and end are used, it seems like a pretty solid bet that the user expects to find that data at runtime.

Anyway, the workaround should be fine for now.

[majnemer]

In D23398#542014, @rnk wrote:

How does gold know to avoid stripping .init_array sections? If it's not a special case, I wonder if we could try to do what's done there.

It is a special case: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gold/object.cc;h=a631c9937c5f589de5db561e381f161c637bdfde;hb=HEAD#l1618

[dberris]

In D23398#542023, @rnk wrote:

My thought was that gold and other linkers could look out for use of the __+section+_start symbol in the final executable, and use that to figure out when to power down --gc-sections. If it's a data (not code) section and the start and end are used, it seems like a pretty solid bet that the user expects to find that data at runtime.

I expected exactly that behaviour. It was unfortunate to learn that this wasn't the case.

Anyway, the workaround should be fine for now.

Thanks for having a look!

[rnk]

In D23398#542025, @majnemer wrote:

It is a special case: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gold/object.cc;h=a631c9937c5f589de5db561e381f161c637bdfde;hb=HEAD#l1618

It says right there that section_start/end symbols will suppress the dead stripping, though:
1625 If the section name XXX can be represented as a C identifier
1626 it cannot be discarded if there are references to
1627 start_XXX and __stop_XXX symbols. These need to be
1628 specially handled.

Maybe we just need to generate two such synthetic references per object file, if the runtime doesn't do enough?

llvm/trunk/lib/Target/X86/X86MCInstLower.cpp

if (Subtarget->isTargetELF()) {
     if (Fn->hasComdat()) {
       Section = OutContext.getELFSection("xray_instr_map", ELF::SHT_PROGBITS,
                                          ELF::SHF_ALLOC | ELF::SHF_GROUP, 0,
                                          Fn->getComdat()->getName());
     } else {
       Section = OutContext.getELFSection("xray_instr_map", ELF::SHT_PROGBITS,
                                          ELF::SHF_ALLOC);
     }
+
+    // Before we switch over, we force a reference to a label inside the
+    // xray_instr_map section. Since EmitXRayTable() is always called just
+    // before the function's end, we assume that this is happening after the
+    // last return instruction.
+    //
+    // We then align the reference to 16 byte boundaries, which we determined
+    // experimentally to be beneficial to avoid causing decoder stalls.
+    MCSymbol *Tmp = OutContext.createTempSymbol("xray_synthetic_", true);
+    OutStreamer->EmitCodeAlignment(16);
+    OutStreamer->EmitSymbolValue(Tmp, 8, false);
     OutStreamer->SwitchSection(Section);
+    OutStreamer->EmitLabel(Tmp);
     for (const auto &Sled : Sleds) {
       OutStreamer->EmitSymbolValue(Sled.Sled, 8);
       OutStreamer->EmitSymbolValue(CurrentFnSym, 8);
       auto Kind = static_cast<uint8_t>(Sled.Kind);
       OutStreamer->EmitBytes(
           StringRef(reinterpret_cast<const char *>(&Kind), 1));
       OutStreamer->EmitBytes(
           StringRef(reinterpret_cast<const char *>(&Sled.AlwaysInstrument), 1));

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

 ; RUN: llc -filetype=asm -o - -mtriple=x86_64-unknown-linux-gnu < %s | FileCheck %s
 
 define i32 @foo() nounwind noinline uwtable "function-instrument"="xray-always" {
 ; CHECK:       .p2align 1, 0x90
 ; CHECK-LABEL: Lxray_sled_0:
 ; CHECK-NEXT:  .ascii "\353\t"
 ; CHECK-NEXT:  nopw 512(%rax,%rax)
 ; CHECK-LABEL: Ltmp0:
   ret i32 0
 ; CHECK:       .p2align 1, 0x90
 ; CHECK-LABEL: Lxray_sled_1:
 ; CHECK-NEXT:  retq
 ; CHECK-NEXT:  nopw %cs:512(%rax,%rax)
 }
+; CHECK:       .p2align 4, 0x90
+; CHECK-NEXT:  .quad .Lxray_synthetic_0
+; CHECK-NEXT:  .section xray_instr_map,{{.*}}
+; CHECK-LABEL: Lxray_synthetic_0:
+; CHECK:       .quad .Lxray_sled_0
+; CHECK:       .quad .Lxray_sled_1