[COFF] Check table ptr more thoroughly and ignore empty sections

[alvinhochun]

Summary

When loading split debug files for PE/COFF executables (produced with
objcopy --only-keep-debug), the tables or directories in such files
may point to data inside sections that may have been stripped.
COFFObjectFile shall detect and gracefully handle this, to allow the
object file be loaded without considering these tables or directories.
This is required for LLDB to load these files for use as debug symbols.

COFFObjectFile shall also check these pointers more carefully to account
for cases in which the section contains less raw data than the size
given by VirtualSize, to prevent going out of bounds.

This commit also changes COFFDump in llvm-objdump to reuse the pointers
that are already range-checked in COFFObjectFile. This fixes a crash
when trying to dump the TLS directory from a stripped file.

Fixes https://github.com/mstorsjo/llvm-mingw/issues/284

[mstorsjo]

llvm/lib/Object/COFFObjectFile.cpp

912

The patch overall looks reasonable to me. The changes to this function feel a bit clunky, but I don't really have any other good suggestions either.

@rnk, what do you think about this patch?

[rnk]

lgtm

llvm/lib/Object/COFFObjectFile.cpp

912

To make it less clunky, I can suggest making a wrapper which consumes stripped section errors, and then call it like:

static Error ignoreStrippedErrors(Error E) {
  if (E.isA<...>()) {
    consumeError(...);
    return Error::success();
  }
  return std::move(E);
}

...
  if (Error E = ignoreStrippedErrors(initDelayImportTablePtr()))
    return E;
...

[alvinhochun]

Applied suggestion to make code less clunky.

[glandium]

I don't have time to provide more details right now, but this change broke Firefox mingw builds with: llvm-strip: error: '../../dist/firefox/uninstall/helper.exe': RVA 0x43000 for base reloc table found but data is incomplete

That helper.exe file is produced by NSIS.

[mstorsjo]

In D126898#3565834, @glandium wrote:

I don't have time to provide more details right now, but this change broke Firefox mingw builds with: llvm-strip: error: '../../dist/firefox/uninstall/helper.exe': RVA 0x43000 for base reloc table found but data is incomplete

That helper.exe file is produced by NSIS.

Can you upload that binary somewhere?

[glandium]

In D126898#3565873, @mstorsjo wrote:

Can you upload that binary somewhere?

This is not the exact binary this happened with, but it's reproducible with this one:

• download https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/CVYLgJ7JTzKIZqeMnxZaXA/runs/0/artifacts/public/build/target.zip
• unzip firefox/uninstall/helper.exe from that zip
• run llvm-strip on it.

[mstorsjo]

In D126898#3566111, @glandium wrote:

In D126898#3565873, @mstorsjo wrote:

Can you upload that binary somewhere?

This is not the exact binary this happened with, but it's reproducible with this one:

• download https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/CVYLgJ7JTzKIZqeMnxZaXA/runs/0/artifacts/public/build/target.zip
• unzip firefox/uninstall/helper.exe from that zip
• run llvm-strip on it.

Thanks! I can see what's happening here. Unfortunately, it seems like that binary is somewhat broken, and libObject used to accept it silently before.

Looking at the binary with llvm-readobj --sections --file-headers, I see this:

  BaseRelocationTableRVA: 0x43000
  BaseRelocationTableSize: 0xB68

