[llvm-objdump][COFF][NFC] Split format-specific interfaces; add namespace

[hubert.reinterpretcast]

Summary

This patch addresses, for the interfaces implemented by COFFDump.cpp, multiple issues identified with the current structure of llvm-objdump.h in the review of D72973.

This patch moves implementation details of the tool into an llvm::objdump namespace for external linkage names, splits the implementation details into separate headers for each implementation file, and uses qualified names when declaring members of the llvm::objdump namespace in place of leaving the namespace definition open.

[hubert.reinterpretcast]

COFFDump.h: Apply clang-format changes for namespace end

[hubert.reinterpretcast]

Hopefully get arc to upload full diff

[MaskRay]

LGTM.

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

78

Add a new line before namespace llvm

[jhenderson]

LGTM, with or without my comments addressed.

llvm/tools/llvm-objdump/COFFDump.h

14

Can't you forward-declare Error to avoid this include?

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

49

It's probably okay to address this clang-format comment here, since you're adding a new include.

[hubert.reinterpretcast]

Please let me know if there is a preference to post additional patches in parts in the same style or to update this one with the changes associated with the other files when they are all done. Thanks.

llvm/tools/llvm-objdump/COFFDump.h

14

Yes, I think so. Will do.

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

49

D72973 already plans to remove this line.

78

Will do; I'll add the same comment to D72973.

[jhenderson]

In D77285#1957066, @hubert.reinterpretcast wrote:

Please let me know if there is a preference to post additional patches in parts in the same style or to update this one with the changes associated with the other files when they are all done. Thanks.

Lots of small changes is fine by me, and probably even preferable overall.

[hubert.reinterpretcast]

In D77285#1957116, @jhenderson wrote:

Lots of small changes is fine by me, and probably even preferable overall.

