Fix a bug in the breakpoint ID verifier in CommandObjectBreakpoint. (#145994)

It was assuming that for any location M.N, N was always less than the
number of breakpoint locations. But if you rebuild the target and rerun
multiple times, when the section backing one of the locations is no
longer valid, we remove the location, but we don't reuse the ID. So you
can have a breakpoint that only has location 1.3. The num_locations
check would say that was an invalid location.

Author: jimingham

lldb/source/Commands/CommandObjectBreakpoint.cpp

@@ -2485,8 +2485,9 @@ void CommandObjectMultiwordBreakpoint::VerifyIDs(
     Breakpoint *breakpoint =
         target.GetBreakpointByID(cur_bp_id.GetBreakpointID()).get();
     if (breakpoint != nullptr) {
-      const size_t num_locations = breakpoint->GetNumLocations();
-      if (static_cast<size_t>(cur_bp_id.GetLocationID()) > num_locations) {
+      lldb::break_id_t cur_loc_id = cur_bp_id.GetLocationID();
+      // GetLocationID returns 0 when the location isn't specified.
+      if (cur_loc_id != 0 && !breakpoint->FindLocationByID(cur_loc_id)) {
         StreamString id_str;
         BreakpointID::GetCanonicalReference(
             &id_str, cur_bp_id.GetBreakpointID(), cur_bp_id.GetLocationID());

lldb/test/API/functionalities/breakpoint/breakpoint_locations/after_rebuild/Makefile

@@ -0,0 +1 @@
+include Makefile.rules

lldb/test/API/functionalities/breakpoint/breakpoint_locations/after_rebuild/TestLocationsAfterRebuild.py

@@ -0,0 +1,56 @@
+"""
+When a rebuild causes a location to be removed, make sure
+we still handle the remaining locations correctly.
+"""
+
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+import os
+
+
+class TestLocationsAfterRebuild(TestBase):
+    # If your test case doesn't stress debug info, then
+    # set this to true.  That way it won't be run once for
+    # each debug info format.
+    NO_DEBUG_INFO_TESTCASE = True
+
+    def test_remaining_location_spec(self):
+        """If we rebuild a couple of times some of the old locations
+        get removed.  Make sure the command-line breakpoint id
+        validator still works correctly."""
+        self.build(dictionary={"C_SOURCES": "main.c", "EXE": "a.out"})
+
+        path_to_exe = self.getBuildArtifact()
+
+        (target, process, thread, bkpt) = lldbutil.run_to_name_breakpoint(self, "main")
+
+        # Let the process continue to exit:
+        process.Continue()
+        self.assertEqual(process.GetState(), lldb.eStateExited, "Ran to completion")
+        os.remove(path_to_exe)
+
+        # We have to rebuild twice with changed sources to get
+        # us to remove the first set of locations:
+        self.build(dictionary={"C_SOURCES": "second_main.c", "EXE": "a.out"})
+
+        (target, process, thread, bkpt) = lldbutil.run_to_breakpoint_do_run(
+            self, target, bkpt
+        )
+
+        # Let the process continue to exit:
+        process.Continue()
+        self.assertEqual(process.GetState(), lldb.eStateExited, "Ran to completion")
+
+        os.remove(path_to_exe)
+
+        self.build(dictionary={"C_SOURCES": "third_main.c", "EXE": "a.out"})
+
+        (target, process, thread, bkpt) = lldbutil.run_to_breakpoint_do_run(
+            self, target, bkpt
+        )
+
+        bkpt_id = bkpt.GetID()
+        loc_string = f"{bkpt_id}.3"
+        self.runCmd(f"break disable {loc_string}")

lldb/test/API/functionalities/breakpoint/breakpoint_locations/after_rebuild/main.c

@@ -0,0 +1 @@
+int main() { return 1111; }

lldb/test/API/functionalities/breakpoint/breakpoint_locations/after_rebuild/second_main.c

@@ -0,0 +1 @@
+int main() { return 22222; }

lldb/test/API/functionalities/breakpoint/breakpoint_locations/after_rebuild/third_main.c

@@ -0,0 +1 @@
+int main() { return 33333; }