Section {
  Number: 4
  Name: .ndata (2E 6E 64 61 74 61 00 00)
  VirtualSize: 0x1C000
  VirtualAddress: 0x40000
  RawDataSize: 512

Section {
  Number: 6
  Name: .reloc (2E 72 65 6C 6F 63 00 00)
  VirtualSize: 0xB68
  VirtualAddress: 0x64000
  RawDataSize: 3072

The base relocation table usually resides in the .reloc section, and as the exact size (0xB68) matches, it looks like it just has got the wrong RVA in the header. With the new, more thorough checks, libObject notices that the RVA points into the .ndata section (within 0x40000 - 0x5C000) but there's only raw data covering 0x40000 - 0x40200, and 0x43000 is outside of that. Previously we'd just set up a pointer into bogus/out of range data in these cases, now we error out.

So the file is clearly buggy here, but it could, in some cases, be quite valid to have RVAs pointing outside of the allocated raw data. (Normally, the data size outside of RawDataSize up until VirtualSize is allocated and readable at runtime, but zero-initialized instead of initialized with data from the file.)

In this case, the check was added to avoid crashes, and for the specific case of SizeOfRawData == 0 we return SectionStrippedError, which then is silently treated as not an error (but not initializing e.g. the BaseRelocHeader pointer, to avoid trying to read from it).

To avoid crashing from RVAs outside of the allocated bounds, but still not flat out erroring out on binaries that are somewhat broken (the same issue causes llvm-readobj to reject the binary, so I had to use an older llvm-readobj binary to look at it), should we just treat all those cases as a silent "section stripped" case?

E.g. this fixes the issue:

diff --git a/llvm/lib/Object/COFFObjectFile.cpp b/llvm/lib/Object/COFFObjectFile.cpp
index 65166b3710a0..f7d957ee2682 100644
--- a/llvm/lib/Object/COFFObjectFile.cpp
+++ b/llvm/lib/Object/COFFObjectFile.cpp
@@ -483,18 +483,12 @@ Error COFFObjectFile::getRvaPtr(uint32_t Addr, uintptr_t &Res,
       // fail, otherwise it will be impossible to use this object as debug info
       // in LLDB. Return SectionStrippedError here so that
       // COFFObjectFile::initialize can ignore the error.
-      if (Section->SizeOfRawData == 0)
-        return make_error<SectionStrippedError>();
+      // Somewhat common binaries may have RVAs pointing outside of the
+      // provided raw data. Instead of rejecting the binaries, just
+      // treat the section as stripped for these purposes.
       if (Section->SizeOfRawData < Section->VirtualSize &&
           Addr >= SectionStart + Section->SizeOfRawData) {
-        if (ErrorContext)
-          return createStringError(object_error::parse_failed,
-                                   "RVA 0x%" PRIx32
-                                   " for %s found but data is incomplete",
-                                   Addr, ErrorContext);
-        return createStringError(
-            object_error::parse_failed,
-            "RVA 0x%" PRIx32 " found but data is incomplete", Addr);
+        return make_error<SectionStrippedError>();
       }
       uint32_t Offset = Addr - SectionStart;
       Res = reinterpret_cast<uintptr_t>(base()) + Section->PointerToRawData +

[alvinhochun]

Yeah, that seems fine as a workaround. https://docs.microsoft.com/en-us/windows/win32/debug/pe-format#section-table-section-headers says:

If this (VirtualSize) value is greater than SizeOfRawData, the section is zero-padded.

This means the RVA should technically be pointing to a block of zeros. If we pretend the section does not exist at all, it is probably still good enough, right?

But I wonder if the code should always just use the .reloc section and ignore the Base Relocation Table RVA if it points to somewhere odd?

[mstorsjo]

In D126898#3566346, @alvinhochun wrote:

Yeah, that seems fine as a workaround. https://docs.microsoft.com/en-us/windows/win32/debug/pe-format#section-table-section-headers says:

If this (VirtualSize) value is greater than SizeOfRawData, the section is zero-padded.

This means the RVA should technically be pointing to a block of zeros. If we pretend the section does not exist at all, it is probably still good enough, right?

Yep, that's probably good enough. For some tables, I guess it could be remotely useful to pretend it exists and allocate a temporary zero block to point into. (Previously you would have gotten random data from the neighbouring sections instead.) But that sounds like something to consider if that actually turns out to be needed...

But I wonder if the code should always just use the .reloc section and ignore the Base Relocation Table RVA if it points to somewhere odd?

No, I think it's safer to use the data directories - that'd be what windows itself uses. After linking, the section names are generally mostly cosmetic. Some things stay in their own sections up until linking but get merged into e.g. the .rdata section when linked. (For dwarf unwind info in PE/COFF we do have a hack in libunwind though - libunwind introspects the modules and locates the section named .eh_frame at runtime. Due to how PE/COFF string tables and all that works, the section name is truncated to .eh_fram in practice.)

[alvinhochun]

I guess you're right. MSVC's dumpbin /relocations helper.exe listed nothing at all.

[glandium]

The base relocation table usually resides in the .reloc section, and as the exact size (0xB68) matches, it looks like it just has got the wrong RVA in the header. With the new, more thorough checks, libObject notices that the RVA points into the .ndata section (within 0x40000 - 0x5C000) but there's only raw data covering 0x40000 - 0x40200, and 0x43000 is outside of that. Previously we'd just set up a pointer into bogus/out of range data in these cases, now we error out.

From a cursory look, it seems to come from NSIS itself not handing BaseRelocationTableRVA. Its prebuild binaries come with stubs that actually have it set to 0 and thus don't have a .reloc section. The stubs we're using in the case of that helper.exe were compiled with mingw-clang and linked with mingw-lld, and do contain a .reloc section. The helper.exe binary ends up having the same BaseRelocationTableRVA as the stub, but NSIS moved the .reloc section, so it's not correct anymore.

So... maybe you shouldn't actually relax things as per D127345, because it seems like a genuine bug in NSIS that should be fixed rather than worked around.

[glandium]

So, looking a little bit deeper, the resource editor in NSIS doesn't handle relocations, and the .reloc section is right after the .rsrc section that the resource editor will grow... So they actually have a build rule to pass /FIXED to link.exe when building their stubs with MSVC. I suppose mingw-binutils defaults to non-relocatable .exes (because the problem doesn't appear when building with mingw-gcc/mingw-binutils), and we only end up with relocatable .exes because we're building with mingw-clang/lld and lld defaults to relocatable .exes.
I'd call this a build system bug.

[mstorsjo]

In D126898#3571941, @glandium wrote:

The base relocation table usually resides in the .reloc section, and as the exact size (0xB68) matches, it looks like it just has got the wrong RVA in the header. With the new, more thorough checks, libObject notices that the RVA points into the .ndata section (within 0x40000 - 0x5C000) but there's only raw data covering 0x40000 - 0x40200, and 0x43000 is outside of that. Previously we'd just set up a pointer into bogus/out of range data in these cases, now we error out.

From a cursory look, it seems to come from NSIS itself not handing BaseRelocationTableRVA. Its prebuild binaries come with stubs that actually have it set to 0 and thus don't have a .reloc section. The stubs we're using in the case of that helper.exe were compiled with mingw-clang and linked with mingw-lld, and do contain a .reloc section. The helper.exe binary ends up having the same BaseRelocationTableRVA as the stub, but NSIS moved the .reloc section, so it's not correct anymore.

Ouch, that does indeed sound like something that should be fixed.

(I don’t remember offhand, but it might be possible to omit the reloc section for an executable by linking with ‘-Wl,-Xlink=-fixed` and maybe -Wl,--disable-dynamicbase. Not sure if it’s actually left out, or if it is just not used.)

Fixing NSIS definitely sounds like something that should be done here.

So... maybe you shouldn't actually relax things as per D127345, because it seems like a genuine bug in NSIS that should be fixed rather than worked around.

Well, while the NSIS bug is real, and it’s great that the restrictive object library caught it, I think we still should go with that patch too anyway - the PE executable isn’t entirely broken structurally, the table just lies in zero-initialized, allocated memory. And that lets the llvm tools operate on files that are potentially valid, instead of flat out rejecting them.

@rnk - what’s your take on this?

[glandium]

Passing -Wl,/fixed as a linker flag when building the NSIS stubs fixes the issue, FWIW.

[mstorsjo]

In D126898#3572344, @glandium wrote:

So, looking a little bit deeper, the resource editor in NSIS doesn't handle relocations, and the .reloc section is right after the .rsrc section that the resource editor will grow... So they actually have a build rule to pass /FIXED to link.exe when building their stubs with MSVC. I suppose mingw-binutils defaults to non-relocatable .exes (because the problem doesn't appear when building with mingw-gcc/mingw-binutils), and we only end up with relocatable .exes because we're building with mingw-clang/lld and lld defaults to relocatable .exes.

Actually, binutils 2.36 and newer also create a reloc section by default now, since this commit: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=514b4e191d5f46de8e142fe216e677a35fa9c4bb;hp=f2e5245f4169c2a1849ba998872e245c1b303950

It can be disabled by passing -Wl,--disable-reloc-section though.

In D126898#3572414, @glandium wrote:

Passing -Wl,/fixed as a linker flag when building the NSIS stubs fixes the issue, FWIW.

It seems like we should implement the --disable-reloc-section in the LLD mingw linker layer then, and pass it onto the lld-link interface as /fixed.

(I'm somewhat amazed that you can pass options directly via the mingw linker layer to the lld-link layer like that. The mingw options interface considers /fixed as an input path name and just passes it on to lld-link, which then considers it an option. The kosher way of doing it is -Wl,-Xlink=/fixed (or -Wl,-Xlink=-fixed if you prefer that form), but that's a bit unwieldy and clumsy. And for any case where one really wants to pass such options outside of just one off testing, we should add a proper mingw linker option for it.)

llvm/include/llvm/Object/COFF.h

struct FpoData {
 
   // fUseBP: true if EBP has been allocated
   bool useBP() const { return (Attributes >> 10) & 1; }
 
   // cbFrame: frame pointer
   frame_type getFP() const { return static_cast<frame_type>(Attributes >> 14); }
 };
 
+class SectionStrippedError
+    : public ErrorInfo<SectionStrippedError, BinaryError> {
+public:
+  SectionStrippedError() { setErrorCode(object_error::section_stripped); }
+};
+
 } // end namespace object
 
 } // end namespace llvm
 
 #endif // LLVM_OBJECT_COFF_H

llvm/include/llvm/Object/Error.h

enum class object_error {
   arch_not_found = 1,
   invalid_file_type,
   parse_failed,
   unexpected_eof,
   string_table_non_null_end,
   invalid_section_index,
   bitcode_section_not_found,
   invalid_symbol_index,
+  section_stripped,
 };
 
 inline std::error_code make_error_code(object_error e) {
   return std::error_code(static_cast<int>(e), object_category());
 }
 
 /// Base class for all errors indicating malformed binary files.
 ///

llvm/lib/Object/COFFObjectFile.cpp

 // Returns the file offset for the given RVA.
 Error COFFObjectFile::getRvaPtr(uint32_t Addr, uintptr_t &Res,
                                 const char *ErrorContext) const {
   for (const SectionRef &S : sections()) {
     const coff_section *Section = getCOFFSection(S);
     uint32_t SectionStart = Section->VirtualAddress;
     uint32_t SectionEnd = Section->VirtualAddress + Section->VirtualSize;
     if (SectionStart <= Addr && Addr < SectionEnd) {
+      // A table/directory entry can be pointing to somewhere in a stripped
+      // section, in an object that went through `objcopy --only-keep-debug`.
+      // In this case we don't want to cause the parsing of the object file to
+      // fail, otherwise it will be impossible to use this object as debug info
+      // in LLDB. Return SectionStrippedError here so that
+      // COFFObjectFile::initialize can ignore the error.
+      if (Section->SizeOfRawData == 0)
+        return make_error<SectionStrippedError>();
+      if (Section->SizeOfRawData < Section->VirtualSize &&
+          Addr >= SectionStart + Section->SizeOfRawData) {
+        if (ErrorContext)
+          return createStringError(object_error::parse_failed,
+                                   "RVA 0x%" PRIx32
+                                   " for %s found but data is incomplete",
+                                   Addr, ErrorContext);
+        return createStringError(
+            object_error::parse_failed,
+            "RVA 0x%" PRIx32 " found but data is incomplete", Addr);
+      }
       uint32_t Offset = Addr - SectionStart;
       Res = reinterpret_cast<uintptr_t>(base()) + Section->PointerToRawData +
             Offset;
       return Error::success();
     }
   }
   if (ErrorContext)
     return createStringError(object_error::parse_failed,
Error COFFObjectFile::initDelayImportTablePtr() {
 
   uint32_t RVA = DataEntry->RelativeVirtualAddress;
   NumberOfDelayImportDirectory = DataEntry->Size /
       sizeof(delay_import_directory_table_entry) - 1;
 
   uintptr_t IntPtr = 0;
   if (Error E = getRvaPtr(RVA, IntPtr, "delay import table"))
     return E;
+  if (Error E = checkOffset(Data, IntPtr, DataEntry->Size))
+    return E;
+
   DelayImportDirectory = reinterpret_cast<
       const delay_import_directory_table_entry *>(IntPtr);
   return Error::success();
 }
 
 // Find the export table.
 Error COFFObjectFile::initExportTablePtr() {
   // First, we get the RVA of the export table. If the file lacks a pointer to
   // the export table, do nothing.
   const data_directory *DataEntry = getDataDirectory(COFF::EXPORT_TABLE);
   if (!DataEntry)
     return Error::success();
 
   // Do nothing if the pointer to export table is NULL.
   if (DataEntry->RelativeVirtualAddress == 0)
     return Error::success();
 
   uint32_t ExportTableRva = DataEntry->RelativeVirtualAddress;
   uintptr_t IntPtr = 0;
   if (Error E = getRvaPtr(ExportTableRva, IntPtr, "export table"))
     return E;
+  if (Error E = checkOffset(Data, IntPtr, DataEntry->Size))
+    return E;
+
   ExportDirectory =
       reinterpret_cast<const export_directory_table_entry *>(IntPtr);
   return Error::success();
 }
 
 Error COFFObjectFile::initBaseRelocPtr() {
   const data_directory *DataEntry =
       getDataDirectory(COFF::BASE_RELOCATION_TABLE);
   if (!DataEntry)
     return Error::success();
   if (DataEntry->RelativeVirtualAddress == 0)
     return Error::success();
 
   uintptr_t IntPtr = 0;
   if (Error E = getRvaPtr(DataEntry->RelativeVirtualAddress, IntPtr,
                           "base reloc table"))
     return E;
+  if (Error E = checkOffset(Data, IntPtr, DataEntry->Size))
+    return E;
+
   BaseRelocHeader = reinterpret_cast<const coff_base_reloc_block_header *>(
       IntPtr);
   BaseRelocEnd = reinterpret_cast<coff_base_reloc_block_header *>(
       IntPtr + DataEntry->Size);
   // FIXME: Verify the section containing BaseRelocHeader has at least
   // DataEntry->Size bytes after DataEntry->RelativeVirtualAddress.
   return Error::success();
 }
Error COFFObjectFile::initDebugDirectoryPtr() {
   if (DataEntry->Size % sizeof(debug_directory) != 0)
     return createStringError(object_error::parse_failed,
                              "debug directory has uneven size");
 
   uintptr_t IntPtr = 0;
   if (Error E = getRvaPtr(DataEntry->RelativeVirtualAddress, IntPtr,
                           "debug directory"))
     return E;
+  if (Error E = checkOffset(Data, IntPtr, DataEntry->Size))
+    return E;
+
   DebugDirectoryBegin = reinterpret_cast<const debug_directory *>(IntPtr);
   DebugDirectoryEnd = reinterpret_cast<const debug_directory *>(
       IntPtr + DataEntry->Size);
   // FIXME: Verify the section containing DebugDirectoryBegin has at least
   // DataEntry->Size bytes after DataEntry->RelativeVirtualAddress.
   return Error::success();
 }
 
return createStringError(
         object_error::parse_failed,
         "TLS Directory size (%u) is not the expected size (%" PRIu64 ").",
         static_cast<uint32_t>(DataEntry->Size), DirSize);
 
   uintptr_t IntPtr = 0;
   if (Error E =
           getRvaPtr(DataEntry->RelativeVirtualAddress, IntPtr, "TLS directory"))
     return E;
+  if (Error E = checkOffset(Data, IntPtr, DataEntry->Size))
+    return E;
 
   if (is64())
     TLSDirectory64 = reinterpret_cast<const coff_tls_directory64 *>(IntPtr);
   else
     TLSDirectory32 = reinterpret_cast<const coff_tls_directory32 *>(IntPtr);
 
   return Error::success();
 }
 
 Error COFFObjectFile::initLoadConfigPtr() {
   // Get the RVA of the debug directory. Do nothing if it does not exist.
   const data_directory *DataEntry = getDataDirectory(COFF::LOAD_CONFIG_TABLE);
   if (!DataEntry)
     return Error::success();
 
   // Do nothing if the RVA is NULL.
   if (DataEntry->RelativeVirtualAddress == 0)
     return Error::success();
   uintptr_t IntPtr = 0;
   if (Error E = getRvaPtr(DataEntry->RelativeVirtualAddress, IntPtr,
                           "load config table"))
     return E;
+  if (Error E = checkOffset(Data, IntPtr, DataEntry->Size))
+    return E;
 
   LoadConfig = (const void *)IntPtr;
   return Error::success();
 }
 
 Expected<std::unique_ptr<COFFObjectFile>>
 COFFObjectFile::create(MemoryBufferRef Object) {
   std::unique_ptr<COFFObjectFile> Obj(new COFFObjectFile(std::move(Object)));
   if (Error E = Obj->initialize())
     return std::move(E);
   return std::move(Obj);
 }
 
 COFFObjectFile::COFFObjectFile(MemoryBufferRef Object)
     : ObjectFile(Binary::ID_COFF, Object), COFFHeader(nullptr),
       COFFBigObjHeader(nullptr), PE32Header(nullptr), PE32PlusHeader(nullptr),
       DataDirectory(nullptr), SectionTable(nullptr), SymbolTable16(nullptr),
       SymbolTable32(nullptr), StringTable(nullptr), StringTableSize(0),
       ImportDirectory(nullptr), DelayImportDirectory(nullptr),
       NumberOfDelayImportDirectory(0), ExportDirectory(nullptr),
       BaseRelocHeader(nullptr), BaseRelocEnd(nullptr),
       DebugDirectoryBegin(nullptr), DebugDirectoryEnd(nullptr),
       TLSDirectory32(nullptr), TLSDirectory64(nullptr) {}
 
+static Error ignoreStrippedErrors(Error E) {
+  if (E.isA<SectionStrippedError>()) {
+    consumeError(std::move(E));
+    return Error::success();
+  }
+  return std::move(E);
+}
+
 Error COFFObjectFile::initialize() {
   // Check that we at least have enough room for a header.
   std::error_code EC;
   if (!checkSize(Data, EC, sizeof(coff_file_header)))
     return errorCodeToError(EC);
 
   // The current location in the file where we are looking at.
   uint64_t CurPtr = 0;
if (getPointerToSymbolTable() != 0) {
     // We had better not have any symbols if we don't have a symbol table.
     if (getNumberOfSymbols() != 0) {
       return createStringError(object_error::parse_failed,
                                "symbol table missing");
     }
   }
 
   // Initialize the pointer to the beginning of the import table.
-  if (Error E = initImportTablePtr())
+  if (Error E = ignoreStrippedErrors(initImportTablePtr()))
     return E;
-  if (Error E = initDelayImportTablePtr())
+  if (Error E = ignoreStrippedErrors(initDelayImportTablePtr()))
     return E;
 
   // Initialize the pointer to the export table.
-  if (Error E = initExportTablePtr())
+  if (Error E = ignoreStrippedErrors(initExportTablePtr()))
     return E;
 
   // Initialize the pointer to the base relocation table.
-  if (Error E = initBaseRelocPtr())
+  if (Error E = ignoreStrippedErrors(initBaseRelocPtr()))
     return E;
 
   // Initialize the pointer to the debug directory.
-  if (Error E = initDebugDirectoryPtr())
+  if (Error E = ignoreStrippedErrors(initDebugDirectoryPtr()))
     return E;
 
   // Initialize the pointer to the TLS directory.
-  if (Error E = initTLSDirectoryPtr())
+  if (Error E = ignoreStrippedErrors(initTLSDirectoryPtr()))
     return E;
 
-  if (Error E = initLoadConfigPtr())
+  if (Error E = ignoreStrippedErrors(initLoadConfigPtr()))
     return E;
 
   return Error::success();
 }
 
 basic_symbol_iterator COFFObjectFile::symbol_begin() const {
   DataRefImpl Ret;
   Ret.p = getSymbolTable();

llvm/lib/Object/Error.cpp

std::string _object_error_category::message(int EV) const {
   case object_error::string_table_non_null_end:
     return "String table must end with a null terminator";
   case object_error::invalid_section_index:
     return "Invalid section index";
   case object_error::bitcode_section_not_found:
     return "Bitcode section not found in object file";
   case object_error::invalid_symbol_index:
     return "Invalid symbol index";
+  case object_error::section_stripped:
+    return "Section has been stripped from the object file";
   }
   llvm_unreachable("An enumerator of object_error does not have a message "
                    "defined.");
 }
 
 void BinaryError::anchor() {}
 char BinaryError::ID = 0;
 char GenericBinaryError::ID = 0;

llvm/test/Object/Inputs/COFF/dwarf-debug-only.yaml

+--- !COFF
+OptionalHeader:
+  AddressOfEntryPoint: 4512
+  ImageBase:       6442450944
+  SectionAlignment: 4096
+  FileAlignment:   512
+  MajorOperatingSystemVersion: 6
+  MinorOperatingSystemVersion: 0
+  MajorImageVersion: 0
+  MinorImageVersion: 0
+  MajorSubsystemVersion: 6
+  MinorSubsystemVersion: 0
+  Subsystem:       IMAGE_SUBSYSTEM_WINDOWS_GUI
+  DLLCharacteristics: [ IMAGE_DLL_CHARACTERISTICS_HIGH_ENTROPY_VA, IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE, IMAGE_DLL_CHARACTERISTICS_NX_COMPAT ]
+  SizeOfStackReserve: 1048576
+  SizeOfStackCommit: 4096
+  SizeOfHeapReserve: 1048576
+  SizeOfHeapCommit: 4096
+  ExportTable:
+    RelativeVirtualAddress: 6962976
+    Size:            75
+  ImportTable:
+    RelativeVirtualAddress: 6963051
+    Size:            420
+  ResourceTable:
+    RelativeVirtualAddress: 0
+    Size:            0
+  ExceptionTable:
+    RelativeVirtualAddress: 6995968
+    Size:            1812
+  CertificateTable:
+    RelativeVirtualAddress: 0
+    Size:            0
+  BaseRelocationTable:
+    RelativeVirtualAddress: 7004160
+    Size:            156
+  Debug:
+    RelativeVirtualAddress: 6987776
+    Size:            28
+  Architecture:
+    RelativeVirtualAddress: 0
+    Size:            0
+  GlobalPtr:
+    RelativeVirtualAddress: 0
+    Size:            0
+  TlsTable:
+    RelativeVirtualAddress: 6962168
+    Size:            40
+  LoadConfigTable:
+    RelativeVirtualAddress: 0
+    Size:            0
+  BoundImport:
+    RelativeVirtualAddress: 0
+    Size:            0
+  IAT:
+    RelativeVirtualAddress: 6965496
+    Size:            2024
+  DelayImportDescriptor:
+    RelativeVirtualAddress: 0
+    Size:            0
+  ClrRuntimeHeader:
+    RelativeVirtualAddress: 0
+    Size:            0
+header:
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_LARGE_ADDRESS_AWARE, IMAGE_FILE_DLL ]
+sections:
+  - Name:            .text
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    VirtualAddress:  4096
+    VirtualSize:     59030
+    SectionData:     ''
+  - Name:            .rdata
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    VirtualAddress:  65536
+    VirtualSize:     6919008
+    SectionData:     ''
+  - Name:            .buildid
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    VirtualAddress:  6987776
+    VirtualSize:     53
+    SectionData:     00000000A9D795620000000002000000190000001CA06A001C040000525344535450075756EAA5DB4C4C44205044422E0100000000
+  - Name:            .data
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ, IMAGE_SCN_MEM_WRITE ]
+    VirtualAddress:  6991872
+    VirtualSize:     1960
+    SectionData:     ''
+  - Name:            .pdata
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    VirtualAddress:  6995968
+    VirtualSize:     1812
+    SectionData:     ''
+  - Name:            .tls
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ, IMAGE_SCN_MEM_WRITE ]
+    VirtualAddress:  7000064
+    VirtualSize:     16
+    SectionData:     ''
+  - Name:            .reloc
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
+    VirtualAddress:  7004160
+    VirtualSize:     156
+    SectionData:     ''
+  - Name:            .debug_info
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
+    VirtualAddress:  7008256
+    VirtualSize:     64
+    SectionData:     DEADBEEFBAADF00D
+symbols: []
+...

llvm/test/Object/coff-dwarf-debug-only.test

+RUN: yaml2obj %p/Inputs/COFF/dwarf-debug-only.yaml | llvm-objdump -p -h - | FileCheck %s
+
+CHECK:      The Data Directory
+CHECK-NEXT: Entry 0 00000000006a3f20 0000004b Export Directory [.edata (or where ever we found it)]
+CHECK-NEXT: Entry 1 00000000006a3f6b 000001a4 Import Directory [parts of .idata]
+CHECK-NEXT: Entry 2 0000000000000000 00000000 Resource Directory [.rsrc]
+CHECK-NEXT: Entry 3 00000000006ac000 00000714 Exception Directory [.pdata]
+CHECK-NEXT: Entry 4 0000000000000000 00000000 Security Directory
+CHECK-NEXT: Entry 5 00000000006ae000 0000009c Base Relocation Directory [.reloc]
+CHECK-NEXT: Entry 6 00000000006aa000 0000001c Debug Directory
+CHECK-NEXT: Entry 7 0000000000000000 00000000 Description Directory
+CHECK-NEXT: Entry 8 0000000000000000 00000000 Special Directory
+CHECK-NEXT: Entry 9 00000000006a3bf8 00000028 Thread Storage Directory [.tls]
+CHECK-NEXT: Entry a 0000000000000000 00000000 Load Configuration Directory
+CHECK-NEXT: Entry b 0000000000000000 00000000 Bound Import Directory
+CHECK-NEXT: Entry c 00000000006a48f8 000007e8 Import Address Table Directory
+CHECK-NEXT: Entry d 0000000000000000 00000000 Delay Import Directory
+CHECK-NEXT: Entry e 0000000000000000 00000000 CLR Runtime Header
+CHECK-NEXT: Entry f 0000000000000000 00000000 Reserved
+
+CHECK:      0 .text         00000000 0000000180001000 TEXT
+CHECK-NEXT: 1 .rdata        00000000 0000000180010000 DATA
+CHECK-NEXT: 2 .buildid      00000035 00000001806aa000 DATA
+CHECK-NEXT: 3 .data         00000000 00000001806ab000 DATA
+CHECK-NEXT: 4 .pdata        00000000 00000001806ac000 DATA
+CHECK-NEXT: 5 .tls          00000000 00000001806ad000 DATA
+CHECK-NEXT: 6 .reloc        00000000 00000001806ae000 DATA
+CHECK-NEXT: 7 .debug_info   00000040 00000001806af000 DATA, DEBUG

llvm/tools/llvm-objdump/COFFDump.cpp

 static void printTLSDirectory(const COFFObjectFile *Obj) {
   const pe32_header *PE32Header = Obj->getPE32Header();
   const pe32plus_header *PE32PlusHeader = Obj->getPE32PlusHeader();
 
   // Skip if it's not executable.
   if (!PE32Header && !PE32PlusHeader)
     return;
 
-  const data_directory *DataDir = Obj->getDataDirectory(COFF::TLS_TABLE);
-  if (!DataDir || DataDir->RelativeVirtualAddress == 0)
-    return;
-
-  uintptr_t IntPtr = 0;
-  if (Error E =
-          Obj->getRvaPtr(DataDir->RelativeVirtualAddress, IntPtr))
-    reportError(std::move(E), Obj->getFileName());
-
   if (PE32Header) {
-    auto *TLSDir = reinterpret_cast<const coff_tls_directory32 *>(IntPtr);
+    if (auto *TLSDir = Obj->getTLSDirectory32())
       printTLSDirectoryT(TLSDir);
   } else {
-    auto *TLSDir = reinterpret_cast<const coff_tls_directory64 *>(IntPtr);
+    if (auto *TLSDir = Obj->getTLSDirectory64())
       printTLSDirectoryT(TLSDir);
   }
 
   outs() << "\n";
 }
 
 static void printLoadConfiguration(const COFFObjectFile *Obj) {
   // Skip if it's not executable.
   if (!Obj->getPE32Header())
     return;
 
   // Currently only x86 is supported
   if (Obj->getMachine() != COFF::IMAGE_FILE_MACHINE_I386)
     return;
 
-  const data_directory *DataDir = Obj->getDataDirectory(COFF::LOAD_CONFIG_TABLE);
-  if (!DataDir)
-    reportError("no load config data dir", Obj->getFileName());
-
-  uintptr_t IntPtr = 0;
-  if (DataDir->RelativeVirtualAddress == 0)
+  auto *LoadConf = Obj->getLoadConfig32();
+  if (!LoadConf)
     return;
 
-  if (Error E =
-          Obj->getRvaPtr(DataDir->RelativeVirtualAddress, IntPtr))
-    reportError(std::move(E), Obj->getFileName());
-
-  auto *LoadConf = reinterpret_cast<const coff_load_configuration32 *>(IntPtr);
   outs() << "Load configuration:"
          << "\n  Timestamp: " << LoadConf->TimeDateStamp
          << "\n  Major Version: " << LoadConf->MajorVersion
          << "\n  Minor Version: " << LoadConf->MinorVersion
          << "\n  GlobalFlags Clear: " << LoadConf->GlobalFlagsClear
          << "\n  GlobalFlags Set: " << LoadConf->GlobalFlagsSet
          << "\n  Critical Section Default Timeout: " << LoadConf->CriticalSectionDefaultTimeout
          << "\n  Decommit Free Block Threshold: " << LoadConf->DeCommitFreeBlockThreshold
for (const ImportDirectoryEntryRef &DirRef : Obj->import_directories()) {
     }
     outs() << "\n";
   }
 }
 
 // Prints export tables. The export table is a table containing the list of
 // exported symbol from the DLL.
 static void printExportTable(const COFFObjectFile *Obj) {
-  outs() << "Export Table:\n";
   export_directory_iterator I = Obj->export_directory_begin();
   export_directory_iterator E = Obj->export_directory_end();
   if (I == E)
     return;
+  outs() << "Export Table:\n";
   StringRef DllName;
   uint32_t OrdinalBase;
   if (I->getDllName(DllName))
     return;
   if (I->getOrdinalBase(OrdinalBase))
     return;
   outs() << " DLL name: " << DllName << "\n";
   outs() << " Ordinal base: " << OrdinalBase << "\n";