Stop emitting a breakpoint for each location in a breakpoint when responding to breakpoint commands.

[clayborg]

Summary

The VS Code DAP expects on response for each breakpoint that was requested. If we responsd with multiple entries for one breakpoint the VS Code UI gets out of date. Currently the VS code DAP doesn't handle one breakpoint with multiple locations. If this ever gets fixed we can modify our code.

[labath]

This is not ideal, but if that's what vscode wants, then I guess we just have to do it.

However, are you sure that this is all that needs to be done here. It seems like also the handling of eBreakpointEventTypeLocationsAdded/Removed events in EventThreadFunction should be changed too. Otherwise, I think the whole breakpoint will disappear whenever a single location goes away...

[clayborg]

In D73665#1848755, @labath wrote:

This is not ideal, but if that's what vscode wants, then I guess we just have to do it.

However, are you sure that this is all that needs to be done here. It seems like also the handling of eBreakpointEventTypeLocationsAdded/Removed events in EventThreadFunction should be changed too. Otherwise, I think the whole breakpoint will disappear whenever a single location goes away...

Yes this needs to be fixed as well. I will switch this to pass in the breakpoint itself to avoid places iterating over the locations...

[clayborg]

Handle breakpoint events in the same way.

[labath]

Sorry about the delay. I think this is better, but the handling of`eBreakpointEventTypeLocationsAdded/Removed` it still does not seem completely right. Though I don't really understand how VSCode works, if my understanding that we are now creating a "vscode" breakpoint for each "lldb breakpoint" (instead of breakpoint _location_) is correct, then it seems that we should treat the "vscode breakpoint" enabled as long as the lldb breakpoint has at least one location.

However, that's not what the code does now, I think. Whenever a single location disappears (eBreakpointEventTypeLocationsRemoved), e.g. in response to a shared library unloading, we will set the reason=removed packet, even if other locations remain. I think that we should send the "removed" message only when the location count goes down to zero (and send the "new" packet whenever it goes non-zero).

Am I misunderstanding something?

[clayborg]

Fixed Pavel's issues. I thoroughly tested this and found that we should be only sending "changed" events for breakpoints since breakpoints never go away.

Changed in this patch:

• add a "vscode" name to each breakpoint so we know which breakpoints were set in the IDE UI. Any breakpoints that don't have this won't get updates to the IDE since the IDE doesn't know about them. This is in case people set breakpoints in the debugger console. I tried reporting such breakpoints as "new" breakpoints, but the IDE UI doesn't do anything with new breakpoint that weren't created in the UI.
• Fixed logic to always report "changed" as the breakpoint event reason. LLDB breakpoints never go away, so they are never "new" or "removed".
• If a breakpoint has no resolved locations, then the breakpoint is marked as "verified": false in the breakpoint responses and events. When a breakpoint resolves then it will get updated.

[clayborg]

Friendly ping.

[labath]

I'm sorry about the delay, I'm quite busy these days.

This code looks good to me now, but it's lacking test coverage. Can you add some test where new breakpoint locations get added or removed and check that the lldb-vscode generates the appropriate kind of dap message. And maybe another where a breakpoint is set via the the command line and check that we _don't_ emit any events...

[clayborg]

Yes, I will add tests now that the code has been finalized! Thanks for your time.

[clayborg]

Added a test to cover breakpoint events. The test has an executable and a shared library and we set breakpoints in the main executable and in the shared library. When we run, we stop at the entry point to the program and then the breakpoints are asked to be set. The breakpoint in the main executable should be resolved, and the one in the shared library should not be resolved since the shared library hasn't been loaded yet. We also set a breakpoint via the "preRunCommands" in the "launch" packet. This ensures we set a breakpoint using a LLDB command for which we expect to not get any breakpoint events for since the breakpoint wasn't set via VS Code DAP packets. After we stop, at the entry point we verify no breakpoint events have been received, and then we run to one of the breakpoints that was set via the VS Code DAP packets and check that we received only one event for the breakpoint set via DAP packets and not for the LLDB command created breakpoint.

Fixed lldbvscode_testcase.py set_source_breakpoints and set_function_breakpoints to return breakpoint IDs and not breakpoint ID + location ID. Also fixed verify_breakpoint_hit so that it actally checks that breakpoints were being set by now using an assertTrue call. Prior to this, this function would return true or false, but no one was looking at the result. Looking around the code it seems this should assert and cause the test to fail.

Modified vscode.py to store any breakpoint events in the object so tests can access these events in the test python code for verification.

Modified the "description" field of "stopped" events when we stop at breakpoints to contain "breakpoint %u" where %u is the breakpoint ID. This allows lldbvscode_testcase.verify_breakpoint_hit() to actually work.

[labath]

This looks good now. Thanks for your patience.

lldb/test/API/tools/lldb-vscode/breakpoint-events/TestVSCode_breakpointEvents.py

69

Under what conditions can response be None? Should this maybe be assertIsNotNone(response) instead ?

[clayborg]

Disabled the new breakpoint event test unless being run on Darwin to keep the buidbots happy until I can figure out what is going wrong:

commit c84a0bd9adb3ad699cd6bd4bf865d8b1ea76f2b0 (HEAD -> master, origin/master, origin/HEAD)
Author: Greg Clayton <[e-mail]>
Date: Thu Feb 13 09:32:19 2020 -0800

Fix buildbots by disabling this new test until I can fix it.

This tests works on Darwin. I will need to check windows and linux.

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

class VSCodeTestCaseBase(TestBase):
 
     def build_and_create_debug_adaptor(self):
         self.build()
         self.create_debug_adaptor()
 
     def set_source_breakpoints(self, source_path, lines, condition=None,
                                hitCondition=None):
         '''Sets source breakpoints and returns an array of strings containing
-           the breakpoint location IDs ("1.1", "1.2") for each breakpoint
-           that was set.
+           the breakpoint IDs ("1", "2") for each breakpoint that was set.
         '''
         response = self.vscode.request_setBreakpoints(
             source_path, lines, condition=condition, hitCondition=hitCondition)
         if response is None:
             return []
         breakpoints = response['body']['breakpoints']
         breakpoint_ids = []
         for breakpoint in breakpoints:
-            response_id = breakpoint['id']
-            bp_id = response_id >> 32
-            bp_loc_id = response_id & 0xffffffff
-            breakpoint_ids.append('%i.%i' % (bp_id, bp_loc_id))
+            breakpoint_ids.append('%i' % (breakpoint['id']))
         return breakpoint_ids
 
     def set_function_breakpoints(self, functions, condition=None,
                                  hitCondition=None):
         '''Sets breakpoints by function name given an array of function names
-           and returns an array of strings containing the breakpoint location
-           IDs ("1.1", "1.2") for each breakpoint that was set.
+           and returns an array of strings containing the breakpoint IDs
+           ("1", "2") for each breakpoint that was set.
         '''
         response = self.vscode.request_setFunctionBreakpoints(
             functions, condition=condition, hitCondition=hitCondition)
         if response is None:
             return []
         breakpoints = response['body']['breakpoints']
         breakpoint_ids = []
         for breakpoint in breakpoints:
-            response_id = breakpoint['id']
-            bp_id = response_id >> 32
-            bp_loc_id = response_id & 0xffffffff
-            breakpoint_ids.append('%i.%i' % (bp_id, bp_loc_id))
+            breakpoint_ids.append('%i' % (breakpoint['id']))
         return breakpoint_ids
 
     def verify_breakpoint_hit(self, breakpoint_ids):
         '''Wait for the process we are debugging to stop, and verify we hit
            any breakpoint location in the "breakpoint_ids" array.
-           "breakpoint_ids" should be a list of breakpoint location ID strings
-           (["1.1", "2.1"]). The return value from
-           self.set_source_breakpoints() can be passed to this function'''
+           "breakpoint_ids" should be a list of breakpoint ID strings
+           (["1", "2"]). The return value from self.set_source_breakpoints()
+           or self.set_function_breakpoints() can be passed to this function'''
         stopped_events = self.vscode.wait_for_stopped()
         for stopped_event in stopped_events:
             if 'body' in stopped_event:
                 body = stopped_event['body']
                 if 'reason' not in body:
                     continue
                 if body['reason'] != 'breakpoint':
                     continue
                 if 'description' not in body:
                     continue
-                # Description is "breakpoint 1.1", so look for any location id
-                # ("1.1") in the description field as verification that one of
-                # the breakpoint locations was hit
+                # Descriptions for breakpoints will be in the form
+                # "breakpoint 1.1", so look for any description that matches
+                # ("breakpoint 1.") in the description field as verification
+                # that one of the breakpoint locations was hit. VSCode doesn't
+                # allow breakpoints to have multiple locations, but LLDB does.
+                # So when looking at the description we just want to make sure
+                # the right breakpoint matches and not worry about the actual
+                # location.
                 description = body['description']
+                print("description: %s" % (description))
                 for breakpoint_id in breakpoint_ids:
-                    if breakpoint_id in description:
-                        return True
-        return False
+                    match_desc = 'breakpoint %s.' % (breakpoint_id)
+                    if match_desc in description:
+                        return
+        self.assertTrue(False, "breakpoint not hit")
 
     def verify_exception_breakpoint_hit(self, filter_label):
         '''Wait for the process we are debugging to stop, and verify the stop
            reason is 'exception' and that the description matches
            'filter_label'
         '''
         stopped_events = self.vscode.wait_for_stopped()
         for stopped_event in stopped_events:

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

def __init__(self, recv, send, init_commands):
         self.recv_packets = []
         self.recv_condition = threading.Condition()
         self.recv_thread = threading.Thread(target=read_packet_thread,
                                             args=(self,))
         self.process_event_body = None
         self.exit_status = None
         self.initialize_body = None
         self.thread_stop_reasons = {}
+        self.breakpoint_events = []
         self.sequence = 1
         self.threads = None
         self.recv_thread.start()
         self.output_condition = threading.Condition()
         self.output = {}
         self.configuration_done_sent = False
         self.frame_scopes = {}
         self.init_commands = init_commands
def handle_recv_packet(self, packet):
                 output = body['output']
                 self.output_condition.acquire()
                 if category in self.output:
                     self.output[category] += output
                 else:
                     self.output[category] = output
                 self.output_condition.notify()
                 self.output_condition.release()
-                # no need to add 'output' packets to our packets list
+                # no need to add 'output' event packets to our packets list
                 return keepGoing
             elif event == 'process':
                 # When a new process is attached or launched, remember the
                 # details that are available in the body of the event
                 self.process_event_body = body
             elif event == 'stopped':
                 # Each thread that stops with a reason will send a
                 # 'stopped' event. We need to remember the thread stop
                 # reasons since the 'threads' command doesn't return
                 # that information.
                 self._process_stopped()
                 tid = body['threadId']
                 self.thread_stop_reasons[tid] = body
+            elif event == 'breakpoint':
+                # Breakpoint events come in when a breakpoint has locations
+                # added or removed. Keep track of them so we can look for them
+                # in tests.
+                self.breakpoint_events.append(packet)
+                # no need to add 'breakpoint' event packets to our packets list
+                return keepGoing
         elif packet_type == 'response':
             if packet['command'] == 'disconnect':
                 keepGoing = False
         self.enqueue_recv_packet(packet)
         return keepGoing
 
     def send_packet(self, command_dict, set_sequence=True):
         '''Take the "command_dict" python dictionary and encode it as a JSON

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

+DYLIB_NAME := unlikely_name
+DYLIB_CXX_SOURCES := foo.cpp
+CXX_SOURCES := main.cpp
+include Makefile.rules

lldb/test/API/tools/lldb-vscode/breakpoint-events/TestVSCode_breakpointEvents.py

+"""
+Test lldb-vscode setBreakpoints request
+"""
+
+
+import unittest2
+import vscode
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import lldbvscode_testcase
+import os
+
+
+class TestVSCode_breakpointEvents(lldbvscode_testcase.VSCodeTestCaseBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    @skipIfWindows
+    def test_breakpoint_events(self):
+        '''
+            This test sets a breakpoint in a shared library and runs and stops
+            at the entry point of a program. When we stop at the entry point,
+            the shared library won't be loaded yet. At this point the
+            breakpoint should set itself, but not be verified because no
+            locations are resolved. We will then continue and expect to get a
+            breakpoint event that informs us that the breakpoint in the shared
+            library is "changed" and the correct line number should be
+            supplied. We also set a breakpoint using a LLDB command using the
+            "preRunCommands" when launching our program. Any breapoints set via
+            the command interpreter should not be have breakpoint events sent
+            back to VS Code as the UI isn't able to add new breakpoints to
+            their UI. Code has been added that tags breakpoints set from VS Code
+            DAP packets so we know the IDE knows about them. If VS Code is ever
+            able to register breakpoints that aren't initially set in the GUI,
+            then we will need to revise this.
+        '''
+        main_source_basename = 'main.cpp'
+        main_source_path = os.path.join(os.getcwd(), main_source_basename)
+        foo_source_basename = 'foo.cpp'
+        foo_source_path = os.path.join(os.getcwd(), foo_source_basename)
+        main_bp_line = line_number('main.cpp', 'main breakpoint 1')
+        foo_bp1_line = line_number('foo.cpp', 'foo breakpoint 1')
+        foo_bp2_line = line_number('foo.cpp', 'foo breakpoint 2')
+
+        # 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.
+        program = self.getBuildArtifact("a.out")
+
+        # Set a breakpoint after creating the target by running a command line
+        # command. It will eventually resolve and cause a breakpoint changed
+        # event to be sent to lldb-vscode. We want to make sure we don't send a
+        # breakpoint any breakpoints that were set from the command line.
+        # Breakpoints that are set via the VS code DAP packets will be
+        # registered and marked with a special keyword to ensure we deliver
+        # breakpoint events for these breakpoints but not for ones that are not
+        # set via the command interpreter.
+        bp_command = 'breakpoint set --file foo.cpp --line %u' % (foo_bp2_line)
+        self.build_and_launch(program, stopOnEntry=True,
+                              preRunCommands=[bp_command])
+        main_bp_id = 0
+        foo_bp_id = 0
+        # Set breakoints and verify that they got set correctly
+        vscode_breakpoint_ids = []
+        response = self.vscode.request_setBreakpoints(main_source_path,
+                                                      [main_bp_line])
+        if response:
+            breakpoints = response['body']['breakpoints']
+            for breakpoint in breakpoints:
+                main_bp_id = breakpoint['id']
+                vscode_breakpoint_ids.append("%i" % (main_bp_id))
+                # line = breakpoint['line']
+                self.assertTrue(breakpoint['verified'],
+                                "expect main breakpoint to be verified")
+
+        response = self.vscode.request_setBreakpoints(foo_source_path,
+                                                      [foo_bp1_line])
+        if response:
+            breakpoints = response['body']['breakpoints']
+            for breakpoint in breakpoints:
+                foo_bp_id = breakpoint['id']
+                vscode_breakpoint_ids.append("%i" % (foo_bp_id))
+                self.assertFalse(breakpoint['verified'],
+                                 "expect foo breakpoint to not be verified")
+
+        # Get the stop at the entry point
+        self.continue_to_next_stop()
+
+        # We are now stopped at the entry point to the program. Shared
+        # libraries are not loaded yet (at least on macOS they aren't) and any
+        # breakpoints set in foo.cpp should not be resolved.
+        self.assertTrue(len(self.vscode.breakpoint_events) == 0,
+                        "no breakpoint events when stopped at entry point")
+
+        # Continue to the breakpoint
+        self.continue_to_breakpoints(vscode_breakpoint_ids)
+
+        # Make sure we only get an event for the breakpoint we set via a call
+        # to self.vscode.request_setBreakpoints(...), not the breakpoint
+        # we set with with a LLDB command in preRunCommands.
+        self.assertTrue(len(self.vscode.breakpoint_events) == 1,
+                        "make sure we got a breakpoint event")
+        event = self.vscode.breakpoint_events[0]
+        # Verify the details of the breakpoint changed notification.
+        body = event['body']
+        self.assertTrue(body['reason'] == 'changed',
+                "breakpoint event is says breakpoint is changed")
+        breakpoint = body['breakpoint']
+        self.assertTrue(breakpoint['verified'] == True,
+                "breakpoint event is says it is verified")
+        self.assertTrue(breakpoint['id'] == foo_bp_id,
+                "breakpoint event is for breakpoint %i" % (foo_bp_id))
+        self.assertTrue('line' in breakpoint and breakpoint['line'] > 0,
+                "breakpoint event is has a line number")
+        self.assertTrue("foo.cpp" in breakpoint['source']['path'],
+                "breakpoint event path contains foo.cpp")
+
+        output = self.get_console() # REMOVE PRIOR TO CHECKIN
+        with open("/tmp/b", "w") as f:
+            f.write(output)

lldb/test/API/tools/lldb-vscode/breakpoint-events/foo.h

+
+int foo(int);

lldb/test/API/tools/lldb-vscode/breakpoint-events/foo.cpp

+#include <stdio.h>
+
+static void unique_function_name() {
+  puts(__PRETTY_FUNCTION__); // foo breakpoint 2
+}
+
+int foo(int x) {
+  // foo breakpoint 1
+  unique_function_name();
+  return x+42;
+}

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

+
+#include "foo.h"
+
+int main(int argc, char const *argv[]) {
+  int f = foo(argc);
+  return 0; // main breakpoint 1
+}

lldb/tools/lldb-vscode/BreakpointBase.h

 #define LLDBVSCODE_BREAKPOINTBASE_H_
 
 #include "JSONUtils.h"
 #include "lldb/API/SBBreakpoint.h"
 #include "llvm/Support/JSON.h"
 #include <string>
 
 namespace lldb_vscode {
-  
 struct BreakpointBase {
 
   // An optional expression for conditional breakpoints.
   std::string condition;
   // An optional expression that controls how many hits of the breakpoint are
   // ignored. The backend is expected to interpret the expression as needed
   std::string hitCondition;
   // If this attribute exists and is non-empty, the backend must not 'break'
   // (stop) but log the message instead. Expressions within {} are
   // interpolated.
   std::string logMessage;
   // The LLDB breakpoint associated wit this source breakpoint
   lldb::SBBreakpoint bp;
 
   BreakpointBase() = default;
   BreakpointBase(const llvm::json::Object &obj);
 
   void SetCondition();
   void SetHitCondition();
   void UpdateBreakpoint(const BreakpointBase &request_bp);
+  static const char *GetBreakpointLabel();
 };
 
 } // namespace lldb_vscode
 
 #endif

lldb/tools/lldb-vscode/BreakpointBase.cpp

 
 BreakpointBase::BreakpointBase(const llvm::json::Object &obj)
     : condition(std::string(GetString(obj, "condition"))),
       hitCondition(std::string(GetString(obj, "hitCondition"))),
       logMessage(std::string(GetString(obj, "logMessage"))) {}
 
 void BreakpointBase::SetCondition() { bp.SetCondition(condition.c_str()); }
 
-void BreakpointBase::SetHitCondition() {  
+void BreakpointBase::SetHitCondition() {
   uint64_t hitCount = 0;
   if (llvm::to_integer(hitCondition, hitCount))
     bp.SetIgnoreCount(hitCount - 1);
 }
 
 void BreakpointBase::UpdateBreakpoint(const BreakpointBase &request_bp) {
   if (condition != request_bp.condition) {
     condition = request_bp.condition;
     SetCondition();
   }
   if (hitCondition != request_bp.hitCondition) {
     hitCondition = request_bp.hitCondition;
     SetHitCondition();
   }
 }
+
+const char *BreakpointBase::GetBreakpointLabel() {
+  // Breakpoints in LLDB can have names added to them which are kind of like
+  // labels or categories. All breakpoints that are set through the IDE UI get
+  // sent through the various VS code DAP set*Breakpoint packets, and these
+  // breakpoints will be labeled with this name so if breakpoint update events
+  // come in for breakpoints that the IDE doesn't know about, like if a
+  // breakpoint is set manually using the debugger console, we won't report any
+  // updates on them and confused the IDE. This function gets called by all of
+  // the breakpoint classes after they set breakpoints to mark a breakpoint as
+  // a UI breakpoint. We can later check a lldb::SBBreakpoint object that comes
+  // in via LLDB breakpoint changed events and check the breakpoint by calling
+  // "bool lldb::SBBreakpoint::MatchesName(const char *)" to check if a
+  // breakpoint in one of the UI breakpoints that we should report changes for.
+  return "vscode";
+}

lldb/tools/lldb-vscode/ExceptionBreakpoint.cpp

 //===-- ExceptionBreakpoint.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 "ExceptionBreakpoint.h"
+#include "BreakpointBase.h"
 #include "VSCode.h"
 
 namespace lldb_vscode {
 
 void ExceptionBreakpoint::SetBreakpoint() {
   if (bp.IsValid())
     return;
   bool catch_value = filter.find("_catch") != std::string::npos;
   bool throw_value = filter.find("_throw") != std::string::npos;
   bp = g_vsc.target.BreakpointCreateForException(language, catch_value,
                                                  throw_value);
+  // See comments in BreakpointBase::GetBreakpointLabel() for details of why
+  // we add a label to our breakpoints.
+  bp.AddName(BreakpointBase::GetBreakpointLabel());
 }
 
 void ExceptionBreakpoint::ClearBreakpoint() {
   if (!bp.IsValid())
     return;
   g_vsc.target.BreakpointDelete(bp.GetID());
   bp = lldb::SBBreakpoint();
 }
 
 } // namespace lldb_vscode
-

lldb/tools/lldb-vscode/FunctionBreakpoint.cpp

 
 FunctionBreakpoint::FunctionBreakpoint(const llvm::json::Object &obj)
     : BreakpointBase(obj), functionName(std::string(GetString(obj, "name"))) {}
 
 void FunctionBreakpoint::SetBreakpoint() {
   if (functionName.empty())
     return;
   bp = g_vsc.target.BreakpointCreateByName(functionName.c_str());
+  // See comments in BreakpointBase::GetBreakpointLabel() for details of why
+  // we add a label to our breakpoints.
+  bp.AddName(GetBreakpointLabel());
   if (!condition.empty())
     SetCondition();
   if (!hitCondition.empty())
     SetHitCondition();
 }
 
 }

lldb/tools/lldb-vscode/JSONUtils.h

 /// \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 breakpoint location to a Visual Studio Code "Breakpoint"
 /// JSON object and appends it to the \a breakpoints array.
 ///
-/// \param[in] bp_loc
-///     A LLDB breakpoint location object to convert into a JSON value
+/// \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::SBBreakpointLocation &bp_loc);
+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

lldb/tools/lldb-vscode/JSONUtils.cpp

 //       "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::SBBreakpointLocation &bp_loc) {
+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_loc.IsValid())
+  if (!bp.IsValid())
     return llvm::json::Value(std::move(object));
 
-  object.try_emplace("verified", true);
-  const auto vs_id = MakeVSCodeBreakpointID(bp_loc);
-  object.try_emplace("id", vs_id);
+  object.try_emplace("verified", bp.GetNumResolvedLocations() > 0);
+  object.try_emplace("id", bp.GetID());
+  // VS Code DAP doesn't currently allow one breakpoint to have multiple
+  // locations so we just report the first one. If we report all locations
+  // then the IDE starts showing the wrong line numbers and locations for
+  // other source file and line breakpoints in the same file.
+
+  // Below we search for the first resolved location in a breakpoint and report
+  // this as the breakpoint location since it will have a complete location
+  // that is at least loaded in the current process.
+  lldb::SBBreakpointLocation bp_loc;
+  const auto num_locs = bp.GetNumLocations();
+  for (size_t i=0; i<num_locs; ++i) {
+    bp_loc = bp.GetLocationAtIndex(i);
+    if (bp_loc.IsResolved())
+      break;
+  }
+  // If not locations are resolved, use the first location.
+  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));
 }
 
 void AppendBreakpoint(lldb::SBBreakpoint &bp, llvm::json::Array &breakpoints) {
-  if (!bp.IsValid())
-    return;
-  const auto num_locations = bp.GetNumLocations();
-  if (num_locations == 0)
-    return;
-  for (size_t i = 0; i < num_locations; ++i) {
-    auto bp_loc = bp.GetLocationAtIndex(i);
-    breakpoints.emplace_back(CreateBreakpoint(bp_loc));
-  }
+  breakpoints.emplace_back(CreateBreakpoint(bp));
 }
 
 // "Event": {
 //   "allOf": [ { "$ref": "#/definitions/ProtocolMessage" }, {
 //     "type": "object",
 //     "description": "Server-initiated event.",
 //     "properties": {
 //       "type": {
