[MC][AMDGPU][llvm-objdump] Synthesized local labels in disassembly

[tpr]

Summary

1. Add an accessor function to MCSymbolizer to retrieve addresses referenced by a symbolizable operand, but not resolved to a symbol. That way, the caller can synthesize labels at those addresses and then retry disassembling the section.

2. Implement that in AMDGPU -- a failed symbol lookup results in the address being added to a vector returned by the new function.

3. Use that in llvm-objdump when using MCSymbolizer (which only happens on AMDGPU).

Change-Id: I19087c3bbfece64bad5a56ee88bcc9110d83989e

[rochauha]

llvm/tools/llvm-objdump/llvm-objdump.cpp

1199

I think it would help if this logic is moved to a separate function in the tool, that is called conditionally. It would make the main loop shorter and perhaps also easier to read?

[rochauha]

llvm/tools/llvm-objdump/llvm-objdump.cpp

1200–1237

I meant this entire snippet in my previous comment, apologies for causing confusion.

[rochauha]

The test in this patch runs with --symbolize-operands.

But we are symbolizing even when --symbolize-operands is not passed to llvm-objdump. Though this doesn't do any harm, I'd lean towards symbolizing only when the switch is passed.

[tpr]

V2: Extract AMDGPU-add-symbolizer code and my new logic into a separate
function. Only enable new functionality with SymbolizeOperands to bring
it into line with X86 (and stop existing tests failing).

[tpr]

llvm/tools/llvm-objdump/llvm-objdump.cpp

1199

I extracted the whole AMDGPU adding symbolizer stuff, including my new code, into a new func. Is that ok?

[rochauha]

LGTM :)

[foad]

llvm/tools/llvm-objdump/llvm-objdump.cpp

1010–1012

Can't you populate this on construction? std::vector<uint64_t> LabelAddrs(LabelAddrsRef.begin(), LabelAddrsRef.end());

llvm/include/llvm/MC/MCDisassembler/MCSymbolizer.h

 // to symbolize instructions decoded from an object, that is, transform their
 // immediate operands to MCExprs.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_MC_MCDISASSEMBLER_MCSYMBOLIZER_H
 #define LLVM_MC_MCDISASSEMBLER_MCSYMBOLIZER_H
 
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/MC/MCDisassembler/MCRelocationInfo.h"
 #include <algorithm>
 #include <cstdint>
 #include <memory>
 
 namespace llvm {
 
 class MCContext;
virtual bool tryAddingSymbolicOperand(MCInst &Inst, raw_ostream &cStream,
                                         uint64_t InstSize) = 0;
 
   /// Try to add a comment on the PC-relative load.
   /// For instance, in Mach-O, this is used to add annotations to instructions
   /// that use C string literals, as found in __cstring.
   virtual void tryAddingPcLoadReferenceComment(raw_ostream &cStream,
                                                int64_t Value,
                                                uint64_t Address) = 0;
+
+  /// Get the MCSymbolizer's list of addresses that were referenced by
+  /// symbolizable operands but not resolved to a symbol. The caller (some
+  /// code that is disassembling a section or other chunk of code) would
+  /// typically create a synthetic label at each address and add them to its
+  /// list of symbols in the section, before creating a new MCSymbolizer with
+  /// the enhanced symbol list and retrying disassembling the section.
+  /// The returned array is unordered and may have duplicates.
+  /// The returned ArrayRef stops being valid on any call to or destruction of
+  /// the MCSymbolizer object.
+  virtual ArrayRef<uint64_t> getReferencedAddresses() const { return {}; }
 };
 
 } // end namespace llvm
 
 #endif // LLVM_MC_MCDISASSEMBLER_MCSYMBOLIZER_H

llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h

 
 //===----------------------------------------------------------------------===//
 // AMDGPUSymbolizer
 //===----------------------------------------------------------------------===//
 
 class AMDGPUSymbolizer : public MCSymbolizer {
 private:
   void *DisInfo;
+  std::vector<uint64_t> ReferencedAddresses;
 
 public:
   AMDGPUSymbolizer(MCContext &Ctx, std::unique_ptr<MCRelocationInfo> &&RelInfo,
                    void *disInfo)
                    : MCSymbolizer(Ctx, std::move(RelInfo)), DisInfo(disInfo) {}
 
   bool tryAddingSymbolicOperand(MCInst &Inst, raw_ostream &cStream,
                                 int64_t Value, uint64_t Address,
                                 bool IsBranch, uint64_t Offset,
                                 uint64_t InstSize) override;
 
   void tryAddingPcLoadReferenceComment(raw_ostream &cStream,
                                        int64_t Value,
                                        uint64_t Address) override;
+
+  ArrayRef<uint64_t> getReferencedAddresses() const override {
+    return ReferencedAddresses;
+  }
 };
 
 } // end namespace llvm
 
 #endif // LLVM_LIB_TARGET_AMDGPU_DISASSEMBLER_AMDGPUDISASSEMBLER_H

llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp

return Val.Addr == static_cast<uint64_t>(Value) &&
            Val.Type == ELF::STT_NOTYPE;
   });
   if (Result != Symbols->end()) {
     auto *Sym = Ctx.getOrCreateSymbol(Result->Name);
     const auto *Add = MCSymbolRefExpr::create(Sym, Ctx);
     Inst.addOperand(MCOperand::createExpr(Add));
     return true;
   }
+  // Add to list of referenced addresses, so caller can synthesize a label.
+  ReferencedAddresses.push_back(static_cast<uint64_t>(Value));
   return false;
 }
 
 void AMDGPUSymbolizer::tryAddingPcLoadReferenceComment(raw_ostream &cStream,
                                                        int64_t Value,
                                                        uint64_t Address) {
   llvm_unreachable("unimplemented");
 }

llvm/test/tools/llvm-objdump/AMDGPU/elf-disassemble-symbolize-operands.yaml

+# RUN: yaml2obj %s -o %t
+# RUN: llvm-objdump %t -d --symbolize-operands --no-show-raw-insn --no-leading-addr | \
+# RUN:   FileCheck %s
+
+## Expect to find the branch labels.
+# CHECK: <break_cond_is_arg>:
+# CHECK:     s_branch L1
+# CHECK: <L0>:
+# CHECK:     s_cbranch_execz L2
+# CHECK: <L1>:
+# CHECK:     s_branch L0
+# CHECKL <L2>:
+
+# I created this YAML starting with this LLVM IR:
+#
+#   define void @break_cond_is_arg(i32 %arg, i1 %breakcond) {
+#   entry:
+#     br label %loop
+#   loop:
+#     %tmp23phi = phi i32 [ %tmp23, %endif ], [ 0, %entry ]
+#     %tmp23 = add nuw i32 %tmp23phi, 1
+#     %tmp27 = icmp ult i32 %arg, %tmp23
+#     br i1 %tmp27, label %then, label %endif
+#   then:                                             ; preds = %bb
+#     call void @llvm.amdgcn.raw.buffer.store.f32(float undef, <4 x i32> undef, i32 0, i32 undef, i32 0)
+#     br label %endif
+#   endif:                                             ; preds = %bb28, %bb
+#     br i1 %breakcond, label %loop, label %loopexit
+#   loopexit:
+#     ret void
+#   }
+#   
+#   declare void @llvm.amdgcn.raw.buffer.store.f32(float, <4 x i32>, i32, i32, i32 immarg) #0
+#   
+#   attributes #0 = { nounwind writeonly }
+#
+# I compiled it to a relocatable ELF:
+#
+#   llc -march=amdgcn -mcpu=gfx1030 llvm/a.ll -filetype=obj -o a.elf
+#
+# then converted it to YAML:
+#
+#   obj2yaml a.elf
+#
+# then manually removed the BB0_1 etc local symbols.
+
+--- !ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_AMDGPU
+  Flags:           [ EF_AMDGPU_MACH_AMDGCN_GFX1030 ]
+Sections:
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    AddressAlign:    0x4
+    Content:         00008CBF0000FDBB81020236810385BE800384BE8102847D6AC10689040082BF7E077E88058105817E047E8A080088BF0500887D7E060787070404886A3C87BEF7FF88BF000070E000000104F4FF82BF7E047E880000FDBB1E2080BE
+  - Name:            .AMDGPU.config
+    Type:            SHT_PROGBITS
+    AddressAlign:    0x1
+    Content:         48B80000000000004CB800000000000060B800000000000004000000000000000800000000000000
+  - Name:            .note.GNU-stack
+    Type:            SHT_PROGBITS
+    AddressAlign:    0x1
+  - Name:            .note
+    Type:            SHT_NOTE
+    AddressAlign:    0x4
+    Notes:
+      - Name:            AMD
+        Desc:            616D6467636E2D756E6B6E6F776E2D6C696E75782D676E752D67667831303330
+        Type:            NT_FREEBSD_PROCSTAT_GROUPS
+  - Type:            SectionHeaderTable
+    Sections:
+      - Name:            .strtab
+      - Name:            .shstrtab
+      - Name:            .text
+      - Name:            .AMDGPU.config
+      - Name:            .note.GNU-stack
+      - Name:            .note
+      - Name:            .symtab
+Symbols:
+  - Name:            break_cond_is_arg
+    Type:            STT_FUNC
+    Section:         .text
+    Binding:         STB_GLOBAL
+    Size:            0x5C
+...

