[xray] Add llvm-xray extract support for 32 bit ARM

[ianlevesque]

Summary

XRay works on 32-bit ARM but extract didn't support it.

See also another previous attempt in D77858.

[MaskRay]

llvm/test/tools/llvm-xray/ARM/Inputs/elf32-pic.yaml

139 ↗

(On Diff #264798)

Content is not significant. Use Size:

llvm/test/tools/llvm-xray/ARM/extract-instrmap.test

5

You can place the content of elf32-pic.yaml in this file. Then you can delete elf32-pic.yaml

[ianlevesque]

Simplify and combine tests per @MaskRay

[MaskRay]

llvm/lib/XRay/InstrumentationMap.cpp

130

R_ARM_RELATIVE does not need this.

138

Perhaps we can special case R_ARM_* for now. This can avoid a getOptionalAddend change.

I am not sure getOptionalAddend will have more use cases.

llvm/test/tools/llvm-xray/ARM/extract-instrmap.test

74

Please check whether more sections can be dropped. For example, .hash is not read. Many tags in .dynamic are probably not needed. xray_fn_idx may not be needed.

Sorry for letting you do more, but I hope we can get a minimal test case which can be used as a template to improve tests for other architectures.

[ianlevesque]

I'll keep looking at this but @MaskRay if you could point me to something in LLVM that checks whether an addend is present that I can actually access from this module that would be helpful. I added getOptionalAddend as a last resort :(

llvm/lib/XRay/InstrumentationMap.cpp

130

This branch still gets hit for the absolute relocations in the xray_fn_idx section, and with the Expected<> return it asserts in debug mode. I could not figure out a way to detect if the Addend was present from this point in the code, because all of the methods you would use to do so are protected visibilty on the ELF* classes. Can you make a more specific suggestion?

138

I am ok with this, but I'm not sure what the check would look like - the same visibility problem from my other comment applies. Can you suggest something?

llvm/test/tools/llvm-xray/ARM/extract-instrmap.test

74

Sure I can delete even more - I would not cut away xray_fn_idx because it forces that other branch to be taken right now, which exposes another assert().

[ianlevesque]

Remove even more sections from the test object. Special case ARM entirely instead of extending the ELFObject classes.

[MaskRay]

Oh, do we still have R_ARM_ABS32 relocating xray_instr_map? Aren't them all converted to R_ARM_RELATIVE?

llvm/lib/XRay/InstrumentationMap.cpp

120

If we only have R_ARM_RELATIVE, you can drop the if branch (always false).

[ianlevesque]

llvm/lib/XRay/InstrumentationMap.cpp

120

It's relocating the xray_fn_idx with absolute relocations still - I think we can just ignore those here but I'd like to do away with them entirely to improve load time.

[ianlevesque]

Remove unnecessary fallback branch.

[ianlevesque]

@MaskRay I think this should cover it now. I cut away all the sections I could from the test and eliminated the unused branch and all the ELFObject modifications.

[ianlevesque]

Can I get a re-review on this?

[MaskRay]

LG, but please remove unneeded re-formatting in this patch.

llvm/lib/XRay/InstrumentationMap.cpp

100

This re-formatting is not needed.

265

The formatting is not needed.

You can ignore clang-format diagnostic for existing problems.

[thakis]

This broke check-llvm on virtually all bots on llvm's buildbot for 3h now. Please fix or revert if it takes a while.

[MaskRay]

In D80185#2061994, @thakis wrote:

This broke check-llvm on virtually all bots on llvm's buildbot for 3h now. Please fix or revert if it takes a while.

Thanks for reporting. Fixed by 59ba12994c07d03ac3b628c05c45a834774f9b17

[ianlevesque]

Ugh, cut a bit too deep when removing formatting. Thanks for fixing forward on the test @MaskRay

llvm/lib/XRay/InstrumentationMap.cpp

if (I != FunctionAddresses.end())
     return I->second;
   return None;
 }
 
 using RelocMap = DenseMap<uint64_t, uint64_t>;
 
 static Error
 loadObj(StringRef Filename, object::OwningBinary<object::ObjectFile> &ObjFile,
-          InstrumentationMap::SledContainer &Sleds,
-          InstrumentationMap::FunctionAddressMap &FunctionAddresses,
-          InstrumentationMap::FunctionAddressReverseMap &FunctionIds) {
   InstrumentationMap Map;
 
   // Find the section named "xray_instr_map".
   if ((!ObjFile.getBinary()->isELF() && !ObjFile.getBinary()->isMachO()) ||
       !(ObjFile.getBinary()->getArch() == Triple::x86_64 ||
         ObjFile.getBinary()->getArch() == Triple::ppc64le ||
+        ObjFile.getBinary()->getArch() == Triple::arm ||
         ObjFile.getBinary()->getArch() == Triple::aarch64))
     return make_error<StringError>(
-        "File format not supported (only does ELF and Mach-O little endian 64-bit).",
+        Twine("File format not supported.  Supports: "
+              "AArch64/ARM/ppc64le/x86-64."),
         std::make_error_code(std::errc::not_supported));
 
   StringRef Contents = "";
   const auto &Sections = ObjFile.getBinary()->sections();
   uint64_t Address = 0;
   auto I = llvm::find_if(Sections, [&](object::SectionRef Section) {
     Expected<StringRef> NameOrErr = Section.getName();
     if (NameOrErr) {
loadObj(StringRef Filename, object::OwningBinary<object::ObjectFile> &ObjFile,
   else
     return E.takeError();
 
   RelocMap Relocs;
   if (ObjFile.getBinary()->isELF()) {
     uint32_t RelativeRelocation = [](object::ObjectFile *ObjFile) {
       if (const auto *ELFObj = dyn_cast<object::ELF32LEObjectFile>(ObjFile))
         return ELFObj->getELFFile()->getRelativeRelocationType();
-      else if (const auto *ELFObj = dyn_cast<object::ELF32BEObjectFile>(ObjFile))
+      else if (const auto *ELFObj =
+                   dyn_cast<object::ELF32BEObjectFile>(ObjFile))
         return ELFObj->getELFFile()->getRelativeRelocationType();
-      else if (const auto *ELFObj = dyn_cast<object::ELF64LEObjectFile>(ObjFile))
+      else if (const auto *ELFObj =
+                   dyn_cast<object::ELF64LEObjectFile>(ObjFile))
         return ELFObj->getELFFile()->getRelativeRelocationType();
-      else if (const auto *ELFObj = dyn_cast<object::ELF64BEObjectFile>(ObjFile))
+      else if (const auto *ELFObj =
+                   dyn_cast<object::ELF64BEObjectFile>(ObjFile))
         return ELFObj->getELFFile()->getRelativeRelocationType();
       else
         return static_cast<uint32_t>(0);
     }(ObjFile.getBinary());
 
     bool (*SupportsRelocation)(uint64_t);
     object::RelocationResolver Resolver;
     std::tie(SupportsRelocation, Resolver) =
         object::getRelocationResolver(*ObjFile.getBinary());
 
     for (const object::SectionRef &Section : Sections) {
       for (const object::RelocationRef &Reloc : Section.relocations()) {
+        if (ObjFile.getBinary()->getArch() == Triple::arm) {
           if (SupportsRelocation && SupportsRelocation(Reloc.getType())) {
+            Expected<uint64_t> ValueOrErr = Reloc.getSymbol()->getValue();
+            if (!ValueOrErr)
+              return ValueOrErr.takeError();
+            Relocs.insert({Reloc.getOffset(), Resolver(Reloc, *ValueOrErr, 0)});
+          } else {
+            Relocs.insert({Reloc.getOffset(), 0});
+          }
+        } else if (SupportsRelocation && SupportsRelocation(Reloc.getType())) {
           auto AddendOrErr = object::ELFRelocationRef(Reloc).getAddend();
           auto A = AddendOrErr ? *AddendOrErr : 0;
           Expected<uint64_t> ValueOrErr = Reloc.getSymbol()->getValue();
           if (!ValueOrErr)
             // TODO: Test this error.
             return ValueOrErr.takeError();
           Relocs.insert({Reloc.getOffset(), Resolver(Reloc, *ValueOrErr, A)});
         } else if (Reloc.getType() == RelativeRelocation) {
           if (auto AddendOrErr = object::ELFRelocationRef(Reloc).getAddend())
             Relocs.insert({Reloc.getOffset(), *AddendOrErr});
         }
       }
     }
   }
 
   // Copy the instrumentation map data into the Sleds data structure.
   auto C = Contents.bytes_begin();
-  static constexpr size_t ELF64SledEntrySize = 32;
+  bool Is32Bit = ObjFile.getBinary()->makeTriple().isArch32Bit();
+  size_t ELFSledEntrySize = Is32Bit ? 16 : 32;
 
-  if ((C - Contents.bytes_end()) % ELF64SledEntrySize != 0)
+  if ((C - Contents.bytes_end()) % ELFSledEntrySize != 0)
     return make_error<StringError>(
         Twine("Instrumentation map entries not evenly divisible by size of "
-              "an XRay sled entry in ELF64."),
+              "an XRay sled entry."),
         std::make_error_code(std::errc::executable_format_error));
 
   auto RelocateOrElse = [&](uint64_t Offset, uint64_t Address) {
     if (!Address) {
       uint64_t A = I->getAddress() + C - Contents.bytes_begin() + Offset;
       RelocMap::const_iterator R = Relocs.find(A);
       if (R != Relocs.end())
         return R->second;
     }
     return Address;
   };
 
-  const int WordSize = 8;
+  const int WordSize = Is32Bit ? 4 : 8;
   int32_t FuncId = 1;
   uint64_t CurFn = 0;
-  for (; C != Contents.bytes_end(); C += ELF64SledEntrySize) {
+  for (; C != Contents.bytes_end(); C += ELFSledEntrySize) {
     DataExtractor Extractor(
-        StringRef(reinterpret_cast<const char *>(C), ELF64SledEntrySize), true,
+        StringRef(reinterpret_cast<const char *>(C), ELFSledEntrySize), true,
         8);
     Sleds.push_back({});
     auto &Entry = Sleds.back();
     uint64_t OffsetPtr = 0;
     uint64_t AddrOff = OffsetPtr;
+    if (Is32Bit)
+      Entry.Address = RelocateOrElse(AddrOff, Extractor.getU32(&OffsetPtr));
+    else
       Entry.Address = RelocateOrElse(AddrOff, Extractor.getU64(&OffsetPtr));
     uint64_t FuncOff = OffsetPtr;
+    if (Is32Bit)
+      Entry.Function = RelocateOrElse(FuncOff, Extractor.getU32(&OffsetPtr));
+    else
       Entry.Function = RelocateOrElse(FuncOff, Extractor.getU64(&OffsetPtr));
     auto Kind = Extractor.getU8(&OffsetPtr);
     static constexpr SledEntry::FunctionKinds Kinds[] = {
         SledEntry::FunctionKinds::ENTRY, SledEntry::FunctionKinds::EXIT,
         SledEntry::FunctionKinds::TAIL,
         SledEntry::FunctionKinds::LOG_ARGS_ENTER,
         SledEntry::FunctionKinds::CUSTOM_EVENT};
     if (Kind >= sizeof(Kinds))
       return errorCodeToError(
llvm::xray::loadInstrumentationMap(StringRef Filename) {
   // work, we treat it as YAML.
   // FIXME: Extend to support non-ELF and non-x86_64 binaries.
 
   InstrumentationMap Map;
   auto ObjectFileOrError = object::ObjectFile::createObjectFile(Filename);
   if (!ObjectFileOrError) {
     auto E = ObjectFileOrError.takeError();
     // We try to load it as YAML if the ELF load didn't work.
-    Expected<sys::fs::file_t> FdOrErr = sys::fs::openNativeFileForRead(Filename);
+    Expected<sys::fs::file_t> FdOrErr =
+        sys::fs::openNativeFileForRead(Filename);
     if (!FdOrErr) {
       // Report the ELF load error if YAML failed.
       consumeError(FdOrErr.takeError());
       return std::move(E);
     }
 
     uint64_t FileSize;
     if (sys::fs::file_size(Filename, FileSize))
       return std::move(E);
 
     // If the file is empty, we return the original error.
     if (FileSize == 0)
       return std::move(E);
 
     // From this point on the errors will be only for the YAML parts, so we
     // consume the errors at this point.
     consumeError(std::move(E));
     if (auto E = loadYAML(*FdOrErr, FileSize, Filename, Map.Sleds,
                           Map.FunctionAddresses, Map.FunctionIds))
       return std::move(E);
   } else if (auto E = loadObj(Filename, *ObjectFileOrError, Map.Sleds,
-                                Map.FunctionAddresses, Map.FunctionIds)) {
     return std::move(E);
   }
   return Map;
 }

llvm/test/tools/llvm-xray/ARM/extract-instrmap.test

+## This test makes sure we can extract the instrumentation map from an
+## XRay-instrumented PIE file.
+
+## Generated from the following source:
+## __attribute__((xray_always_instrument)) void foo() {}
+## __attribute__((xray_always_instrument)) void bar() {}
+## __attribute__((xray_always_instrument)) void jar() {}
+## Built with the following arguments:
+## -target armv7a-linux-androideabi -nostdlib -fxray-instrument -fPIC -shared
+
+--- !ELF
+FileHeader:
+  Class:           ELFCLASS32
+  Data:            ELFDATA2LSB
+  Type:            ET_DYN
+  Machine:         EM_ARM
+  Flags:           [ EF_ARM_SOFT_FLOAT, EF_ARM_EABI_VER5 ]
+  Entry:           0x00000000000012B0
+ProgramHeaders:
+  - Type:            PT_LOAD
+    Flags:           [ PF_R ]
+    Sections:
+      - Section:         .rel.dyn
+    Align:           0x0000000000001000
+  - Type:            PT_LOAD
+    Flags:           [ PF_X, PF_R ]
+    Sections:
+      - Section:         .text
+    VAddr:           0x00000000000012B0
+    Align:           0x0000000000001000
+  - Type:            PT_LOAD
+    Flags:           [ PF_W, PF_R ]
+    Sections:
+      - Section:         xray_instr_map
+      - Section:         xray_fn_idx
+    VAddr:           0x00000000000033CC
+    Align:           0x0000000000001000
+Sections:
+  - Name:            .rel.dyn
+    Type:            SHT_REL
+    Flags:           [ SHF_ALLOC ]
+    Address:         0x0000000000000200
+    Link:            .dynsym
+    AddressAlign:    0x0000000000000004
+    Relocations:
+      - Offset:          0x00000000000033CC
+        Type:            R_ARM_RELATIVE
+      - Offset:          0x00000000000033DC
+        Type:            R_ARM_RELATIVE
+      - Offset:          0x00000000000033EC
+        Type:            R_ARM_RELATIVE
+      - Offset:          0x00000000000033FC
+        Type:            R_ARM_RELATIVE
+      - Offset:          0x000000000000340C
+        Type:            R_ARM_RELATIVE
+      - Offset:          0x000000000000341C
+        Type:            R_ARM_RELATIVE
+      - Offset:          0x0000000000003430
+        Type:            R_ARM_RELATIVE
+      - Offset:          0x0000000000003434
+        Type:            R_ARM_RELATIVE
+      - Offset:          0x0000000000003438
+        Type:            R_ARM_RELATIVE
+      - Offset:          0x000000000000343C
+        Type:            R_ARM_RELATIVE
+      - Offset:          0x0000000000003440
+        Type:            R_ARM_RELATIVE
+      - Offset:          0x0000000000003444
+        Type:            R_ARM_RELATIVE
+      - Offset:          0x00000000000033F0
+        Symbol:          _Z3barv
+        Type:            R_ARM_ABS32
+      - Offset:          0x0000000000003400
+        Symbol:          _Z3barv
+        Type:            R_ARM_ABS32
+      - Offset:          0x00000000000033D0
+        Symbol:          _Z3foov
+        Type:            R_ARM_ABS32
+      - Offset:          0x00000000000033E0
+        Symbol:          _Z3foov
+        Type:            R_ARM_ABS32
+      - Offset:          0x0000000000003410
+        Symbol:          _Z3jarv
+        Type:            R_ARM_ABS32
+      - Offset:          0x0000000000003420
+        Symbol:          _Z3jarv
+        Type:            R_ARM_ABS32
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:         0x00000000000012B0
+    AddressAlign:    0x0000000000000004
+    Size:            180
+  - Name:            xray_instr_map
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_WRITE, SHF_ALLOC, SHF_LINK_ORDER ]
+    Address:         0x00000000000033CC
+    Link:            .text
+    AddressAlign:    0x0000000000000001
+    Content:         B0120000000000000001000000000000CC120000000000000101000000000000EC120000000000000001000000000000081300000000000001010000000000002813000000000000000100000000000044130000000000000101000000000000
+  - Name:            xray_fn_idx
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_WRITE, SHF_ALLOC, SHF_LINK_ORDER ]
+    Address:         0x0000000000003430
+    Link:            .text
+    AddressAlign:    0x0000000000000008
+    Content:         CC330000EC330000EC3300000C3400000C3400002C340000
+DynamicSymbols:
+  - Name:            _Z3barv
+    Type:            STT_FUNC
+    Section:         .text
+    Binding:         STB_GLOBAL
+    Value:           0x00000000000012EC
+    Size:            0x000000000000003C
+  - Name:            _Z3foov
+    Type:            STT_FUNC
+    Section:         .text
+    Binding:         STB_GLOBAL
+    Value:           0x00000000000012B0
+    Size:            0x000000000000003C
+  - Name:            _Z3jarv
+    Type:            STT_FUNC
+    Section:         .text
+    Binding:         STB_GLOBAL
+    Value:           0x0000000000001328
+    Size:            0x000000000000003C
+...
+
+# RUN: yaml2obj %s -o %t.so
+# RUN: llvm-xray extract %t.so -s | FileCheck %s
+
+# CHECK:      ---
+# CHECK-NEXT: - { id: 1, address: 0x00000000000012B0, function: 0x00000000000012B0, kind: function-enter, always-instrument: true, function-name: 'foo()' }
+# CHECK-NEXT: - { id: 1, address: 0x00000000000012CC, function: 0x00000000000012B0, kind: function-exit, always-instrument: true, function-name: 'foo()' }
+# CHECK-NEXT: - { id: 2, address: 0x00000000000012EC, function: 0x00000000000012EC, kind: function-enter, always-instrument: true, function-name: 'bar()' }
+# CHECK-NEXT: - { id: 2, address: 0x0000000000001308, function: 0x00000000000012EC, kind: function-exit, always-instrument: true, function-name: 'bar()' }
+# CHECK-NEXT: - { id: 3, address: 0x0000000000001328, function: 0x0000000000001328, kind: function-enter, always-instrument: true, function-name: 'jar()' }
+# CHECK-NEXT: - { id: 3, address: 0x0000000000001344, function: 0x0000000000001328, kind: function-exit, always-instrument: true, function-name: 'jar()' }
+# CHECK-NEXT: ...
+
+# RUN: llvm-xray extract -s --no-demangle %t.so | FileCheck --check-prefix=MANGLED %s
+
+# MANGLED:      ---
+# MANGLED-NEXT: - { id: 1, address: 0x00000000000012B0, function: 0x00000000000012B0, kind: function-enter, always-instrument: true, function-name: _Z3foov }
+# MANGLED-NEXT: - { id: 1, address: 0x00000000000012CC, function: 0x00000000000012B0, kind: function-exit, always-instrument: true, function-name: _Z3foov }
+# MANGLED-NEXT: - { id: 2, address: 0x00000000000012EC, function: 0x00000000000012EC, kind: function-enter, always-instrument: true, function-name: _Z3barv }
+# MANGLED-NEXT: - { id: 2, address: 0x0000000000001308, function: 0x00000000000012EC, kind: function-exit, always-instrument: true, function-name: _Z3barv }
+# MANGLED-NEXT: - { id: 3, address: 0x0000000000001328, function: 0x0000000000001328, kind: function-enter, always-instrument: true, function-name: _Z3jarv }
+# MANGLED-NEXT: - { id: 3, address: 0x0000000000001344, function: 0x0000000000001328, kind: function-exit, always-instrument: true, function-name: _Z3jarv }
+# MANGLED-NEXT: ...

llvm/test/tools/llvm-xray/X86/bad-instrmap-sizes.txt

 ; RUN: not llvm-xray extract %S/Inputs/elf64-badentrysizes.bin 2>&1 | FileCheck %s
 ; CHECK: llvm-xray: Cannot extract instrumentation map from '{{.*}}elf64-badentrysizes.bin'.
-; CHECK-NEXT: Instrumentation map entries not evenly divisible by size of an XRay sled entry in ELF64.
+; CHECK-NEXT: Instrumentation map entries not evenly divisible by size of an XRay sled entry.

llvm/test/tools/llvm-xray/X86/unsupported-elf32.txt

 ; RUN: not llvm-xray extract %S/Inputs/elf32-noxray.bin 2>&1 | FileCheck %s
 ; CHECK: llvm-xray: Cannot extract instrumentation map from '{{.*}}elf32-noxray.bin'.
-; CHECK-NEXT: File format not supported (only does ELF and Mach-O little endian 64-bit).
+; CHECK-NEXT: File format not supported.  Supports: AArch64/ARM/ppc64le/x86-64.