llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: John Harrison (ashgti) <details> <summary>Changes</summary> This adds new types for setExceptionBreakpoints and adds support for `supportsExceptionFilterOptions`, which allows exception breakpoints to set a condition. While testing this, I noticed that obj-c exception catch breakpoints may not be working correctly in lldb-dap. --- Patch is 42.55 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/144153.diff 18 Files Affected: - (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py (+5-1) - (modified) lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setExceptionBreakpoints.py (+4-7) - (modified) lldb/test/API/tools/lldb-dap/exception/objc/Makefile (+1-1) - (modified) lldb/test/API/tools/lldb-dap/exception/objc/TestDAP_exception_objc.py (+38-1) - (modified) lldb/test/API/tools/lldb-dap/exception/objc/main.m (+9-3) - (modified) lldb/tools/lldb-dap/DAP.cpp (+71-85) - (modified) lldb/tools/lldb-dap/DAP.h (+2-2) - (modified) lldb/tools/lldb-dap/ExceptionBreakpoint.cpp (+17-8) - (modified) lldb/tools/lldb-dap/ExceptionBreakpoint.h (+7-3) - (modified) lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp (-1) - (modified) lldb/tools/lldb-dap/Handler/RequestHandler.h (+11-4) - (modified) lldb/tools/lldb-dap/Handler/SetExceptionBreakpointsRequestHandler.cpp (+41-66) - (modified) lldb/tools/lldb-dap/JSONUtils.cpp (+3) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp (+14) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.h (+50) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp (+31-13) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolTypes.h (+26-7) - (modified) lldb/unittests/DAP/ProtocolTypesTest.cpp (+84-25) ``````````diff diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py index 9786678aa53f9..c1108da17123b 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py @@ -1050,8 +1050,12 @@ def request_setBreakpoints(self, source: Source, line_array, data=None): self._update_verified_breakpoints(response["body"]["breakpoints"]) return response - def request_setExceptionBreakpoints(self, filters): + def request_setExceptionBreakpoints( + self, *, filters: list[str] = [], filter_options: list[dict] = [] + ): args_dict = {"filters": filters} + if filter_options: + args_dict["filterOptions"] = filter_options command_dict = { "command": "setExceptionBreakpoints", "type": "request", diff --git a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setExceptionBreakpoints.py b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setExceptionBreakpoints.py index 4dc8c5b3c7ded..4ca733a9a59ca 100644 --- a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setExceptionBreakpoints.py +++ b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setExceptionBreakpoints.py @@ -1,16 +1,12 @@ """ -Test lldb-dap setBreakpoints request +Test lldb-dap setExceptionBreakpoints request """ - -import dap_server from lldbsuite.test.decorators import * from lldbsuite.test.lldbtest import * -from lldbsuite.test import lldbutil import lldbdap_testcase -@skip("Temporarily disable the breakpoint tests") class TestDAP_setExceptionBreakpoints(lldbdap_testcase.DAPTestCaseBase): @skipIfWindows def test_functionality(self): @@ -33,8 +29,9 @@ def test_functionality(self): program = self.getBuildArtifact("a.out") self.build_and_launch(program) - filters = ["cpp_throw", "cpp_catch"] - response = self.dap_server.request_setExceptionBreakpoints(filters) + response = self.dap_server.request_setExceptionBreakpoints( + filters=["cpp_throw", "cpp_catch"], + ) if response: self.assertTrue(response["success"]) diff --git a/lldb/test/API/tools/lldb-dap/exception/objc/Makefile b/lldb/test/API/tools/lldb-dap/exception/objc/Makefile index 9b6528337cb9d..17e6dc76699ab 100644 --- a/lldb/test/API/tools/lldb-dap/exception/objc/Makefile +++ b/lldb/test/API/tools/lldb-dap/exception/objc/Makefile @@ -1,6 +1,6 @@ OBJC_SOURCES := main.m -CFLAGS_EXTRAS := -w +CFLAGS_EXTRAS := -w -fobjc-exceptions USE_SYSTEM_STDLIB := 1 diff --git a/lldb/test/API/tools/lldb-dap/exception/objc/TestDAP_exception_objc.py b/lldb/test/API/tools/lldb-dap/exception/objc/TestDAP_exception_objc.py index 777d55f48e850..ddedf7a6de8c6 100644 --- a/lldb/test/API/tools/lldb-dap/exception/objc/TestDAP_exception_objc.py +++ b/lldb/test/API/tools/lldb-dap/exception/objc/TestDAP_exception_objc.py @@ -2,7 +2,6 @@ Test exception behavior in DAP with obj-c throw. """ - from lldbsuite.test.decorators import * from lldbsuite.test.lldbtest import * import lldbdap_testcase @@ -25,3 +24,41 @@ def test_stopped_description(self): exception_details = exception_info["details"] self.assertRegex(exception_details["message"], "SomeReason") self.assertRegex(exception_details["stackTrace"], "main.m") + + @skipUnlessDarwin + def test_break_on_throw_and_catch(self): + """ + Test that breakpoints on exceptions work as expected. + """ + program = self.getBuildArtifact("a.out") + self.build_and_launch(program) + + response = self.dap_server.request_setExceptionBreakpoints( + filter_options=[ + { + "filterId": "objc_throw", + "condition": '[[((NSException *)$arg1) name] isEqual:@"ThrownException"]', + }, + ] + ) + if response: + self.assertTrue(response["success"]) + + self.continue_to_exception_breakpoint("Objective-C Throw") + + # FIXME: Catching objc exceptions do not appear to be working. + # Xcode appears to set a breakpoint on '__cxa_begin_catch' for objc + # catch, which is different than + # SBTarget::BreakpointCreateForException(eLanguageObjectiveC, /*catch_bp=*/true, /*throw_bp=*/false); + # self.continue_to_exception_breakpoint("Objective-C Catch") + + self.do_continue() + + self.assertTrue(self.verify_stop_exception_info("signal SIGABRT")) + exception_info = self.get_exceptionInfo() + self.assertEqual(exception_info["breakMode"], "always") + self.assertEqual(exception_info["description"], "signal SIGABRT") + self.assertEqual(exception_info["exceptionId"], "signal") + exception_details = exception_info["details"] + self.assertRegex(exception_details["message"], "SomeReason") + self.assertRegex(exception_details["stackTrace"], "main.m") diff --git a/lldb/test/API/tools/lldb-dap/exception/objc/main.m b/lldb/test/API/tools/lldb-dap/exception/objc/main.m index e8db04fb40de1..bbfa621992799 100644 --- a/lldb/test/API/tools/lldb-dap/exception/objc/main.m +++ b/lldb/test/API/tools/lldb-dap/exception/objc/main.m @@ -1,8 +1,14 @@ #import <Foundation/Foundation.h> int main(int argc, char const *argv[]) { - @throw [[NSException alloc] initWithName:@"ThrownException" - reason:@"SomeReason" - userInfo:nil]; + @try { + NSException *e = [[NSException alloc] initWithName:@"ThrownException" + reason:@"SomeReason" + userInfo:nil]; + @throw e; + } @catch (NSException *e) { + NSLog(@"Caught %@", e); + @throw; // let the process crash... + } return 0; } diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index b034c967594ba..58e0be64ba5b8 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -128,93 +128,81 @@ DAP::DAP(Log *log, const ReplMode default_repl_mode, DAP::~DAP() = default; void DAP::PopulateExceptionBreakpoints() { - llvm::call_once(init_exception_breakpoints_flag, [this]() { - exception_breakpoints = std::vector<ExceptionBreakpoint>{}; - - if (lldb::SBDebugger::SupportsLanguage(lldb::eLanguageTypeC_plus_plus)) { - exception_breakpoints->emplace_back(*this, "cpp_catch", "C++ Catch", - lldb::eLanguageTypeC_plus_plus); - exception_breakpoints->emplace_back(*this, "cpp_throw", "C++ Throw", - lldb::eLanguageTypeC_plus_plus); - } - if (lldb::SBDebugger::SupportsLanguage(lldb::eLanguageTypeObjC)) { - exception_breakpoints->emplace_back( - *this, "objc_catch", "Objective-C Catch", lldb::eLanguageTypeObjC); - exception_breakpoints->emplace_back( - *this, "objc_throw", "Objective-C Throw", lldb::eLanguageTypeObjC); - } - if (lldb::SBDebugger::SupportsLanguage(lldb::eLanguageTypeSwift)) { - exception_breakpoints->emplace_back(*this, "swift_catch", "Swift Catch", - lldb::eLanguageTypeSwift); - exception_breakpoints->emplace_back(*this, "swift_throw", "Swift Throw", - lldb::eLanguageTypeSwift); + if (lldb::SBDebugger::SupportsLanguage(lldb::eLanguageTypeC_plus_plus)) { + exception_breakpoints.emplace_back(*this, "cpp_catch", "C++ Catch", + lldb::eLanguageTypeC_plus_plus, + /*is_throw=*/false, /*is_catch=*/true); + exception_breakpoints.emplace_back(*this, "cpp_throw", "C++ Throw", + lldb::eLanguageTypeC_plus_plus, + /*is_throw=*/true, /*is_catch=*/false); + } + + if (lldb::SBDebugger::SupportsLanguage(lldb::eLanguageTypeObjC)) { + exception_breakpoints.emplace_back(*this, "objc_catch", "Objective-C Catch", + lldb::eLanguageTypeObjC, + /*is_throw=*/false, /*is_catch=*/true); + exception_breakpoints.emplace_back(*this, "objc_throw", "Objective-C Throw", + lldb::eLanguageTypeObjC, + /*is_throw=*/true, /*is_catch=*/false); + } + + if (lldb::SBDebugger::SupportsLanguage(lldb::eLanguageTypeSwift)) { + exception_breakpoints.emplace_back(*this, "swift_catch", "Swift Catch", + lldb::eLanguageTypeSwift, + /*is_throw=*/false, /*is_catch=*/true); + exception_breakpoints.emplace_back(*this, "swift_throw", "Swift Throw", + lldb::eLanguageTypeSwift, + /*is_throw=*/true, /*is_catch=*/false); + } + + // Besides handling the hardcoded list of languages from above, we try to find + // any other languages that support exception breakpoints using the SB API. + for (int raw_lang = lldb::eLanguageTypeUnknown; + raw_lang < lldb::eNumLanguageTypes; ++raw_lang) { + lldb::LanguageType lang = static_cast<lldb::LanguageType>(raw_lang); + + // We first discard any languages already handled above. + if (lldb::SBLanguageRuntime::LanguageIsCFamily(lang) || + lang == lldb::eLanguageTypeSwift) + continue; + + if (!lldb::SBDebugger::SupportsLanguage(lang)) + continue; + + const char *name = lldb::SBLanguageRuntime::GetNameForLanguageType(lang); + if (!name) + continue; + std::string raw_lang_name = name; + std::string capitalized_lang_name = capitalize(name); + + if (lldb::SBLanguageRuntime::SupportsExceptionBreakpointsOnThrow(lang)) { + const char *raw_throw_keyword = + lldb::SBLanguageRuntime::GetThrowKeywordForLanguage(lang); + std::string throw_keyword = + raw_throw_keyword ? raw_throw_keyword : "throw"; + + exception_breakpoints.emplace_back( + *this, raw_lang_name + "_" + throw_keyword, + capitalized_lang_name + " " + capitalize(throw_keyword), lang, + /*is_throw=*/true, /*is_catch=*/false); } - // Besides handling the hardcoded list of languages from above, we try to - // find any other languages that support exception breakpoints using the - // SB API. - for (int raw_lang = lldb::eLanguageTypeUnknown; - raw_lang < lldb::eNumLanguageTypes; ++raw_lang) { - lldb::LanguageType lang = static_cast<lldb::LanguageType>(raw_lang); - - // We first discard any languages already handled above. - if (lldb::SBLanguageRuntime::LanguageIsCFamily(lang) || - lang == lldb::eLanguageTypeSwift) - continue; - - if (!lldb::SBDebugger::SupportsLanguage(lang)) - continue; - - const char *name = lldb::SBLanguageRuntime::GetNameForLanguageType(lang); - if (!name) - continue; - std::string raw_lang_name = name; - std::string capitalized_lang_name = capitalize(name); - - if (lldb::SBLanguageRuntime::SupportsExceptionBreakpointsOnThrow(lang)) { - const char *raw_throw_keyword = - lldb::SBLanguageRuntime::GetThrowKeywordForLanguage(lang); - std::string throw_keyword = - raw_throw_keyword ? raw_throw_keyword : "throw"; - - exception_breakpoints->emplace_back( - *this, raw_lang_name + "_" + throw_keyword, - capitalized_lang_name + " " + capitalize(throw_keyword), lang); - } - if (lldb::SBLanguageRuntime::SupportsExceptionBreakpointsOnCatch(lang)) { - const char *raw_catch_keyword = - lldb::SBLanguageRuntime::GetCatchKeywordForLanguage(lang); - std::string catch_keyword = - raw_catch_keyword ? raw_catch_keyword : "catch"; + if (lldb::SBLanguageRuntime::SupportsExceptionBreakpointsOnCatch(lang)) { + const char *raw_catch_keyword = + lldb::SBLanguageRuntime::GetCatchKeywordForLanguage(lang); + std::string catch_keyword = + raw_catch_keyword ? raw_catch_keyword : "catch"; - exception_breakpoints->emplace_back( - *this, raw_lang_name + "_" + catch_keyword, - capitalized_lang_name + " " + capitalize(catch_keyword), lang); - } + exception_breakpoints.emplace_back( + *this, raw_lang_name + "_" + catch_keyword, + capitalized_lang_name + " " + capitalize(catch_keyword), lang, + /*is_throw=*/true, /*is_catch=*/false); } - assert(!exception_breakpoints->empty() && "should not be empty"); - }); + } } ExceptionBreakpoint *DAP::GetExceptionBreakpoint(llvm::StringRef filter) { - // PopulateExceptionBreakpoints() is called after g_dap.debugger is created - // in a request-initialize. - // - // But this GetExceptionBreakpoint() method may be called before attaching, in - // which case, we may not have populated the filter yet. - // - // We also cannot call PopulateExceptionBreakpoints() in DAP::DAP() because - // we need SBDebugger::Initialize() to have been called before this. - // - // So just calling PopulateExceptionBreakoints(),which does lazy-populating - // seems easiest. Two other options include: - // + call g_dap.PopulateExceptionBreakpoints() in lldb-dap.cpp::main() - // right after the call to SBDebugger::Initialize() - // + Just call PopulateExceptionBreakpoints() to get a fresh list everytime - // we query (a bit overkill since it's not likely to change?) - PopulateExceptionBreakpoints(); - - for (auto &bp : *exception_breakpoints) { + for (auto &bp : exception_breakpoints) { if (bp.GetFilter() == filter) return &bp; } @@ -222,10 +210,7 @@ ExceptionBreakpoint *DAP::GetExceptionBreakpoint(llvm::StringRef filter) { } ExceptionBreakpoint *DAP::GetExceptionBreakpoint(const lldb::break_id_t bp_id) { - // See comment in the other GetExceptionBreakpoint(). - PopulateExceptionBreakpoints(); - - for (auto &bp : *exception_breakpoints) { + for (auto &bp : exception_breakpoints) { if (bp.GetID() == bp_id) return &bp; } @@ -1117,8 +1102,9 @@ protocol::Capabilities DAP::GetCapabilities() { } // Available filters or options for the setExceptionBreakpoints request. + PopulateExceptionBreakpoints(); std::vector<protocol::ExceptionBreakpointsFilter> filters; - for (const auto &exc_bp : *exception_breakpoints) + for (const auto &exc_bp : exception_breakpoints) filters.emplace_back(CreateExceptionBreakpointFilter(exc_bp)); capabilities.exceptionBreakpointFilters = std::move(filters); diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 89bc827c1141f..5ca5822f9bced 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -99,7 +99,7 @@ struct DAP { lldb::SBBroadcaster broadcaster; FunctionBreakpointMap function_breakpoints; InstructionBreakpointMap instruction_breakpoints; - std::optional<std::vector<ExceptionBreakpoint>> exception_breakpoints; + std::vector<ExceptionBreakpoint> exception_breakpoints; llvm::once_flag init_exception_breakpoints_flag; /// Map step in target id to list of function targets that user can choose. @@ -320,7 +320,7 @@ struct DAP { }); } - /// The set of capablities supported by this adapter. + /// The set of capabilities supported by this adapter. protocol::Capabilities GetCapabilities(); /// Debuggee will continue from stopped state. diff --git a/lldb/tools/lldb-dap/ExceptionBreakpoint.cpp b/lldb/tools/lldb-dap/ExceptionBreakpoint.cpp index 9772e7344ced6..2531291fd62cc 100644 --- a/lldb/tools/lldb-dap/ExceptionBreakpoint.cpp +++ b/lldb/tools/lldb-dap/ExceptionBreakpoint.cpp @@ -9,23 +9,32 @@ #include "ExceptionBreakpoint.h" #include "BreakpointBase.h" #include "DAP.h" +#include "Protocol/ProtocolTypes.h" #include "lldb/API/SBMutex.h" #include "lldb/API/SBTarget.h" #include <mutex> +using namespace llvm; +using namespace lldb_dap::protocol; + namespace lldb_dap { -void ExceptionBreakpoint::SetBreakpoint() { +protocol::Breakpoint ExceptionBreakpoint::SetBreakpoint(StringRef condition) { lldb::SBMutex lock = m_dap.GetAPIMutex(); std::lock_guard<lldb::SBMutex> guard(lock); - if (m_bp.IsValid()) - return; - bool catch_value = m_filter.find("_catch") != std::string::npos; - bool throw_value = m_filter.find("_throw") != std::string::npos; - m_bp = m_dap.target.BreakpointCreateForException(m_language, catch_value, - throw_value); - m_bp.AddName(BreakpointBase::kDAPBreakpointLabel); + if (!m_bp.IsValid()) { + m_bp = m_dap.target.BreakpointCreateForException(m_language, m_is_catch, + m_is_throw); + m_bp.AddName(BreakpointBase::kDAPBreakpointLabel); + } + + m_bp.SetCondition(condition.data()); + + protocol::Breakpoint breakpoint; + breakpoint.id = m_bp.GetID(); + breakpoint.verified = m_bp.IsValid(); + return breakpoint; } void ExceptionBreakpoint::ClearBreakpoint() { diff --git a/lldb/tools/lldb-dap/ExceptionBreakpoint.h b/lldb/tools/lldb-dap/ExceptionBreakpoint.h index 319b472a89a34..d453d5fcc4fd3 100644 --- a/lldb/tools/lldb-dap/ExceptionBreakpoint.h +++ b/lldb/tools/lldb-dap/ExceptionBreakpoint.h @@ -10,6 +10,7 @@ #define LLDB_TOOLS_LLDB_DAP_EXCEPTIONBREAKPOINT_H #include "DAPForward.h" +#include "Protocol/ProtocolTypes.h" #include "lldb/API/SBBreakpoint.h" #include "lldb/lldb-enumerations.h" #include "llvm/ADT/StringRef.h" @@ -21,11 +22,12 @@ namespace lldb_dap { class ExceptionBreakpoint { public: ExceptionBreakpoint(DAP &d, std::string f, std::string l, - lldb::LanguageType lang) + lldb::LanguageType lang, bool is_throw, bool is_catch) : m_dap(d), m_filter(std::move(f)), m_label(std::move(l)), - m_language(lang), m_bp() {} + m_language(lang), m_is_throw(is_throw), m_is_catch(is_catch), m_bp() {} - void SetBreakpoint(); + protocol::Breakpoint SetBreakpoint() { return SetBreakpoint(""); }; + protocol::Breakpoint SetBreakpoint(llvm::StringRef condition); void ClearBreakpoint(); lldb::break_id_t GetID() const { return m_bp.GetID(); } @@ -39,6 +41,8 @@ class ExceptionBreakpoint { std::string m_filter; std::string m_label; lldb::LanguageType m_language; + bool m_is_throw; + bool m_is_catch; lldb::SBBreakpoint m_bp; }; diff --git a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp index dcd02d61ca4f4..b499a69876e2c 100644 --- a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp @@ -54,7 +54,6 @@ llvm::Expected<InitializeResponse> InitializeRequestHandler::Run( if (llvm::Error err = dap.RunPreInitCommands()) return err; - dap.PopulateExceptionBreakpoints(); auto cmd = dap.debugger.GetCommandInterpreter().AddMultiwordCommand( "lldb-dap", "Commands for managing lldb-dap."); if (arguments.supportedFeatures.contains( diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h index d3f231589b54c..071a44bd002ad 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.h +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h @@ -386,14 +386,21 @@ class SetBreakpointsRequestHandler Run(const protocol::SetBreakpointsArguments &args) const override; }; -class SetExceptionBreakpointsRequestHandler : public LegacyRequestHandler { +class SetExceptionBreakpointsRequestHandler + : public RequestHandler< + protocol::SetExceptionBreakpointsArguments, + llvm::Expected<protocol::SetExceptionBreakpointsResponseBody>> { public: - using LegacyRequestHandler::LegacyRequestHandler; + using RequestHandler::RequestHandler; static llvm::StringLiteral GetCommand() { return "setExceptionBreakpoints"; } FeatureSet GetSupportedFeatures() const override { - return {protocol::eAdapterFeatureExceptionOptions}; + /// Prefer the `filterOptions` feature over the `exceptionOptions`. + /// exceptionOptions is not supported in VSCode, while `filterOptions` is + /// supported. + re... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/144153 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
