[lldb-dap] Reuse source object logics (#141426)

Refactor code revolving source objects such that most logics will be
reused.

The main change is to expose a single `CreateSource(addr, target)` that
can return either a normal or an assembly source object, and call
`ShouldDisplayAssemblySource()` only from this function instead of
multiple places across the code.

Other functions can use `source.IsAssemblySource()` in order to check
which type the source is.

Author: eronnen

lldb/source/API/SBTarget.cpp

@@ -636,14 +636,17 @@ SBTarget::ResolveSymbolContextForAddress(const SBAddress &addr,
                                          uint32_t resolve_scope) {
   LLDB_INSTRUMENT_VA(this, addr, resolve_scope);
 
-  SBSymbolContext sc;
+  SBSymbolContext sb_sc;
   SymbolContextItem scope = static_cast<SymbolContextItem>(resolve_scope);
   if (addr.IsValid()) {
-    if (TargetSP target_sp = GetSP())
+    if (TargetSP target_sp = GetSP()) {
+      lldb_private::SymbolContext &sc = sb_sc.ref();
+      sc.target_sp = target_sp;
       target_sp->GetImages().ResolveSymbolContextForAddress(addr.ref(), scope,
-                                                            sc.ref());
+                                                            sc);
+    }
   }
-  return sc;
+  return sb_sc;
 }
 
 size_t SBTarget::ReadMemory(const SBAddress addr, void *buf, size_t size,

lldb/source/Core/Module.cpp

@@ -483,6 +483,9 @@ uint32_t Module::ResolveSymbolContextForAddress(
       symfile->SetLoadDebugInfoEnabled();
       resolved_flags |=
           symfile->ResolveSymbolContext(so_addr, resolve_scope, sc);
+
+      if ((resolve_scope & eSymbolContextLineEntry) && sc.line_entry.IsValid())
+        sc.line_entry.ApplyFileMappings(sc.target_sp);
     }
 
     // Resolve the symbol if requested, but don't re-look it up if we've

lldb/tools/lldb-dap/Breakpoint.cpp

@@ -8,13 +8,11 @@
 
 #include "Breakpoint.h"
 #include "DAP.h"
-#include "JSONUtils.h"
-#include "LLDBUtils.h"
+#include "ProtocolUtils.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>
@@ -66,17 +64,15 @@ protocol::Breakpoint Breakpoint::ToProtocolBreakpoint() {
         "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();
-    if (!ShouldDisplayAssemblySource(line_entry, stop_disassembly_display)) {
+    auto source = CreateSource(bp_addr, m_dap.target);
+    if (!IsAssemblySource(source)) {
+      auto line_entry = bp_addr.GetLineEntry();
       const auto line = line_entry.GetLine();
       if (line != LLDB_INVALID_LINE_NUMBER)
         breakpoint.line = line;
       const auto column = line_entry.GetColumn();
       if (column != LLDB_INVALID_COLUMN_NUMBER)
         breakpoint.column = column;
-      breakpoint.source = CreateSource(line_entry);
     } else {
       // Assembly breakpoint.
       auto symbol = bp_addr.GetSymbol();
@@ -86,10 +82,10 @@ protocol::Breakpoint Breakpoint::ToProtocolBreakpoint() {
                 .ReadInstructions(symbol.GetStartAddress(), bp_addr, nullptr)
                 .GetSize() +
             1;
-
-        breakpoint.source = CreateAssemblySource(m_dap.target, bp_addr);
       }
     }
+
+    breakpoint.source = std::move(source);
   }
 
   return breakpoint;

lldb/tools/lldb-dap/CMakeLists.txt

@@ -21,6 +21,7 @@ add_lldb_library(lldbDAP
   LLDBUtils.cpp
   OutputRedirector.cpp
   ProgressEvent.cpp
+  ProtocolUtils.cpp
   RunInTerminal.cpp
   SourceBreakpoint.cpp
   Transport.cpp

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

@@ -9,11 +9,14 @@
 #include "DAP.h"
 #include "EventHelper.h"
 #include "JSONUtils.h"
+#include "LLDBUtils.h"
 #include "Protocol/ProtocolRequests.h"
 #include "Protocol/ProtocolTypes.h"
+#include "ProtocolUtils.h"
 #include "RequestHandler.h"
 #include "lldb/API/SBAddress.h"
 #include "lldb/API/SBInstruction.h"
+#include "lldb/API/SBLineEntry.h"
 #include "lldb/API/SBTarget.h"
 #include "lldb/lldb-types.h"
 #include "llvm/ADT/StringExtras.h"
@@ -139,15 +142,16 @@ static DisassembledInstruction ConvertSBInstructionToDisassembledInstruction(
 
   disassembled_inst.instruction = std::move(instruction);
 
-  auto line_entry = addr.GetLineEntry();
+  protocol::Source source = CreateSource(addr, target);
+  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 (line_entry.GetStartAddress() == addr && line_entry.IsValid() &&
-      line_entry.GetFileSpec().IsValid() && line_entry.GetLine() != 0) {
-    auto source = CreateSource(line_entry);
     disassembled_inst.location = std::move(source);
-
     const auto line = line_entry.GetLine();
     if (line != 0 && line != LLDB_INVALID_LINE_NUMBER)
       disassembled_inst.line = line;
@@ -156,7 +160,8 @@ static DisassembledInstruction ConvertSBInstructionToDisassembledInstruction(
     if (column != 0 && column != LLDB_INVALID_COLUMN_NUMBER)
       disassembled_inst.column = column;
 
-    auto end_line_entry = line_entry.GetEndAddress().GetLineEntry();
+    lldb::SBAddress end_addr = line_entry.GetEndAddress();
+    auto end_line_entry = GetLineEntryForAddress(target, end_addr);
     if (end_line_entry.IsValid() &&
         end_line_entry.GetFileSpec() == line_entry.GetFileSpec()) {
       const auto end_line = end_line_entry.GetLine();

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

@@ -9,8 +9,12 @@
 #include "DAP.h"
 #include "EventHelper.h"
 #include "JSONUtils.h"
+#include "LLDBUtils.h"
+#include "ProtocolUtils.h"
 #include "RequestHandler.h"
+#include "lldb/API/SBAddress.h"
 #include "lldb/API/SBDeclaration.h"
+#include "lldb/API/SBLineEntry.h"
 
 namespace lldb_dap {
 
@@ -122,9 +126,9 @@ void LocationsRequestHandler::operator()(
       return;
     }
 
-    lldb::addr_t addr = variable.GetValueAsAddress();
-    lldb::SBLineEntry line_entry =
-        dap.target.ResolveLoadAddress(addr).GetLineEntry();
+    lldb::addr_t raw_addr = variable.GetValueAsAddress();
+    lldb::SBAddress addr = dap.target.ResolveLoadAddress(raw_addr);
+    lldb::SBLineEntry line_entry = GetLineEntryForAddress(dap.target, addr);
 
     if (!line_entry.IsValid()) {
       response["success"] = false;

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

@@ -9,9 +9,7 @@
 #include "DAP.h"
 #include "EventHelper.h"
 #include "JSONUtils.h"
-#include "LLDBUtils.h"
 #include "RequestHandler.h"
-#include "lldb/lldb-enumerations.h"
 
 namespace lldb_dap {
 
@@ -55,8 +53,6 @@ static bool FillStackFrames(DAP &dap, lldb::SBThread &thread,
                             llvm::json::Array &stack_frames, int64_t &offset,
                             const int64_t start_frame, const int64_t levels,
                             const bool include_all) {
-  lldb::StopDisassemblyType stop_disassembly_display =
-      GetStopDisassemblyDisplay(dap.debugger);
   bool reached_end_of_stack = false;
   for (int64_t i = start_frame;
        static_cast<int64_t>(stack_frames.size()) < levels; i++) {
@@ -73,8 +69,7 @@ static bool FillStackFrames(DAP &dap, lldb::SBThread &thread,
       break;
     }
 
-    stack_frames.emplace_back(
-        CreateStackFrame(frame, frame_format, stop_disassembly_display));
+    stack_frames.emplace_back(CreateStackFrame(frame, frame_format));
   }
 
   if (include_all && reached_end_of_stack) {

lldb/tools/lldb-dap/JSONUtils.cpp

@@ -10,6 +10,7 @@
 #include "DAP.h"
 #include "ExceptionBreakpoint.h"
 #include "LLDBUtils.h"
+#include "ProtocolUtils.h"
 #include "lldb/API/SBAddress.h"
 #include "lldb/API/SBCompileUnit.h"
 #include "lldb/API/SBDeclaration.h"
@@ -490,98 +491,6 @@ CreateExceptionBreakpointFilter(const ExceptionBreakpoint &bp) {
   return filter;
 }
 
-static std::string GetLoadAddressString(const lldb::addr_t addr) {
-  std::string result;
-  llvm::raw_string_ostream os(result);
-  os << llvm::format_hex(addr, 18);
-  return result;
-}
-
-protocol::Source CreateSource(const lldb::SBFileSpec &file) {
-  protocol::Source source;
-  if (file.IsValid()) {
-    const char *name = file.GetFilename();
-    if (name)
-      source.name = name;
-    char path[PATH_MAX] = "";
-    if (file.GetPath(path, sizeof(path)) &&
-        lldb::SBFileSpec::ResolvePath(path, path, PATH_MAX))
-      source.path = path;
-  }
-  return source;
-}
-
-protocol::Source CreateSource(const lldb::SBLineEntry &line_entry) {
-  return CreateSource(line_entry.GetFileSpec());
-}
-
-protocol::Source CreateSource(llvm::StringRef source_path) {
-  protocol::Source source;
-  llvm::StringRef name = llvm::sys::path::filename(source_path);
-  source.name = name;
-  source.path = source_path;
-  return source;
-}
-
-protocol::Source CreateAssemblySource(const lldb::SBTarget &target,
-                                      lldb::SBAddress &address) {
-  protocol::Source source;
-
-  auto symbol = address.GetSymbol();
-  std::string name;
-  if (symbol.IsValid()) {
-    source.sourceReference = symbol.GetStartAddress().GetLoadAddress(target);
-    name = symbol.GetName();
-  } else {
-    const auto load_addr = address.GetLoadAddress(target);
-    source.sourceReference = load_addr;
-    name = GetLoadAddressString(load_addr);
-  }
-
-  lldb::SBModule module = address.GetModule();
-  if (module.IsValid()) {
-    lldb::SBFileSpec file_spec = module.GetFileSpec();
-    if (file_spec.IsValid()) {
-      std::string path = GetSBFileSpecPath(file_spec);
-      if (!path.empty())
-        source.path = path + '`' + name;
-    }
-  }
-
-  source.name = std::move(name);
-
-  // Mark the source as deemphasized since users will only be able to view
-  // assembly for these frames.
-  source.presentationHint =
-      protocol::Source::PresentationHint::eSourcePresentationHintDeemphasize;
-
-  return source;
-}
-
-bool ShouldDisplayAssemblySource(
-    const lldb::SBLineEntry &line_entry,
-    lldb::StopDisassemblyType stop_disassembly_display) {
-  if (stop_disassembly_display == lldb::eStopDisassemblyTypeNever)
-    return false;
-
-  if (stop_disassembly_display == lldb::eStopDisassemblyTypeAlways)
-    return true;
-
-  // A line entry of 0 indicates the line is compiler generated i.e. no source
-  // file is associated with the frame.
-  auto file_spec = line_entry.GetFileSpec();
-  if (!file_spec.IsValid() || line_entry.GetLine() == 0 ||
-      line_entry.GetLine() == LLDB_INVALID_LINE_NUMBER)
-    return true;
-
-  if (stop_disassembly_display == lldb::eStopDisassemblyTypeNoSource &&
-      !file_spec.Exists()) {
-    return true;
-  }
-
-  return false;
-}
-
 // "StackFrame": {
 //   "type": "object",
 //   "description": "A Stackframe contains the source location.",
@@ -643,9 +552,8 @@ bool ShouldDisplayAssemblySource(
 //   },
 //   "required": [ "id", "name", "line", "column" ]
 // }
-llvm::json::Value
-CreateStackFrame(lldb::SBFrame &frame, lldb::SBFormat &format,
-                 lldb::StopDisassemblyType stop_disassembly_display) {
+llvm::json::Value CreateStackFrame(lldb::SBFrame &frame,
+                                   lldb::SBFormat &format) {
   llvm::json::Object object;
   int64_t frame_id = MakeDAPFrameID(frame);
   object.try_emplace("id", frame_id);
@@ -673,22 +581,18 @@ CreateStackFrame(lldb::SBFrame &frame, lldb::SBFormat &format,
 
   EmplaceSafeString(object, "name", frame_name);
 
-  auto line_entry = frame.GetLineEntry();
-  if (!ShouldDisplayAssemblySource(line_entry, stop_disassembly_display)) {
-    object.try_emplace("source", CreateSource(line_entry));
+  auto target = frame.GetThread().GetProcess().GetTarget();
+  auto source = CreateSource(frame.GetPCAddress(), target);
+  if (!IsAssemblySource(source)) {
+    // This is a normal source with a valid line entry.
+    auto line_entry = frame.GetLineEntry();
     object.try_emplace("line", line_entry.GetLine());
     auto column = line_entry.GetColumn();
     object.try_emplace("column", column);
   } else if (frame.GetSymbol().IsValid()) {
-    // If no source is associated with the frame, use the DAPFrameID to track
-    // the 'source' and generate assembly.
-    auto frame_address = frame.GetPCAddress();
-    object.try_emplace("source", CreateAssemblySource(
-                                     frame.GetThread().GetProcess().GetTarget(),
-                                     frame_address));
-
-    // Calculate the line of the current PC from the start of the current
-    // symbol.
+    // 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);
@@ -699,14 +603,12 @@ CreateStackFrame(lldb::SBFrame &frame, lldb::SBFormat &format,
     object.try_emplace("column", 1);
   } else {
     // No valid line entry or symbol.
-    auto frame_address = frame.GetPCAddress();
-    object.try_emplace("source", CreateAssemblySource(
-                                     frame.GetThread().GetProcess().GetTarget(),
-                                     frame_address));
     object.try_emplace("line", 1);
     object.try_emplace("column", 1);
   }
 
+  object.try_emplace("source", std::move(source));
+
   const auto pc = frame.GetPC();
   if (pc != LLDB_INVALID_ADDRESS) {
     std::string formatted_addr = "0x" + llvm::utohexstr(pc);