[lldb-dap] add review changes.

Author: da-viper

lldb/tools/lldb-dap/Breakpoint.cpp

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

lldb/tools/lldb-dap/DAP.cpp

@@ -17,6 +17,7 @@
 #include "Protocol/ProtocolBase.h"
 #include "Protocol/ProtocolRequests.h"
 #include "Protocol/ProtocolTypes.h"
+#include "ProtocolUtils.h"
 #include "Transport.h"
 #include "lldb/API/SBBreakpoint.h"
 #include "lldb/API/SBCommandInterpreter.h"
@@ -511,22 +512,24 @@ DAP::SendFormattedOutput(OutputType o, const char *format, ...) {
 }
 
 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;
+  std::lock_guard<std::mutex> guard(m_source_references_mutex);
+  auto iter = llvm::find(m_source_references, address);
+  if (iter != m_source_references.end())
+    return std::distance(m_source_references.begin(), iter) + 1;
 
-  source_references.emplace_back(address);
-  return static_cast<int32_t>(source_references.size());
+  m_source_references.emplace_back(address);
+  return static_cast<int32_t>(m_source_references.size());
 }
 
 std::optional<lldb::addr_t> DAP::GetSourceReferenceAddress(int32_t reference) {
+  std::lock_guard<std::mutex> guard(m_source_references_mutex);
   if (reference <= LLDB_DAP_INVALID_SRC_REF)
     return std::nullopt;
 
-  if (static_cast<size_t>(reference) > source_references.size())
+  if (static_cast<size_t>(reference) > m_source_references.size())
     return std::nullopt;
 
-  return source_references[reference - 1];
+  return m_source_references[reference - 1];
 }
 
 ExceptionBreakpoint *DAP::GetExceptionBPFromStopReason(lldb::SBThread &thread) {
@@ -634,6 +637,22 @@ ReplMode DAP::DetectReplMode(lldb::SBFrame frame, std::string &expression,
   llvm_unreachable("enum cases exhausted.");
 }
 
+std::optional<protocol::Source> DAP::ResolveSource(lldb::SBAddress address) {
+
+  if (DisplayAssemblySource(debugger, address)) {
+    auto create_reference = [this](lldb::addr_t addr) {
+      return CreateSourceReference(addr);
+    };
+    return CreateAssemblySource(target, address, create_reference);
+  }
+
+  lldb::SBLineEntry line_entry = GetLineEntryForAddress(target, address);
+  if (!line_entry.IsValid())
+    return std::nullopt;
+
+  return CreateSource(line_entry.GetFileSpec());
+}
+
 bool DAP::RunLLDBCommands(llvm::StringRef prefix,
                           llvm::ArrayRef<std::string> commands) {
   bool required_command_failed = false;

lldb/tools/lldb-dap/DAP.h

@@ -105,9 +105,6 @@ 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;
@@ -257,6 +254,17 @@ struct DAP {
   ReplMode DetectReplMode(lldb::SBFrame frame, std::string &expression,
                           bool partial_expression);
 
+  /// Create a "Source" JSON object as described in the debug adapter
+  /// definition.
+  ///
+  /// \param[in] address
+  ///     The address to use when populating out the "Source" object.
+  ///
+  /// \return
+  ///     An optional "Source" JSON object that follows the formal JSON
+  ///     definition outlined by Microsoft.
+  std::optional<protocol::Source> ResolveSource(lldb::SBAddress address);
+
   /// \return
   ///   \b false if a fatal error was found while executing these commands,
   ///   according to the rules of \a LLDBUtils::RunLLDBCommands.
@@ -411,6 +419,10 @@ struct DAP {
   std::thread progress_event_thread;
   /// @}
 
+  /// list of addresses mapped by sourceReference(index - 1)
+  std::vector<lldb::addr_t> m_source_references;
+  std::mutex m_source_references_mutex;
+
   /// Queue for all incoming messages.
   std::deque<protocol::Message> m_queue;
   std::mutex m_queue_mutex;

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

@@ -139,10 +139,7 @@ static DisassembledInstruction ConvertSBInstructionToDisassembledInstruction(
     si << " ; " << c;
   }
 
-  std::optional<protocol::Source> source =
-      CreateSource(addr, target, [&dap](lldb::addr_t addr) {
-        return dap.CreateSourceReference(addr);
-      });
+  std::optional<protocol::Source> source = dap.ResolveSource(addr);
   lldb::SBLineEntry line_entry = GetLineEntryForAddress(target, addr);
 
   // If the line number is 0 then the entry represents a compiler generated

lldb/tools/lldb-dap/JSONUtils.cpp

@@ -583,9 +583,8 @@ llvm::json::Value CreateStackFrame(DAP &dap, lldb::SBFrame &frame,
 
   auto target = frame.GetThread().GetProcess().GetTarget();
   std::optional<protocol::Source> source =
-      CreateSource(frame.GetPCAddress(), target, [&dap](lldb::addr_t addr) {
-        return dap.CreateSourceReference(addr);
-      });
+      dap.ResolveSource(frame.GetPCAddress());
+
   if (source && !IsAssemblySource(*source)) {
     // This is a normal source with a valid line entry.
     auto line_entry = frame.GetLineEntry();

lldb/tools/lldb-dap/ProtocolUtils.cpp

@@ -46,9 +46,9 @@ static bool ShouldDisplayAssemblySource(
   return false;
 }
 
-static std::optional<protocol::Source> CreateAssemblySource(
+std::optional<protocol::Source> CreateAssemblySource(
     const lldb::SBTarget &target, lldb::SBAddress address,
-    llvm::function_ref<uint32_t(lldb::addr_t)> create_reference) {
+    llvm::function_ref<int32_t(lldb::addr_t)> create_reference) {
 
   lldb::SBSymbol symbol = address.GetSymbol();
   lldb::addr_t load_addr = LLDB_INVALID_ADDRESS;
@@ -101,22 +101,6 @@ std::optional<protocol::Source> CreateSource(const lldb::SBFileSpec &file) {
   return source;
 }
 
-std::optional<protocol::Source>
-CreateSource(lldb::SBAddress address, lldb::SBTarget &target,
-             llvm::function_ref<int32_t(lldb::addr_t)> create_reference) {
-  lldb::SBDebugger debugger = target.GetDebugger();
-  lldb::StopDisassemblyType stop_disassembly_display =
-      GetStopDisassemblyDisplay(debugger);
-  if (ShouldDisplayAssemblySource(address, stop_disassembly_display))
-    return CreateAssemblySource(target, address, create_reference);
-
-  lldb::SBLineEntry line_entry = GetLineEntryForAddress(target, address);
-  if (!line_entry.IsValid())
-    return std::nullopt;
-
-  return CreateSource(line_entry.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
@@ -125,6 +109,13 @@ bool IsAssemblySource(const protocol::Source &source) {
          LLDB_DAP_INVALID_SRC_REF;
 }
 
+bool DisplayAssemblySource(lldb::SBDebugger &debugger,
+                           lldb::SBAddress address) {
+  const lldb::StopDisassemblyType stop_disassembly_display =
+      GetStopDisassemblyDisplay(debugger);
+  return ShouldDisplayAssemblySource(address, stop_disassembly_display);
+}
+
 std::string GetLoadAddressString(const lldb::addr_t addr) {
   return "0x" + llvm::utohexstr(addr, false, 16);
 }

lldb/tools/lldb-dap/ProtocolUtils.h

@@ -31,25 +31,27 @@ std::optional<protocol::Source> CreateSource(const lldb::SBFileSpec &file);
 
 /// Create a "Source" JSON object as described in the debug adapter definition.
 ///
-/// \param[in] address
-///     The address to use when populating out the "Source" object.
-///
 /// \param[in] target
 ///     The target that has the address.
 ///
+/// \param[in] address
+///     The address to use when populating out the "Source" object.
+///
 /// \param[in] create_reference
 ///     function used to create a source_reference
 ///
 /// \return
 ///     An optional "Source" JSON object that follows the formal JSON
 ///     definition outlined by Microsoft.
-std::optional<protocol::Source>
-CreateSource(lldb::SBAddress address, lldb::SBTarget &target,
-             llvm::function_ref<int32_t(lldb::addr_t)> create_reference);
+std::optional<protocol::Source> CreateAssemblySource(
+    const lldb::SBTarget &target, lldb::SBAddress address,
+    llvm::function_ref<int32_t(lldb::addr_t)> create_reference);
 
 /// Checks if the given source is for assembly code.
 bool IsAssemblySource(const protocol::Source &source);
 
+bool DisplayAssemblySource(lldb::SBDebugger &debugger, lldb::SBAddress address);
+
 /// Get the address as a 16-digit hex string, e.g. "0x0000000000012345"
 std::string GetLoadAddressString(const lldb::addr_t addr);