[llvm-objdump] [NFC] Use DisassemblerTarget for primary target in disassembleObject.

[jacek]

Summary

Further preparation for ARM64EC/ARM64X support.

[jacek]

Rebased.

[jhenderson]

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

1606

I'm concerned that having the Kind of a MappingSymbol stored as a char is unclear, and a little fragile, if we start diverging from the actual letter used in the mapping symbol as you are doing here in this patch. More generally, I'm not entirely convinced by the fact that here you are making decisions about the implications of the mapping symbols now, whereas before those decisions were made at the point of impact (i.e. the point at which the right target is selected). If possible, I'd prefer it if you removed this change from this patch. I'm not necessarily opposed to it going in in a different patch, but I'd want it reviewed independently and by others who are more familiar with mapping symbols.

[jacek]

I removed mapping change. Handling PrimaryIsThumb while creating mapping felt right to me, because it's not relevant in ARM64EC case and made that code path a bit easier to follow. But as you said, it's a separated change.

[jhenderson]

Thanks, LGTM.

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

fetchBinaryByBuildID(const ObjectFile &Obj) {
   if (!DebugBinary) {
     reportWarning(toString(DebugBinary.takeError()), *Path);
     return std::nullopt;
   }
   return std::move(*DebugBinary);
 }
 
 static void
