llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: John Harrison (ashgti) <details> <summary>Changes</summary> Refactoring `stackTrace` to perform frame look ups in a more on-demand fashion to improve overall performance. Additionally adding additional information to the `exceptionInfo` request to report exception stacks there instead of merging the exception stack into the stack trace. The `exceptionInfo` request is only called if a stop event occurs with `reason='exception'`, which should mitigate the performance of `SBThread::GetCurrentException` calls. Adding unit tests for exception handling and stack trace supporting. --- Patch is 32.45 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/105905.diff 22 Files Affected: - (modified) lldb/packages/Python/lldbsuite/test/lldbplatformutil.py (+16) - (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py (+11) - (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py (+7-2) - (modified) lldb/test/API/tools/lldb-dap/exception/Makefile (+1-1) - (modified) lldb/test/API/tools/lldb-dap/exception/TestDAP_exception.py (+5-3) - (added) lldb/test/API/tools/lldb-dap/exception/cpp/Makefile (+3) - (added) lldb/test/API/tools/lldb-dap/exception/cpp/TestDAP_exception_cpp.py (+26) - (added) lldb/test/API/tools/lldb-dap/exception/cpp/main.cpp (+6) - (renamed) lldb/test/API/tools/lldb-dap/exception/main.c (+1-1) - (added) lldb/test/API/tools/lldb-dap/exception/objc/Makefile (+9) - (added) lldb/test/API/tools/lldb-dap/exception/objc/TestDAP_exception_objc.py (+27) - (added) lldb/test/API/tools/lldb-dap/exception/objc/main.m (+8) - (added) lldb/test/API/tools/lldb-dap/extendedStackTrace/Makefile (+5) - (added) lldb/test/API/tools/lldb-dap/extendedStackTrace/TestDAP_extendedStackTrace.py (+69) - (added) lldb/test/API/tools/lldb-dap/extendedStackTrace/main.m (+28) - (modified) lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py (+1-3) - (modified) lldb/test/API/tools/lldb-dap/stackTraceMissingFunctionName/TestDAP_stackTraceMissingFunctionName.py (-5) - (modified) lldb/tools/lldb-dap/DAP.cpp (-1) - (modified) lldb/tools/lldb-dap/DAP.h (+1-1) - (modified) lldb/tools/lldb-dap/JSONUtils.cpp (+40) - (modified) lldb/tools/lldb-dap/JSONUtils.h (+3) - (modified) lldb/tools/lldb-dap/lldb-dap.cpp (+182-71) ``````````diff diff --git a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py index 602e15d207e94a..3d8c713562e9bf 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py +++ b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py @@ -181,6 +181,22 @@ def findMainThreadCheckerDylib(): return "" +def findBacktraceRecordingDylib(): + if not platformIsDarwin(): + return "" + + if getPlatform() in lldbplatform.translate(lldbplatform.darwin_embedded): + return "/Developer/usr/lib/libBacktraceRecording.dylib" + + with os.popen("xcode-select -p") as output: + xcode_developer_path = output.read().strip() + mtc_dylib_path = "%s/usr/lib/libBacktraceRecording.dylib" % xcode_developer_path + if os.path.isfile(mtc_dylib_path): + return mtc_dylib_path + + return "" + + class _PlatformContext(object): """Value object class which contains platform-specific options.""" 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 874383a13e2bb6..167142779cf12c 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 @@ -707,6 +707,17 @@ def request_evaluate(self, expression, frameIndex=0, threadId=None, context=None } return self.send_recv(command_dict) + def request_exceptionInfo(self, threadId=None): + if threadId is None: + threadId = self.get_thread_id() + args_dict = {"threadId": threadId} + command_dict = { + "command": "exceptionInfo", + "type": "request", + "arguments": args_dict, + } + return self.send_recv(command_dict) + def request_initialize(self, sourceInitFile): command_dict = { "command": "initialize", diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py index 86eba355da83db..40d61636380588 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py @@ -100,13 +100,14 @@ def verify_breakpoint_hit(self, breakpoint_ids): return self.assertTrue(False, "breakpoint not hit") - def verify_stop_exception_info(self, expected_description): + def verify_stop_exception_info(self, expected_description, timeout=timeoutval): """Wait for the process we are debugging to stop, and verify the stop reason is 'exception' and that the description matches 'expected_description' """ - stopped_events = self.dap_server.wait_for_stopped() + stopped_events = self.dap_server.wait_for_stopped(timeout=timeout) for stopped_event in stopped_events: + print("stopped_event", stopped_event) if "body" in stopped_event: body = stopped_event["body"] if "reason" not in body: @@ -177,6 +178,10 @@ def get_stackFrames(self, threadId=None, startFrame=None, levels=None, dump=Fals ) return stackFrames + def get_exceptionInfo(self, threadId=None): + response = self.dap_server.request_exceptionInfo(threadId=threadId) + return self.get_dict_value(response, ["body"]) + def get_source_and_line(self, threadId=None, frameIndex=0): stackFrames = self.get_stackFrames( threadId=threadId, startFrame=frameIndex, levels=1 diff --git a/lldb/test/API/tools/lldb-dap/exception/Makefile b/lldb/test/API/tools/lldb-dap/exception/Makefile index 99998b20bcb050..10495940055b63 100644 --- a/lldb/test/API/tools/lldb-dap/exception/Makefile +++ b/lldb/test/API/tools/lldb-dap/exception/Makefile @@ -1,3 +1,3 @@ -CXX_SOURCES := main.cpp +C_SOURCES := main.c include Makefile.rules diff --git a/lldb/test/API/tools/lldb-dap/exception/TestDAP_exception.py b/lldb/test/API/tools/lldb-dap/exception/TestDAP_exception.py index 8c2c0154ba65c0..39d73737b7e8c0 100644 --- a/lldb/test/API/tools/lldb-dap/exception/TestDAP_exception.py +++ b/lldb/test/API/tools/lldb-dap/exception/TestDAP_exception.py @@ -1,5 +1,5 @@ """ -Test exception behavior in DAP +Test exception behavior in DAP with signal. """ @@ -16,8 +16,10 @@ def test_stopped_description(self): event. """ program = self.getBuildArtifact("a.out") - print("test_stopped_description called", flush=True) self.build_and_launch(program) - self.dap_server.request_continue() self.assertTrue(self.verify_stop_exception_info("signal SIGABRT")) + exceptionInfo = self.get_exceptionInfo() + self.assertEqual(exceptionInfo["breakMode"], "always") + self.assertEqual(exceptionInfo["description"], "signal SIGABRT") + self.assertEqual(exceptionInfo["exceptionId"], "signal") diff --git a/lldb/test/API/tools/lldb-dap/exception/cpp/Makefile b/lldb/test/API/tools/lldb-dap/exception/cpp/Makefile new file mode 100644 index 00000000000000..99998b20bcb050 --- /dev/null +++ b/lldb/test/API/tools/lldb-dap/exception/cpp/Makefile @@ -0,0 +1,3 @@ +CXX_SOURCES := main.cpp + +include Makefile.rules diff --git a/lldb/test/API/tools/lldb-dap/exception/cpp/TestDAP_exception_cpp.py b/lldb/test/API/tools/lldb-dap/exception/cpp/TestDAP_exception_cpp.py new file mode 100644 index 00000000000000..6471e2b87251a7 --- /dev/null +++ b/lldb/test/API/tools/lldb-dap/exception/cpp/TestDAP_exception_cpp.py @@ -0,0 +1,26 @@ +""" +Test exception behavior in DAP with c++ throw. +""" + + +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +import lldbdap_testcase + + +class TestDAP_exception_cpp(lldbdap_testcase.DAPTestCaseBase): + @skipIfWindows + def test_stopped_description(self): + """ + Test that exception description is shown correctly in stopped + event. + """ + program = self.getBuildArtifact("a.out") + self.build_and_launch(program) + self.dap_server.request_continue() + self.assertTrue(self.verify_stop_exception_info("signal SIGABRT")) + exceptionInfo = self.get_exceptionInfo() + self.assertEqual(exceptionInfo["breakMode"], "always") + self.assertEqual(exceptionInfo["description"], "signal SIGABRT") + self.assertEqual(exceptionInfo["exceptionId"], "signal") + self.assertIsNotNone(exceptionInfo["details"]) diff --git a/lldb/test/API/tools/lldb-dap/exception/cpp/main.cpp b/lldb/test/API/tools/lldb-dap/exception/cpp/main.cpp new file mode 100644 index 00000000000000..39d89b95319a8c --- /dev/null +++ b/lldb/test/API/tools/lldb-dap/exception/cpp/main.cpp @@ -0,0 +1,6 @@ +#include <stdexcept> + +int main(int argc, char const *argv[]) { + throw std::invalid_argument("throwing exception for testing"); + return 0; +} diff --git a/lldb/test/API/tools/lldb-dap/exception/main.cpp b/lldb/test/API/tools/lldb-dap/exception/main.c similarity index 56% rename from lldb/test/API/tools/lldb-dap/exception/main.cpp rename to lldb/test/API/tools/lldb-dap/exception/main.c index b940d07c6f2bb3..a653ac5d82aa3a 100644 --- a/lldb/test/API/tools/lldb-dap/exception/main.cpp +++ b/lldb/test/API/tools/lldb-dap/exception/main.c @@ -1,6 +1,6 @@ #include <signal.h> -int main() { +int main(int argc, char const *argv[]) { raise(SIGABRT); return 0; } diff --git a/lldb/test/API/tools/lldb-dap/exception/objc/Makefile b/lldb/test/API/tools/lldb-dap/exception/objc/Makefile new file mode 100644 index 00000000000000..9b6528337cb9d8 --- /dev/null +++ b/lldb/test/API/tools/lldb-dap/exception/objc/Makefile @@ -0,0 +1,9 @@ +OBJC_SOURCES := main.m + +CFLAGS_EXTRAS := -w + +USE_SYSTEM_STDLIB := 1 + +LD_EXTRAS := -framework Foundation + +include Makefile.rules 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 new file mode 100644 index 00000000000000..777d55f48e8504 --- /dev/null +++ b/lldb/test/API/tools/lldb-dap/exception/objc/TestDAP_exception_objc.py @@ -0,0 +1,27 @@ +""" +Test exception behavior in DAP with obj-c throw. +""" + + +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +import lldbdap_testcase + + +class TestDAP_exception_objc(lldbdap_testcase.DAPTestCaseBase): + @skipUnlessDarwin + def test_stopped_description(self): + """ + Test that exception description is shown correctly in stopped event. + """ + program = self.getBuildArtifact("a.out") + self.build_and_launch(program) + self.dap_server.request_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 new file mode 100644 index 00000000000000..e8db04fb40de15 --- /dev/null +++ b/lldb/test/API/tools/lldb-dap/exception/objc/main.m @@ -0,0 +1,8 @@ +#import <Foundation/Foundation.h> + +int main(int argc, char const *argv[]) { + @throw [[NSException alloc] initWithName:@"ThrownException" + reason:@"SomeReason" + userInfo:nil]; + return 0; +} diff --git a/lldb/test/API/tools/lldb-dap/extendedStackTrace/Makefile b/lldb/test/API/tools/lldb-dap/extendedStackTrace/Makefile new file mode 100644 index 00000000000000..e4ee1a0506c0cf --- /dev/null +++ b/lldb/test/API/tools/lldb-dap/extendedStackTrace/Makefile @@ -0,0 +1,5 @@ +OBJC_SOURCES := main.m + +USE_SYSTEM_STDLIB := 1 + +include Makefile.rules diff --git a/lldb/test/API/tools/lldb-dap/extendedStackTrace/TestDAP_extendedStackTrace.py b/lldb/test/API/tools/lldb-dap/extendedStackTrace/TestDAP_extendedStackTrace.py new file mode 100644 index 00000000000000..efc907085b2c21 --- /dev/null +++ b/lldb/test/API/tools/lldb-dap/extendedStackTrace/TestDAP_extendedStackTrace.py @@ -0,0 +1,69 @@ +""" +Test lldb-dap stackTrace request with an extended backtrace thread. +""" + + +import os + +import lldbdap_testcase +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test.lldbplatformutil import * + + +class TestDAP_extendedStackTrace(lldbdap_testcase.DAPTestCaseBase): + @skipUnlessDarwin + def test_stackTrace(self): + """ + Tests the 'stackTrace' packet on a thread with an extended backtrace. + """ + backtrace_recording_lib = findBacktraceRecordingDylib() + if not backtrace_recording_lib: + self.skipTest( + "Skipped because libBacktraceRecording.dylib was present on the system." + ) + + if not os.path.isfile("/usr/lib/system/introspection/libdispatch.dylib"): + self.skipTest( + "Skipped because introspection libdispatch dylib is not present." + ) + + program = self.getBuildArtifact("a.out") + + self.build_and_launch( + program, + env=[ + "DYLD_LIBRARY_PATH=/usr/lib/system/introspection", + "DYLD_INSERT_LIBRARIES=" + backtrace_recording_lib, + ], + ) + source = "main.m" + breakpoint = line_number(source, "breakpoint 1") + lines = [breakpoint] + + breakpoint_ids = self.set_source_breakpoints(source, lines) + self.assertEqual( + len(breakpoint_ids), len(lines), "expect correct number of breakpoints" + ) + + events = self.continue_to_next_stop() + print("huh", events) + stackFrames = self.get_stackFrames(threadId=events[0]["body"]["threadId"]) + self.assertGreaterEqual(len(stackFrames), 3, "expect >= 3 frames") + self.assertEqual(stackFrames[0]["name"], "one") + self.assertEqual(stackFrames[1]["name"], "two") + self.assertEqual(stackFrames[2]["name"], "three") + + stackLabels = [ + frame + for frame in stackFrames + if frame.get("presentationHint", "") == "label" + ] + self.assertEqual(len(stackLabels), 2, "expected two label stack frames") + self.assertRegex( + stackLabels[0]["name"], + "Enqueued from com.apple.root.default-qos \(Thread \d\)", + ) + self.assertRegex( + stackLabels[1]["name"], "Enqueued from com.apple.main-thread \(Thread \d\)" + ) diff --git a/lldb/test/API/tools/lldb-dap/extendedStackTrace/main.m b/lldb/test/API/tools/lldb-dap/extendedStackTrace/main.m new file mode 100644 index 00000000000000..d513880236c517 --- /dev/null +++ b/lldb/test/API/tools/lldb-dap/extendedStackTrace/main.m @@ -0,0 +1,28 @@ +#import <dispatch/dispatch.h> +#include <stdio.h> + +void one() { + printf("one...\n"); // breakpoint 1 +} + +void two() { + printf("two...\n"); + one(); +} + +void three() { + printf("three...\n"); + two(); +} + +int main(int argc, char *argv[]) { + printf("main...\n"); + // Nest from main queue > global queue > main queue. + dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), + ^{ + dispatch_async(dispatch_get_main_queue(), ^{ + three(); + }); + }); + dispatch_main(); +} diff --git a/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py b/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py index 0d7776faa4a9de..fad5419140f454 100644 --- a/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py +++ b/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py @@ -1,13 +1,11 @@ """ -Test lldb-dap setBreakpoints request +Test lldb-dap stackTrace request """ import os -import dap_server import lldbdap_testcase -from lldbsuite.test import lldbutil from lldbsuite.test.decorators import * from lldbsuite.test.lldbtest import * diff --git a/lldb/test/API/tools/lldb-dap/stackTraceMissingFunctionName/TestDAP_stackTraceMissingFunctionName.py b/lldb/test/API/tools/lldb-dap/stackTraceMissingFunctionName/TestDAP_stackTraceMissingFunctionName.py index a04c752764fbb2..f2131d6a821217 100644 --- a/lldb/test/API/tools/lldb-dap/stackTraceMissingFunctionName/TestDAP_stackTraceMissingFunctionName.py +++ b/lldb/test/API/tools/lldb-dap/stackTraceMissingFunctionName/TestDAP_stackTraceMissingFunctionName.py @@ -2,13 +2,8 @@ Test lldb-dap stack trace response """ - -import dap_server from lldbsuite.test.decorators import * -import os - import lldbdap_testcase -from lldbsuite.test import lldbtest, lldbutil class TestDAP_stackTraceMissingFunctionName(lldbdap_testcase.DAPTestCaseBase): diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 57b93c28ce9301..1fd560f21904ab 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -36,7 +36,6 @@ DAP::DAP() focus_tid(LLDB_INVALID_THREAD_ID), stop_at_entry(false), is_attach(false), enable_auto_variable_summaries(false), enable_synthetic_child_debugging(false), - enable_display_extended_backtrace(false), restarting_process_id(LLDB_INVALID_PROCESS_ID), configuration_done_sent(false), waiting_for_run_in_terminal(false), progress_event_reporter( diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 0fc77ac1e81683..c621123f18097f 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -181,7 +181,6 @@ struct DAP { bool is_attach; bool enable_auto_variable_summaries; bool enable_synthetic_child_debugging; - bool enable_display_extended_backtrace; // The process event thread normally responds to process exited events by // shutting down the entire adapter. When we're restarting, we keep the id of // the old process here so we can detect this case and keep running. @@ -193,6 +192,7 @@ struct DAP { // Keep track of the last stop thread index IDs as threads won't go away // unless we send a "thread" event to indicate the thread exited. llvm::DenseSet<lldb::tid_t> thread_ids; + std::map<lldb::tid_t, uint32_t> thread_stack_size_cache; uint32_t reverse_request_seq; std::mutex call_mutex; std::map<int /* request_seq */, ResponseCallback /* reply handler */> diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index a8b85f55939e17..dafe54a49186e0 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -1257,6 +1257,46 @@ llvm::json::Value CreateVariable(lldb::SBValue v, int64_t variablesReference, return llvm::json::Value(std::move(object)); } +// "ExceptionDetails": { +// "type": "object", +// "description": "Detailed information about an exception that has +// occurred.", "properties": { +// "message": { +// "type": "string", +// "description": "Message contained in the exception." +// }, +// "typeName": { +// "type": "string", +// "description": "Short type name of the exception object." +// }, +// "fullTypeName": { +// "type": "string", +// "description": "Fully-qualified type name of the exception object." +// }, +// "evaluateName": { +// "type": "string", +// "description": "An expression that can be evaluated in the current +// scope to obtain the exception object." +// }, +// "stackTrace": { +// "type": "string", +// "description": "Stack trace at the time the exception was thrown." +// }, +// "innerException": { +// "type": "array", +// "items": { +// "$ref": "#/definitions/ExceptionDetails" +// }, +// "description": "Details of the exception contained by this exception, +// if any." +// } +// } +// }, +llvm::json::Value CreateExceptionDetails() { + llvm::json::Object object; + return llvm::json::Value(std::move(object)); +} + llvm::json::Value CreateCompileUnit(lldb::SBCompileUnit unit) { llvm::json::Object object; char unit_path_arr[PATH_MAX]; diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h index 1515f5ba2e5f4d..88df05756a0eaa 100644 --- a/lldb/tools/lldb-dap/JSONUtils.h +++ b/lldb/tools/lldb-dap/JSONUtils.h @@ -467,6 +467,9 @@ llvm::json::Value CreateVariable(lldb::SBValue v, int64_t variablesReference, bool is_name_duplicated = false, std::optional<std::string> custom_name = {}); +/// Create a "ExceptionDetail" object for a LLDB +llvm::json::Value CreateExceptionDetails(); + llvm::json::Value CreateCompileUnit(lldb::SBCompileUnit unit); /// Create a runInTerminal reverse request object diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index 495ed0256120e8..aa34c275fad589 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -701,8 +701,6 @@ void request_attach(const llvm::json::Object &request) { GetBoolean(arguments, "enableAutoVariableSummaries", false); g_dap.enable_synthetic_child_debugging = GetBoolean(arguments, "enableSyntheticChildDebugging", false); - g_dap.enable_display_extended_backtrace = - GetBoolean(arguments, "enableDisplayExtendedBacktrace", false); g_dap.command_escape_prefix = GetString(arguments, "commandEscapePrefix", "`"); g_dap.SetFrameFormat(GetString(arguments, "customFrameFormat")); @@ -1020,6 +1018,68 @@ void request_disconnect(const llvm::json::Object &request) { g_dap.disconnecting = true; } +// "ExceptionInfoRequest": { +// "allOf": [ { "$ref": "#/definitions/Request" }, { +// "type": "object", +// "description": "Retrieves the details of the exception that +// caused this event to be raised. Clients should only call this request if +// the corresponding capability `supportsExceptionInfoR... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/105905 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
