[lldb-vscode] Correctly return source mapped breakpoints for setBreakpoints request

[wallace]

Summary

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.

[wallace]

cleanup

[labath]

lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py

25–43

Please make sure none of these commands modify the source tree. You can look at test/API/source-manager/Makefile for an example of how to build a binary to reference files in the build tree.

lldb/tools/lldb-vscode/JSONUtils.cpp

325

StringRefs are normally passed by value.

lldb/tools/lldb-vscode/lldb-vscode.cpp

1972

What about breakpoints in dynamically loaded shared libraries? Should you be remembering the original path somewhere so that one you can set it here too?

[wallace]

lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py

25–43

thanks, will do

lldb/tools/lldb-vscode/lldb-vscode.cpp

1972

This case is different, as function breakpoints are set by the IDE without referring to any source path, as they are equivalent to making 'b function_name'. Probably function breakpoints are not working correctly, so I'll have to fix it any of these days using the expensive query I mentioned above.

[wallace]

address comments

[clayborg]

The description confused me a bit as I thought we were going to be doing some "CommandObjectSource::DumpLinesInSymbolContexts()" stuff somewhere. But this path is really just "return the same source path from the setBreakpoints" request in the response and I am all for that. So a few nits and this will be good to go. See my inline comments.

lldb/tools/lldb-vscode/JSONUtils.cpp

336–337

There might be a cheaper llvm::sys::path operation that can extract the basename from a StringRef instead of creating a SBFileSpec?

lldb/tools/lldb-vscode/JSONUtils.h

207

Maybe reword a bit to make sure people understand this is the sourcePath that was pass in the setBreakpoint request?

The path that was specified in the setBreakpoint request.

lldb/tools/lldb-vscode/lldb-vscode.cpp

1375

remove white only change

1376

remove white only change

[wallace]

address comments

[wallace]

nit

[clayborg]

After reviewing more, we should just re-use CreateBreakpoint and add a "llvm::Optional<StringRef> request_path" argument. Then all breakpoints use the same function and we avoid duplicated code. Inline comments should detail what should be done. Let me know if you have questions.

lldb/tools/lldb-vscode/JSONUtils.cpp

286

Read my comments below first, then read this:

we should add an extra llvm::Optional<StringRef> arg to this so we can have all code just use this function:

llvm::json::Value CreateBreakpoint(lldb::SBBreakpoint &bp, llvm::Optional<StringRef> request_path)

If "request_path" has a value, then we use this in the "source" response. If it doesn't we look it up in from the location address.

319

we will need to add "llvm::Optional<StringRef> request_path" as an argument to CreateSource here, so it can use request_path if it has a value, else use the source file in the line_entry object.

324

Actually the _only_ difference between this function and "llvm::json::Value CreateBreakpoint(lldb::SBBreakpoint &bp)" is the fact that we pass along the path along. So maybe we can just add a new variable to CreateBreakpoint above so. See my inline comment on CreateBreakpoint().

333

This should do what it was doing before: grab the source line only from the address. Why? If you set a breakpoint on a line that has no code like:

