-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[lldb-dap] Improve stackTrace
and exceptionInfo
DAP request handlers
#105905
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-lldb Author: John Harrison (ashgti) ChangesRefactoring Additionally adding additional information to the 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:
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]
|
This was based on the comments in #104874 LMKWYT |
e22a5dd
to
029a174
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM, and thank you for your diligence with writing tests.
@jeffreytan81 I moved the expensive call into the LMKWYT |
body.try_emplace("totalFrames", totalFrames); | ||
// If we loaded all the frames, set the total frame to the current total, | ||
// otherwise use the totalFrames to indiciate more data is available. | ||
body.try_emplace("totalFrames", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tested this? Since you are returning one more frame, I am not sure if this is causing VSCode client to ask for one more frame when you click "Get More frames" link. I believe @clayborg suggested defining a page size (like 20 frames), then if it is not done, return startFrame + stackFrames.size() + STACK_PAGE_SIZE
instead. Then, client will ask for STACK_PAGE_SIZE
instead of 1 more frame.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
I had tested this but the 20 page size does work better with deep stack traces.
Also updated the tests to verify this as well.
✅ With the latest revision this PR passed the Python code formatter. |
✅ With the latest revision this PR passed the C/C++ code formatter. |
@ashgti, is there any concern to guard this feature behind a launch configuration option? I am happy original profile trace hot path is moved to a rare DAP request, but I am not sure about the cost of Also, if I understand it correctly, the extended backtrace is a plugin style feature which any runtime can implement its own extended backtrace however expensive it is. This means we can't foresee the future cost of enabling this. Adding a knob for it would be useful for people to quickly opt in/out. |
Reverted the removal of |
lldb/tools/lldb-dap/lldb-dap.cpp
Outdated
// *NOTE*: Threads can be chained across mutliple backtraces, so we | ||
// need to keep track of each backtrace we've traversed fully in the | ||
// offset. | ||
while (!frame.IsValid() && current.IsValid() && !threadCluster.empty()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: it may worth moving extended back trace specific code into a dedicated helper function to improve readability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay, I was out for a week.
I refactored the request_stackTrace
implementation and added some additional tests to validate the pagination behavior and ensure stack traces across thread backtraces are handled correctly.
…ers. 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.
…tead of only a +1. This helps improve pagination of deep stack traces.
…ace` is true and document `enableDisplayExtendedBacktrace`.
…to simplify the implementation.
caf309f
to
ecfd5b1
Compare
…ers (llvm#105905) 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. (cherry picked from commit 5b4100c)
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. TheexceptionInfo
request is only called if a stop event occurs withreason='exception'
, which should mitigate the performance ofSBThread::GetCurrentException
calls.Adding unit tests for exception handling and stack trace supporting.