[lldb-dap] Prevent using an implicit step-in.

When there is a function that is inlined at the current program counter. If you get the current `line_entry` using the program counter's address it will point to the location of the inline function that may be in another file. (this is in implicit step-in and should not happen what step over is called).

Use the current frame to get the correct `line_entry`

Author: da-viper

lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py

@@ -344,7 +344,12 @@ def stepOver(
         granularity="statement",
         timeout=DEFAULT_TIMEOUT,
     ):
-        self.dap_server.request_next(threadId=threadId, granularity=granularity)
+        response = self.dap_server.request_next(
+            threadId=threadId, granularity=granularity
+        )
+        self.assertTrue(
+            response["success"], f"next request failed: response {response}"
+        )
         if waitForStop:
             return self.dap_server.wait_for_stopped(timeout)
         return None

lldb/test/API/tools/lldb-dap/step/Number.h

@@ -0,0 +1,57 @@
+#ifndef NUMBER_H
+#define NUMBER_H
+
+#include <cstddef>
+
+template <std::size_t Size> class Numbers {
+public:
+  using ValueType = int;
+  using PointerType = ValueType *;
+  using ReferenceType = ValueType &;
+
+  class Iterator {
+    // all the operators needs to be inlined so that there is no call
+    // instruction.
+  public:
+    __attribute__((always_inline)) explicit Iterator(PointerType ptr)
+        : m_ptr(ptr) {}
+
+    __attribute__((always_inline)) Iterator &operator++() noexcept {
+      ++m_ptr;
+      return *this;
+    };
+
+    __attribute__((always_inline)) Iterator operator++(int) noexcept {
+      Iterator iter = *this;
+      m_ptr++;
+      return iter;
+    }
+
+    __attribute__((always_inline)) ReferenceType operator*() const noexcept {
+      return *m_ptr;
+    }
+    __attribute__((always_inline)) bool
+    operator==(const Iterator &iter) noexcept {
+      return m_ptr == iter.m_ptr;
+    }
+    __attribute__((always_inline)) bool
+    operator!=(const Iterator &iter) noexcept {
+      return !(*this == iter);
+    }
+
+  private:
+    PointerType m_ptr;
+  };
+
+  PointerType data() { return static_cast<PointerType>(m_buffer); }
+
+  Iterator begin() { return Iterator(data()); }
+  Iterator cbegin() { return Iterator(data()); }
+  Iterator end() { return Iterator(data() + Size); }
+  Iterator cend() { return Iterator(data() + Size); }
+
+private:
+  int m_buffer[Size]{};
+};
+
+#endif // NUMBER_H

lldb/test/API/tools/lldb-dap/step/TestDAP_step.py

@@ -83,3 +83,44 @@ def test_step(self):
 
                     # only step one thread that is at the breakpoint and stop
                     break
+
+    def test_step_over(self):
+        """
+        Test stepping over when the program counter is in another file.
+        """
+        # when in a for range loop the program counter will in the iterator header.
+        # make sure the frame source file is correct.
+        program = self.getBuildArtifact("a.out")
+        self.build_and_launch(program)
+        source = "main.cpp"
+        breakpoint1_line = line_number(source, "// breakpoint 2")
+        step_over_pos = line_number(source, "// position_after_step_over")
+        lines = [breakpoint1_line]
+        breakpoint_ids = self.set_source_breakpoints(source, lines)
+        self.assertEqual(
+            len(breakpoint_ids), len(lines), "expect correct number of breakpoints."
+        )
+        self.continue_to_breakpoints(breakpoint_ids)
+
+        thread_id = self.dap_server.get_thread_id()
+        self.stepOver(thread_id)
+        levels = 1
+        frames = self.get_stackFrames(thread_id, 0, levels)
+        self.assertEqual(len(frames), levels, "expect current number of frame levels.")
+        top_frame = frames[0]
+        self.assertEqual(
+            top_frame["source"]["name"], source, "expect we are in the same file."
+        )
+        self.assertTrue(
+            top_frame["source"]["path"].endswith(source),
+            f"expect path ending with '{source}'.",
+        )
+        self.assertEqual(
+            top_frame["line"],
+            step_over_pos,
+            f"expect step_over on line {step_over_pos}",
+        )
+
+        # clear breakpoints
+        self.set_source_breakpoints(source, [])
+        self.continue_to_exit(exitCode=13)

