[Lldb-commits] [PATCH] D108233: WIP: Add minidump save-core functionality to ELF object files
MaskRay added inline comments.
Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:120
+ if (error.Fail()) {
+error.SetErrorString("Unable to convert the csd string to UTF16.");
+return error;
https://llvm.org/docs/CodingStandards.html#error-and-warning-messages
Don't capitalize messages and don't append `.`
Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:732
+ error.SetErrorStringWithFormat(
+ "Unable to write the header. (written %ld/%ld).", bytes_written,
+ header_size);
The format specifiers are wrong. size_t should use %zd and one uint64_t below
should use PRIu64.
I have fixed them.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108233/new/
https://reviews.llvm.org/D108233
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D108233: WIP: Add minidump save-core functionality to ELF object files
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGeee687a66d76: [lldb] Add minidump save-core functionality to
ELF object files (authored by Aj0SK, committed by werat).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108233/new/
https://reviews.llvm.org/D108233
Files:
lldb/include/lldb/Core/PluginManager.h
lldb/source/API/SBProcess.cpp
lldb/source/Commands/CommandObjectProcess.cpp
lldb/source/Commands/Options.td
lldb/source/Core/PluginManager.cpp
lldb/source/Plugins/ObjectFile/CMakeLists.txt
lldb/source/Plugins/ObjectFile/Minidump/CMakeLists.txt
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h
lldb/test/API/functionalities/process_save_core_minidump/Makefile
lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
lldb/test/API/functionalities/process_save_core_minidump/main.cpp
Index: lldb/test/API/functionalities/process_save_core_minidump/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/process_save_core_minidump/main.cpp
@@ -0,0 +1,30 @@
+#include
+#include
+#include
+
+using namespace std;
+
+void g() { assert(false); }
+
+void f() { g(); }
+
+size_t h() {
+ size_t sum = 0;
+ for (size_t i = 0; i < 100; ++i)
+for (size_t j = 0; j < 100; ++j)
+ if ((i * j) % 2 == 0) {
+sum += 1;
+ }
+ return sum;
+}
+
+int main() {
+ thread t1(f);
+
+ size_t x = h();
+
+ t1.join();
+
+ cout << "X is " << x << "\n";
+ return 0;
+}
\ No newline at end of file
Index: lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
===
--- /dev/null
+++ lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -0,0 +1,79 @@
+"""
+Test saving a mini dump.
+"""
+
+
+import os
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class ProcessSaveCoreMinidumpTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipUnlessArch("x86_64")
+@skipUnlessPlatform(["linux"])
+def test_save_linux_mini_dump(self):
+"""Test that we can save a Linux mini dump."""
+self.build()
+exe = self.getBuildArtifact("a.out")
+core = self.getBuildArtifact("core.dmp")
+try:
+target = self.dbg.CreateTarget(exe)
+process = target.LaunchSimple(
+None, None, self.get_process_working_directory())
+self.assertEqual(process.GetState(), lldb.eStateStopped)
+
+# get neccessary data for the verification phase
+process_info = process.GetProcessInfo()
+expected_pid = process_info.GetProcessID() if process_info.IsValid() else -1
+expected_number_of_modules = target.GetNumModules()
+expected_modules = target.modules
+expected_number_of_threads = process.GetNumThreads()
+expected_threads = []
+
+for thread_idx in range(process.GetNumThreads()):
+thread = process.GetThreadAtIndex(thread_idx)
+thread_id = thread.GetThreadID()
+expected_threads.append(thread_id)
+
+# save core and, kill process and verify corefile existence
+self.runCmd("process save-core --plugin-name=minidump --style=stack " + core)
+self.assertTrue(os.path.isfile(core))
+self.assertTrue(process.Kill().Success())
+
+# To verify, we'll launch with the mini dump
+target = self.dbg.CreateTarget(None)
+process = target.LoadCore(core)
+
+# check if the core is in desired state
+self.assertTrue(process, PROCESS_IS_VALID)
+self.assertTrue(process.GetProcessInfo().IsValid())
+self.assertEqual(process.GetProcessInfo().GetProcessID(), expected_pid)
+self.assertTrue(target.GetTriple().find("linux") != -1)
+self.assertTrue(target.GetNumModules(), expected_number_of_modules)
+self.assertEqual(process.GetNumThreads(), expected_number_of_threads)
+
+for module, expected in zip(target.modules, expected_modules):
+self.assertTrue(module.IsValid())
+module_file_name = module.GetFileSpec().GetFilename()
+expected_file_name = expected.GetFileSpec().GetFilename()
+# skip kernel virtual dynamic shared objects
+if "vdso" in expected_file_name:
+continue
+
[Lldb-commits] [PATCH] D108233: WIP: Add minidump save-core functionality to ELF object files
Aj0SK added inline comments.
Comment at:
lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py:17
+
+@skipUnlessArch("x86_64")
+@skipUnlessPlatform(["linux"])
clayborg wrote:
> Do we only support x86_64 right now? If we are actually supporting other
> architectures and emitting a minidump for them, we should be testing them. Do
> we return an error for any architectures that we don't support in the
> ObjectFIleMinidump::SaveCore(...) function?
We only support "full-featured" minidump for the x86_64. For some platforms,
listed in AddSystemInfo method we provide minidump with only basic
information(SystemInfo, ModuleInfo and MiscInfo streams). For all other
platforms, no minidump is created and it will error-out...
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108233/new/
https://reviews.llvm.org/D108233
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D108233: WIP: Add minidump save-core functionality to ELF object files
clayborg added inline comments.
Comment at:
lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py:17
+
+@skipUnlessArch("x86_64")
+@skipUnlessPlatform(["linux"])
Do we only support x86_64 right now? If we are actually supporting other
architectures and emitting a minidump for them, we should be testing them. Do
we return an error for any architectures that we don't support in the
ObjectFIleMinidump::SaveCore(...) function?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108233/new/
https://reviews.llvm.org/D108233
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D108233: WIP: Add minidump save-core functionality to ELF object files
Aj0SK updated this revision to Diff 369722.
Aj0SK added a comment.
Fix not-correctly applied change from review regarding memory reading.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108233/new/
https://reviews.llvm.org/D108233
Files:
lldb/include/lldb/Core/PluginManager.h
lldb/source/API/SBProcess.cpp
lldb/source/Commands/CommandObjectProcess.cpp
lldb/source/Commands/Options.td
lldb/source/Core/PluginManager.cpp
lldb/source/Plugins/ObjectFile/CMakeLists.txt
lldb/source/Plugins/ObjectFile/Minidump/CMakeLists.txt
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h
lldb/test/API/functionalities/process_save_core_minidump/Makefile
lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
lldb/test/API/functionalities/process_save_core_minidump/main.cpp
Index: lldb/test/API/functionalities/process_save_core_minidump/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/process_save_core_minidump/main.cpp
@@ -0,0 +1,30 @@
+#include
+#include
+#include
+
+using namespace std;
+
+void g() { assert(false); }
+
+void f() { g(); }
+
+size_t h() {
+ size_t sum = 0;
+ for (size_t i = 0; i < 100; ++i)
+for (size_t j = 0; j < 100; ++j)
+ if ((i * j) % 2 == 0) {
+sum += 1;
+ }
+ return sum;
+}
+
+int main() {
+ thread t1(f);
+
+ size_t x = h();
+
+ t1.join();
+
+ cout << "X is " << x << "\n";
+ return 0;
+}
\ No newline at end of file
Index: lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
===
--- /dev/null
+++ lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -0,0 +1,79 @@
+"""
+Test saving a mini dump.
+"""
+
+
+import os
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class ProcessSaveCoreMinidumpTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipUnlessArch("x86_64")
+@skipUnlessPlatform(["linux"])
+def test_save_linux_mini_dump(self):
+"""Test that we can save a Linux mini dump."""
+self.build()
+exe = self.getBuildArtifact("a.out")
+core = self.getBuildArtifact("core.dmp")
+try:
+target = self.dbg.CreateTarget(exe)
+process = target.LaunchSimple(
+None, None, self.get_process_working_directory())
+self.assertEqual(process.GetState(), lldb.eStateStopped)
+
+# get neccessary data for the verification phase
+process_info = process.GetProcessInfo()
+expected_pid = process_info.GetProcessID() if process_info.IsValid() else -1
+expected_number_of_modules = target.GetNumModules()
+expected_modules = target.modules
+expected_number_of_threads = process.GetNumThreads()
+expected_threads = []
+
+for thread_idx in range(process.GetNumThreads()):
+thread = process.GetThreadAtIndex(thread_idx)
+thread_id = thread.GetThreadID()
+expected_threads.append(thread_id)
+
+# save core and, kill process and verify corefile existence
+self.runCmd("process save-core --plugin-name=minidump --style=stack " + core)
+self.assertTrue(os.path.isfile(core))
+self.assertTrue(process.Kill().Success())
+
+# To verify, we'll launch with the mini dump
+target = self.dbg.CreateTarget(None)
+process = target.LoadCore(core)
+
+# check if the core is in desired state
+self.assertTrue(process, PROCESS_IS_VALID)
+self.assertTrue(process.GetProcessInfo().IsValid())
+self.assertEqual(process.GetProcessInfo().GetProcessID(), expected_pid)
+self.assertTrue(target.GetTriple().find("linux") != -1)
+self.assertTrue(target.GetNumModules(), expected_number_of_modules)
+self.assertEqual(process.GetNumThreads(), expected_number_of_threads)
+
+for module, expected in zip(target.modules, expected_modules):
+self.assertTrue(module.IsValid())
+module_file_name = module.GetFileSpec().GetFilename()
+expected_file_name = expected.GetFileSpec().GetFilename()
+# skip kernel virtual dynamic shared objects
+if "vdso" in expected_file_name:
+continue
+self.assertEqual(module_file_name, expected_file_name)
+self.assertEqual(module.GetUUIDString(),
[Lldb-commits] [PATCH] D108233: WIP: Add minidump save-core functionality to ELF object files
Aj0SK updated this revision to Diff 369686.
Aj0SK added a comment.
Fixes arm and aarch64 build run fails. Adding aarch64 to the matching in
SystemInfo stream and activating test only on x86_64 as this is the only
platform where also thread info etc. is being saved.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108233/new/
https://reviews.llvm.org/D108233
Files:
lldb/include/lldb/Core/PluginManager.h
lldb/source/API/SBProcess.cpp
lldb/source/Commands/CommandObjectProcess.cpp
lldb/source/Commands/Options.td
lldb/source/Core/PluginManager.cpp
lldb/source/Plugins/ObjectFile/CMakeLists.txt
lldb/source/Plugins/ObjectFile/Minidump/CMakeLists.txt
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h
lldb/test/API/functionalities/process_save_core_minidump/Makefile
lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
lldb/test/API/functionalities/process_save_core_minidump/main.cpp
Index: lldb/test/API/functionalities/process_save_core_minidump/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/process_save_core_minidump/main.cpp
@@ -0,0 +1,30 @@
+#include
+#include
+#include
+
+using namespace std;
+
+void g() { assert(false); }
+
+void f() { g(); }
+
+size_t h() {
+ size_t sum = 0;
+ for (size_t i = 0; i < 100; ++i)
+for (size_t j = 0; j < 100; ++j)
+ if ((i * j) % 2 == 0) {
+sum += 1;
+ }
+ return sum;
+}
+
+int main() {
+ thread t1(f);
+
+ size_t x = h();
+
+ t1.join();
+
+ cout << "X is " << x << "\n";
+ return 0;
+}
\ No newline at end of file
Index: lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
===
--- /dev/null
+++ lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -0,0 +1,79 @@
+"""
+Test saving a mini dump.
+"""
+
+
+import os
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class ProcessSaveCoreMinidumpTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipUnlessArch("x86_64")
+@skipUnlessPlatform(["linux"])
+def test_save_linux_mini_dump(self):
+"""Test that we can save a Linux mini dump."""
+self.build()
+exe = self.getBuildArtifact("a.out")
+core = self.getBuildArtifact("core.dmp")
+try:
+target = self.dbg.CreateTarget(exe)
+process = target.LaunchSimple(
+None, None, self.get_process_working_directory())
+self.assertEqual(process.GetState(), lldb.eStateStopped)
+
+# get neccessary data for the verification phase
+process_info = process.GetProcessInfo()
+expected_pid = process_info.GetProcessID() if process_info.IsValid() else -1
+expected_number_of_modules = target.GetNumModules()
+expected_modules = target.modules
+expected_number_of_threads = process.GetNumThreads()
+expected_threads = []
+
+for thread_idx in range(process.GetNumThreads()):
+thread = process.GetThreadAtIndex(thread_idx)
+thread_id = thread.GetThreadID()
+expected_threads.append(thread_id)
+
+# save core and, kill process and verify corefile existence
+self.runCmd("process save-core --plugin-name=minidump --style=stack " + core)
+self.assertTrue(os.path.isfile(core))
+self.assertTrue(process.Kill().Success())
+
+# To verify, we'll launch with the mini dump
+target = self.dbg.CreateTarget(None)
+process = target.LoadCore(core)
+
+# check if the core is in desired state
+self.assertTrue(process, PROCESS_IS_VALID)
+self.assertTrue(process.GetProcessInfo().IsValid())
+self.assertEqual(process.GetProcessInfo().GetProcessID(), expected_pid)
+self.assertTrue(target.GetTriple().find("linux") != -1)
+self.assertTrue(target.GetNumModules(), expected_number_of_modules)
+self.assertEqual(process.GetNumThreads(), expected_number_of_threads)
+
+for module, expected in zip(target.modules, expected_modules):
+self.assertTrue(module.IsValid())
+module_file_name = module.GetFileSpec().GetFilename()
+expected_file_name = expected.GetFileSpec().GetFilename()
+# skip kernel virtual dynamic shared objects
+if "vdso" in expected_file_name:
+continue
+
[Lldb-commits] [PATCH] D108233: WIP: Add minidump save-core functionality to ELF object files
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGaafa05e03d62: [lldb] Add minidump save-core functionality to
ELF object files (authored by Aj0SK, committed by werat).
Changed prior to commit:
https://reviews.llvm.org/D108233?vs=369410=369673#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108233/new/
https://reviews.llvm.org/D108233
Files:
lldb/include/lldb/Core/PluginManager.h
lldb/source/API/SBProcess.cpp
lldb/source/Commands/CommandObjectProcess.cpp
lldb/source/Commands/Options.td
lldb/source/Core/PluginManager.cpp
lldb/source/Plugins/ObjectFile/CMakeLists.txt
lldb/source/Plugins/ObjectFile/Minidump/CMakeLists.txt
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h
lldb/test/API/functionalities/process_save_core_minidump/Makefile
lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
lldb/test/API/functionalities/process_save_core_minidump/main.cpp
Index: lldb/test/API/functionalities/process_save_core_minidump/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/process_save_core_minidump/main.cpp
@@ -0,0 +1,30 @@
+#include
+#include
+#include
+
+using namespace std;
+
+void g() { assert(false); }
+
+void f() { g(); }
+
+size_t h() {
+ size_t sum = 0;
+ for (size_t i = 0; i < 100; ++i)
+for (size_t j = 0; j < 100; ++j)
+ if ((i * j) % 2 == 0) {
+sum += 1;
+ }
+ return sum;
+}
+
+int main() {
+ thread t1(f);
+
+ size_t x = h();
+
+ t1.join();
+
+ cout << "X is " << x << "\n";
+ return 0;
+}
\ No newline at end of file
Index: lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
===
--- /dev/null
+++ lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -0,0 +1,78 @@
+"""
+Test saving a mini dump.
+"""
+
+
+import os
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class ProcessSaveCoreMinidumpTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipUnlessPlatform(['linux'])
+def test_save_linux_mini_dump(self):
+"""Test that we can save a Linux mini dump."""
+self.build()
+exe = self.getBuildArtifact("a.out")
+core = self.getBuildArtifact("core.dmp")
+try:
+target = self.dbg.CreateTarget(exe)
+process = target.LaunchSimple(
+None, None, self.get_process_working_directory())
+self.assertEqual(process.GetState(), lldb.eStateStopped)
+
+# get neccessary data for the verification phase
+process_info = process.GetProcessInfo()
+expected_pid = process_info.GetProcessID() if process_info.IsValid() else -1
+expected_number_of_modules = target.GetNumModules()
+expected_modules = target.modules
+expected_number_of_threads = process.GetNumThreads()
+expected_threads = []
+
+for thread_idx in range(process.GetNumThreads()):
+thread = process.GetThreadAtIndex(thread_idx)
+thread_id = thread.GetThreadID()
+expected_threads.append(thread_id)
+
+# save core and, kill process and verify corefile existence
+self.runCmd("process save-core --plugin-name=minidump --style=stack " + core)
+self.assertTrue(os.path.isfile(core))
+self.assertTrue(process.Kill().Success())
+
+# To verify, we'll launch with the mini dump
+target = self.dbg.CreateTarget(None)
+process = target.LoadCore(core)
+
+# check if the core is in desired state
+self.assertTrue(process, PROCESS_IS_VALID)
+self.assertTrue(process.GetProcessInfo().IsValid())
+self.assertEqual(process.GetProcessInfo().GetProcessID(), expected_pid)
+self.assertTrue(target.GetTriple().find("linux") != -1)
+self.assertTrue(target.GetNumModules(), expected_number_of_modules)
+self.assertEqual(process.GetNumThreads(), expected_number_of_threads)
+
+for module, expected in zip(target.modules, expected_modules):
+self.assertTrue(module.IsValid())
+module_file_name = module.GetFileSpec().GetFilename()
+expected_file_name = expected.GetFileSpec().GetFilename()
+# skip kernel virtual dynamic shared objects
+if "vdso" in
[Lldb-commits] [PATCH] D108233: WIP: Add minidump save-core functionality to ELF object files
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.
Thanks for all of the changes! Looks great.
Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:549
+bool is_interesting = false;
+for (size_t interesting_address : interesting_addresses)
+ if (interesting_address >= addr && interesting_address < addr + size) {
Aj0SK wrote:
> Aj0SK wrote:
> > clayborg wrote:
> > > Shouldn't this be the main loop that would replace the "while
> > > (range_info.GetRange().GetRangeBase() != LLDB_INVALID_ADDRESS)" loop? I
> > > would think that the main loop wiould be something like:
> > >
> > > ```
> > > std::set visited_region_base_addresses;
> > > for (size_t interesting_address : interesting_addresses) {
> > > error = process_sp->GetMemoryRegionInfo(interesting_address,
> > > range_info);
> > > // Skip failed memory region requests or any regions with no
> > > permissions.
> > > if (error.Fail() || range_info.GetPermissions() == 0)
> > > continue;
> > > const addr_t addr = range_info.GetRange().GetRangeBase();
> > > // Skip any regions we have already saved out.
> > > if (visited_region_base_addresses.insert(addr).second == false)
> > > continue;
> > > const addr_t size = range_info.GetRange().GetByteSize();
> > > if (size == 0)
> > > continue;
> > > auto data_up = std::make_unique(size, 0);
> > > const size_t bytes_read = process_sp->ReadMemory(addr,
> > > data_up->GetBytes(), size, error);
> > > if (bytes_read == 0)
> > > continue;
> > > // We have a good memory region with valid bytes to store.
> > > LocationDescriptor memory_dump;
> > > memory_dump.DataSize = static_cast(size);
> > > memory_dump.RVA
> > > =static_cast(GetCurrentDataEndOffset());
> > > MemoryDescriptor memory_desc;
> > > memory_desc.StartOfMemoryRange =
> > > static_cast(addr);
> > > memory_desc.Memory = memory_dump;
> > > mem_descriptors.push_back(memory_desc);
> > > m_data.AppendData(data_up->GetBytes(), bytes_read);
> > > }
> > > ```
> > >
> > >
> > This kind of an approach was something I also thought about and in fact, I
> > was mainly inspired by this [[
> > https://github.com/llvm-mirror/lldb/blob/d01083a850f577b85501a0902b52fd0930de72c7/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp#L6051
> > | approach ]]. Also, the previous comment about the need of saving the
> > loaded modules addresses in a set doesn't apply to my solution, am I
> > correct? Just to clear it a little bit, I like your solution though and
> > it's better when I think about it. Thanks for the suggestion!
> One thing, that could be a little problematic with the new approach is, if
> the possibility to save full/dirty pages dump would be requested... In this
> case, we still need to go through all of the sections...
Correct, we would just iterate over all memory regions and get the next region
by asking for the region that starts at the end address of the current region.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108233/new/
https://reviews.llvm.org/D108233
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D108233: WIP: Add minidump save-core functionality to ELF object files
Aj0SK updated this revision to Diff 369410.
Aj0SK marked 2 inline comments as done and an inline comment as not done.
Aj0SK added a comment.
Add error for dump core style other than stack and change procedure getting the
memory list stream.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108233/new/
https://reviews.llvm.org/D108233
Files:
lldb/include/lldb/Core/PluginManager.h
lldb/source/API/SBProcess.cpp
lldb/source/Commands/CommandObjectProcess.cpp
lldb/source/Commands/Options.td
lldb/source/Core/PluginManager.cpp
lldb/source/Plugins/ObjectFile/CMakeLists.txt
lldb/source/Plugins/ObjectFile/Minidump/CMakeLists.txt
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h
lldb/test/API/functionalities/process_save_core_minidump/Makefile
lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
lldb/test/API/functionalities/process_save_core_minidump/main.cpp
Index: lldb/test/API/functionalities/process_save_core_minidump/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/process_save_core_minidump/main.cpp
@@ -0,0 +1,30 @@
+#include
+#include
+#include
+
+using namespace std;
+
+void g() { assert(false); }
+
+void f() { g(); }
+
+size_t h() {
+ size_t sum = 0;
+ for (size_t i = 0; i < 100; ++i)
+for (size_t j = 0; j < 100; ++j)
+ if ((i * j) % 2 == 0) {
+sum += 1;
+ }
+ return sum;
+}
+
+int main() {
+ thread t1(f);
+
+ size_t x = h();
+
+ t1.join();
+
+ cout << "X is " << x << "\n";
+ return 0;
+}
\ No newline at end of file
Index: lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
===
--- /dev/null
+++ lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -0,0 +1,78 @@
+"""
+Test saving a mini dump.
+"""
+
+
+import os
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class ProcessSaveCoreMinidumpTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipUnlessPlatform(['linux'])
+def test_save_linux_mini_dump(self):
+"""Test that we can save a Linux mini dump."""
+self.build()
+exe = self.getBuildArtifact("a.out")
+core = self.getBuildArtifact("core.dmp")
+try:
+target = self.dbg.CreateTarget(exe)
+process = target.LaunchSimple(
+None, None, self.get_process_working_directory())
+self.assertEqual(process.GetState(), lldb.eStateStopped)
+
+# get neccessary data for the verification phase
+process_info = process.GetProcessInfo()
+expected_pid = process_info.GetProcessID() if process_info.IsValid() else -1
+expected_number_of_modules = target.GetNumModules()
+expected_modules = target.modules
+expected_number_of_threads = process.GetNumThreads()
+expected_threads = []
+
+for thread_idx in range(process.GetNumThreads()):
+thread = process.GetThreadAtIndex(thread_idx)
+thread_id = thread.GetThreadID()
+expected_threads.append(thread_id)
+
+# save core and, kill process and verify corefile existence
+self.runCmd("process save-core --plugin-name=minidump --style=stack " + core)
+self.assertTrue(os.path.isfile(core))
+self.assertTrue(process.Kill().Success())
+
+# To verify, we'll launch with the mini dump
+target = self.dbg.CreateTarget(None)
+process = target.LoadCore(core)
+
+# check if the core is in desired state
+self.assertTrue(process, PROCESS_IS_VALID)
+self.assertTrue(process.GetProcessInfo().IsValid())
+self.assertEqual(process.GetProcessInfo().GetProcessID(), expected_pid)
+self.assertTrue(target.GetTriple().find("linux") != -1)
+self.assertTrue(target.GetNumModules(), expected_number_of_modules)
+self.assertEqual(process.GetNumThreads(), expected_number_of_threads)
+
+for module, expected in zip(target.modules, expected_modules):
+self.assertTrue(module.IsValid())
+module_file_name = module.GetFileSpec().GetFilename()
+expected_file_name = expected.GetFileSpec().GetFilename()
+# skip kernel virtual dynamic shared objects
+if "vdso" in expected_file_name:
+continue
+self.assertEqual(module_file_name, expected_file_name)
+
[Lldb-commits] [PATCH] D108233: WIP: Add minidump save-core functionality to ELF object files
Aj0SK marked 8 inline comments as done and an inline comment as not done.
Aj0SK added inline comments.
Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:519
+lldb_private::Status
+MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP _sp) {
+ Status error;
clayborg wrote:
> Aj0SK wrote:
> > Aj0SK wrote:
> > > clayborg wrote:
> > > > This should take in the CoreStyle and only emit what was asked for.
> > > >
> > > > If style is set to "eSaveCoreStackOnly", then grab only the memory
> > > > regions for each thread stack pointer and save only those.
> > > >
> > > > We can't tell from LLDB APIs if a page is dirty, so if we get
> > > > "eSaveCoreDirtyOnly", then we will need to save all memory regions that
> > > > have write permissions.
> > > >
> > > > If it is either "eSaveCoreFull" or "eSaveCoreUnspecified" then save
> > > > everything.
> > > I agree that this code should take care of a CoreStyle but it poses a
> > > significant problem to this implementation as it was mainly designed to
> > > save smaller Minidumps. This solution stores all of the information in
> > > memory before dumping them (this eases an implementation quite a bit
> > > because of the way how Minidump pointers (offsets) are working). This
> > > implementation, on my machine, exhausted memory before the minidump could
> > > be written in case of a full memory dump. At this time, we don't plan to
> > > reimplement this solution in the way it would allow to store all the data
> > > on disc at the time of minidump creation so there are two possible
> > > solutions that I see:
> > >
> > > 1. If the type of the CoreStyle is full or dirty, this plugin will return
> > > false from the SaveCore function. Then maybe the default CoreStyle for
> > > this plugin should be changed to "eSaveCoreStackOnly".
> > >
> > > 2. To the best of my knowledge, it should be theoretically possible to
> > > reimplement Dump() method to sort of take special care of a
> > > MemoryListStream, dumping also the memory information at the end of the
> > > minidump file as I think the memory dump is the most stressful for the
> > > memory and otherwise there is no problem with this.
> > To state my preference, I would rather stick to the first option and landed
> > a minimum viable product. The only reason for this is that I have this
> > version way better tested and also I am not entirely sure about how
> > minidump deals with the whole memory dumps as it can be a little
> > problematic for a big memory chunks... Then, I would rather add the
> > necessary changes in the next patch...
> That is fine. I was just looking at the implementation below and I think I
> might know why this was taking up too much space. I will comment below, and
> maybe this will fix the issues of storing way too much info in the miniump
In fact, we are mostly interested in the client-server setup where every spared
MB counts. In case it would be also benefiting for LLDB, I can also come up
with an implementation that doesn't save full 8MB per stack but also limits
this... Most of the solutions, that were used for implementing this
functionality(breakpad, crashpad) limit stack information etc. [[
https://chromium.googlesource.com/breakpad/breakpad/+/refs/heads/main/src/google_breakpad/common/minidump_format.h#276
| breakpad ]].
Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:522
+ MemoryRegionInfo range_info;
+ error = process_sp->GetMemoryRegionInfo(0, range_info);
+
clayborg wrote:
> FYI: memory region for address zero often has no permissions, so there is no
> need to actually try and save this memory. For any memory region be sure the
> check if it has permissions using:
>
> ```
> if (region_info.GetLLDBPermissions() != 0)
> ```
> before trying to read or save this off. I will comment below
Thanks, good to know.
Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:530
+ // Get interesting addresses
+ std::vector interesting_addresses;
+ auto thread_list = process_sp->GetThreadList();
clayborg wrote:
> Should we have a std::set to store the base of any regions that
> have already been saved? Many PC values could be in the same regions (like
> when many threads are waiting in syscalls).
In the proposed approach, I believe so. In the old approach this was taken care
of by the fact that we only went through the every segment only once.
Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:549
+bool is_interesting = false;
+for (size_t interesting_address : interesting_addresses)
+ if (interesting_address >= addr && interesting_address < addr + size) {
Aj0SK wrote:
> clayborg wrote:
> > Shouldn't this be the main loop that would replace the "while
> > (range_info.GetRange().GetRangeBase() !=
[Lldb-commits] [PATCH] D108233: WIP: Add minidump save-core functionality to ELF object files
Aj0SK added inline comments.
Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:549
+bool is_interesting = false;
+for (size_t interesting_address : interesting_addresses)
+ if (interesting_address >= addr && interesting_address < addr + size) {
clayborg wrote:
> Shouldn't this be the main loop that would replace the "while
> (range_info.GetRange().GetRangeBase() != LLDB_INVALID_ADDRESS)" loop? I would
> think that the main loop wiould be something like:
>
> ```
> std::set visited_region_base_addresses;
> for (size_t interesting_address : interesting_addresses) {
> error = process_sp->GetMemoryRegionInfo(interesting_address, range_info);
> // Skip failed memory region requests or any regions with no permissions.
> if (error.Fail() || range_info.GetPermissions() == 0)
> continue;
> const addr_t addr = range_info.GetRange().GetRangeBase();
> // Skip any regions we have already saved out.
> if (visited_region_base_addresses.insert(addr).second == false)
> continue;
> const addr_t size = range_info.GetRange().GetByteSize();
> if (size == 0)
> continue;
> auto data_up = std::make_unique(size, 0);
> const size_t bytes_read = process_sp->ReadMemory(addr, data_up->GetBytes(),
> size, error);
> if (bytes_read == 0)
> continue;
> // We have a good memory region with valid bytes to store.
> LocationDescriptor memory_dump;
> memory_dump.DataSize = static_cast(size);
> memory_dump.RVA
> =static_cast(GetCurrentDataEndOffset());
> MemoryDescriptor memory_desc;
> memory_desc.StartOfMemoryRange =
> static_cast(addr);
> memory_desc.Memory = memory_dump;
> mem_descriptors.push_back(memory_desc);
> m_data.AppendData(data_up->GetBytes(), bytes_read);
> }
> ```
>
>
This kind of an approach was something I also thought about and in fact, I was
mainly inspired by this [[
https://github.com/llvm-mirror/lldb/blob/d01083a850f577b85501a0902b52fd0930de72c7/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp#L6051
| approach ]]. Also, the previous comment about the need of saving the loaded
modules addresses in a set doesn't apply to my solution, am I correct? Just to
clear it a little bit, I like your solution though and it's better when I think
about it. Thanks for the suggestion!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108233/new/
https://reviews.llvm.org/D108233
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D108233: WIP: Add minidump save-core functionality to ELF object files
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
No worries on only saving out the minimal for now, but just make sure that you
error out if CoreStyle is anything but "eSaveCoreUnspecified" or
"eSaveCoreStackOnly".
Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:519
+lldb_private::Status
+MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP _sp) {
+ Status error;
Aj0SK wrote:
> Aj0SK wrote:
> > clayborg wrote:
> > > This should take in the CoreStyle and only emit what was asked for.
> > >
> > > If style is set to "eSaveCoreStackOnly", then grab only the memory
> > > regions for each thread stack pointer and save only those.
> > >
> > > We can't tell from LLDB APIs if a page is dirty, so if we get
> > > "eSaveCoreDirtyOnly", then we will need to save all memory regions that
> > > have write permissions.
> > >
> > > If it is either "eSaveCoreFull" or "eSaveCoreUnspecified" then save
> > > everything.
> > I agree that this code should take care of a CoreStyle but it poses a
> > significant problem to this implementation as it was mainly designed to
> > save smaller Minidumps. This solution stores all of the information in
> > memory before dumping them (this eases an implementation quite a bit
> > because of the way how Minidump pointers (offsets) are working). This
> > implementation, on my machine, exhausted memory before the minidump could
> > be written in case of a full memory dump. At this time, we don't plan to
> > reimplement this solution in the way it would allow to store all the data
> > on disc at the time of minidump creation so there are two possible
> > solutions that I see:
> >
> > 1. If the type of the CoreStyle is full or dirty, this plugin will return
> > false from the SaveCore function. Then maybe the default CoreStyle for this
> > plugin should be changed to "eSaveCoreStackOnly".
> >
> > 2. To the best of my knowledge, it should be theoretically possible to
> > reimplement Dump() method to sort of take special care of a
> > MemoryListStream, dumping also the memory information at the end of the
> > minidump file as I think the memory dump is the most stressful for the
> > memory and otherwise there is no problem with this.
> To state my preference, I would rather stick to the first option and landed a
> minimum viable product. The only reason for this is that I have this version
> way better tested and also I am not entirely sure about how minidump deals
> with the whole memory dumps as it can be a little problematic for a big
> memory chunks... Then, I would rather add the necessary changes in the next
> patch...
That is fine. I was just looking at the implementation below and I think I
might know why this was taking up too much space. I will comment below, and
maybe this will fix the issues of storing way too much info in the miniump
Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:522
+ MemoryRegionInfo range_info;
+ error = process_sp->GetMemoryRegionInfo(0, range_info);
+
FYI: memory region for address zero often has no permissions, so there is no
need to actually try and save this memory. For any memory region be sure the
check if it has permissions using:
```
if (region_info.GetLLDBPermissions() != 0)
```
before trying to read or save this off. I will comment below
Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:530
+ // Get interesting addresses
+ std::vector interesting_addresses;
+ auto thread_list = process_sp->GetThreadList();
Should we have a std::set to store the base of any regions that
have already been saved? Many PC values could be in the same regions (like when
many threads are waiting in syscalls).
Comment at:
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:544-546
+ while (range_info.GetRange().GetRangeBase() != LLDB_INVALID_ADDRESS) {
+const addr_t addr = range_info.GetRange().GetRangeBase();
+const addr_t size = range_info.GetRange().GetByteSize();
Why are we checking the region_info for address zero here? The first time
through this loop, it will contain information for address zero which will be a
region that has no permissions. I suggest a new main loop in the comment below
which simplifies this entire logic and also makes sure we don't emit the same
region more than once.
Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:549
+bool is_interesting = false;
+for (size_t interesting_address : interesting_addresses)
+ if (interesting_address >= addr && interesting_address < addr + size) {
Shouldn't this be the main loop that would replace the "while
(range_info.GetRange().GetRangeBase() != LLDB_INVALID_ADDRESS)" loop? I
[Lldb-commits] [PATCH] D108233: WIP: Add minidump save-core functionality to ELF object files
Aj0SK added inline comments.
Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:519
+lldb_private::Status
+MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP _sp) {
+ Status error;
Aj0SK wrote:
> clayborg wrote:
> > This should take in the CoreStyle and only emit what was asked for.
> >
> > If style is set to "eSaveCoreStackOnly", then grab only the memory regions
> > for each thread stack pointer and save only those.
> >
> > We can't tell from LLDB APIs if a page is dirty, so if we get
> > "eSaveCoreDirtyOnly", then we will need to save all memory regions that
> > have write permissions.
> >
> > If it is either "eSaveCoreFull" or "eSaveCoreUnspecified" then save
> > everything.
> I agree that this code should take care of a CoreStyle but it poses a
> significant problem to this implementation as it was mainly designed to save
> smaller Minidumps. This solution stores all of the information in memory
> before dumping them (this eases an implementation quite a bit because of the
> way how Minidump pointers (offsets) are working). This implementation, on my
> machine, exhausted memory before the minidump could be written in case of a
> full memory dump. At this time, we don't plan to reimplement this solution in
> the way it would allow to store all the data on disc at the time of minidump
> creation so there are two possible solutions that I see:
>
> 1. If the type of the CoreStyle is full or dirty, this plugin will return
> false from the SaveCore function. Then maybe the default CoreStyle for this
> plugin should be changed to "eSaveCoreStackOnly".
>
> 2. To the best of my knowledge, it should be theoretically possible to
> reimplement Dump() method to sort of take special care of a MemoryListStream,
> dumping also the memory information at the end of the minidump file as I
> think the memory dump is the most stressful for the memory and otherwise
> there is no problem with this.
To state my preference, I would rather stick to the first option and landed a
minimum viable product. The only reason for this is that I have this version
way better tested and also I am not entirely sure about how minidump deals with
the whole memory dumps as it can be a little problematic for a big memory
chunks... Then, I would rather add the necessary changes in the next patch...
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108233/new/
https://reviews.llvm.org/D108233
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D108233: WIP: Add minidump save-core functionality to ELF object files
Aj0SK added inline comments.
Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:225
+m.SizeOfImage = static_cast(
+mod->GetObjectFile()->GetByteSize());
+m.Checksum = static_cast(0);
clayborg wrote:
> I am not sure if the object file's full size is the right value here. I think
> the right value would be the last contiguous loaded address range for a given
> file. This information is going to be used to set the load addresses of any
> sections in the object file when it is loaded, and if we have a huge ELF file
> that has tons of debug info, this is going to make the loaded address range
> way too big.
>
> So the algorithm I would suggest is:
> 1 - grab the section that contains base address:
> 2 - in a loop, grab the next section that starts at the end of this section,
> and as long as the addresses are contiguous, increase the SizeOfImage:
>
> So something like:
> ```
> lldb::SectionSP sect_sp = mod->GetObjectFile()->GetBaseAddress().GetSection();
> uint64_t SizeOfImage = 0;
> if (sect_sp) {
> lldb::addr_t sect_addr = sect_sp->GetLoadAddress();
> // Use memory size since zero fill sections, like ".bss", will be smaller
> on disk.
> lldb::addr_t sect_size = sect_sp->MemorySize();
> // This will usually be zero, but make sure to calculate the BaseOfImage
> offset.
> const lldb::addr_t base_sect_offset = m.BaseOfImage - sect_addr;
> SizeOfImage = sect_size - base_sect_offset;
> lldb::addr_t next_sect_addr = sect_addr + sect_size;
> Address sect_so_addr;
> target.ResolveLoadAddress(next_sect_addr, sect_so_addr);
> lldb::SectionSP next_sect_sp = sect_so_addr.GetSections();
> while (next_sect_sp && next_sect_sp->GetLoadBaseAddress() ==
> next_sect_addr)) {
> sect_size = sect_sp->MemorySize();
> SizeOfImage += sect_size;
> next_sect_addr += sect_size;
> target.ResolveLoadAddress(next_sect_addr, sect_so_addr);
> next_sect_sp = sect_so_addr.GetSections();
> }
> m.SizeOfImage = static_cast(SizeOfImage);
> } else {
> m.SizeOfImage = static_cast(sect_size);
> }
> ```
>
Thank you for the suggestion. I will add it to the next revision.
Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:519
+lldb_private::Status
+MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP _sp) {
+ Status error;
clayborg wrote:
> This should take in the CoreStyle and only emit what was asked for.
>
> If style is set to "eSaveCoreStackOnly", then grab only the memory regions
> for each thread stack pointer and save only those.
>
> We can't tell from LLDB APIs if a page is dirty, so if we get
> "eSaveCoreDirtyOnly", then we will need to save all memory regions that have
> write permissions.
>
> If it is either "eSaveCoreFull" or "eSaveCoreUnspecified" then save
> everything.
I agree that this code should take care of a CoreStyle but it poses a
significant problem to this implementation as it was mainly designed to save
smaller Minidumps. This solution stores all of the information in memory before
dumping them (this eases an implementation quite a bit because of the way how
Minidump pointers (offsets) are working). This implementation, on my machine,
exhausted memory before the minidump could be written in case of a full memory
dump. At this time, we don't plan to reimplement this solution in the way it
would allow to store all the data on disc at the time of minidump creation so
there are two possible solutions that I see:
1. If the type of the CoreStyle is full or dirty, this plugin will return false
from the SaveCore function. Then maybe the default CoreStyle for this plugin
should be changed to "eSaveCoreStackOnly".
2. To the best of my knowledge, it should be theoretically possible to
reimplement Dump() method to sort of take special care of a MemoryListStream,
dumping also the memory information at the end of the minidump file as I think
the memory dump is the most stressful for the memory and otherwise there is no
problem with this.
Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:667
+if (memory_buffer) {
+ size_t size = memory_buffer->getBufferSize();
+ AddDirectory(stream, size);
clayborg wrote:
> Do we need to make sure size is not zero?
I can add this. The idea was that even if the file is "empty", we at least give
the user opening the minidump the information that we have been able to find
this file and add it to the minidump instead of only not providing any
information at all.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108233/new/
https://reviews.llvm.org/D108233
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D108233: WIP: Add minidump save-core functionality to ELF object files
clayborg added a comment.
Very nice! Structure is good now. I found a few other issue we should fix as
well.
Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:225
+m.SizeOfImage = static_cast(
+mod->GetObjectFile()->GetByteSize());
+m.Checksum = static_cast(0);
I am not sure if the object file's full size is the right value here. I think
the right value would be the last contiguous loaded address range for a given
file. This information is going to be used to set the load addresses of any
sections in the object file when it is loaded, and if we have a huge ELF file
that has tons of debug info, this is going to make the loaded address range way
too big.
So the algorithm I would suggest is:
1 - grab the section that contains base address:
2 - in a loop, grab the next section that starts at the end of this section,
and as long as the addresses are contiguous, increase the SizeOfImage:
So something like:
```
lldb::SectionSP sect_sp = mod->GetObjectFile()->GetBaseAddress().GetSection();
uint64_t SizeOfImage = 0;
if (sect_sp) {
lldb::addr_t sect_addr = sect_sp->GetLoadAddress();
// Use memory size since zero fill sections, like ".bss", will be smaller on
disk.
lldb::addr_t sect_size = sect_sp->MemorySize();
// This will usually be zero, but make sure to calculate the BaseOfImage
offset.
const lldb::addr_t base_sect_offset = m.BaseOfImage - sect_addr;
SizeOfImage = sect_size - base_sect_offset;
lldb::addr_t next_sect_addr = sect_addr + sect_size;
Address sect_so_addr;
target.ResolveLoadAddress(next_sect_addr, sect_so_addr);
lldb::SectionSP next_sect_sp = sect_so_addr.GetSections();
while (next_sect_sp && next_sect_sp->GetLoadBaseAddress() ==
next_sect_addr)) {
sect_size = sect_sp->MemorySize();
SizeOfImage += sect_size;
next_sect_addr += sect_size;
target.ResolveLoadAddress(next_sect_addr, sect_so_addr);
next_sect_sp = sect_so_addr.GetSections();
}
m.SizeOfImage = static_cast(SizeOfImage);
} else {
m.SizeOfImage = static_cast(sect_size);
}
```
Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:519
+lldb_private::Status
+MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP _sp) {
+ Status error;
This should take in the CoreStyle and only emit what was asked for.
If style is set to "eSaveCoreStackOnly", then grab only the memory regions for
each thread stack pointer and save only those.
We can't tell from LLDB APIs if a page is dirty, so if we get
"eSaveCoreDirtyOnly", then we will need to save all memory regions that have
write permissions.
If it is either "eSaveCoreFull" or "eSaveCoreUnspecified" then save everything.
Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:667
+if (memory_buffer) {
+ size_t size = memory_buffer->getBufferSize();
+ AddDirectory(stream, size);
Do we need to make sure size is not zero?
Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h:1-3
+//===-- MinidumpFileBuilder.h -- -*- C++
+//-*-===//
+//
Fix this header comment line to be on one line.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108233/new/
https://reviews.llvm.org/D108233
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D108233: WIP: Add minidump save-core functionality to ELF object files
Aj0SK updated this revision to Diff 368765.
Aj0SK added a comment.
Change ObjectFileMinidump plugin to inherit from PluginInterface
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108233/new/
https://reviews.llvm.org/D108233
Files:
lldb/include/lldb/Core/PluginManager.h
lldb/source/API/SBProcess.cpp
lldb/source/Commands/CommandObjectProcess.cpp
lldb/source/Commands/Options.td
lldb/source/Core/PluginManager.cpp
lldb/source/Plugins/ObjectFile/CMakeLists.txt
lldb/source/Plugins/ObjectFile/Minidump/CMakeLists.txt
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h
lldb/test/API/functionalities/process_save_core_minidump/Makefile
lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
lldb/test/API/functionalities/process_save_core_minidump/main.cpp
Index: lldb/test/API/functionalities/process_save_core_minidump/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/process_save_core_minidump/main.cpp
@@ -0,0 +1,30 @@
+#include
+#include
+#include
+
+using namespace std;
+
+void g() { assert(false); }
+
+void f() { g(); }
+
+size_t h() {
+ size_t sum = 0;
+ for (size_t i = 0; i < 100; ++i)
+for (size_t j = 0; j < 100; ++j)
+ if ((i * j) % 2 == 0) {
+sum += 1;
+ }
+ return sum;
+}
+
+int main() {
+ thread t1(f);
+
+ size_t x = h();
+
+ t1.join();
+
+ cout << "X is " << x << "\n";
+ return 0;
+}
\ No newline at end of file
Index: lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
===
--- /dev/null
+++ lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -0,0 +1,78 @@
+"""
+Test saving a mini dump.
+"""
+
+
+import os
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class ProcessSaveCoreMinidumpTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipUnlessPlatform(['linux'])
+def test_save_linux_mini_dump(self):
+"""Test that we can save a Linux mini dump."""
+self.build()
+exe = self.getBuildArtifact("a.out")
+core = self.getBuildArtifact("core.dmp")
+try:
+target = self.dbg.CreateTarget(exe)
+process = target.LaunchSimple(
+None, None, self.get_process_working_directory())
+self.assertEqual(process.GetState(), lldb.eStateStopped)
+
+# get neccessary data for the verification phase
+process_info = process.GetProcessInfo()
+expected_pid = process_info.GetProcessID() if process_info.IsValid() else -1
+expected_number_of_modules = target.GetNumModules()
+expected_modules = target.modules
+expected_number_of_threads = process.GetNumThreads()
+expected_threads = []
+
+for thread_idx in range(process.GetNumThreads()):
+thread = process.GetThreadAtIndex(thread_idx)
+thread_id = thread.GetThreadID()
+expected_threads.append(thread_id)
+
+# save core and, kill process and verify corefile existence
+self.runCmd("process save-core --plugin-name=minidump " + core)
+self.assertTrue(os.path.isfile(core))
+self.assertTrue(process.Kill().Success())
+
+# To verify, we'll launch with the mini dump
+target = self.dbg.CreateTarget(None)
+process = target.LoadCore(core)
+
+# check if the core is in desired state
+self.assertTrue(process, PROCESS_IS_VALID)
+self.assertTrue(process.GetProcessInfo().IsValid())
+self.assertEqual(process.GetProcessInfo().GetProcessID(), expected_pid)
+self.assertTrue(target.GetTriple().find("linux") != -1)
+self.assertTrue(target.GetNumModules(), expected_number_of_modules)
+self.assertEqual(process.GetNumThreads(), expected_number_of_threads)
+
+for module, expected in zip(target.modules, expected_modules):
+self.assertTrue(module.IsValid())
+module_file_name = module.GetFileSpec().GetFilename()
+expected_file_name = expected.GetFileSpec().GetFilename()
+# skip kernel virtual dynamic shared objects
+if "vdso" in expected_file_name:
+continue
+self.assertEqual(module_file_name, expected_file_name)
+self.assertEqual(module.GetUUIDString(), expected.GetUUIDString())
+
+for thread_idx in
[Lldb-commits] [PATCH] D108233: WIP: Add minidump save-core functionality to ELF object files
clayborg added inline comments.
Comment at: lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp:31
+ /*create_memory_callback=*/nullptr,
+ /*get_module_specifications=*/nullptr, SaveCore);
+}
It should be fine to not inherit from ObjectFile and yes I agree, probably best
to not modify other code.
All you need to provide is static function callbacks that have the correct
static function signature, and that doesn't require your class to inherit from
ObjectFile. You already have a CreateInstance function to see how to do this.
The CreateInstance(...) function you have can do nothing and just return
nullptr.
Then you need to make a create memory callback and a get module specifications:
```
ObjectFile *ObjectFileMinidump::CreateMemoryInstance(
const lldb::ModuleSP _sp, DataBufferSP _sp,
const ProcessSP _sp, lldb::addr_t header_addr) {
return nullptr;
}
size_t ObjectFileMinidump::GetModuleSpecifications(
const lldb_private::FileSpec , lldb::DataBufferSP _sp,
lldb::offset_t data_offset, lldb::offset_t file_offset,
lldb::offset_t length, lldb_private::ModuleSpecList ) {
specs.Clear();
return 0;
}
```
Then your registration call looks like:
```
PluginManager::RegisterPlugin(
GetPluginNameStatic(), GetPluginDescriptionStatic(), CreateInstance,
CreateMemoryInstance, GetModuleSpecifications, SaveCore);
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108233/new/
https://reviews.llvm.org/D108233
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D108233: WIP: Add minidump save-core functionality to ELF object files
Aj0SK marked 9 inline comments as done. Aj0SK added a comment. Thanks for the review! Some requested changes need to be clarified for me. I have a problem mainly with successfully registering the Plugin when it inherits only from PluginInterface. Comment at: lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp:30-31 + GetPluginNameStatic(), GetPluginDescriptionStatic(), CreateInstance, + /*create_memory_callback=*/nullptr, + /*get_module_specifications=*/nullptr, SaveCore); +} clayborg wrote: > We have two options here: > 1 - if we want to pass NULL for any callbacks, then we need to fix all > callsites to that use > PluginManager::GetObjectFileCreateMemoryCallbackAtIndex(...) and > PluginManager::GetObjectFileGetModuleSpecificationsCallbackAtIndex(...) to > have this done inside of new plug-in manager > PluginManager::CreateMemoryCallbackAtIndex(...) and > PluginManager::GetModuleSpecifications(...) where these new functions iterate > over all plug-ins and skip any plug-ins that have NULL callbacks. > 2 - add valid create_memory_callback and get_module_specifications callbacks > that return invalid values > > Otherwise this plug-in might stop the iteration early for existing callers of > PluginManager::GetObjectFileCreateMemoryCallbackAtIndex(...) and > PluginManager::GetObjectFileGetModuleSpecificationsCallbackAtIndex(...). I would prefer modifying code outside of this functionality as little as possible so I would stick to the second option. However, maybe returning the invalid values is also not the best solution. Also, does this option works with ObjectFileMinidump inheriting from PluginInterface? I think that I am unable to use the same PluginManager::RegisterPlugin method, so not sure how this plugin gets to ObjectFilesInstances... Maybe I am missing out something. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108233/new/ https://reviews.llvm.org/D108233 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D108233: WIP: Add minidump save-core functionality to ELF object files
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
Thanks for moving over into object file. See inlined comments, we should be
able to get this down to just the SaveCore and other static functions that just
return nothing. Let me know if the inlined comments don't make sense.
Comment at: lldb/source/Commands/CommandObjectProcess.cpp:1235
SaveCoreStyle m_requested_save_core_style;
+std::string m_requested_plugin_name;
};
Make this a ConstString
Comment at: lldb/source/Commands/CommandObjectProcess.cpp:1242
if (process_sp) {
- if (command.GetArgumentCount() == 1) {
+ ConstString plugin_name(m_options.m_requested_plugin_name.c_str());
+ if (command.GetArgumentCount() <= 2) {
Remove this if we switch m_requested_plugin_name to be a ConstString. Also a
quick FYI: ConstString has a std::string constructor, so there is no need to
add ".c_str()" here if this code was going to stay.
Comment at: lldb/source/Commands/CommandObjectProcess.cpp:1243
+ ConstString plugin_name(m_options.m_requested_plugin_name.c_str());
+ if (command.GetArgumentCount() <= 2) {
FileSpec output_file(command.GetArgumentAtIndex(0));
We haven't changed the number of arguments here right? Options and their option
values to no count towards the argument count. So all of these have just one
argument:
```
(lldb) process save-core
(lldb) process save-core -p
(lldb) proicess save-core -s
[Lldb-commits] [PATCH] D108233: WIP: Add minidump save-core functionality to ELF object files
Aj0SK updated this revision to Diff 368110.
Aj0SK added a comment.
Herald added a subscriber: dang.
Move minidump save-core functionality to separate plugin
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108233/new/
https://reviews.llvm.org/D108233
Files:
lldb/include/lldb/Core/PluginManager.h
lldb/source/API/SBProcess.cpp
lldb/source/Commands/CommandObjectProcess.cpp
lldb/source/Commands/Options.td
lldb/source/Core/PluginManager.cpp
lldb/source/Plugins/ObjectFile/CMakeLists.txt
lldb/source/Plugins/ObjectFile/Minidump/CMakeLists.txt
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h
lldb/test/API/functionalities/process_save_core_minidump/Makefile
lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
lldb/test/API/functionalities/process_save_core_minidump/main.cpp
Index: lldb/test/API/functionalities/process_save_core_minidump/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/process_save_core_minidump/main.cpp
@@ -0,0 +1,30 @@
+#include
+#include
+#include
+
+using namespace std;
+
+void g() { assert(false); }
+
+void f() { g(); }
+
+size_t h() {
+ size_t sum = 0;
+ for (size_t i = 0; i < 100; ++i)
+for (size_t j = 0; j < 100; ++j)
+ if ((i * j) % 2 == 0) {
+sum += 1;
+ }
+ return sum;
+}
+
+int main() {
+ thread t1(f);
+
+ size_t x = h();
+
+ t1.join();
+
+ cout << "X is " << x << "\n";
+ return 0;
+}
\ No newline at end of file
Index: lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
===
--- /dev/null
+++ lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -0,0 +1,78 @@
+"""
+Test saving a mini dump.
+"""
+
+
+import os
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class ProcessSaveCoreMinidumpTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipUnlessPlatform(['linux'])
+def test_save_linux_mini_dump(self):
+"""Test that we can save a Linux mini dump."""
+self.build()
+exe = self.getBuildArtifact("a.out")
+core = self.getBuildArtifact("core.dmp")
+try:
+target = self.dbg.CreateTarget(exe)
+process = target.LaunchSimple(
+None, None, self.get_process_working_directory())
+self.assertEqual(process.GetState(), lldb.eStateStopped)
+
+# get neccessary data for the verification phase
+process_info = process.GetProcessInfo()
+expected_pid = process_info.GetProcessID() if process_info.IsValid() else -1
+expected_number_of_modules = target.GetNumModules()
+expected_modules = target.modules
+expected_number_of_threads = process.GetNumThreads()
+expected_threads = []
+
+for thread_idx in range(process.GetNumThreads()):
+thread = process.GetThreadAtIndex(thread_idx)
+thread_id = thread.GetThreadID()
+expected_threads.append(thread_id)
+
+# save core and, kill process and verify corefile existence
+self.runCmd("process save-core --plugin-name=minidump " + core)
+self.assertTrue(os.path.isfile(core))
+self.assertTrue(process.Kill().Success())
+
+# To verify, we'll launch with the mini dump
+target = self.dbg.CreateTarget(None)
+process = target.LoadCore(core)
+
+# check if the core is in desired state
+self.assertTrue(process, PROCESS_IS_VALID)
+self.assertTrue(process.GetProcessInfo().IsValid())
+self.assertEqual(process.GetProcessInfo().GetProcessID(), expected_pid)
+self.assertTrue(target.GetTriple().find("linux") != -1)
+self.assertTrue(target.GetNumModules(), expected_number_of_modules)
+self.assertEqual(process.GetNumThreads(), expected_number_of_threads)
+
+for module, expected in zip(target.modules, expected_modules):
+self.assertTrue(module.IsValid())
+module_file_name = module.GetFileSpec().GetFilename()
+expected_file_name = expected.GetFileSpec().GetFilename()
+# skip kernel virtual dynamic shared objects
+if "vdso" in expected_file_name:
+continue
+self.assertEqual(module_file_name, expected_file_name)
+self.assertEqual(module.GetUUIDString(), expected.GetUUIDString())
+
+
[Lldb-commits] [PATCH] D108233: WIP: Add minidump save-core functionality to ELF object files
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
I like the idea of this! I have a minidump creation tool I wrote in python for
making tests.
ELF files support core files in their native file format, so I think the
ObjectFileELF::SaveCore(...) should actually save an ELF core file if it saves
anything. So we should move the minidump creation to another location. I
mention ideas below.
When running "process save-core" we can add a new option like
"--plugin=", and if this option is specified we will look for a plug-in
name that matches when iterating over the instances that support core file
exporting. Right now the save core calls the plugin manager:
Status PluginManager::SaveCore(const lldb::ProcessSP _sp,
const FileSpec ,
lldb::SaveCoreStyle _style) {
Status error;
auto = GetObjectFileInstances().GetInstances();
for (auto : instances) {
if (instance.save_core &&
instance.save_core(process_sp, outfile, core_style, error))
return error;
}
error.SetErrorString(
"no ObjectFile plugins were able to save a core for this process");
return error;
}
Currently we are expecting only one ObjectFile class to return true for any
given process. But since ObjectFileELF::SaveCore(...) could eventually be added
to ObjectFileELF, we need a way to force a plug-in to be considered. If we add
an extra "ConstString plugin_name" to the arguments, we can check the name and
target a different plug-in. It should be possible to save a core file in
different formats.
So I propose:
- remove code from ELF ObjectFile plug-in
- modify PluginManager::SaveCore(...) to take an extra "ConstString
plugin_name" parameter.
- If this parameter is valid, skip any "instance" whose name doesn't match
- Modify the ObjectFileSaveCore callback definition to take a "bool force"
parameter which will make the plug-ins skip the triple detection stuff that
will skip saving the core file if each SaveCore(...) function didn't like the
triple it was passed. It is fine for a plug-in to return an error stating that
saving a core file with the current process doesn't make sense since each core
file has to have support for saving registers.
- make a new Minidump ObjectFile plug-in at
"lldb/source/Plugins/ObjectFile/Minidump"
- Create a lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp and
lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h
- when you register the callbacks for this plug-in, only register the
ObjectFileMinidump::SaveCore callback (see example code below)
void ObjectFileMinidump::Initialize() {
PluginManager::RegisterPlugin(
GetPluginNameStatic(),
GetPluginDescriptionStatic(),
/*create_callback=*/nullptr,
/*create_memory_callback=*/nullptr,
/*get_module_specifications=*/nullptr,
SaveCore);
}
Then hopefully you can just make a very ObjectFileMinidump with a few plug-in
template functions and the SaveCore(...) function. Let me know if any of this
is not clear.
Comment at: lldb/source/Plugins/ObjectFile/ELF/MinidumpFileBuilder.cpp:70
+ default:
+arch = ProcessorArchitecture::AMD64;
+break;
Seems like we should return an error here if the architecture isn't supported
with a nice error string that gets displayed. We shouldn't default to AMD64
right?
Comment at: lldb/source/Plugins/ObjectFile/ELF/MinidumpFileBuilder.cpp:80
+ GetCurrentDataEndOffset() + sizeof(llvm::minidump::SystemInfo));
+ sys_info.PlatformId = OSPlatform::Linux;
+ m_data.AppendData(_info, sizeof(llvm::minidump::SystemInfo));
We should be able to populate this correctly using the triple right? And error
out if we don't support the OS?
Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:725-727
+ if (error.Fail()) {
+return false;
+ }
remove braces for single statement if (LLVM coding guidelines)
Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:730-732
+ if (error.Fail()) {
+return false;
+ }
remove braces for single statement if (LLVM coding guidelines)
Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:738-740
+if (error.Fail()) {
+ return false;
+}
remove braces for single statement if (LLVM coding guidelines)
Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:743-745
+if (error.Fail()) {
+ return false;
+}
remove braces for single statement if (LLVM coding guidelines)
Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:748-750
+if (error.Fail()) {
+ return false;
+}
remove braces for
[Lldb-commits] [PATCH] D108233: WIP: Add minidump save-core functionality to ELF object files
Aj0SK created this revision.
Aj0SK added a reviewer: clayborg.
Herald added subscribers: pengfei, mgorny, emaste.
Aj0SK requested review of this revision.
Herald added subscribers: lldb-commits, MaskRay.
Herald added a project: LLDB.
This change adds save-core functionality into the ObjectFileELF that enables
saving minidump of a stopped process. This change is mainly targeting Linux
running on x86_64 machines. Minidump should contain basic information needed
to examine state of threads, local variables and stack traces. Full support
for other platforms is not so far implemented. API tests are using LLDB's
MinidumpParser.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D108233
Files:
lldb/source/Plugins/ObjectFile/ELF/CMakeLists.txt
lldb/source/Plugins/ObjectFile/ELF/MinidumpFileBuilder.cpp
lldb/source/Plugins/ObjectFile/ELF/MinidumpFileBuilder.h
lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
lldb/test/API/functionalities/process_save_core_minidump/Makefile
lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
lldb/test/API/functionalities/process_save_core_minidump/main.cpp
Index: lldb/test/API/functionalities/process_save_core_minidump/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/process_save_core_minidump/main.cpp
@@ -0,0 +1,30 @@
+#include
+#include
+#include
+
+using namespace std;
+
+void g() { assert(false); }
+
+void f() { g(); }
+
+size_t h() {
+ size_t sum = 0;
+ for (size_t i = 0; i < 100; ++i)
+for (size_t j = 0; j < 100; ++j)
+ if ((i * j) % 2 == 0) {
+sum += 1;
+ }
+ return sum;
+}
+
+int main() {
+ thread t1(f);
+
+ size_t x = h();
+
+ t1.join();
+
+ cout << "X is " << x << "\n";
+ return 0;
+}
\ No newline at end of file
Index: lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
===
--- /dev/null
+++ lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -0,0 +1,78 @@
+"""
+Test saving a mini dump.
+"""
+
+
+import os
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class ProcessSaveCoreMinidumpTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipUnlessPlatform(['linux'])
+def test_save_linux_mini_dump(self):
+"""Test that we can save a Linux mini dump."""
+self.build()
+exe = self.getBuildArtifact("a.out")
+core = self.getBuildArtifact("core.dmp")
+try:
+target = self.dbg.CreateTarget(exe)
+process = target.LaunchSimple(
+None, None, self.get_process_working_directory())
+self.assertEqual(process.GetState(), lldb.eStateStopped)
+
+# get neccessary data for the verification phase
+process_info = process.GetProcessInfo()
+expected_pid = process_info.GetProcessID() if process_info.IsValid() else -1
+expected_number_of_modules = target.GetNumModules()
+expected_modules = target.modules
+expected_number_of_threads = process.GetNumThreads()
+expected_threads = []
+
+for thread_idx in range(process.GetNumThreads()):
+thread = process.GetThreadAtIndex(thread_idx)
+thread_id = thread.GetThreadID()
+expected_threads.append(thread_id)
+
+# save core and, kill process and verify corefile existence
+self.assertTrue(process.SaveCore(core))
+self.assertTrue(os.path.isfile(core))
+self.assertTrue(process.Kill().Success())
+
+# To verify, we'll launch with the mini dump
+target = self.dbg.CreateTarget(None)
+process = target.LoadCore(core)
+
+# check if the core is in desired state
+self.assertTrue(process, PROCESS_IS_VALID)
+self.assertTrue(process.GetProcessInfo().IsValid())
+self.assertEqual(process.GetProcessInfo().GetProcessID(), expected_pid)
+self.assertTrue(target.GetTriple().find("linux") != -1)
+self.assertTrue(target.GetNumModules(), expected_number_of_modules)
+self.assertEqual(process.GetNumThreads(), expected_number_of_threads)
+
+for module, expected in zip(target.modules, expected_modules):
+self.assertTrue(module.IsValid())
+module_file_name = module.GetFileSpec().GetFilename()
+expected_file_name = expected.GetFileSpec().GetFilename()
+# skip kernel virtual dynamic shared objects
+if "vdso" in expected_file_name:
+continue
+self.assertEqual(module_file_name,