Okay. I will address the comments on the commit (and change the patch description accordingly) if it goes smoothly. Would you like to see the other patches (if they don't have additional complications) posted for pre-commit review?

[hubert.reinterpretcast]

• Address comments: Forward-declare llvm::Error; add newline

[jhenderson]

In D77285#1957277, @hubert.reinterpretcast wrote:

In D77285#1957116, @jhenderson wrote:

Lots of small changes is fine by me, and probably even preferable overall.

Okay. I will address the comments on the commit (and change the patch description accordingly) if it goes smoothly. Would you like to see the other patches (if they don't have additional complications) posted for pre-commit review?

Yes, please.

llvm/tools/llvm-objdump/COFFDump.h

+//===-- COFFDump.h ----------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_TOOLS_LLVM_OBJDUMP_COFFDUMP_H
+#define LLVM_TOOLS_LLVM_OBJDUMP_COFFDUMP_H
+
+#include "llvm/ADT/SmallVector.h"
+
+namespace llvm {
+
+class Error;
+
+namespace object {
+class COFFObjectFile;
+class COFFImportFile;
+class ObjectFile;
+class RelocationRef;
+} // namespace object
+
+namespace objdump {
+Error getCOFFRelocationValueString(const object::COFFObjectFile *Obj,
+                                   const object::RelocationRef &Rel,
+                                   llvm::SmallVectorImpl<char> &Result);
+
+void printCOFFUnwindInfo(const object::COFFObjectFile *O);
+void printCOFFFileHeader(const object::ObjectFile *O);
+void printCOFFSymbolTable(const object::COFFImportFile *I);
+void printCOFFSymbolTable(const object::COFFObjectFile *O);
+} // namespace objdump
+} // namespace llvm
+
+#endif

llvm/tools/llvm-objdump/COFFDump.cpp

 //===-- COFFDump.cpp - COFF-specific dumper ---------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
 /// This file implements the COFF-specific dumper for llvm-objdump.
 /// It outputs the Win64 EH data structures as plain text.
 /// The encoding of the unwind codes is described in MSDN:
 /// http://msdn.microsoft.com/en-us/library/ck9asaa9.aspx
 ///
 //===----------------------------------------------------------------------===//
 
+#include "COFFDump.h"
+
 #include "llvm-objdump.h"
 #include "llvm/Demangle/Demangle.h"
 #include "llvm/Object/COFF.h"
 #include "llvm/Object/COFFImportFile.h"
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/Win64EH.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 
+using namespace llvm;
 using namespace llvm::object;
 using namespace llvm::Win64EH;
 
-namespace llvm {
 // Returns the name of the unwind code.
 static StringRef getUnwindCodeTypeName(uint8_t Code) {
   switch(Code) {
   default: llvm_unreachable("Invalid unwind code");
   case UOP_PushNonVol: return "UOP_PushNonVol";
   case UOP_AllocLarge: return "UOP_AllocLarge";
   case UOP_AllocSmall: return "UOP_AllocSmall";
   case UOP_SetFPReg: return "UOP_SetFPReg";
for (const SectionRef &Section : Obj->sections()) {
 
     RFStart = reinterpret_cast<const RuntimeFunction *>(Contents.data());
     NumRFs = Contents.size() / sizeof(RuntimeFunction);
     return true;
   }
   return false;
 }
 
-Error getCOFFRelocationValueString(const COFFObjectFile *Obj,
+Error objdump::getCOFFRelocationValueString(const COFFObjectFile *Obj,
                                             const RelocationRef &Rel,
                                             SmallVectorImpl<char> &Result) {
   symbol_iterator SymI = Rel.getSymbol();
   Expected<StringRef> SymNameOrErr = SymI->getName();
   if (!SymNameOrErr)
     return SymNameOrErr.takeError();
   StringRef SymName = *SymNameOrErr;
   Result.append(SymName.begin(), SymName.end());
   return Error::success();
 }
static void printRuntimeFunctionRels(const COFFObjectFile *Obj,
   if (UnwindInfoOffset > XContents.size())
     return;
 
   auto *UI = reinterpret_cast<const Win64EH::UnwindInfo *>(XContents.data() +
                                                            UnwindInfoOffset);
   printWin64EHUnwindInfo(UI);
 }
 
-void printCOFFUnwindInfo(const COFFObjectFile *Obj) {
+void objdump::printCOFFUnwindInfo(const COFFObjectFile *Obj) {
   if (Obj->getMachine() != COFF::IMAGE_FILE_MACHINE_AMD64) {
     WithColor::error(errs(), "llvm-objdump")
         << "unsupported image machine type "
            "(currently only AMD64 is supported).\n";
     return;
   }
 
   std::vector<RelocationRef> Rels;
void objdump::printCOFFUnwindInfo(const COFFObjectFile *Obj) {
 
   for (const RuntimeFunction &RF : RFs) {
     uint64_t SectionOffset =
         std::distance(RFs.begin(), &RF) * sizeof(RuntimeFunction);
     printRuntimeFunctionRels(Obj, RF, SectionOffset, Rels);
   }
 }
 
-void printCOFFFileHeader(const object::ObjectFile *Obj) {
+void objdump::printCOFFFileHeader(const object::ObjectFile *Obj) {
   const COFFObjectFile *file = dyn_cast<const COFFObjectFile>(Obj);
   printTLSDirectory(file);
   printLoadConfiguration(file);
   printImportTables(file);
   printExportTable(file);
 }
 
-void printCOFFSymbolTable(const object::COFFImportFile *i) {
+void objdump::printCOFFSymbolTable(const object::COFFImportFile *i) {
   unsigned Index = 0;
   bool IsCode = i->getCOFFImportHeader()->getType() == COFF::IMPORT_CODE;
 
   for (const object::BasicSymbolRef &Sym : i->symbols()) {
     std::string Name;
     raw_string_ostream NS(Name);
 
     cantFail(Sym.printName(NS));
     NS.flush();
 
     outs() << "[" << format("%2d", Index) << "]"
            << "(sec " << format("%2d", 0) << ")"
            << "(fl 0x00)" // Flag bits, which COFF doesn't have.
            << "(ty " << format("%3x", (IsCode && Index) ? 32 : 0) << ")"
            << "(scl " << format("%3x", 0) << ") "
            << "(nx " << 0 << ") "
            << "0x" << format("%08x", 0) << " " << Name << '\n';
 
     ++Index;
   }
 }
 
-void printCOFFSymbolTable(const COFFObjectFile *coff) {
+void objdump::printCOFFSymbolTable(const COFFObjectFile *coff) {
   for (unsigned SI = 0, SE = coff->getNumberOfSymbols(); SI != SE; ++SI) {
     Expected<COFFSymbolRef> Symbol = coff->getSymbol(SI);
     if (!Symbol)
       reportError(Symbol.takeError(), coff->getFileName());
 
     StringRef Name;
     if (std::error_code EC = coff->getSymbolName(*Symbol, Name))
       reportError(errorCodeToError(EC), coff->getFileName());
for (unsigned AI = 0, AE = Symbol->getNumberOfAuxSymbols(); AI < AE; ++AI, ++SI) {
                                    static_cast<uint32_t>(awe->TagIndex),
                                    static_cast<uint32_t>(awe->Characteristics));
       } else {
         outs() << "AUX Unknown\n";
       }
     }
   }
 }
-} // namespace llvm

llvm/tools/llvm-objdump/llvm-objdump.h

 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/DataTypes.h"
 #include "llvm/Object/Archive.h"
 
 namespace llvm {
 class StringRef;
 
 namespace object {
-class COFFObjectFile;
-class COFFImportFile;
 class ELFObjectFileBase;
 class ELFSectionRef;
 class MachOObjectFile;
 class MachOUniversalBinary;
 class RelocationRef;
 class XCOFFObjectFile;
 }
 

 /// this is. This may be different than the actual section number, as some
 /// sections may be filtered (e.g. symbol tables).
 SectionFilter ToolSectionFilter(llvm::object::ObjectFile const &O,
                                 uint64_t *Idx = nullptr);
 
 Error getELFRelocationValueString(const object::ELFObjectFileBase *Obj,
                                   const object::RelocationRef &Rel,
                                   llvm::SmallVectorImpl<char> &Result);
-Error getCOFFRelocationValueString(const object::COFFObjectFile *Obj,
-                                   const object::RelocationRef &Rel,
-                                   llvm::SmallVectorImpl<char> &Result);
 Error getWasmRelocationValueString(const object::WasmObjectFile *Obj,
                                    const object::RelocationRef &RelRef,
                                    llvm::SmallVectorImpl<char> &Result);
 Error getMachORelocationValueString(const object::MachOObjectFile *Obj,
                                     const object::RelocationRef &RelRef,
                                     llvm::SmallVectorImpl<char> &Result);
 Error getXCOFFRelocationValueString(const object::XCOFFObjectFile *Obj,
                                     const object::RelocationRef &RelRef,
                                     llvm::SmallVectorImpl<char> &Result);
 
 uint64_t getELFSectionLMA(const object::ELFSectionRef& Sec);
 
 bool isRelocAddressLess(object::RelocationRef A, object::RelocationRef B);
 void parseInputMachO(StringRef Filename);
 void parseInputMachO(object::MachOUniversalBinary *UB);
-void printCOFFUnwindInfo(const object::COFFObjectFile *O);
 void printMachOUnwindInfo(const object::MachOObjectFile *O);
 void printMachOExportsTrie(const object::MachOObjectFile *O);
 void printMachORebaseTable(object::MachOObjectFile *O);
 void printMachOBindTable(object::MachOObjectFile *O);
 void printMachOLazyBindTable(object::MachOObjectFile *O);
 void printMachOWeakBindTable(object::MachOObjectFile *O);
 void printELFFileHeader(const object::ObjectFile *O);
 void printELFDynamicSection(const object::ObjectFile *Obj);
 void printELFSymbolVersionInfo(const object::ObjectFile *Obj);
-void printCOFFFileHeader(const object::ObjectFile *O);
-void printCOFFSymbolTable(const object::COFFImportFile *I);
-void printCOFFSymbolTable(const object::COFFObjectFile *O);
 void printMachOFileHeader(const object::ObjectFile *O);
 void printMachOLoadCommands(const object::ObjectFile *O);
 void printWasmFileHeader(const object::ObjectFile *O);
 void printExportsTrie(const object::ObjectFile *O);
 void printRebaseTable(object::ObjectFile *O);
 void printBindTable(object::ObjectFile *O);
 void printLazyBindTable(object::ObjectFile *O);
 void printWeakBindTable(object::ObjectFile *O);

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

 // flags.
 //
 // The flags and output of this program should be near identical to those of
 // binutils objdump.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm-objdump.h"
+#include "COFFDump.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SetOperations.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/CodeGen/FaultMaps.h"
 #include "llvm/DebugInfo/DWARF/DWARFContext.h"

 #include "llvm/MC/MCTargetOptions.h"
 #include "llvm/Object/Archive.h"
 #include "llvm/Object/COFF.h"
 #include "llvm/Object/COFFImportFile.h"
 #include "llvm/Object/ELFObjectFile.h"
 #include "llvm/Object/MachO.h"
 #include "llvm/Object/MachOUniversal.h"
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Object/XCOFFObjectFile.h"
 #include "llvm/Object/Wasm.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/FormatVariadic.h"

 #include <algorithm>
 #include <cctype>
 #include <cstring>
 #include <system_error>
 #include <unordered_map>
 #include <utility>
 
 using namespace llvm::object;
+using namespace llvm::objdump;
 
 namespace llvm {
 
 cl::OptionCategory ObjdumpCat("llvm-objdump Options");
 
 // MachO specific
 extern cl::OptionCategory MachOCat;
 extern cl::opt<bool> Bind;
 extern cl::opt<bool> DataInCode;
 extern cl::opt<bool> DylibsUsed;