[Lldb-commits] [lldb] r354263 - minidump: Add ability to attach (breakpad) symbol files to placeholder modules
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Mon Feb 18 03:55:02 PST 2019
Author: labath
Date: Mon Feb 18 03:55:01 2019
New Revision: 354263
URL: http://llvm.org/viewvc/llvm-project?rev=354263&view=rev
Log:
minidump: Add ability to attach (breakpad) symbol files to placeholder modules
This re-commits r353677, which was reverted due to test failures on the
windows bot. The issue there was that ObjectFilePECOFF vended its base
address through the incorrect interface. SymbolFilePDB depended on that,
which lead to assertion failures when SymbolFilePDB was attempting to
use the placeholder object files as a base. This has been fixed in
r354258
It also fixes one small problem in the original patch. The issue was that the
Module class would attempt to overwrite the object file we created in
CreateModuleFromObjectFile if the file corresponding to the placeholder object
file happened to exist (but we have already disqualified it due to UUID
mismatch. The fix is simple -- we set the m_did_load_objfile flag to properly
record the fact that we have already created an object file for the module.
The original commit message was:
The reason this wasn't working was that ProcessMinidump was creating odd
object-file-less modules, and SymbolFileBreakpad required the module to
have an associated object file because it needed to get its base
address.
This fixes that by introducing a PlaceholderObjectFile to serve as a
dummy object file. The general idea for this is taken from D55142, but
I've reworked it a bit to avoid the need for the PlaceholderModule
class. Now that we have an object file, our modules are sufficiently
similar to regular modules that we can use the regular Module class
almost out of the box -- the only thing I needed to tweak was the
Module::CreateModuleFromObjectFile functon to set the module's FileSpec
in addition to it's architecture. This wasn't needed for ObjectFileJIT
(the other user of CreateModuleFromObjectFile), but it shouldn't hurt it
either, and the change seems like a straightforward extension of this
function.
Reviewers: clayborg, lemo, amccarth
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D57751
Added:
lldb/trunk/lit/Minidump/Inputs/linux-x86_64.dmp
lldb/trunk/lit/Minidump/Inputs/linux-x86_64.syms
lldb/trunk/lit/Minidump/breakpad-symbols.test
Modified:
lldb/trunk/include/lldb/Core/Module.h
lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp
Modified: lldb/trunk/include/lldb/Core/Module.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Module.h?rev=354263&r1=354262&r2=354263&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Core/Module.h (original)
+++ lldb/trunk/include/lldb/Core/Module.h Mon Feb 18 03:55:01 2019
@@ -163,15 +163,19 @@ public:
lldb::ModuleSP module_sp(new Module());
module_sp->m_objfile_sp =
std::make_shared<ObjFilePlugin>(module_sp, std::forward<Args>(args)...);
+ module_sp->m_did_load_objfile.store(true, std::memory_order_relaxed);
- // 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;
}
//------------------------------------------------------------------
Added: lldb/trunk/lit/Minidump/Inputs/linux-x86_64.dmp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Minidump/Inputs/linux-x86_64.dmp?rev=354263&view=auto
==============================================================================
Binary files lldb/trunk/lit/Minidump/Inputs/linux-x86_64.dmp (added) and lldb/trunk/lit/Minidump/Inputs/linux-x86_64.dmp Mon Feb 18 03:55:01 2019 differ
Added: lldb/trunk/lit/Minidump/Inputs/linux-x86_64.syms
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Minidump/Inputs/linux-x86_64.syms?rev=354263&view=auto
==============================================================================
--- lldb/trunk/lit/Minidump/Inputs/linux-x86_64.syms (added)
+++ lldb/trunk/lit/Minidump/Inputs/linux-x86_64.syms Mon Feb 18 03:55:01 2019
@@ -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
Added: lldb/trunk/lit/Minidump/breakpad-symbols.test
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Minidump/breakpad-symbols.test?rev=354263&view=auto
==============================================================================
--- lldb/trunk/lit/Minidump/breakpad-symbols.test (added)
+++ lldb/trunk/lit/Minidump/breakpad-symbols.test Mon Feb 18 03:55:01 2019
@@ -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)
Modified: lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp?rev=354263&r1=354262&r2=354263&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp (original)
+++ lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp Mon Feb 18 03:55:01 2019
@@ -41,51 +41,80 @@ using namespace lldb;
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_up = llvm::make_unique<Symtab>(this);
}
-ObjectFile *GetObjectFile() override { return nullptr; }
+ 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; }
+ UUID GetUUID() override { return m_uuid; }
+ Symtab *GetSymtab() override { return m_symtab_up.get(); }
+ bool IsStripped() override { return true; }
+ ByteOrder GetByteOrder() const override { return m_arch.GetByteOrder(); }
+
+ uint32_t GetAddressByteSize() const override {
+ return m_arch.GetAddressByteSize();
+ }
- SectionList *GetSectionList() override {
- return Module::GetUnifiedSectionList();
+ Address GetBaseAddress() override {
+ return Address(m_sections_up->GetSectionAtIndex(0), 0);
}
+
+ void CreateSections(SectionList &unified_section_list) override {
+ m_sections_up = 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_up->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_up->GetNumSections(0) == 1);
+
+ target.GetSectionLoadList().SetSectionLoadAddress(
+ m_sections_up->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 +386,7 @@ void ProcessMinidump::ReadModuleList() {
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 +403,8 @@ void ProcessMinidump::ReadModuleList() {
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);
}
More information about the lldb-commits
mailing list