case lldb::eStopReasonPlanComplete:
     break;
   case lldb::eStopReasonBreakpoint: {
     ExceptionBreakpoint *exc_bp = g_vsc.GetExceptionBPFromStopReason(thread);
     if (exc_bp) {
       body.try_emplace("reason", "exception");
       EmplaceSafeString(body, "description", exc_bp->label);
     } else {
       body.try_emplace("reason", "breakpoint");
+      char desc_str[64];
+      uint64_t bp_id = thread.GetStopReasonDataAtIndex(0);
+      uint64_t bp_loc_id = thread.GetStopReasonDataAtIndex(1);
+      snprintf(desc_str, sizeof(desc_str), "breakpoint %" PRIu64 ".%" PRIu64,
+               bp_id, bp_loc_id);
+      EmplaceSafeString(body, "description", desc_str);
     }
   } break;
   case lldb::eStopReasonWatchpoint:
   case lldb::eStopReasonInstrumentation:
     body.try_emplace("reason", "breakpoint");
     break;
   case lldb::eStopReasonSignal:
     body.try_emplace("reason", "exception");
llvm::json::Value CreateVariable(lldb::SBValue v, int64_t variablesReference,
   v.GetExpressionPath(evaluateStream);
   const char *evaluateName = evaluateStream.GetData();
   if (evaluateName && evaluateName[0])
     EmplaceSafeString(object, "evaluateName", std::string(evaluateName));
   return llvm::json::Value(std::move(object));
 }
 
 } // namespace lldb_vscode