llvm/tools/llvm-objdump/llvm-objdump.cpp

if (Disassembled && MIA) {
           !Labels.count(Target))
         Labels[Target] = ("L" + Twine(LabelCount++)).str();
     }
 
     Index += Size;
   }
 }
 
+// Create an MCSymbolizer for the target and add it to the MCDisassembler.
+// This is currently only used on AMDGPU, and assumes the format of the
+// void * argument passed to AMDGPU's createMCSymbolizer.
+static void addSymbolizer(MCContext &Ctx, const Target *Target,
+                          StringRef TripleName, MCDisassembler *DisAsm,
+                          uint64_t SectionAddr, ArrayRef<uint8_t> Bytes,
+                          SectionSymbolsTy &Symbols,
+                          std::vector<std::string *> &SynthesizedLabelNames) {
+
+  std::unique_ptr<MCRelocationInfo> RelInfo(
+      Target->createMCRelocationInfo(TripleName, Ctx));
+  if (!RelInfo)
+    return;
+  std::unique_ptr<MCSymbolizer> Symbolizer(Target->createMCSymbolizer(
+      TripleName, nullptr, nullptr, &Symbols, &Ctx, std::move(RelInfo)));
+  MCSymbolizer *SymbolizerPtr = &*Symbolizer;
+  DisAsm->setSymbolizer(std::move(Symbolizer));
+
+  if (!SymbolizeOperands)
+    return;
+
+  // Synthesize labels referenced by branch instructions by
+  // disassembling, discarding the output, and collecting the referenced
+  // addresses from the symbolizer.
+  for (size_t Index = 0; Index != Bytes.size();) {
+    MCInst Inst;
+    uint64_t Size;
+    DisAsm->getInstruction(Inst, Size, Bytes.slice(Index), SectionAddr + Index,
+                           nulls());
+    if (Size == 0)
+      Size = 1;
+    Index += Size;
+  }
+  ArrayRef<uint64_t> LabelAddrsRef = SymbolizerPtr->getReferencedAddresses();
+  // Copy and sort to remove duplicates.
+  std::vector<uint64_t> LabelAddrs;
+  LabelAddrs.insert(LabelAddrs.end(), LabelAddrsRef.begin(),
+                    LabelAddrsRef.end());
+  llvm::sort(LabelAddrs);
+  LabelAddrs.resize(std::unique(LabelAddrs.begin(), LabelAddrs.end()) -
+                    LabelAddrs.begin());
+  // Add the labels.
+  for (unsigned LabelNum = 0; LabelNum != LabelAddrs.size(); ++LabelNum) {
+    SynthesizedLabelNames.push_back(
+        new std::string((Twine("L") + Twine(LabelNum)).str()));
+    Symbols.push_back(SymbolInfoTy(
+        LabelAddrs[LabelNum], *SynthesizedLabelNames.back(), ELF::STT_NOTYPE));
+  }
+  llvm::stable_sort(Symbols);
+  // Recreate the symbolizer with the new symbols list.
+  RelInfo.reset(Target->createMCRelocationInfo(TripleName, Ctx));
+  Symbolizer.reset(Target->createMCSymbolizer(
+      TripleName, nullptr, nullptr, &Symbols, &Ctx, std::move(RelInfo)));
+  DisAsm->setSymbolizer(std::move(Symbolizer));
+}
+
 static StringRef getSegmentName(const MachOObjectFile *MachO,
                                 const SectionRef &Section) {
   if (MachO) {
     DataRefImpl DR = Section.getRawDataRefImpl();
     StringRef SegmentName = MachO->getSectionFinalSegmentName(DR);
     return SegmentName;
   }
   return "";
if (hasMappingSymbols(Obj)) {
           MappingSymbols.emplace_back(Address - SectionAddr, 'a');
         if (Name.startswith("$t"))
           MappingSymbols.emplace_back(Address - SectionAddr, 't');
       }
     }
 
     llvm::sort(MappingSymbols);
 
