https://github.com/eronnen updated https://github.com/llvm/llvm-project/pull/136494
>From caf6db4c4ef8caea027bb7909a723bf24ff881ad Mon Sep 17 00:00:00 2001 From: Ely Ronnen <elyron...@gmail.com> Date: Sun, 20 Apr 2025 17:07:09 +0200 Subject: [PATCH] fallback to assembly when source code is not available fix TestDAP_coreFile.py with source maps use stop-disassembly-display setting to determine when to show disassembly * Use GetSetting instead of Handle command * Implement OptionValueEnumeration::ToJSON export StopDisassemblyType enum fix comment cleaner stackTraceDisassemblyDisplay tests remove include in main.c in test --- lldb/include/lldb/Core/Debugger.h | 10 +- lldb/include/lldb/lldb-enumerations.h | 8 + lldb/source/Core/CoreProperties.td | 12 +- lldb/source/Core/Debugger.cpp | 14 +- lldb/source/Target/StackFrame.cpp | 11 +- .../stackTraceDisassemblyDisplay/Makefile | 3 + .../TestDAP_stackTraceDisassemblyDisplay.py | 161 ++++++++++++++++++ .../stackTraceDisassemblyDisplay/main.c | 6 + .../Handler/StackTraceRequestHandler.cpp | 7 +- lldb/tools/lldb-dap/JSONUtils.cpp | 35 +++- lldb/tools/lldb-dap/JSONUtils.h | 22 ++- lldb/tools/lldb-dap/LLDBUtils.cpp | 27 +++ lldb/tools/lldb-dap/LLDBUtils.h | 9 + 13 files changed, 288 insertions(+), 37 deletions(-) create mode 100644 lldb/test/API/tools/lldb-dap/stackTraceDisassemblyDisplay/Makefile create mode 100644 lldb/test/API/tools/lldb-dap/stackTraceDisassemblyDisplay/TestDAP_stackTraceDisassemblyDisplay.py create mode 100644 lldb/test/API/tools/lldb-dap/stackTraceDisassemblyDisplay/main.c diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h index c2e7d6f1d64b7..0595125b1813d 100644 --- a/lldb/include/lldb/Core/Debugger.h +++ b/lldb/include/lldb/Core/Debugger.h @@ -232,14 +232,6 @@ class Debugger : public std::enable_shared_from_this<Debugger>, void SetLoggingCallback(lldb::LogOutputCallback log_callback, void *baton); - // Properties Functions - enum StopDisassemblyType { - eStopDisassemblyTypeNever = 0, - eStopDisassemblyTypeNoDebugInfo, - eStopDisassemblyTypeNoSource, - eStopDisassemblyTypeAlways - }; - Status SetPropertyValue(const ExecutionContext *exe_ctx, VarSetOperationType op, llvm::StringRef property_path, llvm::StringRef value) override; @@ -336,7 +328,7 @@ class Debugger : public std::enable_shared_from_this<Debugger>, uint64_t GetStopSourceLineCount(bool before) const; - StopDisassemblyType GetStopDisassemblyDisplay() const; + lldb::StopDisassemblyType GetStopDisassemblyDisplay() const; uint64_t GetDisassemblyLineCount() const; diff --git a/lldb/include/lldb/lldb-enumerations.h b/lldb/include/lldb/lldb-enumerations.h index 8e962428260f8..6d10cc8bcffcb 100644 --- a/lldb/include/lldb/lldb-enumerations.h +++ b/lldb/include/lldb/lldb-enumerations.h @@ -1383,6 +1383,14 @@ enum CommandReturnObjectCallbackResult { eCommandReturnObjectPrintCallbackHandled = 1, }; +/// Used to determine when to show disassembly. +enum StopDisassemblyType { + eStopDisassemblyTypeNever = 0, + eStopDisassemblyTypeNoDebugInfo, + eStopDisassemblyTypeNoSource, + eStopDisassemblyTypeAlways +}; + } // namespace lldb #endif // LLDB_LLDB_ENUMERATIONS_H diff --git a/lldb/source/Core/CoreProperties.td b/lldb/source/Core/CoreProperties.td index f5d86b663de13..a1a4e994c3b9c 100644 --- a/lldb/source/Core/CoreProperties.td +++ b/lldb/source/Core/CoreProperties.td @@ -91,11 +91,13 @@ let Definition = "debugger" in { Global, DefaultUnsignedValue<4>, Desc<"The number of disassembly lines to show when displaying a stopped context.">; - def StopDisassemblyDisplay: Property<"stop-disassembly-display", "Enum">, - Global, - DefaultEnumValue<"Debugger::eStopDisassemblyTypeNoDebugInfo">, - EnumValues<"OptionEnumValues(g_show_disassembly_enum_values)">, - Desc<"Control when to display disassembly when displaying a stopped context.">; + def StopDisassemblyDisplay + : Property<"stop-disassembly-display", "Enum">, + Global, + DefaultEnumValue<"eStopDisassemblyTypeNoDebugInfo">, + EnumValues<"OptionEnumValues(g_show_disassembly_enum_values)">, + Desc<"Control when to display disassembly when displaying a stopped " + "context.">; def StopDisassemblyMaxSize: Property<"stop-disassembly-max-size", "UInt64">, Global, DefaultUnsignedValue<32000>, diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp index cd8726eeba632..1a0723a2f3b3f 100644 --- a/lldb/source/Core/Debugger.cpp +++ b/lldb/source/Core/Debugger.cpp @@ -112,24 +112,24 @@ static llvm::DefaultThreadPool *g_thread_pool = nullptr; static constexpr OptionEnumValueElement g_show_disassembly_enum_values[] = { { - Debugger::eStopDisassemblyTypeNever, + lldb::eStopDisassemblyTypeNever, "never", "Never show disassembly when displaying a stop context.", }, { - Debugger::eStopDisassemblyTypeNoDebugInfo, + lldb::eStopDisassemblyTypeNoDebugInfo, "no-debuginfo", "Show disassembly when there is no debug information.", }, { - Debugger::eStopDisassemblyTypeNoSource, + lldb::eStopDisassemblyTypeNoSource, "no-source", "Show disassembly when there is no source information, or the source " "file " "is missing when displaying a stop context.", }, { - Debugger::eStopDisassemblyTypeAlways, + lldb::eStopDisassemblyTypeAlways, "always", "Always show disassembly when displaying a stop context.", }, @@ -611,10 +611,10 @@ uint64_t Debugger::GetStopSourceLineCount(bool before) const { idx, g_debugger_properties[idx].default_uint_value); } -Debugger::StopDisassemblyType Debugger::GetStopDisassemblyDisplay() const { +lldb::StopDisassemblyType Debugger::GetStopDisassemblyDisplay() const { const uint32_t idx = ePropertyStopDisassemblyDisplay; - return GetPropertyAtIndexAs<Debugger::StopDisassemblyType>( - idx, static_cast<Debugger::StopDisassemblyType>( + return GetPropertyAtIndexAs<lldb::StopDisassemblyType>( + idx, static_cast<lldb::StopDisassemblyType>( g_debugger_properties[idx].default_uint_value)); } diff --git a/lldb/source/Target/StackFrame.cpp b/lldb/source/Target/StackFrame.cpp index 0306f68169a98..691541f0ec5bb 100644 --- a/lldb/source/Target/StackFrame.cpp +++ b/lldb/source/Target/StackFrame.cpp @@ -2030,8 +2030,7 @@ bool StackFrame::GetStatus(Stream &strm, bool show_frame_info, bool show_source, if (show_source) { ExecutionContext exe_ctx(shared_from_this()); bool have_source = false, have_debuginfo = false; - Debugger::StopDisassemblyType disasm_display = - Debugger::eStopDisassemblyTypeNever; + lldb::StopDisassemblyType disasm_display = lldb::eStopDisassemblyTypeNever; Target *target = exe_ctx.GetTargetPtr(); if (target) { Debugger &debugger = target->GetDebugger(); @@ -2064,20 +2063,20 @@ bool StackFrame::GetStatus(Stream &strm, bool show_frame_info, bool show_source, } } switch (disasm_display) { - case Debugger::eStopDisassemblyTypeNever: + case lldb::eStopDisassemblyTypeNever: break; - case Debugger::eStopDisassemblyTypeNoDebugInfo: + case lldb::eStopDisassemblyTypeNoDebugInfo: if (have_debuginfo) break; [[fallthrough]]; - case Debugger::eStopDisassemblyTypeNoSource: + case lldb::eStopDisassemblyTypeNoSource: if (have_source) break; [[fallthrough]]; - case Debugger::eStopDisassemblyTypeAlways: + case lldb::eStopDisassemblyTypeAlways: if (target) { const uint32_t disasm_lines = debugger.GetDisassemblyLineCount(); if (disasm_lines > 0) { diff --git a/lldb/test/API/tools/lldb-dap/stackTraceDisassemblyDisplay/Makefile b/lldb/test/API/tools/lldb-dap/stackTraceDisassemblyDisplay/Makefile new file mode 100644 index 0000000000000..118f0aa59ef6f --- /dev/null +++ b/lldb/test/API/tools/lldb-dap/stackTraceDisassemblyDisplay/Makefile @@ -0,0 +1,3 @@ +C_SOURCES := main.c other.c + +include Makefile.rules diff --git a/lldb/test/API/tools/lldb-dap/stackTraceDisassemblyDisplay/TestDAP_stackTraceDisassemblyDisplay.py b/lldb/test/API/tools/lldb-dap/stackTraceDisassemblyDisplay/TestDAP_stackTraceDisassemblyDisplay.py new file mode 100644 index 0000000000000..d47e485c7f9d9 --- /dev/null +++ b/lldb/test/API/tools/lldb-dap/stackTraceDisassemblyDisplay/TestDAP_stackTraceDisassemblyDisplay.py @@ -0,0 +1,161 @@ +""" +Test lldb-dap stack trace when some of the source paths are missing +""" + +from lldbsuite.test.lldbtest import line_number +import lldbdap_testcase +from contextlib import contextmanager +import os + + +OTHER_C_SOURCE_CODE = """ +int no_source_func(int n) { + return n + 1; // Break here +} +""" + + +@contextmanager +def delete_file_on_exit(path): + try: + yield path + finally: + if os.path.exists(path): + os.remove(path) + + +class TestDAP_stackTraceMissingSourcePath(lldbdap_testcase.DAPTestCaseBase): + def build_and_run_until_breakpoint(self): + """ + Build the program and run until the breakpoint is hit, and return the stack frames. + """ + other_source_file = "other.c" + with delete_file_on_exit(other_source_file): + with open(other_source_file, "w") as f: + f.write(OTHER_C_SOURCE_CODE) + + breakpoint_line = line_number(other_source_file, "// Break here") + + program = self.getBuildArtifact("a.out") + self.build_and_launch(program, commandEscapePrefix="") + + breakpoint_ids = self.set_source_breakpoints( + other_source_file, [breakpoint_line] + ) + self.assertEqual( + len(breakpoint_ids), 1, "expect correct number of breakpoints" + ) + + self.continue_to_breakpoints(breakpoint_ids) + + frames = self.get_stackFrames() + self.assertLessEqual(2, len(frames), "expect at least 2 frames") + + self.assertIn( + "path", + frames[0]["source"], + "Expect source path to always be in frame (other.c)", + ) + self.assertIn( + "path", + frames[1]["source"], + "Expect source path in always be in frame (main.c)", + ) + + return frames + + def verify_frames_source( + self, frames, main_frame_assembly: bool, other_frame_assembly: bool + ): + if other_frame_assembly: + self.assertFalse( + frames[0]["source"]["path"].endswith("other.c"), + "Expect original source path to not be in unavailable source frame (other.c)", + ) + self.assertIn( + "sourceReference", + frames[0]["source"], + "Expect sourceReference to be in unavailable source frame (other.c)", + ) + else: + self.assertTrue( + frames[0]["source"]["path"].endswith("other.c"), + "Expect original source path to be in normal source frame (other.c)", + ) + self.assertNotIn( + "sourceReference", + frames[0]["source"], + "Expect sourceReference to not be in normal source frame (other.c)", + ) + + if main_frame_assembly: + self.assertFalse( + frames[1]["source"]["path"].endswith("main.c"), + "Expect original source path to not be in unavailable source frame (main.c)", + ) + self.assertIn( + "sourceReference", + frames[1]["source"], + "Expect sourceReference to be in unavailable source frame (main.c)", + ) + else: + self.assertTrue( + frames[1]["source"]["path"].endswith("main.c"), + "Expect original source path to be in normal source frame (main.c)", + ) + self.assertNotIn( + "sourceReference", + frames[1]["source"], + "Expect sourceReference to not be in normal source code frame (main.c)", + ) + + def test_stopDisassemblyDisplay(self): + """ + Test that with with all stop-disassembly-display values you get correct assembly / no assembly source code. + """ + self.build_and_run_until_breakpoint() + frames = self.get_stackFrames() + self.assertLessEqual(2, len(frames), "expect at least 2 frames") + + self.assertIn( + "path", + frames[0]["source"], + "Expect source path to always be in frame (other.c)", + ) + self.assertIn( + "path", + frames[1]["source"], + "Expect source path in always be in frame (main.c)", + ) + + self.dap_server.request_evaluate( + "settings set stop-disassembly-display never", context="repl" + ) + frames = self.get_stackFrames() + self.verify_frames_source( + frames, main_frame_assembly=False, other_frame_assembly=False + ) + + self.dap_server.request_evaluate( + "settings set stop-disassembly-display always", context="repl" + ) + frames = self.get_stackFrames() + self.verify_frames_source( + frames, main_frame_assembly=True, other_frame_assembly=True + ) + + self.dap_server.request_evaluate( + "settings set stop-disassembly-display no-source", context="repl" + ) + frames = self.get_stackFrames() + self.verify_frames_source( + frames, main_frame_assembly=False, other_frame_assembly=True + ) + + self.dap_server.request_evaluate( + "settings set stop-disassembly-display no-debuginfo", context="repl" + ) + frames = self.get_stackFrames() + self.verify_frames_source( + frames, main_frame_assembly=False, other_frame_assembly=False + ) diff --git a/lldb/test/API/tools/lldb-dap/stackTraceDisassemblyDisplay/main.c b/lldb/test/API/tools/lldb-dap/stackTraceDisassemblyDisplay/main.c new file mode 100644 index 0000000000000..f4c6337f48b22 --- /dev/null +++ b/lldb/test/API/tools/lldb-dap/stackTraceDisassemblyDisplay/main.c @@ -0,0 +1,6 @@ +int no_source_func(int n); + +int main(int argc, char const *argv[]) { + no_source_func(10); + return 0; +} diff --git a/lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp index 4ea4cd1e517d4..ff326a47d9a48 100644 --- a/lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp @@ -9,7 +9,9 @@ #include "DAP.h" #include "EventHelper.h" #include "JSONUtils.h" +#include "LLDBUtils.h" #include "RequestHandler.h" +#include "lldb/lldb-enumerations.h" namespace lldb_dap { @@ -53,6 +55,8 @@ static bool FillStackFrames(DAP &dap, lldb::SBThread &thread, llvm::json::Array &stack_frames, int64_t &offset, const int64_t start_frame, const int64_t levels, const bool include_all) { + lldb::StopDisassemblyType stop_disassembly_display = + GetStopDisassemblyDisplay(dap.debugger); bool reached_end_of_stack = false; for (int64_t i = start_frame; static_cast<int64_t>(stack_frames.size()) < levels; i++) { @@ -69,7 +73,8 @@ static bool FillStackFrames(DAP &dap, lldb::SBThread &thread, break; } - stack_frames.emplace_back(CreateStackFrame(frame, frame_format)); + stack_frames.emplace_back( + CreateStackFrame(frame, frame_format, stop_disassembly_display)); } if (include_all && reached_end_of_stack) { diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index e4351c0f86cf4..4409cf5b27e5b 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -659,6 +659,30 @@ llvm::json::Value CreateSource(llvm::StringRef source_path) { return llvm::json::Value(std::move(source)); } +bool ShouldDisplayAssemblySource( + const lldb::SBLineEntry &line_entry, + lldb::StopDisassemblyType stop_disassembly_display) { + if (stop_disassembly_display == lldb::eStopDisassemblyTypeNever) + return false; + + if (stop_disassembly_display == lldb::eStopDisassemblyTypeAlways) + return true; + + // A line entry of 0 indicates the line is compiler generated i.e. no source + // file is associated with the frame. + auto file_spec = line_entry.GetFileSpec(); + if (!file_spec.IsValid() || line_entry.GetLine() == 0 || + line_entry.GetLine() == LLDB_INVALID_LINE_NUMBER) + return true; + + if (stop_disassembly_display == lldb::eStopDisassemblyTypeNoSource && + !file_spec.Exists()) { + return true; + } + + return false; +} + // "StackFrame": { // "type": "object", // "description": "A Stackframe contains the source location.", @@ -720,8 +744,9 @@ llvm::json::Value CreateSource(llvm::StringRef source_path) { // }, // "required": [ "id", "name", "line", "column" ] // } -llvm::json::Value CreateStackFrame(lldb::SBFrame &frame, - lldb::SBFormat &format) { +llvm::json::Value +CreateStackFrame(lldb::SBFrame &frame, lldb::SBFormat &format, + lldb::StopDisassemblyType stop_disassembly_display) { llvm::json::Object object; int64_t frame_id = MakeDAPFrameID(frame); object.try_emplace("id", frame_id); @@ -751,11 +776,7 @@ llvm::json::Value CreateStackFrame(lldb::SBFrame &frame, EmplaceSafeString(object, "name", frame_name); auto line_entry = frame.GetLineEntry(); - // A line entry of 0 indicates the line is compiler generated i.e. no source - // file is associated with the frame. - if (line_entry.GetFileSpec().IsValid() && - (line_entry.GetLine() != 0 || - line_entry.GetLine() != LLDB_INVALID_LINE_NUMBER)) { + if (!ShouldDisplayAssemblySource(line_entry, stop_disassembly_display)) { object.try_emplace("source", CreateSource(line_entry)); object.try_emplace("line", line_entry.GetLine()); auto column = line_entry.GetColumn(); diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h index 3eb445d8c2f40..d0e20729f4ed9 100644 --- a/lldb/tools/lldb-dap/JSONUtils.h +++ b/lldb/tools/lldb-dap/JSONUtils.h @@ -346,6 +346,21 @@ llvm::json::Value CreateSource(const lldb::SBLineEntry &line_entry); /// definition outlined by Microsoft. llvm::json::Value CreateSource(llvm::StringRef source_path); +/// Return true if the given line entry should be displayed as assembly. +/// +/// \param[in] line_entry +/// The LLDB line entry to check. +/// +/// \param[in] stop_disassembly_display +/// The value of the "stop-disassembly-display" setting. +/// +/// \return +/// True if the line entry should be displayed as assembly, false +/// otherwise. +bool ShouldDisplayAssemblySource( + const lldb::SBLineEntry &line_entry, + lldb::StopDisassemblyType stop_disassembly_display); + /// Create a "StackFrame" object for a LLDB frame object. /// /// This function will fill in the following keys in the returned @@ -364,11 +379,14 @@ llvm::json::Value CreateSource(llvm::StringRef source_path); /// The LLDB format to use when populating out the "StackFrame" /// object. /// +/// \param[in] stop_disassembly_display +/// The value of the "stop-disassembly-display" setting. +/// /// \return /// A "StackFrame" JSON object with that follows the formal JSON /// definition outlined by Microsoft. -llvm::json::Value CreateStackFrame(lldb::SBFrame &frame, - lldb::SBFormat &format); +llvm::json::Value CreateStackFrame(lldb::SBFrame &frame, lldb::SBFormat &format, + lldb::StopDisassemblyType); /// Create a "StackFrame" label object for a LLDB thread. /// diff --git a/lldb/tools/lldb-dap/LLDBUtils.cpp b/lldb/tools/lldb-dap/LLDBUtils.cpp index 803320f8810fd..7c71d385e9f7e 100644 --- a/lldb/tools/lldb-dap/LLDBUtils.cpp +++ b/lldb/tools/lldb-dap/LLDBUtils.cpp @@ -186,6 +186,33 @@ GetEnvironmentFromArguments(const llvm::json::Object &arguments) { return envs; } +lldb::StopDisassemblyType +GetStopDisassemblyDisplay(lldb::SBDebugger &debugger) { + lldb::StopDisassemblyType result = + lldb::StopDisassemblyType::eStopDisassemblyTypeNoDebugInfo; + lldb::SBStructuredData string_result = + debugger.GetSetting("stop-disassembly-display"); + const size_t result_length = string_result.GetStringValue(nullptr, 0); + if (result_length > 0) { + std::string result_string(result_length, '\0'); + string_result.GetStringValue(result_string.data(), result_length + 1); + + result = + llvm::StringSwitch<lldb::StopDisassemblyType>(result_string) + .Case("never", lldb::StopDisassemblyType::eStopDisassemblyTypeNever) + .Case("always", + lldb::StopDisassemblyType::eStopDisassemblyTypeAlways) + .Case("no-source", + lldb::StopDisassemblyType::eStopDisassemblyTypeNoSource) + .Case("no-debuginfo", + lldb::StopDisassemblyType::eStopDisassemblyTypeNoDebugInfo) + .Default( + lldb::StopDisassemblyType::eStopDisassemblyTypeNoDebugInfo); + } + + return result; +} + llvm::Error ToError(const lldb::SBError &error) { if (error.Success()) return llvm::Error::success(); diff --git a/lldb/tools/lldb-dap/LLDBUtils.h b/lldb/tools/lldb-dap/LLDBUtils.h index 1e49305df5c18..7ce242ec887ae 100644 --- a/lldb/tools/lldb-dap/LLDBUtils.h +++ b/lldb/tools/lldb-dap/LLDBUtils.h @@ -159,6 +159,15 @@ uint32_t GetLLDBFrameID(uint64_t dap_frame_id); lldb::SBEnvironment GetEnvironmentFromArguments(const llvm::json::Object &arguments); +/// Get the stop-disassembly-display settings +/// +/// \param[in] debugger +/// The debugger that will execute the lldb commands. +/// +/// \return +/// The value of the stop-disassembly-display setting +lldb::StopDisassemblyType GetStopDisassemblyDisplay(lldb::SBDebugger &debugger); + /// Take ownership of the stored error. llvm::Error ToError(const lldb::SBError &error); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits