[FaultsMaps][llvm-objdump] Move FaultMapParser to Object/. Remove CodeGen dependency from llvm-objdump

[craig.topper]

Summary

FaultsMapParser lived in CodeGen and was forcing llvm-objdump to
link CodeGen and everything CodeGen depends on.

This was previously attempted in r240364 to fix a link failure.
The CodeGen dependency was independently added to fix the same
link failure, and that ended up being kept.

Removing the dependency seems like the correct layering for
llvm-objdump.

[MaskRay]

I agree that lib/Object is a better place.

llvm/include/llvm/Object/FaultMapParser.h

7

The license header should be Apache-2.0 WITH LLVM-exception

llvm/lib/Object/FaultMapParser.cpp

6

Apache-2.0 WITH LLVM-exception

[craig.topper]

llvm/include/llvm/Object/FaultMapParser.h

7

Doh! I started from reverting the original revert and forgot that the headers would be old. Thanks.

[MaskRay]

llvm/include/llvm/Object/FaultMapParser.h

19–20

clang-tidy: warning: code/includes outside of area guarded by header guard; consider moving it [llvm-header-guard]

Move the guard before other includes.

157–158

Consider reformat it while you are moving code.

[craig.topper]

Address review comments

[MaskRay]

Thanks! Please wait a bit for other reviewers.

[jhenderson]

Have you made sure the includes in both the files being moved from and being moved to are minimal? Seems like an easy thing to forget to do, but I have no specific example of any that are incorrect (I haven't gone digging). LGTM, aside from that and the inline comments.

llvm/lib/Object/FaultMapParser.cpp

46

Consider fixing these clang-tidy warnings whilst you're moving the code.

60

Ditto.

llvm/include/llvm/CodeGen/FaultMaps.h

 //===- FaultMaps.h - The "FaultMaps" section --------------------*- 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_CODEGEN_FAULTMAPS_H
 #define LLVM_CODEGEN_FAULTMAPS_H
 
 #include "llvm/MC/MCSymbol.h"
 #include "llvm/Support/Endian.h"
-#include <cassert>
-#include <cstddef>
-#include <cstdint>
 #include <map>
 #include <vector>
 
 namespace llvm {
 
 class AsmPrinter;
 class MCExpr;
 class raw_ostream;
private:
 
   std::map<const MCSymbol *, FunctionFaultInfos, MCSymbolComparator>
       FunctionInfos;
   AsmPrinter &AP;
 
   void emitFunctionInfo(const MCSymbol *FnLabel, const FunctionFaultInfos &FFI);
 };
 
-/// A parser for the __llvm_faultmaps section generated by the FaultMaps class
-/// above.  This parser is version locked with with the __llvm_faultmaps section
-/// generated by the version of LLVM that includes it.  No guarantees are made
-/// with respect to forward or backward compatibility.
-class FaultMapParser {
-  using FaultMapVersionType = uint8_t;
-  using Reserved0Type = uint8_t;
-  using Reserved1Type = uint16_t;
-  using NumFunctionsType = uint32_t;
-
-  static const size_t FaultMapVersionOffset = 0;
-  static const size_t Reserved0Offset =
-      FaultMapVersionOffset + sizeof(FaultMapVersionType);
-  static const size_t Reserved1Offset = Reserved0Offset + sizeof(Reserved0Type);
-  static const size_t NumFunctionsOffset =
-      Reserved1Offset + sizeof(Reserved1Type);
-  static const size_t FunctionInfosOffset =
-      NumFunctionsOffset + sizeof(NumFunctionsType);
-
-  const uint8_t *P;
-  const uint8_t *E;
-
-  template <typename T> static T read(const uint8_t *P, const uint8_t *E) {
-    assert(P + sizeof(T) <= E && "out of bounds read!");
-    return support::endian::read<T, support::little, 1>(P);
-  }
-
-public:
-  class FunctionFaultInfoAccessor {
-    using FaultKindType = uint32_t;
-    using FaultingPCOffsetType = uint32_t;
-    using HandlerPCOffsetType = uint32_t;
-
-    static const size_t FaultKindOffset = 0;
-    static const size_t FaultingPCOffsetOffset =
-        FaultKindOffset + sizeof(FaultKindType);
-    static const size_t HandlerPCOffsetOffset =
-        FaultingPCOffsetOffset + sizeof(FaultingPCOffsetType);
-
-    const uint8_t *P;
-    const uint8_t *E;
-
-  public:
-    static const size_t Size =
-        HandlerPCOffsetOffset + sizeof(HandlerPCOffsetType);
-
-    explicit FunctionFaultInfoAccessor(const uint8_t *P, const uint8_t *E)
-        : P(P), E(E) {}
-
-    FaultKindType getFaultKind() const {
-      return read<FaultKindType>(P + FaultKindOffset, E);
-    }
-
-    FaultingPCOffsetType getFaultingPCOffset() const {
-      return read<FaultingPCOffsetType>(P + FaultingPCOffsetOffset, E);
-    }
-
-    HandlerPCOffsetType getHandlerPCOffset() const {
-      return read<HandlerPCOffsetType>(P + HandlerPCOffsetOffset, E);
-    }
-  };
-
-  class FunctionInfoAccessor {
-    using FunctionAddrType = uint64_t;
-    using NumFaultingPCsType = uint32_t;
-    using ReservedType = uint32_t;
-
-    static const size_t FunctionAddrOffset = 0;
-    static const size_t NumFaultingPCsOffset =
-        FunctionAddrOffset + sizeof(FunctionAddrType);
-    static const size_t ReservedOffset =
-        NumFaultingPCsOffset + sizeof(NumFaultingPCsType);
-    static const size_t FunctionFaultInfosOffset =
-        ReservedOffset + sizeof(ReservedType);
-    static const size_t FunctionInfoHeaderSize = FunctionFaultInfosOffset;
-
-    const uint8_t *P = nullptr;
-    const uint8_t *E = nullptr;
-
-  public:
-    FunctionInfoAccessor() = default;
-
-    explicit FunctionInfoAccessor(const uint8_t *P, const uint8_t *E)
-        : P(P), E(E) {}
-
-    FunctionAddrType getFunctionAddr() const {
-      return read<FunctionAddrType>(P + FunctionAddrOffset, E);
-    }
-
-    NumFaultingPCsType getNumFaultingPCs() const {
-      return read<NumFaultingPCsType>(P + NumFaultingPCsOffset, E);
-    }
-
-    FunctionFaultInfoAccessor getFunctionFaultInfoAt(uint32_t Index) const {
-      assert(Index < getNumFaultingPCs() && "index out of bounds!");
-      const uint8_t *Begin = P + FunctionFaultInfosOffset +
-                             FunctionFaultInfoAccessor::Size * Index;
-      return FunctionFaultInfoAccessor(Begin, E);
-    }
-
-    FunctionInfoAccessor getNextFunctionInfo() const {
-      size_t MySize = FunctionInfoHeaderSize +
-                      getNumFaultingPCs() * FunctionFaultInfoAccessor::Size;
-
-      const uint8_t *Begin = P + MySize;
-      assert(Begin < E && "out of bounds!");
-      return FunctionInfoAccessor(Begin, E);
-    }
-  };
-
-  explicit FaultMapParser(const uint8_t *Begin, const uint8_t *End)
-      : P(Begin), E(End) {}
-
-  FaultMapVersionType getFaultMapVersion() const {
-    auto Version = read<FaultMapVersionType>(P + FaultMapVersionOffset, E);
-    assert(Version == 1 && "only version 1 supported!");
-    return Version;
-  }
-
-  NumFunctionsType getNumFunctions() const {
-    return read<NumFunctionsType>(P + NumFunctionsOffset, E);
-  }
-
-  FunctionInfoAccessor getFirstFunctionInfo() const {
-    const uint8_t *Begin = P + FunctionInfosOffset;
-    return FunctionInfoAccessor(Begin, E);
-  }
-};
-
-raw_ostream &
-operator<<(raw_ostream &OS, const FaultMapParser::FunctionFaultInfoAccessor &);
-
-raw_ostream &operator<<(raw_ostream &OS,
-                        const FaultMapParser::FunctionInfoAccessor &);
-
-raw_ostream &operator<<(raw_ostream &OS, const FaultMapParser &);
-
 } // end namespace llvm
 
 #endif // LLVM_CODEGEN_FAULTMAPS_H

