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.

6 files changed, 63 insertions, 2 deletions

diff --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp b/lldb/source/Commands/CommandObjectBreakpoint.cpp
index a0c39cf..4631c97 100644
--- a/lldb/source/Commands/CommandObjectBreakpoint.cpp
+++ b/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());
diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_locations/after_rebuild/Makefile b/lldb/test/API/functionalities/breakpoint/breakpoint_locations/after_rebuild/Makefile
new file mode 100644
index 0000000..22f1051
--- /dev/null
+++ b/lldb/test/API/functionalities/breakpoint/breakpoint_locations/after_rebuild/Makefile
@@ -0,0 +1 @@
+include Makefile.rules
diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_locations/after_rebuild/TestLocationsAfterRebuild.py b/lldb/test/API/functionalities/breakpoint/breakpoint_locations/after_rebuild/TestLocationsAfterRebuild.py
new file mode 100644
index 0000000..c42fe33
--- /dev/null
+++ b/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}")
diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_locations/after_rebuild/main.c b/lldb/test/API/functionalities/breakpoint/breakpoint_locations/after_rebuild/main.c
new file mode 100644
index 0000000..d06ee7a
--- /dev/null
+++ b/lldb/test/API/functionalities/breakpoint/breakpoint_locations/after_rebuild/main.c
@@ -0,0 +1 @@
+int main() { return 1111; }
diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_locations/after_rebuild/second_main.c b/lldb/test/API/functionalities/breakpoint/breakpoint_locations/after_rebuild/second_main.c
new file mode 100644
index 0000000..a3090f9
--- /dev/null
+++ b/lldb/test/API/functionalities/breakpoint/breakpoint_locations/after_rebuild/second_main.c
@@ -0,0 +1 @@
+int main() { return 22222; }
diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_locations/after_rebuild/third_main.c b/lldb/test/API/functionalities/breakpoint/breakpoint_locations/after_rebuild/third_main.c
new file mode 100644
index 0000000..07027ea
--- /dev/null
+++ b/lldb/test/API/functionalities/breakpoint/breakpoint_locations/after_rebuild/third_main.c
@@ -0,0 +1 @@
+int main() { return 33333; }