wallace created this revision.
wallace added reviewers: clayborg, kusmour, labath, aadsm.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
wallace updated this revision to Diff 253283.
wallace added a comment.

cleanup


When using source maps for a breakpoint, in order to find the actual source 
that breakpoint has resolved, we
need to use a query similar to what 
CommandObjectSource::DumpLinesInSymbolContexts does, which is the logic
used by the CLI to display the source line at a given breakpoint. That's 
necessary because from a breakpoint
location you only have an address, which points to the original source 
location, not the source mapped one.

in the setBreakpoints request handler, we haven't been doing such query and we 
were returning the original
source location, which was breaking the UX of VSCode, as many breakpoints were 
being set but not displayed
in the source file next to each line. Besides, clicking the source path of a 
breakpoint in the breakpoints
view in the debug tab was not working for this case, as VSCode was trying to 
open a non-existent file, thus
showing an error to the user.

Ideally, we should do the query mentioned above to find the actual source 
location to respond to the IDE,
however, that query is expensive and users can have an arbitrary number of 
breakpoints set. As a simpler fix,
the request sent by VSCode already contains the full source path, which exists 
because the user set it from
the IDE itself, so we can simply reuse it instead of querying from the SB API.

I wrote a test for this, and found out that I had to move 
SetSourceMapFromArguments after RunInitCommands in
lldb-vscode.cpp, because an init command used in all tests is `settings clear 
-all`, which would clear the
source map unless specified after initCommands. And in any case, I think it 
makes sense to have initCommands
run before anything the debugger would do.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76968

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
  lldb/tools/lldb-vscode/JSONUtils.cpp
  lldb/tools/lldb-vscode/JSONUtils.h
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===================================================================
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1364,15 +1364,17 @@
     llvm::sys::fs::set_current_path(debuggerRoot.data());
   }
 
-  SetSourceMapFromArguments(*arguments);
-
   // Run any initialize LLDB commands the user specified in the launch.json.
   // This is run before target is created, so commands can't do anything with
   // the targets - preRunCommands are run with the target.
   g_vsc.RunInitCommands();
 
+  SetSourceMapFromArguments(*arguments);
+
   lldb::SBError status;
+
   g_vsc.SetTarget(g_vsc.CreateTargetFromArguments(*arguments, status));
+
   if (status.Fail()) {
     response["success"] = llvm::json::Value(false);
     EmplaceSafeString(response, "message", status.GetCString());
@@ -1748,13 +1750,15 @@
         const auto &existing_bp = existing_source_bps->second.find(src_bp.line);
         if (existing_bp != existing_source_bps->second.end()) {
           existing_bp->second.UpdateBreakpoint(src_bp);
-          AppendBreakpoint(existing_bp->second.bp, response_breakpoints);
+          AppendSourceBreakpoint(existing_bp->second.bp, path, src_bp.line,
+                                 response_breakpoints);
           continue;
         }
       }
       // At this point the breakpoint is new
       src_bp.SetBreakpoint(path.data());
-      AppendBreakpoint(src_bp.bp, response_breakpoints);
+      AppendSourceBreakpoint(src_bp.bp, path, src_bp.line,
+                             response_breakpoints);
       g_vsc.source_breakpoints[path][src_bp.line] = std::move(src_bp);
     }
   }
Index: lldb/tools/lldb-vscode/JSONUtils.h
===================================================================
--- lldb/tools/lldb-vscode/JSONUtils.h
+++ lldb/tools/lldb-vscode/JSONUtils.h
@@ -196,6 +196,26 @@
 ///     appended to it.
 void AppendBreakpoint(lldb::SBBreakpoint &bp, llvm::json::Array &breakpoints);
 
+/// Converts \a bp to a JSON value using the provided source line information to
+/// the \a breakpoints array.
+///
+/// \param[in] bp
+///     A LLDB breakpoint object which will provide basic information as ID and
+///     resolved status.
+///
+/// \param[in] sourcePath
+///     The full path to the source to store in the JSON value.
+///
+/// \param[in] line
+///     The line to store in the JSON value.
+///
+/// \param[in] breakpoints
+///     A JSON array that will get a llvm::json::Value for \a bp
+///     appended to it.
+void AppendSourceBreakpoint(lldb::SBBreakpoint &bp,
+                            const llvm::StringRef &sourcePath, int line,
+                            llvm::json::Array &breakpoints);
+
 /// Converts breakpoint location to a Visual Studio Code "Breakpoint"
 /// JSON object and appends it to the \a breakpoints array.
 ///
Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===================================================================
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -12,6 +12,7 @@
 
 #include "lldb/API/SBBreakpoint.h"
 #include "lldb/API/SBBreakpointLocation.h"
+#include "lldb/API/SBFileSpec.h"
 #include "lldb/API/SBValue.h"
 #include "lldb/Host/PosixApi.h"
 
@@ -319,10 +320,37 @@
   return llvm::json::Value(std::move(object));
 }
 
