fix resolving of assembly source breakpoints

Author: eronnen

lldb/tools/lldb-dap/Breakpoint.cpp

@@ -9,10 +9,12 @@
 #include "Breakpoint.h"
 #include "DAP.h"
 #include "JSONUtils.h"
+#include "LLDBUtils.h"
 #include "lldb/API/SBAddress.h"
 #include "lldb/API/SBBreakpointLocation.h"
 #include "lldb/API/SBLineEntry.h"
 #include "lldb/API/SBMutex.h"
+#include "lldb/lldb-enumerations.h"
 #include "llvm/ADT/StringExtras.h"
 #include <cstddef>
 #include <cstdint>
@@ -63,14 +65,43 @@ protocol::Breakpoint Breakpoint::ToProtocolBreakpoint() {
     std::string formatted_addr =
         "0x" + llvm::utohexstr(bp_addr.GetLoadAddress(m_bp.GetTarget()));
     breakpoint.instructionReference = formatted_addr;
+
+    lldb::StopDisassemblyType stop_disassembly_display =
+        GetStopDisassemblyDisplay(m_dap.debugger);
     auto line_entry = bp_addr.GetLineEntry();
-    const auto line = line_entry.GetLine();
-    if (line != UINT32_MAX)
-      breakpoint.line = line;
-    const auto column = line_entry.GetColumn();
-    if (column != 0)
-      breakpoint.column = column;
-    breakpoint.source = CreateSource(line_entry);
+    if (!ShouldDisplayAssemblySource(line_entry, stop_disassembly_display)) {
+      const auto line = line_entry.GetLine();
+      if (line != UINT32_MAX)
+        breakpoint.line = line;
+      const auto column = line_entry.GetColumn();
+      if (column != 0)
+        breakpoint.column = column;
+      breakpoint.source = CreateSource(line_entry);
+    } else {
+      // Breakpoint made by assembly
+      auto symbol_context = bp_addr.GetSymbolContext(
+          lldb::eSymbolContextSymbol | lldb::eSymbolContextModule);
+      if (symbol_context.IsValid()) {
+        auto symbol = symbol_context.GetSymbol();
+        breakpoint.line =
+            m_bp.GetTarget()
+                .ReadInstructions(symbol.GetStartAddress(), bp_addr, nullptr)
+                .GetSize() +
+            1;
+        protocol::Source source;
+        source.name = symbol.GetName();
+
+        auto module = symbol_context.GetModule();
+        if (module.IsValid()) {
+          std::string path = module.GetFileSpec().GetDirectory();
+          path += "/";
+          path += module.GetFileSpec().GetFilename();
+          source.path = std::move(path);
+        }
+
+        breakpoint.source = std::move(source);
+      }
+    }
   }
 
   return breakpoint;

lldb/tools/lldb-dap/DAP.h

