This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB353677: minidump: Add ability to attach (breakpad) symbol 
files to placeholder modules (authored by labath, committed by ).
Herald added a subscriber: abidh.
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D57751?vs=185513&id=186195#toc

Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57751/new/

https://reviews.llvm.org/D57751

Files:
  include/lldb/Core/Module.h
  lit/Minidump/Inputs/linux-x86_64.dmp
  lit/Minidump/Inputs/linux-x86_64.syms
  lit/Minidump/breakpad-symbols.test
  source/Plugins/Process/minidump/ProcessMinidump.cpp

Index: source/Plugins/Process/minidump/ProcessMinidump.cpp
===================================================================
--- source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -41,51 +41,84 @@
 using namespace lldb_private;
 using namespace minidump;
 
-//------------------------------------------------------------------
-/// A placeholder module used for minidumps, where the original
-/// object files may not be available (so we can't parse the object
-/// files to extract the set of sections/segments)
-///
-/// This placeholder module has a single synthetic section (.module_image)
-/// which represents the module memory range covering the whole module.
-//------------------------------------------------------------------
-class PlaceholderModule : public Module {
+namespace {
+
+/// A minimal ObjectFile implementation providing a dummy object file for the
+/// cases when the real module binary is not available. This allows the module
+/// to show up in "image list" and symbols to be added to it.
+class PlaceholderObjectFile : public ObjectFile {
 public:
-  PlaceholderModule(const ModuleSpec &module_spec) :
-    Module(module_spec.GetFileSpec(), module_spec.GetArchitecture()) {
-    if (module_spec.GetUUID().IsValid())
-      SetUUID(module_spec.GetUUID());
-  }
-
-  // Creates a synthetic module section covering the whole module image (and
-  // sets the section load address as well)
-  void CreateImageSection(const MinidumpModule *module, Target& target) {
-    const ConstString section_name(".module_image");
-    lldb::SectionSP section_sp(new Section(
-        shared_from_this(),     // Module to which this section belongs.
-        nullptr,                // ObjectFile
-        0,                      // Section ID.
-        section_name,           // Section name.
-        eSectionTypeContainer,  // Section type.
-        module->base_of_image,  // VM address.
-        module->size_of_image,  // VM size in bytes of this section.
-        0,                      // Offset of this section in the file.
-        module->size_of_image,  // Size of the section as found in the file.
-        12,                     // Alignment of the section (log2)
-        0,                      // Flags for this section.
-        1));                    // Number of host bytes per target byte
-    section_sp->SetPermissions(ePermissionsExecutable | ePermissionsReadable);
-    GetSectionList()->AddSection(section_sp);
-    target.GetSectionLoadList().SetSectionLoadAddress(
-        section_sp, module->base_of_image);
+  PlaceholderObjectFile(const lldb::ModuleSP &module_sp,
+                        const ModuleSpec &module_spec, lldb::offset_t base,
+                        lldb::offset_t size)
+      : ObjectFile(module_sp, &module_spec.GetFileSpec(), /*file_offset*/ 0,
+                   /*length*/ 0, /*data_sp*/ nullptr, /*data_offset*/ 0),
+        m_arch(module_spec.GetArchitecture()), m_uuid(module_spec.GetUUID()),
+        m_base(base), m_size(size) {
+    m_symtab_ap = llvm::make_unique<Symtab>(this);
+  }
+
+  ConstString GetPluginName() override { return ConstString("placeholder"); }
+  uint32_t GetPluginVersion() override { return 1; }
+  bool ParseHeader() override { return true; }
+  Type CalculateType() override { return eTypeUnknown; }
+  Strata CalculateStrata() override { return eStrataUnknown; }
+  uint32_t GetDependentModules(FileSpecList &file_list) override { return 0; }
+  bool IsExecutable() const override { return false; }
+  ArchSpec GetArchitecture() override { return m_arch; }
+  Symtab *GetSymtab() override { return m_symtab_ap.get(); }
+  bool IsStripped() override { return true; }
+  ByteOrder GetByteOrder() const override { return m_arch.GetByteOrder(); }
+
+  uint32_t GetAddressByteSize() const override {
+    return m_arch.GetAddressByteSize();
+  }
+
+  bool GetUUID(UUID *uuid) override {
+    *uuid = m_uuid;
+    return true;
+  }
+
+  Address GetBaseAddress() override {
+    return Address(m_sections_ap->GetSectionAtIndex(0), 0);
   }
 
-ObjectFile *GetObjectFile() override { return nullptr; }
+  void CreateSections(SectionList &unified_section_list) override {
+    m_sections_ap = llvm::make_unique<SectionList>();
+    auto section_sp = std::make_shared<Section>(
+        GetModule(), this, /*sect_id*/ 0, ConstString(".module_image"),
+        eSectionTypeOther, m_base, m_size, /*file_offset*/ 0, /*file_size*/ 0,
+        /*log2align*/ 0, /*flags*/ 0);
+    m_sections_ap->AddSection(section_sp);
+    unified_section_list.AddSection(std::move(section_sp));
+  }
+
+  bool SetLoadAddress(Target &target, addr_t value,
+                      bool value_is_offset) override {
+    assert(!value_is_offset);
+    assert(value == m_base);
+
+    // Create sections if they haven't been created already.
+    GetModule()->GetSectionList();
+    assert(m_sections_ap->GetNumSections(0) == 1);
 
-  SectionList *GetSectionList() override {
-    return Module::GetUnifiedSectionList();
+    target.GetSectionLoadList().SetSectionLoadAddress(
+        m_sections_ap->GetSectionAtIndex(0), m_base);
+    return true;
   }
+
+  void Dump(Stream *s) override {
+    s->Format("Placeholder object file for {0} loaded at [{1:x}-{2:x})\n",
+              GetFileSpec(), m_base, m_base + m_size);
+  }
+
+private:
+  ArchSpec m_arch;
+  UUID m_uuid;
+  lldb::offset_t m_base;
+  lldb::offset_t m_size;
 };
+} // namespace
 
 ConstString ProcessMinidump::GetPluginNameStatic() {
   static ConstString g_name("minidump");
@@ -357,6 +390,7 @@
     auto file_spec = FileSpec(name.getValue(), GetArchitecture().GetTriple());
     FileSystem::Instance().Resolve(file_spec);
     ModuleSpec module_spec(file_spec, uuid);
+    module_spec.GetArchitecture() = GetArchitecture();
     Status error;
     lldb::ModuleSP module_sp = GetTarget().GetSharedModule(module_spec, &error);
     if (!module_sp || error.Fail()) {
@@ -373,10 +407,8 @@
                     name.getValue().c_str());
       }
 
-      auto placeholder_module =
-          std::make_shared<PlaceholderModule>(module_spec);
-      placeholder_module->CreateImageSection(module, GetTarget());
-      module_sp = placeholder_module;
+      module_sp = Module::CreateModuleFromObjectFile<PlaceholderObjectFile>(
+          module_spec, module->base_of_image, module->size_of_image);
       GetTarget().GetImages().Append(module_sp);
     }
 
Index: lit/Minidump/Inputs/linux-x86_64.syms
===================================================================
--- lit/Minidump/Inputs/linux-x86_64.syms
+++ lit/Minidump/Inputs/linux-x86_64.syms
@@ -0,0 +1,4 @@
+MODULE Linux x86_64 3B285CE327C387C262DB788BF5A4078B0 linux-x86_64
+INFO CODE_ID E35C283BC327C28762DB788BF5A4078BE2351448
+FUNC 3d0 18 0 crash()
+FUNC 3f0 10 0 _start
Index: lit/Minidump/breakpad-symbols.test
===================================================================
--- lit/Minidump/breakpad-symbols.test
+++ lit/Minidump/breakpad-symbols.test
@@ -0,0 +1,26 @@
+# Test that we can attach breakpad symbols to the "placeholder" modules created
+# for minidump files.
+#
+# The minidump input for this test taken from packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new
+
+# RUN: %lldb -c %S/Inputs/linux-x86_64.dmp \
+# RUN:   -o "target symbols add -s /tmp/test/linux-x86_64 %S/Inputs/linux-x86_64.syms" \
+# RUN:   -s %s -o exit | FileCheck %s
+
+image list
+# CHECK-LABEL: image list
+# CHECK: E35C283B-C327-C287-62DB-788BF5A4078B-E2351448 0x0000000000400000 /tmp/test/linux-x86_64
+
+image dump symtab /tmp/test/linux-x86_64
+# CHECK-LABEL: image dump symtab /tmp/test/linux-x86_64
+# CHECK: Symtab, file = /tmp/test/linux-x86_64, num_symbols = 2:
+# CHECK: [    0]      0   X Code            0x00000000004003d0 0x00000000004003d0 0x0000000000000018 0x00000000 crash()
+# CHECK: [    1]      0   X Code            0x00000000004003f0 0x00000000004003f0 0x0000000000000010 0x00000000 _start
+
+image lookup -a 0x4003f3
+# CHECK-LABEL: image lookup -a 0x4003f3
+# CHECK: Summary: linux-x86_64`_start + 3
+
+image dump objfile /tmp/test/linux-x86_64
+# CHECK-LABEL: image dump objfile /tmp/test/linux-x86_64
+# CHECK: Placeholder object file for /tmp/test/linux-x86_64 loaded at [0x400000-0x401000)
Index: include/lldb/Core/Module.h
===================================================================
--- include/lldb/Core/Module.h
+++ include/lldb/Core/Module.h
@@ -164,14 +164,17 @@
     module_sp->m_objfile_sp =
         std::make_shared<ObjFilePlugin>(module_sp, std::forward<Args>(args)...);
 
-    // Once we get the object file, update our module with the object file's
-    // architecture since it might differ in vendor/os if some parts were
-    // unknown.
-    if (ArchSpec arch = module_sp->m_objfile_sp->GetArchitecture()) {
-      module_sp->m_arch = arch;
-      return module_sp;
-    }
-    return nullptr;
+    // Once we get the object file, set module ArchSpec to the one we get from
+    // the object file. If the object file does not have an architecture, we
+    // consider the creation a failure.
+    ArchSpec arch = module_sp->m_objfile_sp->GetArchitecture();
+    if (!arch)
+      return nullptr;
+    module_sp->m_arch = arch;
+
+    // Also copy the object file's FileSpec.
+    module_sp->m_file = module_sp->m_objfile_sp->GetFileSpec();
+    return module_sp;
   }
 
   //------------------------------------------------------------------
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to