llvm/include/llvm/Object/FaultMapParser.h

-//===- FaultMaps.h - The "FaultMaps" section --------------------*- C++ -*-===//
+//===-- FaultMapParser.h - Parser for the  "FaultMaps" section --*- 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_CODEGEN_FAULTMAPS_H
-#define LLVM_CODEGEN_FAULTMAPS_H
+#ifndef LLVM_OBJECT_FAULTMAPPARSER_H
+#define LLVM_OBJECT_FAULTMAPPARSER_H
 
-#include "llvm/MC/MCSymbol.h"
 #include "llvm/Support/Endian.h"
 #include <cassert>
-#include <cstddef>
 #include <cstdint>
-#include <map>
-#include <vector>
 
 namespace llvm {
 
-class AsmPrinter;
-class MCExpr;
 class raw_ostream;
 
-class FaultMaps {
-public:
-  enum FaultKind {
-    FaultingLoad = 1,
-    FaultingLoadStore,
-    FaultingStore,
-    FaultKindMax
-  };
-
-  explicit FaultMaps(AsmPrinter &AP);
-
-  static const char *faultTypeToString(FaultKind);
-
-  void recordFaultingOp(FaultKind FaultTy, const MCSymbol *FaultingLabel,
-                        const MCSymbol *HandlerLabel);
-  void serializeToFaultMapSection();
-  void reset() {
-    FunctionInfos.clear();
-  }
-
-private:
-  static const char *WFMP;
-
-  struct FaultInfo {
-    FaultKind Kind = FaultKindMax;
-    const MCExpr *FaultingOffsetExpr = nullptr;
-    const MCExpr *HandlerOffsetExpr = nullptr;
-
-    FaultInfo() = default;
-
-    explicit FaultInfo(FaultMaps::FaultKind Kind, const MCExpr *FaultingOffset,
-                       const MCExpr *HandlerOffset)
-        : Kind(Kind), FaultingOffsetExpr(FaultingOffset),
-          HandlerOffsetExpr(HandlerOffset) {}
-  };
-
-  using FunctionFaultInfos = std::vector<FaultInfo>;
-
-  // We'd like to keep a stable iteration order for FunctionInfos to help
-  // FileCheck based testing.
-  struct MCSymbolComparator {
-    bool operator()(const MCSymbol *LHS, const MCSymbol *RHS) const {
-      return LHS->getName() < RHS->getName();
-    }
-  };
-
-  std::map<const MCSymbol *, FunctionFaultInfos, MCSymbolComparator>
-      FunctionInfos;
-  AsmPrinter &AP;
-
-  void emitFunctionInfo(const MCSymbol *FnLabel, const FunctionFaultInfos &FFI);
-};
-
 /// A parser for the __llvm_faultmaps section generated by the FaultMaps class
-/// above.  This parser is version locked with with the __llvm_faultmaps section
-/// generated by the version of LLVM that includes it.  No guarantees are made
-/// with respect to forward or backward compatibility.
+/// declared in llvm/CodeGen/FaultMaps.h.  This parser is version locked with
+/// with the __llvm_faultmaps section generated by the version of LLVM that
+/// includes it.  No guarantees are made with respect to forward or backward
+/// compatibility.
 class FaultMapParser {
   using FaultMapVersionType = uint8_t;
   using Reserved0Type = uint8_t;
   using Reserved1Type = uint16_t;
   using NumFunctionsType = uint32_t;
 
   static const size_t FaultMapVersionOffset = 0;
   static const size_t Reserved0Offset =
       FaultMapVersionOffset + sizeof(FaultMapVersionType);
   static const size_t Reserved1Offset = Reserved0Offset + sizeof(Reserved0Type);
   static const size_t NumFunctionsOffset =
       Reserved1Offset + sizeof(Reserved1Type);
   static const size_t FunctionInfosOffset =
       NumFunctionsOffset + sizeof(NumFunctionsType);
 
   const uint8_t *P;
   const uint8_t *E;
 
   template <typename T> static T read(const uint8_t *P, const uint8_t *E) {
     assert(P + sizeof(T) <= E && "out of bounds read!");
     return support::endian::read<T, support::little, 1>(P);
   }
 
 public:
+  enum FaultKind {
+    FaultingLoad = 1,
+    FaultingLoadStore,
+    FaultingStore,
+    FaultKindMax
+  };
+
   class FunctionFaultInfoAccessor {
     using FaultKindType = uint32_t;
     using FaultingPCOffsetType = uint32_t;
     using HandlerPCOffsetType = uint32_t;
 
     static const size_t FaultKindOffset = 0;
     static const size_t FaultingPCOffsetOffset =
         FaultKindOffset + sizeof(FaultKindType);
public:
   }
 
   FunctionInfoAccessor getFirstFunctionInfo() const {
     const uint8_t *Begin = P + FunctionInfosOffset;
     return FunctionInfoAccessor(Begin, E);
   }
 };
 
-raw_ostream &
-operator<<(raw_ostream &OS, const FaultMapParser::FunctionFaultInfoAccessor &);
+raw_ostream &operator<<(raw_ostream &OS,
+                        const FaultMapParser::FunctionFaultInfoAccessor &);
 
 raw_ostream &operator<<(raw_ostream &OS,
                         const FaultMapParser::FunctionInfoAccessor &);
 
 raw_ostream &operator<<(raw_ostream &OS, const FaultMapParser &);
 
-} // end namespace llvm
+} // namespace llvm
 
