[lldb][COFF] Rewrite ParseSymtab to list both export and symbol tables

This reimplements `ObjectFilePECOFF::ParseSymtab` to replace the manual
data extraction with what `COFFObjectFile` already provides. Also use
`SymTab::AddSymbol` instead of resizing the SymTab then assigning each
elements afterwards.

Previously, ParseSymTab loads symbols from both the COFF symbol table
and the export table, but if there are any entries in the export table,
it overwrites all the symbols already loaded from the COFF symbol table.
Due to the change to use AddSymbols, this no longer happens, and so the
SymTab now contains all symbols from both tables as expected.

The export symbols are now ordered by ordinal, instead of by the name
table order.

In its current state, it is possible for symbols in the COFF symbol
table to be duplicated by those in the export table. This behaviour will
be modified in a separate change.

Reviewed By: labath

Differential Revision: https://reviews.llvm.org/D134196

1 files changed, 76 insertions, 113 deletions

diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
index 5e7a921..3890e8b 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -30,11 +30,12 @@
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/Timer.h"
 #include "lldb/Utility/UUID.h"
-#include "llvm/BinaryFormat/COFF.h"
 
+#include "llvm/BinaryFormat/COFF.h"
 #include "llvm/Object/COFFImportFile.h"
 #include "llvm/Support/CRC.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/FormatAdapters.h"
 #include "llvm/Support/Host.h"
 #include "llvm/Support/MemoryBuffer.h"
 
