[Lldb-commits] [lldb] 6870ac2 - Fix saving minidump from lldb-dap (#89113)
via lldb-commits
lldb-commits at lists.llvm.org
Thu Apr 18 09:24:38 PDT 2024
Author: jeffreytan81
Date: 2024-04-18T09:24:33-07:00
New Revision: 6870ac201f9f50a13313213fea7a14b198d7280a
URL: https://github.com/llvm/llvm-project/commit/6870ac201f9f50a13313213fea7a14b198d7280a
DIFF: https://github.com/llvm/llvm-project/commit/6870ac201f9f50a13313213fea7a14b198d7280a.diff
LOG: Fix saving minidump from lldb-dap (#89113)
There are users reporting saving minidump from lldb-dap does not work.
Turns out our stack trace request always evaluate a function call which
caused JIT object file like "__lldb_caller_function" to be created which
can fail minidump builder to get its module size.
This patch fixes "getModuleFileSize" for ObjectFileJIT so that module
list can be saved. I decided to create a lldb-dap test so that this
end-to-end functionality can be validated from our tests (instead of
only command line lldb).
The patch also improves several other small things in the workflow:
1. It logs any minidump stream failure so that it is easier to find out
what stream saving fails. In future, we should show process and error to
end users.
2. It handles error from "getModuleFileSize" llvm::Expected<T> otherwise
it will complain the error is not handled.
---------
Co-authored-by: jeffreytan81 <jeffreytan at fb.com>
Added:
lldb/test/API/tools/lldb-dap/save-core/Makefile
lldb/test/API/tools/lldb-dap/save-core/TestDAP_save_core.py
lldb/test/API/tools/lldb-dap/save-core/main.cpp
Modified:
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
Removed:
################################################################################
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 50d1b563f469cf..cefd4cb22b6bae 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -20,6 +20,7 @@
#include "lldb/Target/StopInfo.h"
#include "lldb/Target/ThreadList.h"
#include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/RegisterValue.h"
#include "llvm/ADT/StringRef.h"
@@ -154,8 +155,16 @@ Status WriteString(const std::string &to_write,
llvm::Expected<uint64_t> getModuleFileSize(Target &target,
const ModuleSP &mod) {
- SectionSP sect_sp = mod->GetObjectFile()->GetBaseAddress().GetSection();
+ // JIT module has the same vm and file size.
uint64_t SizeOfImage = 0;
+ if (mod->GetObjectFile()->CalculateType() == ObjectFile::Type::eTypeJIT) {
+ for (const auto §ion : *mod->GetObjectFile()->GetSectionList()) {
+ SizeOfImage += section->GetByteSize();
+ }
+ return SizeOfImage;
+ }
+
+ SectionSP sect_sp = mod->GetObjectFile()->GetBaseAddress().GetSection();
if (!sect_sp) {
return llvm::createStringError(std::errc::operation_not_supported,
@@ -226,8 +235,13 @@ Status MinidumpFileBuilder::AddModuleList(Target &target) {
std::string module_name = mod->GetSpecificationDescription();
auto maybe_mod_size = getModuleFileSize(target, mod);
if (!maybe_mod_size) {
- error.SetErrorStringWithFormat("Unable to get the size of module %s.",
- module_name.c_str());
+ llvm::Error mod_size_err = maybe_mod_size.takeError();
+ llvm::handleAllErrors(std::move(mod_size_err),
+ [&](const llvm::ErrorInfoBase &E) {
+ error.SetErrorStringWithFormat(
+ "Unable to get the size of module %s: %s.",
+ module_name.c_str(), E.message().c_str());
+ });
return error;
}
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
index fe609c7f3d2001..1af5d99f0b1604 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
@@ -14,6 +14,7 @@
#include "lldb/Core/PluginManager.h"
#include "lldb/Core/Section.h"
#include "lldb/Target/Process.h"
+#include "lldb/Utility/LLDBLog.h"
#include "llvm/Support/FileSystem.h"
@@ -68,26 +69,35 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
Target &target = process_sp->GetTarget();
+ Log *log = GetLog(LLDBLog::Object);
error = builder.AddSystemInfo(target.GetArchitecture().GetTriple());
- if (error.Fail())
+ if (error.Fail()) {
+ LLDB_LOG(log, "AddSystemInfo failed: %s", error.AsCString());
return false;
+ }
error = builder.AddModuleList(target);
- if (error.Fail())
+ if (error.Fail()) {
+ LLDB_LOG(log, "AddModuleList failed: %s", error.AsCString());
return false;
+ }
builder.AddMiscInfo(process_sp);
error = builder.AddThreadList(process_sp);
- if (error.Fail())
+ if (error.Fail()) {
+ LLDB_LOG(log, "AddThreadList failed: %s", error.AsCString());
return false;
+ }
// Add any exceptions but only if there are any in any threads.
builder.AddExceptions(process_sp);
error = builder.AddMemoryList(process_sp, core_style);
- if (error.Fail())
+ if (error.Fail()) {
+ LLDB_LOG(log, "AddMemoryList failed: %s", error.AsCString());
return false;
+ }
if (target.GetArchitecture().GetTriple().getOS() ==
llvm::Triple::OSType::Linux) {
diff --git a/lldb/test/API/tools/lldb-dap/save-core/Makefile b/lldb/test/API/tools/lldb-dap/save-core/Makefile
new file mode 100644
index 00000000000000..b7f3072e28c9d9
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/save-core/Makefile
@@ -0,0 +1,5 @@
+ENABLE_THREADS := YES
+
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/tools/lldb-dap/save-core/TestDAP_save_core.py b/lldb/test/API/tools/lldb-dap/save-core/TestDAP_save_core.py
new file mode 100644
index 00000000000000..4045dd8fb6569f
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/save-core/TestDAP_save_core.py
@@ -0,0 +1,79 @@
+"""
+Test saving core minidump from lldb-dap
+"""
+
+import dap_server
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+import lldbdap_testcase
+from lldbsuite.test import lldbutil
+
+
+class TestDAP_save_core(lldbdap_testcase.DAPTestCaseBase):
+ @skipUnlessArch("x86_64")
+ @skipUnlessPlatform(["linux"])
+ def test_save_core(self):
+ """
+ Tests saving core minidump from lldb-dap.
+ """
+ program = self.getBuildArtifact("a.out")
+ self.build_and_launch(program)
+ source = "main.cpp"
+ # source_path = os.path.join(os.getcwd(), source)
+ breakpoint1_line = line_number(source, "// breakpoint 1")
+ lines = [breakpoint1_line]
+ # Set breakpoint in the thread function so we can step the threads
+ breakpoint_ids = self.set_source_breakpoints(source, lines)
+ self.assertEqual(
+ len(breakpoint_ids), len(lines), "expect correct number of breakpoints"
+ )
+ self.continue_to_breakpoints(breakpoint_ids)
+
+ # Getting dap stack trace may trigger __lldb_caller_function JIT module to be created.
+ self.get_stackFrames(startFrame=0)
+
+ # Evaluating an expression that cause "_$__lldb_valid_pointer_check" JIT module to be created.
+ expression = 'printf("this is a test")'
+ self.dap_server.request_evaluate(expression, context="watch")
+
+ # Verify "_$__lldb_valid_pointer_check" JIT module is created.
+ modules = self.dap_server.get_modules()
+ self.assertTrue(modules["_$__lldb_valid_pointer_check"])
+ thread_count = len(self.dap_server.get_threads())
+
+ core_stack = self.getBuildArtifact("core.stack.dmp")
+ core_dirty = self.getBuildArtifact("core.dirty.dmp")
+ core_full = self.getBuildArtifact("core.full.dmp")
+
+ base_command = "`process save-core --plugin-name=minidump "
+ self.dap_server.request_evaluate(
+ base_command + " --style=stack '%s'" % (core_stack), context="repl"
+ )
+
+ self.assertTrue(os.path.isfile(core_stack))
+ self.verify_core_file(core_stack, len(modules), thread_count)
+
+ self.dap_server.request_evaluate(
+ base_command + " --style=modified-memory '%s'" % (core_dirty),
+ context="repl",
+ )
+ self.assertTrue(os.path.isfile(core_dirty))
+ self.verify_core_file(core_dirty, len(modules), thread_count)
+
+ self.dap_server.request_evaluate(
+ base_command + " --style=full '%s'" % (core_full), context="repl"
+ )
+ self.assertTrue(os.path.isfile(core_full))
+ self.verify_core_file(core_full, len(modules), thread_count)
+
+ def verify_core_file(self, core_path, expected_module_count, expected_thread_count):
+ # To verify, we'll launch with the mini dump
+ target = self.dbg.CreateTarget(None)
+ process = target.LoadCore(core_path)
+
+ # check if the core is in desired state
+ self.assertTrue(process, PROCESS_IS_VALID)
+ self.assertTrue(process.GetProcessInfo().IsValid())
+ self.assertNotEqual(target.GetTriple().find("linux"), -1)
+ self.assertTrue(target.GetNumModules(), expected_module_count)
+ self.assertEqual(process.GetNumThreads(), expected_thread_count)
diff --git a/lldb/test/API/tools/lldb-dap/save-core/main.cpp b/lldb/test/API/tools/lldb-dap/save-core/main.cpp
new file mode 100644
index 00000000000000..8905beb5e7eff6
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/save-core/main.cpp
@@ -0,0 +1,8 @@
+int function(int x) {
+ if ((x % 2) == 0)
+ return function(x - 1) + x; // breakpoint 1
+ else
+ return x;
+}
+
+int main(int argc, char const *argv[]) { return function(2); }
More information about the lldb-commits
mailing list