@@ -169,7 +169,8 @@ struct DAP {
   Variables variables;
   lldb::SBBroadcaster broadcaster;
   llvm::StringMap<SourceBreakpointMap> source_breakpoints;
-  llvm::DenseMap<int64_t, SourceBreakpointMap> assembly_breakpoints;
+  llvm::DenseMap<int64_t, llvm::DenseMap<uint32_t, SourceBreakpoint>>
+      assembly_breakpoints;
   FunctionBreakpointMap function_breakpoints;
   InstructionBreakpointMap instruction_breakpoints;
   std::optional<std::vector<ExceptionBreakpoint>> exception_breakpoints;

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

@@ -122,9 +122,9 @@ void BreakpointLocationsRequestHandler::AddAssemblyBreakpointLocations(
 
   // start_line is relative to the symbol's start address
   lldb::SBInstructionList insts = symbol.GetInstructions(dap.target);
-  for (uint32_t i = start_line - 1; i < insts.GetSize() && i < (end_line - 1);
+  for (uint32_t i = start_line - 1; i < insts.GetSize() && i <= (end_line - 1);
        ++i) {
-    locations.emplace_back(i, 0);
+    locations.emplace_back(i, 1);
   }
 }
 

lldb/tools/lldb-dap/Handler/RequestHandler.h

@@ -381,11 +381,11 @@ class SetBreakpointsRequestHandler
   Run(const protocol::SetBreakpointsArguments &args) const override;
 
   std::vector<protocol::Breakpoint> SetSourceBreakpoints(
-      const std::string &path,
+      const protocol::Source &source,
       const std::optional<std::vector<protocol::SourceBreakpoint>> &breakpoints)
       const;
   std::vector<protocol::Breakpoint> SetAssemblyBreakpoints(
-      int64_t sourceReference,
+      const protocol::Source &source,
       const std::optional<std::vector<protocol::SourceBreakpoint>> &breakpoints)
       const;
 };

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

@@ -28,21 +28,20 @@ SetBreakpointsRequestHandler::Run(
   const auto &source = args.source;
   std::vector<protocol::Breakpoint> response_breakpoints;
   if (source.sourceReference)
-    response_breakpoints = SetAssemblyBreakpoints(
-        source.sourceReference.value(), args.breakpoints);
+    response_breakpoints = SetAssemblyBreakpoints(source, args.breakpoints);
   else if (source.path)
-    response_breakpoints =
-        SetSourceBreakpoints(source.path.value(), args.breakpoints);
+    response_breakpoints = SetSourceBreakpoints(source, args.breakpoints);
 
   return protocol::SetBreakpointsResponseBody{std::move(response_breakpoints)};
 }
 
 std::vector<protocol::Breakpoint>
 SetBreakpointsRequestHandler::SetSourceBreakpoints(
-    const std::string &path,
+    const protocol::Source &source,
     const std::optional<std::vector<protocol::SourceBreakpoint>> &breakpoints)
     const {
   std::vector<protocol::Breakpoint> response_breakpoints;
+  std::string path = source.path.value_or("");
 
   // Decode the source breakpoint infos for this "setBreakpoints" request
   SourceBreakpointMap request_bps;
@@ -96,10 +95,11 @@ SetBreakpointsRequestHandler::SetSourceBreakpoints(
 
 std::vector<protocol::Breakpoint>
 SetBreakpointsRequestHandler::SetAssemblyBreakpoints(
-    int64_t sourceReference,
+    const protocol::Source &source,
     const std::optional<std::vector<protocol::SourceBreakpoint>> &breakpoints)
     const {
   std::vector<protocol::Breakpoint> response_breakpoints;
+  int64_t sourceReference = source.sourceReference.value_or(0);
 
   lldb::SBProcess process = dap.target.GetProcess();
   lldb::SBThread thread =
@@ -114,30 +114,26 @@ SetBreakpointsRequestHandler::SetAssemblyBreakpoints(
     return response_breakpoints; // Not yet supporting breakpoints in assembly
                                  // without a valid symbol
 
-  SourceBreakpointMap request_bps;
+  llvm::DenseMap<uint32_t, SourceBreakpoint> request_bps;
   if (breakpoints) {
     for (const auto &bp : *breakpoints) {
       SourceBreakpoint src_bp(dap, bp);
-      std::pair<uint32_t, uint32_t> bp_pos(src_bp.GetLine(), 0);
-      request_bps.try_emplace(bp_pos, src_bp);
+      request_bps.try_emplace(src_bp.GetLine(), src_bp);
       const auto [iv, inserted] =
-          dap.assembly_breakpoints[sourceReference].try_emplace(bp_pos, src_bp);
+          dap.assembly_breakpoints[sourceReference].try_emplace(
+              src_bp.GetLine(), src_bp);
       // We check if this breakpoint already exists to update it
       if (inserted)
         iv->getSecond().SetBreakpoint(symbol);
       else
         iv->getSecond().UpdateBreakpoint(src_bp);
 
       protocol::Breakpoint response_bp = iv->getSecond().ToProtocolBreakpoint();
-      protocol::Source source;
-      source.sourceReference = sourceReference;
-      source.name = symbol.GetName();
-      response_bp.source = std::move(source);
-
+      response_bp.source = source;
       if (!response_bp.line)
         response_bp.line = src_bp.GetLine();
-      if (!response_bp.column)
-        response_bp.column = src_bp.GetColumn();
+      if (bp.column)
+        response_bp.column = *bp.column;
       response_breakpoints.push_back(response_bp);
     }
   }

lldb/tools/lldb-dap/package-lock.json

@@ -1,7 +1,7 @@
 {
   "name": "lldb-dap",
   "displayName": "LLDB DAP",
-  "version": "0.2.13",
+  "version": "0.2.14",
   "publisher": "llvm-vs-code-extensions",
   "homepage": "https://lldb.llvm.org",
   "description": "Debugging with LLDB in Visual Studio Code",
@@ -265,6 +265,9 @@
       ]
     },
     "breakpoints": [
+      {
+        "language": "lldb.disassembly"
+      },
       {
         "language": "ada"
       },