[lldb-dap] Fix source references

The protocol expects that `sourceReference` be less than `(2^31)-1`, but we currently represent memory address as source reference, this can be truncated either when sending through json or by the client. Instead, generate new source references based on the memory address.

Make the `CreateSource` function return an optional source.

# Conflicts:
#	lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp

Author: da-viper

lldb/test/API/tools/lldb-dap/breakpoint-assembly/TestDAP_breakpointAssembly.py

@@ -67,19 +67,19 @@ def test_break_on_invalid_source_reference(self):
             "Invalid sourceReference.",
         )
 
-        # Verify that setting a breakpoint on a source reference without a symbol also fails
+        # Verify that setting a breakpoint on a source reference that is not created fails
         response = self.dap_server.request_setBreakpoints(
-            Source(source_reference=0), [1]
+            Source(source_reference=200), [1]
         )
         self.assertIsNotNone(response)
         breakpoints = response["body"]["breakpoints"]
         self.assertEqual(len(breakpoints), 1)
-        breakpoint = breakpoints[0]
+        break_point = breakpoints[0]
         self.assertFalse(
-            breakpoint["verified"], "Expected breakpoint to not be verified"
+            break_point["verified"], "Expected breakpoint to not be verified"
         )
-        self.assertIn("message", breakpoint, "Expected message to be present")
+        self.assertIn("message", break_point, "Expected message to be present")
         self.assertEqual(
-            breakpoint["message"],
-            "Breakpoints in assembly without a valid symbol are not supported yet.",
+            break_point["message"],
+            "Invalid sourceReference.",
         )

lldb/tools/lldb-dap/Breakpoint.cpp