lldb/test/API/tools/lldb-dap/step/main.cpp

@@ -1,8 +1,23 @@
+#include "Number.h"
+
 int function(int x) {
   if ((x % 2) == 0)
     return function(x - 1) + x; // breakpoint 1
   else
     return x;
 }
 
-int main(int argc, char const *argv[]) { return function(2); }
+int function2() {
+  Numbers<10> list;
+
+  int result = 0;
+  for (const int val : list) // position_after_step_over
+    result++;                // breakpoint 2
+
+  return result;
+}
+
+int main(int argc, char const *argv[]) {
+  int func_result = function2();
+  return function(2) + func_result; // returns 13
+}

lldb/tools/lldb-dap/JSONUtils.cpp

@@ -581,19 +581,19 @@ llvm::json::Value CreateStackFrame(lldb::SBFrame &frame,
 
   EmplaceSafeString(object, "name", frame_name);
 
-  auto target = frame.GetThread().GetProcess().GetTarget();
-  auto source = CreateSource(frame.GetPCAddress(), target);
+  lldb::SBTarget target = frame.GetThread().GetProcess().GetTarget();
+  protocol::Source source = CreateSource(frame);
+
   if (!IsAssemblySource(source)) {
     // This is a normal source with a valid line entry.
-    auto line_entry = frame.GetLineEntry();
+    lldb::SBLineEntry line_entry = frame.GetLineEntry();
     object.try_emplace("line", line_entry.GetLine());
-    auto column = line_entry.GetColumn();
+    uint32_t column = line_entry.GetColumn();
     object.try_emplace("column", column);
   } else if (frame.GetSymbol().IsValid()) {
     // This is a source where the disassembly is used, but there is a valid
     // symbol. Calculate the line of the current PC from the start of the
     // current symbol.
-    lldb::SBTarget target = frame.GetThread().GetProcess().GetTarget();
     lldb::SBInstructionList inst_list = target.ReadInstructions(
         frame.GetSymbol().GetStartAddress(), frame.GetPCAddress(), nullptr);
     size_t inst_line = inst_list.GetSize();

lldb/tools/lldb-dap/ProtocolUtils.cpp

@@ -105,6 +105,21 @@ protocol::Source CreateSource(lldb::SBAddress address, lldb::SBTarget &target) {
   return CreateSource(line_entry.GetFileSpec());
 }
 
+protocol::Source CreateSource(lldb::SBFrame frame) {
+  if (!frame.IsValid())
+    return {};
+
+  const lldb::SBTarget target = frame.GetThread().GetProcess().GetTarget();
+  lldb::SBDebugger debugger = target.GetDebugger();
+  lldb::StopDisassemblyType stop_disassembly_display =
+      GetStopDisassemblyDisplay(debugger);
+  const lldb::SBAddress frame_pc = frame.GetPCAddress();
+  if (ShouldDisplayAssemblySource(frame_pc, stop_disassembly_display))
+    return CreateAssemblySource(target, frame_pc);
+
+  return CreateSource(frame.GetLineEntry().GetFileSpec());
+}
+
 bool IsAssemblySource(const protocol::Source &source) {
   // According to the specification, a source must have either `path` or
   // `sourceReference` specified. We use `path` for sources with known source

lldb/tools/lldb-dap/ProtocolUtils.h

@@ -42,6 +42,17 @@ protocol::Source CreateSource(const lldb::SBFileSpec &file);
 ///     definition outlined by Microsoft.
 protocol::Source CreateSource(lldb::SBAddress address, lldb::SBTarget &target);
 
+/// Create a `protocol::Source` object as described in the debug adapter
+/// definition.
+///
+/// \param[in] frame
+///     The frame to use when populating the "Source" object.
+///
+/// \return
+///     A `protcol::Source` object that follows the formal JSON
+///     definition outlined by Microsoft.
+protocol::Source CreateSource(lldb::SBFrame frame);
+
 /// Checks if the given source is for assembly code.
 bool IsAssemblySource(const protocol::Source &source);