[lldb] Read modules from memory when a local copy is not available

[wallace]

Summary

When the local lldb doesn't have access to a copy of the modules in the target, e.g. winphone, with this change now we read these modules from memory.

There are mainly 2 changes:

1. create pecoff object files from memory
2. read from memory when the local file is not available

[zturner]

Can you rebase on top of tunk and re-upload? Also please use -U999999 when generating the diff (assuming you are using git) so we can see context.

[zturner]

This comment was removed by zturner.

[wallace]

rebase

[zturner]

source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp

88 ↗

(On Diff #71265)

auto_ptr should never be used under any circumstances. Prefer std::unique_ptr any time you want to use an auto_ptr.

Also, can you do an early return here?

if (!data_sp || !ObjectFilePECOFF::MagicBytesMatch(data_sp))
  return nullptr;

To create the unique_ptr, the preferred syntax (similar for shared_ptr) is this:

auto objfile_ap = llvm::make_unique<ObjectFilePECOFF>(module_sp, data_sp, process_sp, header_addr);

423 ↗

(On Diff #71265)

Early return here.

return DataExtractor(buffer_sp, GetByteOrder(), getAddressByteSize());

427 ↗

(On Diff #71265)

See earlier comment about make_unique. Not critical, but a good habit to get into.

[wallace]

fix pointers

[wallace]

rebase

[wallace]

ping

[zturner]

source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp

89 ↗

(On Diff #71409)

s/NULL/nullptr/

[wallace]

I'm obliged, your grace

[wallace]

s/NULL/nullptr/

[wallace]

rebase

[zturner]

When I run ninja check-lldb with this patch, every single test crashes. It's crashing at ObjectFilePECOFF.cpp:583, which does this:

const char *symbol_name_cstr =
    symtab_data.PeekCStr(name_address - data_start);
symbol_name.assign(symbol_name_cstr);

the symbol name returned is null, which it's trying to assign to a std::string.

I confirmed that without your patch everything works.

The CMake command line I am using is

cmake -G Ninja -DLLDB_TEST_COMPILER=D:\src\llvmbuild\ninja_release\bin\clang.exe -DPYTHON_HOME=C:\Python35

Let me know if you have trouble reproducing this crash, but it will need to be fixed before this CL can go in.

[wallace]

I was using this patch as well https://reviews.llvm.org/D24530, which might be the reason why it fails on your side. I'll double check, but I'm pretty sure it's just that

[wallace]

rebase

[wallace]

This time I'm calculating the address of the exports header correctly because it is
an RVA address.

[wallace]

minor nits

[wallace]

I've run the tests and it's the same with and without this diff :)

[wallace]

before.txt68 KBDownload

after.txt136 KBDownload

These are the output from running the tests. Btw, this was failing before because it was reading the export table incorrectly, but I updated it and it should be correct now

[wallace]

pingping

[zturner]

This looks fine now. Sorry for the delay, I had forgotten about it. I think you still don't have commit access right? Now would be a good time to request it, and this can be your first commit.

lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h

typedef enum MachineType {
     MachineWcemIpsv2 = 0x169
   } MachineType;
 
   ObjectFilePECOFF(const lldb::ModuleSP &module_sp, lldb::DataBufferSP &data_sp,
                    lldb::offset_t data_offset,
                    const lldb_private::FileSpec *file,
                    lldb::offset_t file_offset, lldb::offset_t length);
 
+  ObjectFilePECOFF(const lldb::ModuleSP &module_sp,
+                   lldb::DataBufferSP &header_data_sp,
+                   const lldb::ProcessSP &process_sp, lldb::addr_t header_addr);
+
   ~ObjectFilePECOFF() override;
 
   //------------------------------------------------------------------
   // Static Functions
   //------------------------------------------------------------------
   static void Initialize();
 
   static void Terminate();
public:
   // PluginInterface protocol
   //------------------------------------------------------------------
   lldb_private::ConstString GetPluginName() override;
 
   uint32_t GetPluginVersion() override;
 
   bool IsWindowsSubsystem();
 
+  lldb_private::DataExtractor ReadImageData(uint32_t offset, size_t size);
+
 protected:
   bool NeedsEndianSwap() const;
 
   typedef struct dos_header { // DOS .EXE header
     uint16_t e_magic;         // Magic number
     uint16_t e_cblp;          // Bytes on last page of file
     uint16_t e_cp;            // Pages in file
     uint16_t e_crlc;          // Relocations

lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp

 //===-- ObjectFilePECOFF.cpp ------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "ObjectFilePECOFF.h"
 #include "WindowsMiniDump.h"
 
 #include "llvm/Support/COFF.h"
 
 #include "lldb/Core/ArchSpec.h"
 #include "lldb/Core/DataBuffer.h"
+#include "lldb/Core/DataBufferHeap.h"
 #include "lldb/Core/FileSpecList.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Core/StreamString.h"
 #include "lldb/Core/Timer.h"
if (objfile_ap.get() && objfile_ap->ParseHeader())
       return objfile_ap.release();
   }
   return NULL;
 }
 
 ObjectFile *ObjectFilePECOFF::CreateMemoryInstance(
     const lldb::ModuleSP &module_sp, lldb::DataBufferSP &data_sp,
     const lldb::ProcessSP &process_sp, lldb::addr_t header_addr) {
-  return NULL;
+  if (!data_sp || !ObjectFilePECOFF::MagicBytesMatch(data_sp))
+    return nullptr;
+  auto objfile_ap = llvm::make_unique<ObjectFilePECOFF>(
+      module_sp, data_sp, process_sp, header_addr);
+  if (objfile_ap.get() && objfile_ap->ParseHeader()) {
+    return objfile_ap.release();
+  }
+  return nullptr;
 }
 
 size_t ObjectFilePECOFF::GetModuleSpecifications(
     const lldb_private::FileSpec &file, lldb::DataBufferSP &data_sp,
     lldb::offset_t data_offset, lldb::offset_t file_offset,
     lldb::offset_t length, lldb_private::ModuleSpecList &specs) {
   const size_t initial_count = specs.GetSize();
 
ObjectFilePECOFF::ObjectFilePECOFF(const lldb::ModuleSP &module_sp,
     : ObjectFile(module_sp, file, file_offset, length, data_sp, data_offset),
       m_dos_header(), m_coff_header(), m_coff_header_opt(), m_sect_headers(),
       m_entry_point_address() {
   ::memset(&m_dos_header, 0, sizeof(m_dos_header));
   ::memset(&m_coff_header, 0, sizeof(m_coff_header));
   ::memset(&m_coff_header_opt, 0, sizeof(m_coff_header_opt));
 }
 
+ObjectFilePECOFF::ObjectFilePECOFF(const lldb::ModuleSP &module_sp,
+                                   DataBufferSP &header_data_sp,
+                                   const lldb::ProcessSP &process_sp,
+                                   addr_t header_addr)
+    : ObjectFile(module_sp, process_sp, header_addr, header_data_sp),
+      m_dos_header(), m_coff_header(), m_coff_header_opt(), m_sect_headers(),
+      m_entry_point_address() {
+  ::memset(&m_dos_header, 0, sizeof(m_dos_header));
+  ::memset(&m_coff_header, 0, sizeof(m_coff_header));
+  ::memset(&m_coff_header_opt, 0, sizeof(m_coff_header_opt));
+}
+
 ObjectFilePECOFF::~ObjectFilePECOFF() {}
 
 bool ObjectFilePECOFF::ParseHeader() {
   ModuleSP module_sp(GetModule());
   if (module_sp) {
     std::lock_guard<std::recursive_mutex> guard(module_sp->GetMutex());
     m_sect_headers.clear();
     m_data.SetByteOrder(eByteOrderLittle);
if (*offset_ptr < end_offset) {
       }
     }
   }
   // Make sure we are on track for section data which follows
   *offset_ptr = end_offset;
   return success;
 }
 
+DataExtractor ObjectFilePECOFF::ReadImageData(uint32_t offset, size_t size) {
+  if (m_file) {
+    DataBufferSP buffer_sp(m_file.ReadFileContents(offset, size));
+    return DataExtractor(buffer_sp, GetByteOrder(), GetAddressByteSize());
+  }
+  ProcessSP process_sp(m_process_wp.lock());
+  DataExtractor data;
+  if (process_sp) {
+    auto data_ap = llvm::make_unique<DataBufferHeap>(size, 0);
+    Error readmem_error;
+    size_t bytes_read =
+        process_sp->ReadMemory(m_image_base + offset, data_ap->GetBytes(),
+                               data_ap->GetByteSize(), readmem_error);
+    if (bytes_read == size) {
+      DataBufferSP buffer_sp(data_ap.release());
+      data.SetData(buffer_sp, 0, buffer_sp->GetByteSize());
+    }
+  }
+  return data;
+}
+
 //----------------------------------------------------------------------
 // ParseSectionHeaders
 //----------------------------------------------------------------------
 bool ObjectFilePECOFF::ParseSectionHeaders(
     uint32_t section_header_data_offset) {
   const uint32_t nsects = m_coff_header.nsects;
   m_sect_headers.clear();
 
   if (nsects > 0) {
-    const uint32_t addr_byte_size = GetAddressByteSize();
     const size_t section_header_byte_size = nsects * sizeof(section_header_t);
-    DataBufferSP section_header_data_sp(m_file.ReadFileContents(
-        section_header_data_offset, section_header_byte_size));
-    DataExtractor section_header_data(section_header_data_sp, GetByteOrder(),
-                                      addr_byte_size);
+    DataExtractor section_header_data =
+        ReadImageData(section_header_data_offset, section_header_byte_size);
 
     lldb::offset_t offset = 0;
     if (section_header_data.ValidOffsetForDataOfSize(
             offset, section_header_byte_size)) {
       m_sect_headers.resize(nsects);
 
       for (uint32_t idx = 0; idx < nsects; ++idx) {
         const void *name_data = section_header_data.GetData(&offset, 8);
if (module_sp) {
     std::lock_guard<std::recursive_mutex> guard(module_sp->GetMutex());
     if (m_symtab_ap.get() == NULL) {
       SectionList *sect_list = GetSectionList();
       m_symtab_ap.reset(new Symtab(this));
       std::lock_guard<std::recursive_mutex> guard(m_symtab_ap->GetMutex());
 
       const uint32_t num_syms = m_coff_header.nsyms;
 
-      if (num_syms > 0 && m_coff_header.symoff > 0) {
+      if (m_file && num_syms > 0 && m_coff_header.symoff > 0) {
         const uint32_t symbol_size = 18;
-        const uint32_t addr_byte_size = GetAddressByteSize();
         const size_t symbol_data_size = num_syms * symbol_size;
         // Include the 4-byte string table size at the end of the symbols
-        DataBufferSP symtab_data_sp(m_file.ReadFileContents(
-            m_coff_header.symoff, symbol_data_size + 4));
-        DataExtractor symtab_data(symtab_data_sp, GetByteOrder(),
-                                  addr_byte_size);
+        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);
-        DataBufferSP strtab_data_sp(m_file.ReadFileContents(
-            m_coff_header.symoff + symbol_data_size, strtab_size));
-        DataExtractor strtab_data(strtab_data_sp, GetByteOrder(),
-                                  addr_byte_size);
+        if (strtab_size > 0) {
+          DataExtractor strtab_data = ReadImageData(
+              m_coff_header.symoff + symbol_data_size, strtab_size);
 
           // First 4 bytes should be zeroed after strtab_size has been read,
           // because it is used as offset 0 to encode a NULL string.
-        uint32_t *strtab_data_start = (uint32_t *)strtab_data_sp->GetBytes();
+          uint32_t *strtab_data_start = (uint32_t *)strtab_data.GetDataStart();
           strtab_data_start[0] = 0;
 
           offset = 0;
           std::string symbol_name;
           Symbol *symbols = m_symtab_ap->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 = NULL;
             // 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 == NULL)
                 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->GetSectionAtIndex(symbol.sect - 1),
                                   symbol.value);
               symbols[i].GetAddressRef() = symbol_addr;
               symbols[i].SetType(MapSymbolType(symbol.type));
             }
 
             if (symbol.naux > 0) {
               i += symbol.naux;
               offset += symbol_size;
             }
           }
         }
+      }
 
       // 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;
+
+        uint32_t address_rva = data_start;
+        if (m_file) {
           Address address(m_coff_header_opt.image_base + data_start, sect_list);
-        DataBufferSP symtab_data_sp(m_file.ReadFileContents(
-            address.GetSection()->GetFileOffset() + address.GetOffset(),
-            m_coff_header_opt.data_dirs[0].vmsize));
-        DataExtractor symtab_data(symtab_data_sp, GetByteOrder(),
-                                  GetAddressByteSize());
+          address_rva =
+              address.GetSection()->GetFileOffset() + address.GetOffset();
+        }
+        DataExtractor symtab_data =
+            ReadImageData(address_rva, m_coff_header_opt.data_dirs[0].vmsize);
         lldb::offset_t offset = 0;
 
         // 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);