+llvm::json::Value CreateSourceBreakpoint(lldb::SBBreakpoint &bp,
+                                         const llvm::StringRef &sourcePath,
+                                         int line) {
+  llvm::json::Object object;
+  if (!bp.IsValid())
+    return llvm::json::Value(std::move(object));
+
+  object.try_emplace("verified", bp.GetNumResolvedLocations() > 0);
+  object.try_emplace("id", bp.GetID());
+
+  object.try_emplace("line", line);
+
+  llvm::json::Object source;
+  lldb::SBFileSpec file(sourcePath.str().c_str());
+  const char *name = file.GetFilename();
+  EmplaceSafeString(source, "name", name);
+  EmplaceSafeString(source, "path", sourcePath);
+  object.try_emplace("source", llvm::json::Value(std::move(source)));
+  return llvm::json::Value(std::move(object));
+}
+
 void AppendBreakpoint(lldb::SBBreakpoint &bp, llvm::json::Array &breakpoints) {
   breakpoints.emplace_back(CreateBreakpoint(bp));
 }
 
+void AppendSourceBreakpoint(lldb::SBBreakpoint &bp,
+                            const llvm::StringRef &sourcePath, int line,
+                            llvm::json::Array &breakpoints) {
+  breakpoints.emplace_back(CreateSourceBreakpoint(bp, sourcePath, line));
+}
+
 // "Event": {
 //   "allOf": [ { "$ref": "#/definitions/ProtocolMessage" }, {
 //     "type": "object",
Index: lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
===================================================================
--- lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
+++ lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
@@ -5,6 +5,7 @@
 
 import unittest2
 import vscode
+import shutil
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
@@ -18,6 +19,45 @@
 
     @skipIfWindows
     @skipIfRemote
+    def test_source_map(self):
+        self.build_and_create_debug_adaptor()
+
+        source_basename = 'main.cpp'
+        source_path = os.path.join(os.getcwd(), source_basename)
+
+        new_source_folder = os.path.join(os.path.dirname(os.getcwd()), 'moved_location')
+        new_source_path = os.path.join(new_source_folder, source_basename)
+
+        def cleanup():
+            shutil.move(new_source_path, source_path)
+            os.rmdir(new_source_folder)
+
+        self.addTearDownHook(cleanup)
+
+        lines = [line_number('main.cpp', 'break 12')]
+
+        os.mkdir(new_source_folder)
+        shutil.move(source_path, new_source_path)
+
+        program = self.getBuildArtifact("a.out")
+        self.launch(program, sourceMap=[[os.getcwd(), new_source_folder]])
+
+        response = self.vscode.request_setBreakpoints(new_source_path, lines)
+        if response:
+            breakpoints = response['body']['breakpoints']
+            self.assertEquals(len(breakpoints), len(lines),
+                            "expect %u source breakpoints" % (len(lines)))
+            for (breakpoint, index) in zip(breakpoints, range(len(lines))):
+                line = breakpoint['line']
+                self.assertTrue(line, lines[index])
+                self.assertTrue(line in lines, "line expected in lines array")
+                self.assertTrue(breakpoint['verified'],
+                                "expect breakpoint verified")
+                self.assertEqual('main.cpp', breakpoint['source']['name'])
+                self.assertEqual(new_source_path, breakpoint['source']['path'])
+
+    @skipIfWindows
+    @skipIfRemote
     def test_set_and_clear(self):
         '''Tests setting and clearing source file and line breakpoints.
            This packet is a bit tricky on the debug adaptor side since there
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -570,7 +570,7 @@
                        disableSTDIO=False, shellExpandArguments=False,
                        trace=False, initCommands=None, preRunCommands=None,
                        stopCommands=None, exitCommands=None, sourcePath=None,
-                       debuggerRoot=None, launchCommands=None):
+                       debuggerRoot=None, launchCommands=None, sourceMap=None):
         args_dict = {
             'program': program
         }
@@ -605,6 +605,8 @@
             args_dict['debuggerRoot'] = debuggerRoot
         if launchCommands:
             args_dict['launchCommands'] = launchCommands
+        if sourceMap:
+            args_dict['sourceMap'] = sourceMap
         command_dict = {
             'command': 'launch',
             'type': 'request',
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
@@ -266,8 +266,8 @@
                stopOnEntry=False, disableASLR=True,
                disableSTDIO=False, shellExpandArguments=False,
                trace=False, initCommands=None, preRunCommands=None,
-               stopCommands=None, exitCommands=None,sourcePath= None,
-               debuggerRoot=None, launchCommands=None):
+               stopCommands=None, exitCommands=None,sourcePath=None,
+               debuggerRoot=None, launchCommands=None, sourceMap=None):
         '''Sending launch request to vscode
         '''
 
@@ -298,7 +298,8 @@
             exitCommands=exitCommands,
             sourcePath=sourcePath,
             debuggerRoot=debuggerRoot,
-            launchCommands=launchCommands)
+            launchCommands=launchCommands,
+            sourceMap=sourceMap)
         if not (response and response['success']):
             self.assertTrue(response['success'],
                             'launch failed (%s)' % (response['message']))
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to