[llvm-objdump] Teach -d about AArch64 mapping symbols

[davide]

Summary

AArch64 uses $d* and $x* to interleave between text and data.
llvm-objdump didn't know about this so it ended up printing garbage. This is the initial support to to teach llvm-objdump how to behave correctly in this situation.
I think llvm-objdump needs a lot of love. For now, this is a missing functionality that blocks my lld AArch64 testing, so I implemented it in the most straightforward way. Comments are welcome, and appreciated.

[Bigcheese]

I'd rather we not loop over all the mapping symbols each time. Or at least do a binary search. Other than that it looks fine.

[davide]

Reworked as per Rafael request.

[• rafael]

Much better, but I think it can still be improved.

auto DAI =
          std::lower_bound(Symbols.begin(), Symbols.end(), Index, SymCmp);

Index is >= Start, so you don't need to search past Symbols[si], no?

In fact, why do you need to search at all? When you are about to print a symbol from Start to End, you can check if the range for that symbol is data or not and you can do that by just maintaining a single "InCode" variable that is updated every time you pass a $x or $d.

[davide]

Updated. Thank you for your help, Rafael!
This also now matches better with GNU objdump does, not showing mapping symbols.

[davide]

git-clang-format.

[• rafael]

LGTM

[jmolloy]

tools/llvm-objdump/llvm-objdump.cpp

988

As this is a target-independent file, shouldn't we also make sure $a and $t (for AArch32) are covered here? Are you going to do this in another patch?

[davide]

tools/llvm-objdump/llvm-objdump.cpp

988

Hi James, I think I can take of this separately. Please file a PR and assign to me so I won't forget.

Thanks,

Davide

test/tools/llvm-objdump/AArch64/elf-aarch64-mapping-symbols.test

 # CHECK: _start:
 # CHECK:        0:       21 00 00 10     adr     x1, #4
 # CHECK: msg:
 # CHECK:        4:       48 65 6c 6c     .word
 # CHECK:        8:       6f 2c 20 77     .word
 # CHECK:        c:       6f 72 6c 64     .word
 # CHECK:       10:       0a 00   .short
 # CHECK: Disassembly of section .myothersection:
-# CHECK: $x.2:
 # CHECK:        0:       01 00 00 90     adrp    x1, #0
 # CHECK: mystr:
 # CHECK:        4:       62 6c 61 68     .word
 # CHECK:        8:       00      .byte

tools/llvm-objdump/llvm-objdump.cpp

 
     uint64_t SectionAddr = Section.getAddress();
     uint64_t SectSize = Section.getSize();
     if (!SectSize)
       continue;
 
     // Make a list of all the symbols in this section.
     std::vector<std::pair<uint64_t, StringRef>> Symbols;
-    std::vector<uint64_t> DataMappingSymsAddr;
-    std::vector<uint64_t> TextMappingSymsAddr;
     for (const SymbolRef &Symbol : Obj->symbols()) {
       if (Section.containsSymbol(Symbol)) {
         ErrorOr<uint64_t> AddressOrErr = Symbol.getAddress();
         error(AddressOrErr.getError());
         uint64_t Address = *AddressOrErr;
         Address -= SectionAddr;
         if (Address >= SectSize)
           continue;
 
         ErrorOr<StringRef> Name = Symbol.getName();
         error(Name.getError());
         Symbols.push_back(std::make_pair(Address, *Name));
-        if (Obj->isELF() && Obj->getArch() == Triple::aarch64) {
-          if (Name->startswith("$d"))
-            DataMappingSymsAddr.push_back(Address);
-          if (Name->startswith("$x"))
-            TextMappingSymsAddr.push_back(Address);
-        }
       }
     }
 
     // Sort the symbols by address, just in case they didn't come in that way.