@@ -760,131 +761,93 @@ llvm::StringRef ObjectFilePECOFF::GetSectionName(const section_header_t &sect) {
 
 void ObjectFilePECOFF::ParseSymtab(Symtab &symtab) {
   SectionList *sect_list = GetSectionList();
-  const uint32_t num_syms = m_coff_header.nsyms;
-  if (m_file && num_syms > 0 && m_coff_header.symoff > 0) {
-    const uint32_t symbol_size = 18;
-    const size_t symbol_data_size = num_syms * symbol_size;
-    // Include the 4-byte string table size at the end of the symbols
-    DataExtractor symtab_data =
-        ReadImageData(m_coff_header.symoff, symbol_data_size + 4);
-    lldb::offset_t offset = symbol_data_size;
-    const uint32_t strtab_size = symtab_data.GetU32(&offset);
-    if (strtab_size > 0) {
-      DataExtractor strtab_data = ReadImageData(
-          m_coff_header.symoff + symbol_data_size, strtab_size);
-
-      offset = 0;
-      std::string symbol_name;
-      Symbol *symbols = symtab.Resize(num_syms);
-      for (uint32_t i = 0; i < num_syms; ++i) {
-        coff_symbol_t symbol;
-        const uint32_t symbol_offset = offset;
-        const char *symbol_name_cstr = nullptr;
-        // If the first 4 bytes of the symbol string are zero, then they
-        // are followed by a 4-byte string table offset. Else these
-        // 8 bytes contain the symbol name
-        if (symtab_data.GetU32(&offset) == 0) {
-          // Long string that doesn't fit into the symbol table name, so
-          // now we must read the 4 byte string table offset
-          uint32_t strtab_offset = symtab_data.GetU32(&offset);
-          symbol_name_cstr = strtab_data.PeekCStr(strtab_offset);
-          symbol_name.assign(symbol_name_cstr);
-        } else {
-          // Short string that fits into the symbol table name which is 8
-          // bytes
-          offset += sizeof(symbol.name) - 4; // Skip remaining
-          symbol_name_cstr = symtab_data.PeekCStr(symbol_offset);
-          if (symbol_name_cstr == nullptr)
-            break;
-          symbol_name.assign(symbol_name_cstr, sizeof(symbol.name));
-        }
-        symbol.value = symtab_data.GetU32(&offset);
-        symbol.sect = symtab_data.GetU16(&offset);
-        symbol.type = symtab_data.GetU16(&offset);
-        symbol.storage = symtab_data.GetU8(&offset);
-        symbol.naux = symtab_data.GetU8(&offset);
-        symbols[i].GetMangled().SetValue(ConstString(symbol_name.c_str()));
-        if ((int16_t)symbol.sect >= 1) {
-          Address symbol_addr(sect_list->FindSectionByID(symbol.sect),
-                              symbol.value);
-          symbols[i].GetAddressRef() = symbol_addr;
-          symbols[i].SetType(MapSymbolType(symbol.type));
-        }
+  AppendFromExportTable(sect_list, symtab);
+  AppendFromCOFFSymbolTable(sect_list, symtab);
+}
 
-        if (symbol.naux > 0) {
-          i += symbol.naux;
-          offset += symbol.naux * symbol_size;
-        }
-      }
+void ObjectFilePECOFF::AppendFromCOFFSymbolTable(SectionList *sect_list,
+                                                 Symtab &symtab) {
+  const uint32_t num_syms = m_binary->getNumberOfSymbols();
+  if (num_syms == 0)
+    return;
+  // Check that this is not a bigobj; we do not support bigobj.
+  if (m_binary->getSymbolTableEntrySize() !=
+      sizeof(llvm::object::coff_symbol16))
+    return;
+
+  Log *log = GetLog(LLDBLog::Object);
+  symtab.Reserve(symtab.GetNumSymbols() + num_syms);
+  for (const auto &sym_ref : m_binary->symbols()) {
+    const auto coff_sym_ref = m_binary->getCOFFSymbol(sym_ref);
+    auto name_or_error = sym_ref.getName();
+    if (auto err = name_or_error.takeError()) {
+      LLDB_LOG(log,
+               "ObjectFilePECOFF::AppendFromCOFFSymbolTable - failed to get "
+               "symbol table entry name: {0}",
+               llvm::fmt_consume(std::move(err)));
+      continue;
+    }
+    const llvm::StringRef sym_name = *name_or_error;
+    Symbol symbol;
+    symbol.GetMangled().SetValue(ConstString(sym_name));
+    int16_t section_number =
+        static_cast<int16_t>(coff_sym_ref.getSectionNumber());
+    if (section_number >= 1) {
+      symbol.GetAddressRef() = Address(
+          sect_list->FindSectionByID(section_number), coff_sym_ref.getValue());
+      symbol.SetType(MapSymbolType(coff_sym_ref.getType()));
     }
+    symtab.AddSymbol(symbol);
   }
+}
 
-  // Read export header
-  if (coff_data_dir_export_table < m_coff_header_opt.data_dirs.size() &&
-      m_coff_header_opt.data_dirs[coff_data_dir_export_table].vmsize > 0 &&
-      m_coff_header_opt.data_dirs[coff_data_dir_export_table].vmaddr > 0) {
-    export_directory_entry export_table;
-    uint32_t data_start =
-        m_coff_header_opt.data_dirs[coff_data_dir_export_table].vmaddr;
-
-    DataExtractor symtab_data = ReadImageDataByRVA(
-        data_start, m_coff_header_opt.data_dirs[0].vmsize);
-    lldb::offset_t offset = 0;
+void ObjectFilePECOFF::AppendFromExportTable(SectionList *sect_list,
+                                             Symtab &symtab) {
+  const auto *export_table = m_binary->getExportTable();
+  if (!export_table)
+    return;
+  const uint32_t num_syms = export_table->AddressTableEntries;
+  if (num_syms == 0)
+    return;
 
-    // Read export_table header
-    export_table.characteristics = symtab_data.GetU32(&offset);
-    export_table.time_date_stamp = symtab_data.GetU32(&offset);
-    export_table.major_version = symtab_data.GetU16(&offset);
-    export_table.minor_version = symtab_data.GetU16(&offset);
-    export_table.name = symtab_data.GetU32(&offset);
-    export_table.base = symtab_data.GetU32(&offset);
-    export_table.number_of_functions = symtab_data.GetU32(&offset);
-    export_table.number_of_names = symtab_data.GetU32(&offset);
-    export_table.address_of_functions = symtab_data.GetU32(&offset);
-    export_table.address_of_names = symtab_data.GetU32(&offset);
-    export_table.address_of_name_ordinals = symtab_data.GetU32(&offset);
-
-    bool has_ordinal = export_table.address_of_name_ordinals != 0;
-
-    lldb::offset_t name_offset = export_table.address_of_names - data_start;
-    lldb::offset_t name_ordinal_offset =
-        export_table.address_of_name_ordinals - data_start;
-
-    Symbol *symbols = symtab.Resize(export_table.number_of_names);
-
-    std::string symbol_name;
-
-    // Read each export table entry
-    for (size_t i = 0; i < export_table.number_of_names; ++i) {
-      uint32_t name_ordinal =
-          has_ordinal ? symtab_data.GetU16(&name_ordinal_offset) : i;
-      uint32_t name_address = symtab_data.GetU32(&name_offset);
-
-      const char *symbol_name_cstr =
-          symtab_data.PeekCStr(name_address - data_start);
-      symbol_name.assign(symbol_name_cstr);
-
-      lldb::offset_t function_offset = export_table.address_of_functions -
-                                        data_start +
-                                        sizeof(uint32_t) * name_ordinal;
-      uint32_t function_rva = symtab_data.GetU32(&function_offset);
-
-      Address symbol_addr(m_coff_header_opt.image_base + function_rva,
-                          sect_list);
-      symbols[i].GetMangled().SetValue(ConstString(symbol_name.c_str()));
-      symbols[i].GetAddressRef() = symbol_addr;
-      symbols[i].SetType(lldb::eSymbolTypeCode);
-      symbols[i].SetDebug(true);
+  Log *log = GetLog(LLDBLog::Object);
+  symtab.Reserve(symtab.GetNumSymbols() + num_syms);
+  // Read each export table entry, ordered by ordinal instead of by name.
+  for (const auto &entry : m_binary->export_directories()) {
+    llvm::StringRef sym_name;
+    if (auto err = entry.getSymbolName(sym_name)) {
+      LLDB_LOG(log,
+               "ObjectFilePECOFF::AppendFromExportTable - failed to get export "
+               "table entry name: {0}",
+               llvm::fmt_consume(std::move(err)));
+      continue;
+    }
+    Symbol symbol;
+    // Note: symbol name may be empty if it is only exported by ordinal.
+    symbol.GetMangled().SetValue(ConstString(sym_name));
+
+    uint32_t function_rva;
+    if (auto err = entry.getExportRVA(function_rva)) {
+      LLDB_LOG(log,
+               "ObjectFilePECOFF::AppendFromExportTable - failed to get "
+               "address of export entry '{0}': {1}",
+               sym_name, llvm::fmt_consume(std::move(err)));
+      continue;
     }
+    symbol.GetAddressRef() =
+        Address(m_coff_header_opt.image_base + function_rva, sect_list);
+    symbol.SetType(lldb::eSymbolTypeCode);
+    symbol.SetDebug(true);
+    symtab.AddSymbol(symbol);
   }
 }
 
 std::unique_ptr<CallFrameInfo> ObjectFilePECOFF::CreateCallFrameInfo() {
-  if (coff_data_dir_exception_table >= m_coff_header_opt.data_dirs.size())
+  if (llvm::COFF::EXCEPTION_TABLE >= m_coff_header_opt.data_dirs.size())
     return {};
 
   data_directory data_dir_exception =
-      m_coff_header_opt.data_dirs[coff_data_dir_exception_table];
+      m_coff_header_opt.data_dirs[llvm::COFF::EXCEPTION_TABLE];
   if (!data_dir_exception.vmaddr)
     return {};