-

lldb/tools/lldb-vscode/LLDBUtils.h

 ///
 /// \param[in] dap_frame_id
 ///     The VSCode frame ID to convert to a frame ID.
 ///
 /// \return
 ///     The LLDB frame index ID.
 uint32_t GetLLDBFrameID(uint64_t dap_frame_id);
 
-/// Given a LLDB breakpoint, make a breakpoint ID that is unique to a
-/// specific breakpoint and breakpoint location.
-///
-/// VSCode requires a Breakpoint "id" to be unique, so we use the
-/// breakpoint ID in the lower BREAKPOINT_ID_SHIFT bits and the
-/// breakpoint location ID in the upper BREAKPOINT_ID_SHIFT bits.
-///
-/// \param[in] bp_loc
-///     The LLDB break point location.
-///
-/// \return
-///     A unique integer that allows us to easily find the right
-///     stack frame within a thread on subsequent VS code requests.
-int64_t MakeVSCodeBreakpointID(lldb::SBBreakpointLocation &bp_loc);
-
-/// Given a VSCode breakpoint ID, convert to a LLDB breakpoint ID.
-///
-/// VSCode requires a Breakpoint "id" to be unique, so we use the
-/// breakpoint ID in the lower BREAKPOINT_ID_SHIFT bits and the
-/// breakpoint location ID in the upper BREAKPOINT_ID_SHIFT bits.
-///
-/// \param[in] dap_breakpoint_id
-///     The VSCode breakpoint ID to convert to an LLDB breakpoint ID.
-///
-/// \return
-///     The LLDB breakpoint ID.
-uint32_t GetLLDBBreakpointID(uint64_t dap_breakpoint_id);
-
-/// Given a VSCode breakpoint ID, convert to a LLDB breakpoint location ID.
-///
-/// VSCode requires a Breakpoint "id" to be unique, so we use the
-/// breakpoint ID in the lower BREAKPOINT_ID_SHIFT bits and the
-/// breakpoint location ID in the upper BREAKPOINT_ID_SHIFT bits.
-///
-/// \param[in] dap_breakpoint_id
-///     The VSCode frame ID to convert to a breakpoint location ID.
-///
-/// \return
-///     The LLDB breakpoint location ID.
-uint32_t GetLLDBBreakpointLocationID(uint64_t dap_breakpoint_id);
 } // namespace lldb_vscode
 
 #endif

lldb/tools/lldb-vscode/LLDBUtils.cpp