-disassembleObject(const Target *TheTarget, ObjectFile &Obj,
-                  const ObjectFile &DbgObj, MCContext &Ctx,
-                  MCDisassembler *PrimaryDisAsm,
+disassembleObject(ObjectFile &Obj, const ObjectFile &DbgObj,
+                  DisassemblerTarget &PrimaryTarget,
                   std::optional<DisassemblerTarget> &SecondaryTarget,
-                  const MCInstrAnalysis *MIA, MCInstPrinter *IP,
-                  const MCSubtargetInfo *PrimarySTI, PrettyPrinter &PIP,
                   SourcePrinter &SP, bool InlineRelocs) {
-  const MCSubtargetInfo *STI = PrimarySTI;
-  MCDisassembler *DisAsm = PrimaryDisAsm;
+  DisassemblerTarget *DT = &PrimaryTarget;
   bool PrimaryIsThumb = false;
   if (isArmElf(Obj))
-    PrimaryIsThumb = STI->checkFeatures("+thumb-mode");
+    PrimaryIsThumb = PrimaryTarget.SubtargetInfo->checkFeatures("+thumb-mode");
 
   std::map<SectionRef, std::vector<RelocationRef>> RelocMap;
   if (InlineRelocs)
     RelocMap = getRelocsMap(Obj);
   bool Is64Bits = Obj.getBytesInAddress() > 4;
 
   // Create a mapping from virtual address to symbol name.  This is used to
   // pretty print the symbols while disassembling.
disassembleObject(ObjectFile &Obj, const ObjectFile &DbgObj,
   // Multiple symbols can have the same address. Use a stable sort to stabilize
   // the output.
   StringSet<> FoundDisasmSymbolSet;
   for (std::pair<const SectionRef, SectionSymbolsTy> &SecSyms : AllSymbols)
     llvm::stable_sort(SecSyms.second);
   llvm::stable_sort(AbsoluteSymbols);
 
   std::unique_ptr<DWARFContext> DICtx;
-  LiveVariablePrinter LVP(*Ctx.getRegisterInfo(), *STI);
+  LiveVariablePrinter LVP(*DT->Context->getRegisterInfo(), *DT->SubtargetInfo);
 
   if (DbgVariables != DVDisabled) {
     DICtx = DWARFContext::create(DbgObj);
     for (const std::unique_ptr<DWARFUnit> &CU : DICtx->compile_units())
       LVP.addCompileUnit(CU->getUnitDIE(false));
   }
 
   LLVM_DEBUG(LVP.dump());
for (const SectionRef &Section : ToolSectionFilter(Obj)) {
 
     // For relocatable object files, read the LLVM_BB_ADDR_MAP section
     // corresponding to this section, if present.
     if (SymbolizeOperands && Obj.isRelocatableObject())
       ReadBBAddrMap(Section.getIndex());
 
     // Get the list of all the symbols in this section.
     SectionSymbolsTy &Symbols = AllSymbols[Section];
     auto &MappingSymbols = AllMappingSymbols[Section];
     llvm::sort(MappingSymbols);
 
     ArrayRef<uint8_t> Bytes = arrayRefFromStringRef(
         unwrapOrError(Section.getContents(), Obj.getFileName()));
 
     std::vector<std::unique_ptr<std::string>> SynthesizedLabelNames;
     if (Obj.isELF() && Obj.getArch() == Triple::amdgcn) {
       // AMDGPU disassembler uses symbolizer for printing labels
-      addSymbolizer(Ctx, TheTarget, TripleName, DisAsm, SectionAddr, Bytes,
-                    Symbols, SynthesizedLabelNames);
+      addSymbolizer(*DT->Context, DT->TheTarget, TripleName, DT->DisAsm.get(),
+                    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.
     // Without --show-all-symbols, also insert one if all symbols at the start
     // are mapping symbols.
     bool CreateDummy = Symbols.empty();
for (size_t SI = 0, SE = Symbols.size(); SI != SE;) {
       // interesting behavior (if any), on the assumption that if two symbols
       // defined at the same address trigger two conflicting symbol handlers,
       // the object file is probably confused anyway, and it would make even
       // less sense to present the output of _both_ handlers, because that
       // would describe the same data twice.
       for (size_t SHI = 0; SHI < SymbolsHere.size(); ++SHI) {
         SymbolInfoTy Symbol = SymbolsHere[SHI];
 
-        auto Status =
-            DisAsm->onSymbolStart(Symbol, Size, Bytes.slice(Start, End - Start),
-                                  SectionAddr + Start, CommentStream);
+        auto Status = DT->DisAsm->onSymbolStart(
+            Symbol, Size, Bytes.slice(Start, End - Start), SectionAddr + Start,
+            CommentStream);
 
         if (!Status) {
           // If onSymbolStart returns std::nullopt, that means it didn't trigger
           // any interesting handling for this symbol. Try the other symbols
           // defined at this address.
           continue;
         }
 
         if (*Status == MCDisassembler::Fail) {
           // If onSymbolStart returns Fail, that means it identified some kind
           // of special data at this address, but wasn't able to disassemble it
           // meaningfully. So we fall back to disassembling the failed region
           // as bytes, assuming that the target detected the failure before
           // printing anything.
           //
           // Return values Success or SoftFail (i.e no 'real' failure) are
           // expected to mean that the target has emitted its own output.
           //
           // Either way, 'Size' will have been set to the amount of data
           // covered by whatever prologue the target identified. So we advance
           // our own position to beyond that. Sometimes that will be the entire
           // distance to the next symbol, and sometimes it will be just a
           // prologue and we should start disassembling instructions from where
           // it left off.
-          outs() << Ctx.getAsmInfo()->getCommentString()
+          outs() << DT->Context->getAsmInfo()->getCommentString()
                  << " error in decoding " << SymNamesHere[SHI]
                  << " : decoding failed region as bytes.\n";
           for (uint64_t I = 0; I < Size; ++I) {
             outs() << "\t.byte\t " << format_hex(Bytes[I], 1, /*Upper=*/true)
                    << "\n";
           }
         }
         Start += Size;
for (size_t SI = 0, SE = Symbols.size(); SI != SE;) {
           Symbols[SI - 1].XCOFFSymInfo.StorageMappingClass &&
           (*Symbols[SI - 1].XCOFFSymInfo.StorageMappingClass == XCOFF::XMC_PR);
 
       formatted_raw_ostream FOS(outs());
 
       std::unordered_map<uint64_t, std::string> AllLabels;
       std::unordered_map<uint64_t, std::vector<std::string>> BBAddrMapLabels;
       if (SymbolizeOperands) {
-        collectLocalBranchTargets(Bytes, MIA, DisAsm, IP, PrimarySTI,
+        collectLocalBranchTargets(Bytes, DT->InstrAnalysis.get(),
+                                  DT->DisAsm.get(), DT->InstPrinter.get(),
+                                  PrimaryTarget.SubtargetInfo.get(),
                                   SectionAddr, Index, End, AllLabels);
         collectBBAddrMapLabels(AddrToBBAddrMap, SectionAddr, Index, End,
                                BBAddrMapLabels);
       }
 
       while (Index < End) {
         // ARM and 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 the data marker is within a function, it is
         // denoted as a word/short etc.
         if (!MappingSymbols.empty()) {
           char Kind = getMappingSymbolKind(MappingSymbols, Index);
           DumpARMELFData = Kind == 'd';
           if (SecondaryTarget) {
             if (Kind == 'a') {
-              STI = PrimaryIsThumb ? SecondaryTarget->SubtargetInfo.get()
-                                   : PrimarySTI;
-              DisAsm = PrimaryIsThumb ? SecondaryTarget->DisAsm.get()
-                                      : PrimaryDisAsm;
+              DT = PrimaryIsThumb ? &*SecondaryTarget : &PrimaryTarget;
             } else if (Kind == 't') {
-              STI = PrimaryIsThumb ? PrimarySTI
-                                   : SecondaryTarget->SubtargetInfo.get();
-              DisAsm = PrimaryIsThumb ? PrimaryDisAsm
-                                      : SecondaryTarget->DisAsm.get();
+              DT = PrimaryIsThumb ? &PrimaryTarget : &*SecondaryTarget;
             }
           }
         }
 
         if (DumpARMELFData) {
           Size = dumpARMELFData(SectionAddr, Index, End, Obj, Bytes,
-                                MappingSymbols, *STI, FOS);
+                                MappingSymbols, *DT->SubtargetInfo, FOS);
         } else {
           // When -z or --disassemble-zeroes are given we always dissasemble
           // them. Otherwise we might want to skip zero bytes we see.
           if (!DisassembleZeroes) {
             uint64_t MaxOffset = End - Index;
             // For --reloc: print zero blocks patched by relocations, so that
             // relocations can be shown in the dump.
             if (RelCur != RelEnd)
               MaxOffset = std::min(RelCur->getOffset() - RelAdjustment - Index,
                                    MaxOffset);
 
             if (size_t N =
                     countSkippableZeroBytes(Bytes.slice(Index, MaxOffset))) {
               FOS << "\t\t..." << '\n';
               Index += N;
               continue;
             }
           }
 
           if (DumpTracebackTableForXCOFFFunction &&
               doesXCOFFTracebackTableBegin(Bytes.slice(Index, 4))) {
             dumpTracebackTable(Bytes.slice(Index),
                                SectionAddr + Index + VMAAdjustment, FOS,
-                               SectionAddr + End + VMAAdjustment, *STI,
-                               cast<XCOFFObjectFile>(&Obj));
+                               SectionAddr + End + VMAAdjustment,
+                               *DT->SubtargetInfo, cast<XCOFFObjectFile>(&Obj));
             Index = End;
             continue;
           }
 
           // Print local label if there's any.
           auto Iter1 = BBAddrMapLabels.find(SectionAddr + Index);
           if (Iter1 != BBAddrMapLabels.end()) {
             for (StringRef Label : Iter1->second)
               FOS << "<" << Label << ">:\n";
           } else {
             auto Iter2 = AllLabels.find(SectionAddr + Index);
             if (Iter2 != AllLabels.end())
               FOS << "<" << Iter2->second << ">:\n";
           }
 
           // Disassemble a real instruction or a data when disassemble all is
           // provided
           MCInst Inst;
           ArrayRef<uint8_t> ThisBytes = Bytes.slice(Index);
           uint64_t ThisAddr = SectionAddr + Index;
-          bool Disassembled = DisAsm->getInstruction(Inst, Size, ThisBytes,
-                                                     ThisAddr, CommentStream);
+          bool Disassembled = DT->DisAsm->getInstruction(
+              Inst, Size, ThisBytes, ThisAddr, CommentStream);
           if (Size == 0)
             Size = std::min<uint64_t>(
                 ThisBytes.size(),
-                DisAsm->suggestBytesToSkip(ThisBytes, ThisAddr));
+                DT->DisAsm->suggestBytesToSkip(ThisBytes, ThisAddr));
 
           LVP.update({Index, Section.getIndex()},
                      {Index + Size, Section.getIndex()}, Index + Size != End);
 
-          IP->setCommentStream(CommentStream);
+          DT->InstPrinter->setCommentStream(CommentStream);
 
-          PIP.printInst(
-              *IP, Disassembled ? &Inst : nullptr, Bytes.slice(Index, Size),
+          DT->Printer->printInst(
+              *DT->InstPrinter, Disassembled ? &Inst : nullptr,
+              Bytes.slice(Index, Size),
               {SectionAddr + Index + VMAAdjustment, Section.getIndex()}, FOS,
-              "", *STI, &SP, Obj.getFileName(), &Rels, LVP);
+              "", *DT->SubtargetInfo, &SP, Obj.getFileName(), &Rels, LVP);
 
-          IP->setCommentStream(llvm::nulls());
+          DT->InstPrinter->setCommentStream(llvm::nulls());
 
           // If disassembly has failed, avoid analysing invalid/incomplete
           // instruction information. Otherwise, try to resolve the target
           // address (jump target or memory operand address) and print it on the
           // right of the instruction.
-          if (Disassembled && MIA) {
+          if (Disassembled && DT->InstrAnalysis) {
             // Branch targets are printed just after the instructions.
             llvm::raw_ostream *TargetOS = &FOS;
             uint64_t Target;
-            bool PrintTarget =
-                MIA->evaluateBranch(Inst, SectionAddr + Index, Size, Target);
+            bool PrintTarget = DT->InstrAnalysis->evaluateBranch(
+                Inst, SectionAddr + Index, Size, Target);
             if (!PrintTarget)
               if (std::optional<uint64_t> MaybeTarget =
-                      MIA->evaluateMemoryOperandAddress(
-                          Inst, STI, SectionAddr + Index, Size)) {
+                      DT->InstrAnalysis->evaluateMemoryOperandAddress(
+                          Inst, DT->SubtargetInfo.get(), SectionAddr + Index,
+                          Size)) {
                 Target = *MaybeTarget;
                 PrintTarget = true;
                 // Do not print real address when symbolizing.
                 if (!SymbolizeOperands) {
                   // Memory operand addresses are printed as comments.
                   TargetOS = &CommentStream;
                   *TargetOS << "0x" << Twine::utohexstr(Target);
                 }
for (size_t SI = 0, SE = Symbols.size(); SI != SE;) {
               // By convention, each record in the comment stream should be
               // terminated.
               if (TargetOS == &CommentStream)
                 *TargetOS << "\n";
             }
           }
         }
 
-        assert(Ctx.getAsmInfo());
-        emitPostInstructionInfo(FOS, *Ctx.getAsmInfo(), *STI,
-                                CommentStream.str(), LVP);
+        assert(DT->Context->getAsmInfo());
+        emitPostInstructionInfo(FOS, *DT->Context->getAsmInfo(),
+                                *DT->SubtargetInfo, CommentStream.str(), LVP);
         Comments.clear();
 
         // Hexagon does this in pretty printer
         if (Obj.getArch() != Triple::hexagon) {
           // Print relocation for instruction and data.
           while (RelCur != RelEnd) {
             uint64_t Offset = RelCur->getOffset() - RelAdjustment;
             // If this relocation is hidden, skip it.
static void disassembleObject(ObjectFile *Obj, bool InlineRelocs) {
 
   SourcePrinter SP(DbgObj, TheTarget->getName());
 
   for (StringRef Opt : DisassemblerOptions)
     if (!PrimaryTarget.InstPrinter->applyTargetSpecificCLOption(Opt))
       reportError(Obj->getFileName(),
                   "Unrecognized disassembler option: " + Opt);
 
-  disassembleObject(TheTarget, *Obj, *DbgObj, *PrimaryTarget.Context.get(),
-                    PrimaryTarget.DisAsm.get(), SecondaryTarget,
-                    PrimaryTarget.InstrAnalysis.get(),
-                    PrimaryTarget.InstPrinter.get(),
-                    PrimaryTarget.SubtargetInfo.get(),
-                    *PrimaryTarget.Printer, SP, InlineRelocs);
+  disassembleObject(*Obj, *DbgObj, PrimaryTarget, SecondaryTarget, SP,
+                    InlineRelocs);
 }
 
 void Dumper::printRelocations() {
   StringRef Fmt = O.getBytesInAddress() > 4 ? "%016" PRIx64 : "%08" PRIx64;
 
   // Build a mapping from relocation target to a vector of relocation
   // sections. Usually, there is an only one relocation section for
   // each relocated section.