-#endif // LLVM_CODEGEN_FAULTMAPS_H
+#endif

llvm/lib/CodeGen/FaultMaps.cpp

 #include "llvm/ADT/Twine.h"
 #include "llvm/CodeGen/AsmPrinter.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCExpr.h"
 #include "llvm/MC/MCObjectFileInfo.h"
 #include "llvm/MC/MCStreamer.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
-#include "llvm/Support/Format.h"
-#include "llvm/Support/raw_ostream.h"
 
 using namespace llvm;
 
 #define DEBUG_TYPE "faultmaps"
 
 static const int FaultMapVersion = 1;
 const char *FaultMaps::WFMP = "Fault Maps: ";
 
const char *FaultMaps::faultTypeToString(FaultMaps::FaultKind FT) {
   case FaultMaps::FaultingLoad:
     return "FaultingLoad";
   case FaultMaps::FaultingLoadStore:
     return "FaultingLoadStore";
   case FaultMaps::FaultingStore:
     return "FaultingStore";
   }
 }
-
-raw_ostream &llvm::
-operator<<(raw_ostream &OS,
-           const FaultMapParser::FunctionFaultInfoAccessor &FFI) {
-  OS << "Fault kind: "
-     << FaultMaps::faultTypeToString((FaultMaps::FaultKind)FFI.getFaultKind())
-     << ", faulting PC offset: " << FFI.getFaultingPCOffset()
-     << ", handling PC offset: " << FFI.getHandlerPCOffset();
-  return OS;
-}
-
-raw_ostream &llvm::
-operator<<(raw_ostream &OS, const FaultMapParser::FunctionInfoAccessor &FI) {
-  OS << "FunctionAddress: " << format_hex(FI.getFunctionAddr(), 8)
-     << ", NumFaultingPCs: " << FI.getNumFaultingPCs() << "\n";
-  for (unsigned i = 0, e = FI.getNumFaultingPCs(); i != e; ++i)
-    OS << FI.getFunctionFaultInfoAt(i) << "\n";
-  return OS;
-}
-
-raw_ostream &llvm::operator<<(raw_ostream &OS, const FaultMapParser &FMP) {
-  OS << "Version: " << format_hex(FMP.getFaultMapVersion(), 2) << "\n";
-  OS << "NumFunctions: " << FMP.getNumFunctions() << "\n";
-
-  if (FMP.getNumFunctions() == 0)
-    return OS;
-
-  FaultMapParser::FunctionInfoAccessor FI;
-
-  for (unsigned i = 0, e = FMP.getNumFunctions(); i != e; ++i) {
-    FI = (i == 0) ? FMP.getFirstFunctionInfo() : FI.getNextFunctionInfo();
-    OS << FI;
-  }
-
-  return OS;
-}