uint32_t GetLLDBFrameID(uint64_t dap_frame_id) {
   return dap_frame_id & ((1u << THREAD_INDEX_SHIFT) - 1);
 }
 
 int64_t MakeVSCodeFrameID(lldb::SBFrame &frame) {
   return (int64_t)(frame.GetThread().GetIndexID() << THREAD_INDEX_SHIFT |
                    frame.GetFrameID());
 }
 
-static uint32_t constexpr BREAKPOINT_ID_SHIFT = 22;
-
-uint32_t GetLLDBBreakpointID(uint64_t dap_breakpoint_id) {
-  return dap_breakpoint_id >> BREAKPOINT_ID_SHIFT;
-}
-
-uint32_t GetLLDBBreakpointLocationID(uint64_t dap_breakpoint_id) {
-  return dap_breakpoint_id & ((1u << BREAKPOINT_ID_SHIFT) - 1);
-}
-
-int64_t MakeVSCodeBreakpointID(lldb::SBBreakpointLocation &bp_loc) {
-  return (int64_t)(bp_loc.GetBreakpoint().GetID() << BREAKPOINT_ID_SHIFT |
-                   bp_loc.GetID());
-}
-
 } // namespace lldb_vscode

lldb/tools/lldb-vscode/SourceBreakpoint.cpp

 namespace lldb_vscode {
 
 SourceBreakpoint::SourceBreakpoint(const llvm::json::Object &obj)
     : BreakpointBase(obj), line(GetUnsigned(obj, "line", 0)),
       column(GetUnsigned(obj, "column", 0)) {}
 
 void SourceBreakpoint::SetBreakpoint(const llvm::StringRef source_path) {
   bp = g_vsc.target.BreakpointCreateByLocation(source_path.str().c_str(), line);
+  // See comments in BreakpointBase::GetBreakpointLabel() for details of why
+  // we add a label to our breakpoints.
+  bp.AddName(GetBreakpointLabel());
   if (!condition.empty())
     SetCondition();
   if (!hitCondition.empty())
     SetHitCondition();
 }
 
 } // namespace lldb_vscode

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

if (listener.WaitForEvent(1, event)) {
         } else if ((event_mask & lldb::SBProcess::eBroadcastBitSTDOUT) ||
                    (event_mask & lldb::SBProcess::eBroadcastBitSTDERR)) {
           SendStdOutStdErr(process);
         }
       } else if (lldb::SBBreakpoint::EventIsBreakpointEvent(event)) {
         if (event_mask & lldb::SBTarget::eBroadcastBitBreakpointChanged) {
           auto event_type =
               lldb::SBBreakpoint::GetBreakpointEventTypeFromEvent(event);
-          const auto num_locs =
-              lldb::SBBreakpoint::GetNumBreakpointLocationsFromEvent(event);
           auto bp = lldb::SBBreakpoint::GetBreakpointFromEvent(event);
-          bool added = event_type & lldb::eBreakpointEventTypeLocationsAdded;
-          bool removed =
-              event_type & lldb::eBreakpointEventTypeLocationsRemoved;
-          if (added || removed) {
-            for (size_t i = 0; i < num_locs; ++i) {
-              auto bp_loc =
-                  lldb::SBBreakpoint::GetBreakpointLocationAtIndexFromEvent(
-                      event, i);
-              auto bp_event = CreateEventObject("breakpoint");
-              llvm::json::Object body;
-              body.try_emplace("breakpoint", CreateBreakpoint(bp_loc));
-              if (added)
-                body.try_emplace("reason", "new");
-              else
-                body.try_emplace("reason", "removed");
-              bp_event.try_emplace("body", std::move(body));
-              g_vsc.SendJSON(llvm::json::Value(std::move(bp_event)));
-            }
-          }
-        }
+          // If the breakpoint was originated from the IDE, it will have the
+          // 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())) {
+            body.try_emplace("breakpoint", CreateBreakpoint(bp));
+            body.try_emplace("reason", "changed");
       } else if (event.BroadcasterMatchesRef(g_vsc.broadcaster)) {
         if (event_mask & eBroadcastBitStopEventThread) {
           done = true;
         }
       }
     }
   }
 }