-    array_pod_sort(Symbols.begin(), Symbols.end());
-    std::sort(DataMappingSymsAddr.begin(), DataMappingSymsAddr.end());
-    std::sort(TextMappingSymsAddr.begin(), TextMappingSymsAddr.end());
+    // Also make sure that mapping symbols come first.
+    std::sort(Symbols.begin(), Symbols.end(), [](
+      std::pair<uint64_t, StringRef> A, std::pair<uint64_t, StringRef> B) {
+        return (A.first < B.first) || (A.first == B.first && A.second.startswith("$"));});
 
     // Make a list of all the relocations for this section.
     std::vector<RelocationRef> Rels;
     if (InlineRelocs) {
       for (const SectionRef &RelocSec : SectionRelocMap[Section]) {
         for (const RelocationRef &Reloc : RelocSec.relocations()) {
           Rels.push_back(Reloc);
         }

 
     StringRef BytesStr;
     error(Section.getContents(BytesStr));
     ArrayRef<uint8_t> Bytes(reinterpret_cast<const uint8_t *>(BytesStr.data()),
                             BytesStr.size());
 
     uint64_t Size;
     uint64_t Index;
+    bool DataInCode = false;
 
     std::vector<RelocationRef>::const_iterator rel_cur = Rels.begin();
     std::vector<RelocationRef>::const_iterator rel_end = Rels.end();
     // Disassemble symbol by symbol.
     for (unsigned si = 0, se = Symbols.size(); si != se; ++si) {
 
+      // Mapping symbols.
+      if (Symbols[si].second.startswith("$d")) {
+        DataInCode = true;
+        continue;
+      }
+      if (Symbols[si].second.startswith("$x")) {
+        DataInCode = false;
+        continue;
+      }
+
       uint64_t Start = Symbols[si].first;
       // The end is either the section end or the beginning of the next symbol.
       uint64_t End = (si == se - 1) ? SectSize : Symbols[si + 1].first;
       // If this symbol has the same address as the next symbol, then skip it.
       if (Start == End)
         continue;
 
       outs() << '\n' << Symbols[si].second << ":\n";
 
 #ifndef NDEBUG
       raw_ostream &DebugOut = DebugFlag ? dbgs() : nulls();
 #else
       raw_ostream &DebugOut = nulls();
 #endif
 
       for (Index = Start; Index < End; Index += Size) {
         MCInst Inst;
 
         // AArch64 ELF binaries can interleave data and text in the
         // same section. We rely on the markers introduced to
         // understand what we need to dump.
-        if (Obj->isELF() && Obj->getArch() == Triple::aarch64) {
-          uint64_t Stride = 0;
-
-          auto DAI = std::lower_bound(DataMappingSymsAddr.begin(),
-                                      DataMappingSymsAddr.end(), Index);
-          if (DAI != DataMappingSymsAddr.end() && *DAI == Index) {
-            // Switch to data.
-            while (Index < End) {
-              outs() << format("%8" PRIx64 ":", SectionAddr + Index);
-              outs() << "\t";
-              if (Index + 4 <= End) {
-                Stride = 4;
-                dumpBytes(Bytes.slice(Index, 4), outs());
-                outs() << "\t.word";
-              } else if (Index + 2 <= End) {
-                Stride = 2;
-                dumpBytes(Bytes.slice(Index, 2), outs());
-                outs() << "\t.short";
-              } else {
-                Stride = 1;
-                dumpBytes(Bytes.slice(Index, 1), outs());
-                outs() << "\t.byte";
-              }
-              Index += Stride;
-              outs() << "\n";
-              auto TAI = std::lower_bound(TextMappingSymsAddr.begin(),
-                                          TextMappingSymsAddr.end(), Index);
-              if (TAI != TextMappingSymsAddr.end() && *TAI == Index)
-                break;
-            }
+        if (DataInCode) {
+          outs() << format("%8" PRIx64 ":", SectionAddr + Index);
+          outs() << "\t";
+          if (Index + 4 <= End) {
+            Size = 4;
+            dumpBytes(Bytes.slice(Index, 4), outs());
+            outs() << "\t.word";
+          } else if (Index + 2 <= End) {
+            Size = 2;
+            dumpBytes(Bytes.slice(Index, 2), outs());
+            outs() << "\t.short";
+          } else {
+            Size = 1;
+            dumpBytes(Bytes.slice(Index, 1), outs());
+            outs() << "\t.byte";
           }
+          outs() << "\n";
+          continue;
         }
 
-        if (Index >= End)
-          break;
-
         if (DisAsm->getInstruction(Inst, Size, Bytes.slice(Index),
                                    SectionAddr + Index, DebugOut,
                                    CommentStream)) {
           PIP.printInst(*IP, &Inst,
                         Bytes.slice(Index, Size),
                         SectionAddr + Index, outs(), "", *STI);
           outs() << CommentStream.str();
           Comments.clear();