[lldb] When starting in a hidden frame, don't skip over hidden frames when navigating up/down

[Michael137]

When stopped in a hidden frame (either because we selected the hidden frame or hit a breakpoint inside it), a user most likely is intersted in exploring the immediate frames around it. But currently issuing up/down commands will unconditionally skip over all hidden frames.

This patch makes it so up/down commands don't skip hidden frames if the frame we started it was a hidden frame.

[adrian-prantl]

I think that is a good behavior change.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

When stopped in a hidden frame (either because we selected the hidden frame or hit a breakpoint inside it), a user most likely is intersted in exploring the immediate frames around it. But currently issuing up/down commands will unconditionally skip over all hidden frames.

This patch makes it so up/down commands don't skip hidden frames if the frame we started it was a hidden frame.

---

Full diff: https://github.com/llvm/llvm-project/pull/166394.diff

4 Files Affected:

• (modified) lldb/source/Commands/CommandObjectFrame.cpp (+30-22)
• (added) lldb/test/API/commands/frame/select-hidden/Makefile (+3)
• (added) lldb/test/API/commands/frame/select-hidden/TestNavigateHiddenFrame.py (+32)
• (added) lldb/test/API/commands/frame/select-hidden/main.cpp (+13)

diff --git a/lldb/source/Commands/CommandObjectFrame.cpp b/lldb/source/Commands/CommandObjectFrame.cpp
index 88a02dce35b9d..9133359fbf537 100644
--- a/lldb/source/Commands/CommandObjectFrame.cpp
+++ b/lldb/source/Commands/CommandObjectFrame.cpp
@@ -265,6 +265,29 @@ class CommandObjectFrameSelect : public CommandObjectParsed {
 
   Options *GetOptions() override { return &m_options; }
 
+private:
+  void SkipHiddenFrames(Thread &thread, uint32_t frame_idx) {
+    uint32_t candidate_idx = frame_idx;
+    const unsigned max_depth = 12;
+    for (unsigned num_try = 0; num_try < max_depth; ++num_try) {
+      if (candidate_idx == 0 && *m_options.relative_frame_offset == -1) {
+        candidate_idx = UINT32_MAX;
+        break;
+      }
+      candidate_idx += *m_options.relative_frame_offset;
+      if (auto candidate_sp = thread.GetStackFrameAtIndex(candidate_idx)) {
+        if (candidate_sp->IsHidden())
+          continue;
+        // Now candidate_idx is the first non-hidden frame.
+        break;
+      }
+      candidate_idx = UINT32_MAX;
+      break;
+    };
+    if (candidate_idx != UINT32_MAX)
+      m_options.relative_frame_offset = candidate_idx - frame_idx;
+  }
+
 protected:
   void DoExecute(Args &command, CommandReturnObject &result) override {
     // No need to check "thread" for validity as eCommandRequiresThread ensures
@@ -278,28 +301,13 @@ class CommandObjectFrameSelect : public CommandObjectParsed {
       if (frame_idx == UINT32_MAX)
         frame_idx = 0;
 
-      // If moving up/down by one, skip over hidden frames.
-      if (*m_options.relative_frame_offset == 1 ||
-          *m_options.relative_frame_offset == -1) {
-        uint32_t candidate_idx = frame_idx;
-        const unsigned max_depth = 12;
-        for (unsigned num_try = 0; num_try < max_depth; ++num_try) {
-          if (candidate_idx == 0 && *m_options.relative_frame_offset == -1) {
-            candidate_idx = UINT32_MAX;
-            break;
-          }
-          candidate_idx += *m_options.relative_frame_offset;
-          if (auto candidate_sp = thread->GetStackFrameAtIndex(candidate_idx)) {
-            if (candidate_sp->IsHidden())
-              continue;
-            // Now candidate_idx is the first non-hidden frame.
-            break;
-          }
-          candidate_idx = UINT32_MAX;
-          break;
-        };
-        if (candidate_idx != UINT32_MAX)
-          m_options.relative_frame_offset = candidate_idx - frame_idx;
+      // If moving up/down by one, skip over hidden frames, unless we started
+      // in a hidden frame.
+      if ((*m_options.relative_frame_offset == 1 ||
+           *m_options.relative_frame_offset == -1)) {
+        if (auto current_frame_sp = thread->GetStackFrameAtIndex(frame_idx);
+            !current_frame_sp->IsHidden())
+          SkipHiddenFrames(*thread, frame_idx);
       }
 
       if (*m_options.relative_frame_offset < 0) {
diff --git a/lldb/test/API/commands/frame/select-hidden/Makefile b/lldb/test/API/commands/frame/select-hidden/Makefile
new file mode 100644
index 0000000000000..99998b20bcb05
--- /dev/null
+++ b/lldb/test/API/commands/frame/select-hidden/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/commands/frame/select-hidden/TestNavigateHiddenFrame.py b/lldb/test/API/commands/frame/select-hidden/TestNavigateHiddenFrame.py
new file mode 100644
index 0000000000000..698447b552877
--- /dev/null
+++ b/lldb/test/API/commands/frame/select-hidden/TestNavigateHiddenFrame.py
@@ -0,0 +1,32 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class NavigateHiddenFrameTestCase(TestBase):
+    NO_DEBUG_INFO_TESTCASE = True
+
+    @add_test_categories(["libc++"])
+    def test(self):
+        """Test going up/down a backtrace but we started in a hidden frame."""
+        self.build()
+        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+            self, "Break here", lldb.SBFileSpec("main.cpp")
+        )
+        # up
+        self.assertIn("__impl2", thread.selected_frame.GetFunctionName())
+        self.expect("up")
+        self.assertIn("__impl1", thread.selected_frame.GetFunctionName())
+        self.expect("up")
+        self.assertIn("__impl", thread.selected_frame.GetFunctionName())
+        self.expect("up")
+        self.assertIn("non_impl", thread.selected_frame.GetFunctionName())
+
+        # Back down again.
+        self.expect("down")
+        self.assertIn("__impl", thread.selected_frame.GetFunctionName())
+        self.expect("down")
+        self.assertIn("__impl1", thread.selected_frame.GetFunctionName())
+        self.expect("down")
+        self.assertIn("__impl2", thread.selected_frame.GetFunctionName())
diff --git a/lldb/test/API/commands/frame/select-hidden/main.cpp b/lldb/test/API/commands/frame/select-hidden/main.cpp
new file mode 100644
index 0000000000000..dc97abb6323a4
--- /dev/null
+++ b/lldb/test/API/commands/frame/select-hidden/main.cpp
@@ -0,0 +1,13 @@
+namespace std {
+namespace __1 {
+static const char *__impl2() { return "Break here"; }
+static const char *__impl1() { return __impl2(); }
+static const char *__impl() { return __impl1(); }
+static const char *non_impl() { return __impl(); }
+} // namespace __1
+} // namespace std
+
+int main() {
+  std::__1::non_impl();
+  __builtin_debugtrap();
+}