+    ArrayRef<uint8_t> Bytes = arrayRefFromStringRef(
+        unwrapOrError(Section.getContents(), Obj->getFileName()));
+
+    std::vector<std::string *> SynthesizedLabelNames;
     if (Obj->isELF() && Obj->getArch() == Triple::amdgcn) {
       // AMDGPU disassembler uses symbolizer for printing labels
-      std::unique_ptr<MCRelocationInfo> RelInfo(
-        TheTarget->createMCRelocationInfo(TripleName, Ctx));
-      if (RelInfo) {
-        std::unique_ptr<MCSymbolizer> Symbolizer(
-          TheTarget->createMCSymbolizer(
-            TripleName, nullptr, nullptr, &Symbols, &Ctx, std::move(RelInfo)));
-        DisAsm->setSymbolizer(std::move(Symbolizer));
-      }
+      addSymbolizer(Ctx, TheTarget, TripleName, DisAsm, SectionAddr, Bytes,
+                    Symbols, SynthesizedLabelNames);
     }
 
     StringRef SegmentName = getSegmentName(MachO, Section);
     StringRef SectionName = unwrapOrError(Section.getName(), Obj->getFileName());
     // If the section has no symbol at the start, just insert a dummy one.
     if (Symbols.empty() || Symbols[0].Addr != 0) {
       Symbols.insert(Symbols.begin(),
                      createDummySymbolInfo(Obj, SectionAddr, SectionName,
                                            Section.isText() ? ELF::STT_FUNC
                                                             : ELF::STT_OBJECT));
     }
 
     SmallString<40> Comments;
     raw_svector_ostream CommentStream(Comments);
 
-    ArrayRef<uint8_t> Bytes = arrayRefFromStringRef(
-        unwrapOrError(Section.getContents(), Obj->getFileName()));
-
     uint64_t VMAAdjustment = 0;
     if (shouldAdjustVA(Section))
       VMAAdjustment = AdjustVMA;
 
     uint64_t Size;
     uint64_t Index;
     bool PrintedSection = false;
     std::vector<RelocationRef> Rels = RelocMap[Section];
     std::vector<RelocationRef>::const_iterator RelCur = Rels.begin();
     std::vector<RelocationRef>::const_iterator RelEnd = Rels.end();
     // Disassemble symbol by symbol.
     for (unsigned SI = 0, SE = Symbols.size(); SI != SE; ++SI) {
       std::string SymbolName = Symbols[SI].Name.str();
       if (Demangle)
         SymbolName = demangle(SymbolName);
 
       // Skip if --disassemble-symbols is not empty and the symbol is not in
       // the list.
       if (!DisasmSymbolSet.empty() && !DisasmSymbolSet.count(SymbolName))
         continue;
 
       uint64_t Start = Symbols[SI].Addr;
       if (Start < SectionAddr || StopAddress <= Start)
         continue;
       else
         FoundDisasmSymbolSet.insert(SymbolName);
 
       // The end is the section end, the beginning of the next symbol, or
       // --stop-address.
       uint64_t End = std::min<uint64_t>(SectionAddr + SectSize, StopAddress);