@@ -64,8 +64,11 @@ protocol::Breakpoint Breakpoint::ToProtocolBreakpoint() {
         "0x" + llvm::utohexstr(bp_addr.GetLoadAddress(m_bp.GetTarget()));
     breakpoint.instructionReference = formatted_addr;
 
-    auto source = CreateSource(bp_addr, m_dap.target);
-    if (!IsAssemblySource(source)) {
+    std::optional<protocol::Source> source =
+        CreateSource(bp_addr, m_dap.target, [this](lldb::addr_t addr) {
+          return m_dap.CreateSourceReference(addr);
+        });
+    if (source && !IsAssemblySource(*source)) {
       auto line_entry = bp_addr.GetLineEntry();
       const auto line = line_entry.GetLine();
       if (line != LLDB_INVALID_LINE_NUMBER)

lldb/tools/lldb-dap/DAP.cpp

@@ -510,6 +510,25 @@ DAP::SendFormattedOutput(OutputType o, const char *format, ...) {
       o, llvm::StringRef(buffer, std::min<int>(actual_length, sizeof(buffer))));
 }
 
+int32_t DAP::CreateSourceReference(lldb::addr_t address) {
+  auto iter = llvm::find(source_references, address);
+  if (iter != source_references.end())
+    return std::distance(source_references.begin(), iter) + 1;
+
+  source_references.emplace_back(address);
+  return static_cast<int32_t>(source_references.size());
+}
+
+std::optional<lldb::addr_t> DAP::GetSourceReferenceAddress(int32_t reference) {
+  if (reference <= LLDB_DAP_INVALID_SRC_REF)
+    return std::nullopt;
+
+  if (static_cast<size_t>(reference) > source_references.size())
+    return std::nullopt;
+
+  return source_references[reference - 1];
+}
+
 ExceptionBreakpoint *DAP::GetExceptionBPFromStopReason(lldb::SBThread &thread) {
   const auto num = thread.GetStopReasonDataCount();
   // Check to see if have hit an exception breakpoint and change the

lldb/tools/lldb-dap/DAP.h

@@ -105,6 +105,9 @@ struct DAP {
   /// Map step in target id to list of function targets that user can choose.
   llvm::DenseMap<lldb::addr_t, std::string> step_in_targets;
 
+  /// list of addresses mapped by sourceReference(index - 1)
+  std::vector<lldb::addr_t> source_references;
+
   /// A copy of the last LaunchRequest so we can reuse its arguments if we get a
   /// RestartRequest. Restarting an AttachRequest is not supported.
   std::optional<protocol::LaunchRequestArguments> last_launch_request;
@@ -219,7 +222,9 @@ struct DAP {
   void __attribute__((format(printf, 3, 4)))
   SendFormattedOutput(OutputType o, const char *format, ...);
 
-  static int64_t GetNextSourceReference();
+  int32_t CreateSourceReference(lldb::addr_t address);
+
+  std::optional<lldb::addr_t> GetSourceReferenceAddress(int32_t reference);
 
   ExceptionBreakpoint *GetExceptionBPFromStopReason(lldb::SBThread &thread);
 

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

@@ -85,7 +85,8 @@ static lldb::SBAddress GetDisassembleStartAddress(lldb::SBTarget target,
 }
 
 static DisassembledInstruction ConvertSBInstructionToDisassembledInstruction(
-    lldb::SBTarget &target, lldb::SBInstruction &inst, bool resolve_symbols) {
+    DAP &dap, lldb::SBInstruction &inst, bool resolve_symbols) {
+  lldb::SBTarget target = dap.target;
   if (!inst.IsValid())
     return GetInvalidInstruction();
 
@@ -138,14 +139,17 @@ static DisassembledInstruction ConvertSBInstructionToDisassembledInstruction(
     si << " ; " << c;
   }
 
-  protocol::Source source = CreateSource(addr, target);
+  std::optional<protocol::Source> source =
+      CreateSource(addr, target, [&dap](lldb::addr_t addr) {
+        return dap.CreateSourceReference(addr);
+      });
   lldb::SBLineEntry line_entry = GetLineEntryForAddress(target, addr);
 
   // If the line number is 0 then the entry represents a compiler generated
   // location.
-  if (!IsAssemblySource(source) && line_entry.GetStartAddress() == addr &&
-      line_entry.IsValid() && line_entry.GetFileSpec().IsValid() &&
-      line_entry.GetLine() != 0) {
+  if (source && !IsAssemblySource(*source) &&
+      line_entry.GetStartAddress() == addr && line_entry.IsValid() &&
+      line_entry.GetFileSpec().IsValid() && line_entry.GetLine() != 0) {
 
     disassembled_inst.location = std::move(source);
     const auto line = line_entry.GetLine();
@@ -221,7 +225,7 @@ DisassembleRequestHandler::Run(const DisassembleArguments &args) const {
       original_address_index = i;
 
     instructions.push_back(ConvertSBInstructionToDisassembledInstruction(
-        dap.target, inst, resolve_symbols));
+        dap, inst, resolve_symbols));
   }
 
   // Check if we miss instructions at the beginning.

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

@@ -137,7 +137,16 @@ void LocationsRequestHandler::operator()(
       return;
     }
 
-    body.try_emplace("source", CreateSource(line_entry.GetFileSpec()));
+    const std::optional<protocol::Source> source =
+        CreateSource(line_entry.GetFileSpec());
+    if (!source) {
+      response["success"] = false;
+      response["message"] = "Failed to resolve file path for location";
+      dap.SendJSON(llvm::json::Value(std::move(response)));
+      return;
+    }
+
+    body.try_emplace("source", *source);
     if (int line = line_entry.GetLine())
       body.try_emplace("line", line);
     if (int column = line_entry.GetColumn())
@@ -152,7 +161,16 @@ void LocationsRequestHandler::operator()(
       return;
     }
 
-    body.try_emplace("source", CreateSource(decl.GetFileSpec()));
+    const std::optional<protocol::Source> source =
+        CreateSource(decl.GetFileSpec());
+    if (!source) {
+      response["success"] = false;
+      response["message"] = "Failed to resolve file path for location";
+      dap.SendJSON(llvm::json::Value(std::move(response)));
+      return;
+    }
+
+    body.try_emplace("source", *source);
     if (int line = decl.GetLine())
       body.try_emplace("line", line);
     if (int column = decl.GetColumn())

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

@@ -29,34 +29,42 @@ namespace lldb_dap {
 /// the source code for a given source reference.
 llvm::Expected<protocol::SourceResponseBody>
 SourceRequestHandler::Run(const protocol::SourceArguments &args) const {
-  const auto source =
+
+  uint32_t source_ref =
       args.source->sourceReference.value_or(args.sourceReference);
+  const std::optional<lldb::addr_t> source_addr_opt =
+      dap.GetSourceReferenceAddress(source_ref);
 
-  if (!source)
+  if (!source_addr_opt)
     return llvm::make_error<DAPError>(
-        "invalid arguments, expected source.sourceReference to be set");
+        llvm::formatv("Unknown source reference {}", source_ref));
 
-  lldb::SBAddress address(source, dap.target);
+  lldb::SBAddress address(*source_addr_opt, dap.target);
   if (!address.IsValid())
     return llvm::make_error<DAPError>("source not found");
 
   lldb::SBSymbol symbol = address.GetSymbol();
-
-  lldb::SBStream stream;
-  lldb::SBExecutionContext exe_ctx(dap.target);
+  lldb::SBInstructionList insts;
 
   if (symbol.IsValid()) {
-    lldb::SBInstructionList insts = symbol.GetInstructions(dap.target);
-    insts.GetDescription(stream, exe_ctx);
+    insts = symbol.GetInstructions(dap.target);
   } else {
     // No valid symbol, just return the disassembly.
-    lldb::SBInstructionList insts = dap.target.ReadInstructions(
+    insts = dap.target.ReadInstructions(
         address, dap.k_number_of_assembly_lines_for_nodebug);
-    insts.GetDescription(stream, exe_ctx);
   }
 
+  if (!insts || insts.GetSize() == 0)
+    return llvm::make_error<DAPError>(
+        llvm::formatv("no instruction source for address {}",
+                      address.GetLoadAddress(dap.target)));
+
+  lldb::SBStream stream;
+  lldb::SBExecutionContext exe_ctx(dap.target);
+  insts.GetDescription(stream, exe_ctx);
   return protocol::SourceResponseBody{/*content=*/stream.GetData(),
-                                      /*mimeType=*/"text/x-lldb.disassembly"};
+                                      /*mimeType=*/
+                                      "text/x-lldb.disassembly"};
 }
 
 } // namespace lldb_dap

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

@@ -69,7 +69,7 @@ static bool FillStackFrames(DAP &dap, lldb::SBThread &thread,
       break;
     }
 
-    stack_frames.emplace_back(CreateStackFrame(frame, frame_format));
+    stack_frames.emplace_back(CreateStackFrame(dap, frame, frame_format));
   }
 
   if (include_all && reached_end_of_stack) {

lldb/tools/lldb-dap/JSONUtils.cpp

@@ -552,7 +552,7 @@ CreateExceptionBreakpointFilter(const ExceptionBreakpoint &bp) {
 //   },
 //   "required": [ "id", "name", "line", "column" ]
 // }
-llvm::json::Value CreateStackFrame(lldb::SBFrame &frame,
+llvm::json::Value CreateStackFrame(DAP &dap, lldb::SBFrame &frame,
                                    lldb::SBFormat &format) {
   llvm::json::Object object;
   int64_t frame_id = MakeDAPFrameID(frame);
@@ -582,8 +582,11 @@ llvm::json::Value CreateStackFrame(lldb::SBFrame &frame,
   EmplaceSafeString(object, "name", frame_name);
 
   auto target = frame.GetThread().GetProcess().GetTarget();
-  auto source = CreateSource(frame.GetPCAddress(), target);
-  if (!IsAssemblySource(source)) {
+  std::optional<protocol::Source> source =
+      CreateSource(frame.GetPCAddress(), target, [&dap](lldb::addr_t addr) {
+        return dap.CreateSourceReference(addr);
+      });
+  if (source && !IsAssemblySource(*source)) {
     // This is a normal source with a valid line entry.
     auto line_entry = frame.GetLineEntry();
     object.try_emplace("line", line_entry.GetLine());
@@ -607,7 +610,8 @@ llvm::json::Value CreateStackFrame(lldb::SBFrame &frame,
     object.try_emplace("column", 1);
   }
 
-  object.try_emplace("source", std::move(source));
+  if (source)
+    object.try_emplace("source", std::move(source).value());
 
   const auto pc = frame.GetPC();
   if (pc != LLDB_INVALID_ADDRESS) {

lldb/tools/lldb-dap/JSONUtils.h

@@ -246,6 +246,9 @@ CreateExceptionBreakpointFilter(const ExceptionBreakpoint &bp);
 ///   "line" - the source file line number as an integer
 ///   "column" - the source file column number as an integer
 ///
+/// \param[in] dap
+///     The DAP session associated with the stopped thread.
+///
 /// \param[in] frame
 ///     The LLDB stack frame to use when populating out the "StackFrame"
 ///     object.
@@ -257,7 +260,7 @@ CreateExceptionBreakpointFilter(const ExceptionBreakpoint &bp);
 /// \return
 ///     A "StackFrame" JSON object with that follows the formal JSON
 ///     definition outlined by Microsoft.
-llvm::json::Value CreateStackFrame(lldb::SBFrame &frame,
+llvm::json::Value CreateStackFrame(DAP &dap, lldb::SBFrame &frame,
                                    lldb::SBFormat &format);
 
 /// Create a "StackFrame" label object for a LLDB thread.

lldb/tools/lldb-dap/Protocol/ProtocolRequests.h

@@ -468,7 +468,7 @@ struct SourceArguments {
   /// The reference to the source. This is the same as `source.sourceReference`.
   /// This is provided for backward compatibility since old clients do not
   /// understand the `source` attribute.
-  int64_t sourceReference;
+  int64_t sourceReference = LLDB_DAP_INVALID_SRC_REF;
 };
 bool fromJSON(const llvm::json::Value &, SourceArguments &, llvm::json::Path);
 

lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp

@@ -65,7 +65,7 @@ llvm::json::Value toJSON(const Source &S) {
     result.insert({"name", *S.name});
   if (S.path)
     result.insert({"path", *S.path});
-  if (S.sourceReference)
+  if (S.sourceReference && (*S.sourceReference > LLDB_DAP_INVALID_SRC_REF))
     result.insert({"sourceReference", *S.sourceReference});
   if (S.presentationHint)
     result.insert({"presentationHint", *S.presentationHint});

lldb/tools/lldb-dap/Protocol/ProtocolTypes.h

@@ -28,6 +28,7 @@
 #include <string>
 
 #define LLDB_DAP_INVALID_VARRERF UINT64_MAX
+#define LLDB_DAP_INVALID_SRC_REF 0
 
 namespace lldb_dap::protocol {
 
@@ -309,7 +310,7 @@ struct Source {
   /// `source` request (even if a path is specified). Since a `sourceReference`
   /// is only valid for a session, it can not be used to persist a source. The
   /// value should be less than or equal to 2147483647 (2^31-1).
-  std::optional<int64_t> sourceReference;
+  std::optional<int32_t> sourceReference;
 
   /// A hint for how to present the source in the UI. A value of `deemphasize`
   /// can be used to indicate that the source is not available or that it is