llvm/lib/Object/CMakeLists.txt

 add_llvm_component_library(LLVMObject
   Archive.cpp
   ArchiveWriter.cpp
   Binary.cpp
   COFFImportFile.cpp
   COFFModuleDefinition.cpp
   COFFObjectFile.cpp
   Decompressor.cpp
   ELF.cpp
   ELFObjectFile.cpp
   Error.cpp
+  FaultMapParser.cpp
   IRObjectFile.cpp
   IRSymtab.cpp
   MachOObjectFile.cpp
   MachOUniversal.cpp
   Minidump.cpp
   ModuleSymbolTable.cpp
   Object.cpp
   ObjectFile.cpp

llvm/lib/Object/FaultMapParser.cpp

+//===----------------------- FaultMapParser.cpp ---------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Object/FaultMapParser.h"
+#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/Format.h"
+#include "llvm/Support/raw_ostream.h"
+
+using namespace llvm;
+
+void printFaultType(FaultMapParser::FaultKind FT, raw_ostream &OS) {
+  switch (FT) {
+  default:
+    llvm_unreachable("unhandled fault type!");
+  case FaultMapParser::FaultingLoad:
+    OS << "FaultingLoad";
+    break;
+  case FaultMapParser::FaultingLoadStore:
+    OS << "FaultingLoadStore";
+    break;
+  case FaultMapParser::FaultingStore:
+    OS << "FaultingStore";
+    break;
+  }
+}
+
+raw_ostream &
+llvm::operator<<(raw_ostream &OS,
+                 const FaultMapParser::FunctionFaultInfoAccessor &FFI) {
+  OS << "Fault kind: ";
+  printFaultType((FaultMapParser::FaultKind)FFI.getFaultKind(), OS);
+  OS << ", faulting PC offset: " << FFI.getFaultingPCOffset()
+     << ", handling PC offset: " << FFI.getHandlerPCOffset();
+  return OS;
+}
+
+raw_ostream &llvm::operator<<(raw_ostream &OS,
+                              const FaultMapParser::FunctionInfoAccessor &FI) {
+  OS << "FunctionAddress: " << format_hex(FI.getFunctionAddr(), 8)
+     << ", NumFaultingPCs: " << FI.getNumFaultingPCs() << "\n";
+  for (unsigned I = 0, E = FI.getNumFaultingPCs(); I != E; ++I)
+    OS << FI.getFunctionFaultInfoAt(I) << "\n";
+  return OS;
+}
+
+raw_ostream &llvm::operator<<(raw_ostream &OS, const FaultMapParser &FMP) {
+  OS << "Version: " << format_hex(FMP.getFaultMapVersion(), 2) << "\n";
+  OS << "NumFunctions: " << FMP.getNumFunctions() << "\n";
+
+  if (FMP.getNumFunctions() == 0)
+    return OS;
+
+  FaultMapParser::FunctionInfoAccessor FI;
+
+  for (unsigned I = 0, E = FMP.getNumFunctions(); I != E; ++I) {
+    FI = (I == 0) ? FMP.getFirstFunctionInfo() : FI.getNextFunctionInfo();
+    OS << FI;
+  }
+
+  return OS;
+}

llvm/tools/llvm-objdump/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   AllTargetsDescs
   AllTargetsDisassemblers
   AllTargetsInfos
   BinaryFormat
-  CodeGen
   DebugInfoDWARF
   DebugInfoPDB
   Demangle
   MC
   MCDisassembler
   Object
   Support
   Symbolize

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

 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallSet.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/ADT/Twine.h"
-#include "llvm/CodeGen/FaultMaps.h"
 #include "llvm/DebugInfo/DWARF/DWARFContext.h"
 #include "llvm/DebugInfo/Symbolize/Symbolize.h"
 #include "llvm/Demangle/Demangle.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCDisassembler/MCDisassembler.h"
 #include "llvm/MC/MCDisassembler/MCRelocationInfo.h"
 #include "llvm/MC/MCInst.h"
 #include "llvm/MC/MCInstPrinter.h"
 #include "llvm/MC/MCInstrAnalysis.h"
 #include "llvm/MC/MCInstrInfo.h"
 #include "llvm/MC/MCObjectFileInfo.h"
 #include "llvm/MC/MCRegisterInfo.h"
 #include "llvm/MC/MCSubtargetInfo.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/FaultMapParser.h"
 #include "llvm/Object/MachO.h"
 #include "llvm/Object/MachOUniversal.h"
 #include "llvm/Object/ObjectFile.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"