1 int main(int argc, ...)
2 {                              /// <-- set bp here
3   int foo = arg * 10; /// <-- bp will end up here

So we should still be looking up the line number from the Address of the first resolved location so that the source line does get adjusted in the IDE.

494

add llvm::Optional<StringRef> request_path argument for CreateBreakpoint

[wallace]

lldb/tools/lldb-vscode/JSONUtils.cpp

333

TIL

[wallace]

address comments

[clayborg]

Looks good as long as with we have "llvm::Optional<llvm::StringRef> request_path = {}" in the arguments for a function, that "{}" will turn into llvm::None and not an empty StringRef? Unclear to me. As long as this is what is happening and as long is this coding convention is used elsewhere in llvm (using "{}" instead of "llvm::None") I am ok. Pavel?

lldb/tools/lldb-vscode/JSONUtils.h

204

s/{}/llvm::None/ if {} doesn't construct to llvm::None?

229

s/{}/llvm::None/ if {} doesn't construct to llvm::None

[labath]

In D76968#1951707, @clayborg wrote:

Looks good as long as with we have "llvm::Optional<llvm::StringRef> request_path = {}" in the arguments for a function, that "{}" will turn into llvm::None and not an empty StringRef? Unclear to me. As long as this is what is happening and as long is this coding convention is used elsewhere in llvm (using "{}" instead of "llvm::None") I am ok. Pavel?

The two expressions are equivalent here. We're currently using both styles in lldb. I have a slight preference for the explicit None version, but I don't think it's worth the trouble to standardize on one or the other.

lldb/tools/lldb-vscode/lldb-vscode.cpp

421

The comment was supposed to be here.

1972

Ah, sorry. I misplaced this comment. It was meant to go above to line 417, where the eBreakpointEventTypeLocationsAdded event is handled. That code should be reached for "regular" file+line breakpoints in case a breakpoint gets new locations (e.g.) in response to a shared library being loaded.

[wallace]

Account for dynamically loaded libraries and update the logic for breakpoint events

[wallace]

cleanup

[wallace]

Some included changes:

• Removed the "source" element if the "breakpoint" object returned in breakpoint events
• CreateBreakpoint now accepts an optional line number, which is used as fallback if no location is valid
• Added a test that asserts breakpoints in dynamically loaded libraries

I also tested the entire flow in VSCode and it was all fine

[clayborg]

Just use "llvm::None" instead of "{}" and this is good to go.

lldb/tools/lldb-vscode/JSONUtils.h

229

lets use "llvm::None" here instead of "{}" for clarity

230

ditto

[labath]

Thanks for the updates and for writing the test. The code looks fine to me, the "request changes" is for making sure the test follows best practices and is portable.

lldb/test/API/tools/lldb-vscode/breakpoint/Makefile

12–19

This is not the right way to build binaries -- it will fail on macos for instance.
If we were not dlopening, you should set CXX_SOURCES, and DYLIB_C_SOURCES and let the normal rules do build for you, but for dlopen you need to do something a tad more complicated and invoke a submake with the right arguments you can look at functionalities/load_unload/Makefile for an example of how to do that.

I don't think we currently have a test which uses both shared libraries, and relocated sources files, so it's possible that this may fail for some reason. But if that's the case, then I want to understand what's the problem first (so we can try to fix it).

lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py

32–33

If I follow this correctly this will escape from the build folder for this test (two dirnames). Can you stay within the confines of the build folder?

Among other things, then you won't need to rmtree these things as the build folder is automatically cleaned before each test.

199–201

maybe move these into the setUp function?

lldb/test/API/tools/lldb-vscode/breakpoint/main.cpp

19 ↗

(On Diff #254041)

Looking at the other dlopen tests (e.g., functionalities/load_unload/main.cpp), it looks like you'll need to ifdef the correct library name here.

[wallace]

address comments

[wallace]

Ping ping. This will fix some existing issues reported by users :)

[labath]

Looks good now. Sorry about the delay, I have a lot of things in my queue now and this got lost at the bottom of the stack.

[wallace]

rebase

lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py

def attach(self, program=None, pid=None, waitFor=None, trace=None,
         if not (response and response['success']):
             self.assertTrue(response['success'],
                             'attach failed (%s)' % (response['message']))
 
     def launch(self, program=None, args=None, cwd=None, env=None,
                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
         '''
 
         # Make sure we disconnect and terminate the VSCode debug adapter,
         # if we throw an exception during the test case
         def cleanup():
             self.vscode.request_disconnect(terminateDebuggee=True)
             self.vscode.terminate()
def launch(self, program=None, args=None, cwd=None, env=None,
             shellExpandArguments=shellExpandArguments,
             trace=trace,
             initCommands=initCommands,
             preRunCommands=preRunCommands,
             stopCommands=stopCommands,
             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']))
 
     def build_and_launch(self, program, args=None, cwd=None, env=None,
                          stopOnEntry=False, disableASLR=True,
                          disableSTDIO=False, shellExpandArguments=False,
                          trace=False, initCommands=None, preRunCommands=None,

lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py

def request_initialize(self):
                 self.initialize_body = response['body']
         return response
 
     def request_launch(self, program, args=None, cwd=None, env=None,
                        stopOnEntry=False, disableASLR=True,
                        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
         }
         if args:
             args_dict['args'] = args
         if cwd:
             args_dict['cwd'] = cwd
         if env:
def request_launch(self, program, args=None, cwd=None, env=None,
         if exitCommands:
             args_dict['exitCommands'] = exitCommands
         if sourcePath:
             args_dict['sourcePath'] = sourcePath
         if debuggerRoot:
             args_dict['debuggerRoot'] = debuggerRoot
         if launchCommands:
             args_dict['launchCommands'] = launchCommands
+        if sourceMap:
+            args_dict['sourceMap'] = sourceMap
         command_dict = {
             'command': 'launch',
             'type': 'request',
             'arguments': args_dict
         }
         response = self.send_recv(command_dict)
 
         # Wait for a 'process' and 'initialized' event in any order

lldb/test/API/tools/lldb-vscode/breakpoint/Makefile

-CXX_SOURCES := main.cpp
+CXX_SOURCES := main-copy.cpp
 
 include Makefile.rules
+
+# Copy file into the build folder to enable the test to modify it.
+main-copy.cpp: main.cpp
+  cp -f $< $@

lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py

 """
 Test lldb-vscode setBreakpoints request
 """
 
 
 import unittest2
 import vscode
+import shutil
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 import lldbvscode_testcase
 import os
 
 
 class TestVSCode_setBreakpoints(lldbvscode_testcase.VSCodeTestCaseBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
     @skipIfWindows
     @skipIfRemote
+    def test_source_map(self):
+        self.build_and_create_debug_adaptor()
+
+        source_basename = 'main-copy.cpp'
+        source_path = self.getBuildArtifact(source_basename)
+        source_folder = os.path.dirname(source_path)
+
+        new_source_folder = os.path.join(os.path.dirname(source_folder), '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=[[source_folder, 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(source_basename, 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
            is no "clearBreakpoints" packet. Source file and line breakpoints
            are set by sending a "setBreakpoints" packet with a source file
            specified and zero or more source lines. If breakpoints have been
            set in the source file before, any exising breakpoints must remain
            set, and any new breakpoints must be created, and any breakpoints
            that were in previous requests and are not in the current request
            must be removed. This function tests this setting and clearing
            and makes sure things happen correctly. It doesn't test hitting
            breakpoints and the functionality of each breakpoint, like
            'conditions' and 'hitCondition' settings.'''
-        source_basename = 'main.cpp'
-        source_path = os.path.join(os.getcwd(), source_basename)
+        source_basename = 'main-copy.cpp'
+        source_path = self.getBuildArtifact(source_basename)
         first_line = line_number('main.cpp', 'break 12')
         second_line = line_number('main.cpp', 'break 13')
         third_line = line_number('main.cpp', 'break 14')
         lines = [first_line, third_line, second_line]
 
         # Visual Studio Code Debug Adaptors have no way to specify the file
         # without launching or attaching to a process, so we must start a
         # process in order to be able to set breakpoints.
def test_set_and_clear(self):
                 self.assertTrue(breakpoint['verified'],
                                 "expect breakpoint still verified")
 
     @skipIfWindows
     @skipIfRemote
     def test_functionality(self):
         '''Tests hitting breakpoints and the functionality of a single
            breakpoint, like 'conditions' and 'hitCondition' settings.'''
-        source_basename = 'main.cpp'
-        source_path = os.path.join(os.getcwd(), source_basename)
+        source_basename = 'main-copy.cpp'
+        source_path = self.getBuildArtifact(source_basename)
         loop_line = line_number('main.cpp', '// break loop')
 
         program = self.getBuildArtifact("a.out")
         self.build_and_launch(program)
         # Set a breakpoint at the loop line with no condition and no
         # hitCondition
         breakpoint_ids = self.set_source_breakpoints(source_path, [loop_line])
         self.assertEquals(len(breakpoint_ids), 1, "expect one breakpoint")
         self.vscode.request_continue()

lldb/tools/lldb-vscode/JSONUtils.h

 ///     A LLDB breakpoint object which will get all locations extracted
 ///     and converted into a JSON objects in the \a breakpoints array
 ///
 /// \param[in] breakpoints
 ///     A JSON array that will get a llvm::json::Value for \a bp
 ///     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, 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.
 ///
 /// \param[in] bp
 ///     A LLDB breakpoint object to convert into a JSON value
 ///
 /// \return
 ///     A "Breakpoint" JSON object with that follows the formal JSON
 ///     definition outlined by Microsoft.
 llvm::json::Value CreateBreakpoint(lldb::SBBreakpoint &bp);
 
 /// Create a "Event" JSON object using \a event_name as the event name
 ///
 /// \param[in] event_name
 ///     The string value to use for the "event" key in the JSON object.
 ///
 /// \return
 ///     A "Event" JSON object with that follows the formal JSON
 ///     definition outlined by Microsoft.
 llvm::json::Object CreateEventObject(const llvm::StringRef event_name);
 

lldb/tools/lldb-vscode/JSONUtils.cpp

 //===-- JSONUtils.cpp -------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include <algorithm>
 
 #include "llvm/Support/FormatAdapters.h"
+#include "llvm/Support/Path.h"
 
 #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"
 
 #include "ExceptionBreakpoint.h"
 #include "JSONUtils.h"
 #include "LLDBUtils.h"
 #include "VSCode.h"
 

 //       "type": "integer",
 //       "description": "An optional end column of the actual range covered by
 //                       the breakpoint. If no end line is given, then the end
 //                       column is assumed to be in the start line."
 //     }
 //   },
 //   "required": [ "verified" ]
 // }
 llvm::json::Value CreateBreakpoint(lldb::SBBreakpoint &bp) {
   // Each breakpoint location is treated as a separate breakpoint for VS code.
   // They don't have the notion of a single breakpoint with multiple locations.
   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());
llvm::json::Value CreateBreakpoint(lldb::SBBreakpoint &bp) {
   if (!bp_loc.IsResolved())
     bp_loc = bp.GetLocationAtIndex(0);
   auto bp_addr = bp_loc.GetAddress();
   if (bp_addr.IsValid()) {
     auto line_entry = bp_addr.GetLineEntry();
     const auto line = line_entry.GetLine();
     if (line != UINT32_MAX)
       object.try_emplace("line", line);
     object.try_emplace("source", CreateSource(line_entry));
   }
   return llvm::json::Value(std::move(object));
 }
 
+llvm::json::Value CreateSourceBreakpoint(lldb::SBBreakpoint &bp,
+                                         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;
+  llvm::StringRef name = llvm::sys::path::filename(sourcePath);
+  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, llvm::StringRef sourcePath,
+                            int line, llvm::json::Array &breakpoints) {
+  breakpoints.emplace_back(CreateSourceBreakpoint(bp, sourcePath, line));
+}
+
 // "Event": {
 //   "allOf": [ { "$ref": "#/definitions/ProtocolMessage" }, {
 //     "type": "object",
 //     "description": "Server-initiated event.",
 //     "properties": {
 //       "type": {
 //         "type": "string",
 //         "enum": [ "event" ]

 //       "type": "array",
 //       "items": {
 //         "$ref": "#/definitions/Checksum"
 //       },
 //       "description": "The checksums associated with this file."
 //     }
 //   }
 // }
 llvm::json::Value CreateSource(lldb::SBLineEntry &line_entry) {
   llvm::json::Object object;
   lldb::SBFileSpec file = line_entry.GetFileSpec();
   if (file.IsValid()) {
     const char *name = file.GetFilename();
     if (name)
       EmplaceSafeString(object, "name", name);
     char path[PATH_MAX] = "";
     file.GetPath(path, sizeof(path));

lldb/tools/lldb-vscode/lldb-vscode.cpp

if (listener.WaitForEvent(1, event)) {
           // BreakpointBase::GetBreakpointLabel() label attached. Regardless
           // of wether the locations were added or removed, the breakpoint
           // ins't going away, so we the reason is always "changed".
           if ((event_type & lldb::eBreakpointEventTypeLocationsAdded ||
                event_type & lldb::eBreakpointEventTypeLocationsRemoved) &&
               bp.MatchesName(BreakpointBase::GetBreakpointLabel())) {
             auto bp_event = CreateEventObject("breakpoint");
             llvm::json::Object body;
             body.try_emplace("breakpoint", CreateBreakpoint(bp));
             body.try_emplace("reason", "changed");
             bp_event.try_emplace("body", std::move(body));
             g_vsc.SendJSON(llvm::json::Value(std::move(bp_event)));
           }
         }
       } else if (event.BroadcasterMatchesRef(g_vsc.broadcaster)) {
         if (event_mask & eBroadcastBitStopEventThread) {
           done = true;
void request_launch(const llvm::json::Object &request) {
   // This is a hack for loading DWARF in .o files on Mac where the .o files
   // in the debug map of the main executable have relative paths which require
   // the lldb-vscode binary to have its working directory set to that relative
   // root for the .o files in order to be able to load debug info.
   if (!debuggerRoot.empty()) {
     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());
     g_vsc.SendJSON(llvm::json::Value(std::move(response)));
     return;
   }
 
   // Instantiate a launch info instance for the target.
   auto launch_info = g_vsc.target.GetLaunchInfo();
if (bp_obj) {
       request_bps[src_bp.line] = src_bp;
 
       // We check if this breakpoint already exists to update it
       auto existing_source_bps = g_vsc.source_breakpoints.find(path);
       if (existing_source_bps != g_vsc.source_breakpoints.end()) {
         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);
     }
   }
 
   // Delete any breakpoints in this source file that aren't in the
   // request_bps set. There is no call to remove breakpoints other than
   // calling this function with a smaller or empty "breakpoints" list.
   auto old_src_bp_pos = g_vsc.source_breakpoints.find(path);
for (auto &pair: g_vsc.function_breakpoints) {
     } else {
       // Update the existing breakpoint as any setting withing the function
       // breakpoint might have changed.
       pair.second.UpdateBreakpoint(request_pos->second);
       // Remove this breakpoint from the request breakpoints since we have
       // handled it here and we don't need to set a new breakpoint below.
       request_bps.erase(request_pos);
       // Add this breakpoint info to the response
       AppendBreakpoint(pair.second.bp, response_breakpoints);
     }
   }
   // Remove any breakpoints that are no longer in our list
   for (const auto &name: remove_names)
     g_vsc.function_breakpoints.erase(name);
 
   // Any breakpoints that are left in "request_bps" are breakpoints that
   // need to be set.