Fix incorrect interpretation of includeAll

Author: JDevlieghere

lldb/test/API/tools/lldb-dap/extendedStackTrace/TestDAP_extendedStackTrace.py

@@ -2,7 +2,6 @@
 Test lldb-dap stackTrace request with an extended backtrace thread.
 """
 
-
 import os
 
 import lldbdap_testcase
@@ -12,11 +11,8 @@
 
 
 class TestDAP_extendedStackTrace(lldbdap_testcase.DAPTestCaseBase):
-    @skipUnlessDarwin
-    def test_stackTrace(self):
-        """
-        Tests the 'stackTrace' packet on a thread with an extended backtrace.
-        """
+
+    def build_and_run(self, displayExtendedBacktrace=True):
         backtrace_recording_lib = findBacktraceRecordingDylib()
         if not backtrace_recording_lib:
             self.skipTest(
@@ -36,7 +32,7 @@ def test_stackTrace(self):
                 "DYLD_LIBRARY_PATH=/usr/lib/system/introspection",
                 "DYLD_INSERT_LIBRARIES=" + backtrace_recording_lib,
             ],
-            displayExtendedBacktrace=True,
+            displayExtendedBacktrace=displayExtendedBacktrace,
         )
         source = "main.m"
         breakpoint = line_number(source, "breakpoint 1")
@@ -47,6 +43,12 @@ def test_stackTrace(self):
             len(breakpoint_ids), len(lines), "expect correct number of breakpoints"
         )
 
+    @skipUnlessDarwin
+    def test_stackTrace(self):
+        """
+        Tests the 'stackTrace' packet on a thread with an extended backtrace.
+        """
+        self.build_and_run()
         events = self.continue_to_next_stop()
 
         stackFrames, totalFrames = self.get_stackFrames_and_totalFramesCount(
@@ -102,3 +104,23 @@ def test_stackTrace(self):
             self.assertGreaterEqual(
                 totalFrames, i, "total frames should include a pagination offset"
             )
+
+    @skipIfWindows
+    def test_stackTraceWithFormat(self):
+        """
+        Tests the 'stackTrace' packet on a thread with an extended backtrace using stack trace formats.
+        """
+        self.build_and_run(displayExtendedBacktrace=False)
+        events = self.continue_to_next_stop()
+
+        stackFrames, _ = self.get_stackFrames_and_totalFramesCount(
+            threadId=events[0]["body"]["threadId"], format={"includeAll": True}
+        )
+
+        stackLabels = [
+            (i, frame)
+            for i, frame in enumerate(stackFrames)
+            if frame.get("presentationHint", "") == "label"
+        ]
+
+        self.assertEqual(len(stackLabels), 2, "expected two label stack frames")

lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py

@@ -2,7 +2,6 @@
 Test lldb-dap stackTrace request
 """
 
-
 import os
 
 import lldbdap_testcase
@@ -230,9 +229,6 @@ def test_StackFrameFormat(self):
         self.set_source_breakpoints(source, [line_number(source, "recurse end")])
 
         self.continue_to_next_stop()
-        frame = self.get_stackFrames(format={"includeAll": True})[0]
-        self.assertEqual(frame["name"], "a.out main.c:6:5 recurse(x=1)")
-
         frame = self.get_stackFrames(format={"parameters": True})[0]
         self.assertEqual(frame["name"], "recurse(x=1)")
 

lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp

@@ -51,7 +51,8 @@ static constexpr int StackPageSize = 20;
 static bool FillStackFrames(DAP &dap, lldb::SBThread &thread,
                             lldb::SBFormat &frame_format,
                             llvm::json::Array &stack_frames, int64_t &offset,
-                            const int64_t start_frame, const int64_t levels) {
+                            const int64_t start_frame, const int64_t levels,
+                            const bool include_all) {
   bool reached_end_of_stack = false;
   for (int64_t i = start_frame;
        static_cast<int64_t>(stack_frames.size()) < levels; i++) {
@@ -71,7 +72,9 @@ static bool FillStackFrames(DAP &dap, lldb::SBThread &thread,
     stack_frames.emplace_back(CreateStackFrame(frame, frame_format));
   }
 
-  if (dap.configuration.displayExtendedBacktrace && reached_end_of_stack) {
+  const bool include_extended_backtrace =
+      include_all || dap.configuration.displayExtendedBacktrace;
+  if (include_extended_backtrace && reached_end_of_stack) {
     // Check for any extended backtraces.
     for (uint32_t bt = 0;
          bt < thread.GetProcess().GetNumExtendedBacktraceTypes(); bt++) {
@@ -82,7 +85,8 @@ static bool FillStackFrames(DAP &dap, lldb::SBThread &thread,
 
       reached_end_of_stack = FillStackFrames(
           dap, backtrace, frame_format, stack_frames, offset,
-          (start_frame - offset) > 0 ? start_frame - offset : -1, levels);
+          (start_frame - offset) > 0 ? start_frame - offset : -1, levels,
+          include_all);
       if (static_cast<int64_t>(stack_frames.size()) >= levels)
         break;
     }
@@ -180,44 +184,53 @@ void StackTraceRequestHandler::operator()(
   llvm::json::Object body;
 
   lldb::SBFormat frame_format = dap.frame_format;
+  bool include_all = false;
 
   if (const auto *format = arguments->getObject("format")) {
+    // Indicates that all stack frames should be included, even those the debug
+    // adapter might otherwise hide.
+    include_all = GetBoolean(format, "includeAll").value_or(false);
+
+    // Parse the properties that have a corresponding format string.
+    // FIXME: Support "parameterTypes" and "hex".
+    const bool module = GetBoolean(format, "module").value_or(false);
+    const bool line = GetBoolean(format, "line").value_or(false);
     const bool parameters = GetBoolean(format, "parameters").value_or(false);
     const bool parameter_names =
         GetBoolean(format, "parameterNames").value_or(false);
     const bool parameter_values =
         GetBoolean(format, "parameterValues").value_or(false);
-    const bool line = GetBoolean(format, "line").value_or(false);
-    const bool module = GetBoolean(format, "module").value_or(false);
-    const bool include_all = GetBoolean(format, "includeAll").value_or(false);
 
-    std::string format_str;
-    llvm::raw_string_ostream os(format_str);
+    // Only change the format string if we have to.
+    if (module || line || parameters || parameter_names || parameter_values) {
+      std::string format_str;
+      llvm::raw_string_ostream os(format_str);
 
-    if (include_all || module)
-      os << "{${module.file.basename} }";
+      if (module)
+        os << "{${module.file.basename} }";
 
-    if (include_all || line)
-      os << "{${line.file.basename}:${line.number}:${line.column} }";
+      if (line)
+        os << "{${line.file.basename}:${line.number}:${line.column} }";
 
-    if (include_all || parameters || parameter_names || parameter_values)
-      os << "{${function.name-with-args}}";
-    else
-      os << "{${function.name-without-args}}";
+      if (parameters || parameter_names || parameter_values)
+        os << "{${function.name-with-args}}";
+      else
+        os << "{${function.name-without-args}}";
 
-    lldb::SBError error;
-    frame_format = lldb::SBFormat(format_str.c_str(), error);
-    assert(error.Success());
+      lldb::SBError error;
+      frame_format = lldb::SBFormat(format_str.c_str(), error);
+      assert(error.Success());
+    }
   }
 
   if (thread.IsValid()) {
     const auto start_frame =
         GetInteger<uint64_t>(arguments, "startFrame").value_or(0);
     const auto levels = GetInteger<uint64_t>(arguments, "levels").value_or(0);
     int64_t offset = 0;
-    bool reached_end_of_stack =
-        FillStackFrames(dap, thread, frame_format, stack_frames, offset,
-                        start_frame, levels == 0 ? INT64_MAX : levels);
+    bool reached_end_of_stack = FillStackFrames(
+        dap, thread, frame_format, stack_frames, offset, start_frame,
+        levels == 0 ? INT64_MAX : levels, include_all);
     body.try_emplace("totalFrames",
                      start_frame + stack_frames.size() +
                          (reached_end_of_stack ? 0 : StackPageSize));