Merge pull request #3751 from adrian-prantl/83503919

adrian-prantl · web-flow · commit 143f1389db63 · 2022-01-07T10:24:40.000-08:00
Cherry-pick fix for relative breakpoints
diff --git a/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp b/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
@@ -188,7 +188,7 @@ void BreakpointResolverFileLine::FilterContexts(SymbolContextList &sc_list,
     // is 0, then we can't do this calculation.  That can happen if
     // GetStartLineSourceInfo gets an error, or if the first line number in
     // the function really is 0 - which happens for some languages.
-    
+
     // But only do this calculation if the line number we found in the SC
     // was different from the one requested in the source file.  If we actually
     // found an exact match it must be valid.
@@ -229,18 +229,25 @@ Searcher::CallbackReturn BreakpointResolverFileLine::SearchCallback(
   const uint32_t line = m_location_spec.GetLine().getValueOr(0);
   const llvm::Optional<uint16_t> column = m_location_spec.GetColumn();
 
+  // We'll create a new SourceLocationSpec that can take into account the
+  // relative path case, and we'll use it to resolve the symbol context
+  // of the CUs.
   FileSpec search_file_spec = m_location_spec.GetFileSpec();
   const bool is_relative = search_file_spec.IsRelative();
   if (is_relative)
     search_file_spec.GetDirectory().Clear();
+  SourceLocationSpec search_location_spec(
+      search_file_spec, m_location_spec.GetLine().getValueOr(0),
+      m_location_spec.GetColumn(), m_location_spec.GetCheckInlines(),
+      m_location_spec.GetExactMatch());
 
   const size_t num_comp_units = context.module_sp->GetNumCompileUnits();
   for (size_t i = 0; i < num_comp_units; i++) {
     CompUnitSP cu_sp(context.module_sp->GetCompileUnitAtIndex(i));
     if (cu_sp) {
       if (filter.CompUnitPasses(*cu_sp))
-        cu_sp->ResolveSymbolContext(m_location_spec, eSymbolContextEverything,
-                                    sc_list);
+        cu_sp->ResolveSymbolContext(search_location_spec,
+                                    eSymbolContextEverything, sc_list);
     }
   }
 
diff --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp b/lldb/source/Commands/CommandObjectBreakpoint.cpp
@@ -1463,6 +1463,7 @@ class CommandObjectBreakpointDelete : public CommandObjectParsed {
       return false;
     }
 
+    // Handle the delete all breakpoints case:
     if (command.empty() && !m_options.m_delete_disabled) {
       if (!m_options.m_force &&
           !m_interpreter.Confirm(
@@ -1476,67 +1477,73 @@ class CommandObjectBreakpointDelete : public CommandObjectParsed {
             (uint64_t)num_breakpoints, num_breakpoints > 1 ? "s" : "");
       }
       result.SetStatus(eReturnStatusSuccessFinishNoResult);
-    } else {
-      // Particular breakpoint selected; disable that breakpoint.
-      BreakpointIDList valid_bp_ids;
-      
-      if (m_options.m_delete_disabled) {
-        BreakpointIDList excluded_bp_ids;
+      return result.Succeeded();
+    }
+ 
+    // Either we have some kind of breakpoint specification(s),
+    // or we are handling "break disable --deleted".  Gather the list
+    // of breakpoints to delete here, the we'll delete them below.
+    BreakpointIDList valid_bp_ids;
+    
+    if (m_options.m_delete_disabled) {
+      BreakpointIDList excluded_bp_ids;
 
-        if (!command.empty()) {
-          CommandObjectMultiwordBreakpoint::VerifyBreakpointOrLocationIDs(
-              command, &target, result, &excluded_bp_ids,
-              BreakpointName::Permissions::PermissionKinds::deletePerm);
-        }
-        for (auto breakpoint_sp : breakpoints.Breakpoints()) {
-          if (!breakpoint_sp->IsEnabled() && breakpoint_sp->AllowDelete()) {
-            BreakpointID bp_id(breakpoint_sp->GetID());
-            size_t pos = 0;
-            if (!excluded_bp_ids.FindBreakpointID(bp_id, &pos))
-              valid_bp_ids.AddBreakpointID(breakpoint_sp->GetID());
-          }
-        }
-        if (valid_bp_ids.GetSize() == 0) {
-          result.AppendError("No disabled breakpoints.");
-          return false;
-        }
-      } else {
+      if (!command.empty()) {
         CommandObjectMultiwordBreakpoint::VerifyBreakpointOrLocationIDs(
-            command, &target, result, &valid_bp_ids,
+            command, &target, result, &excluded_bp_ids,
             BreakpointName::Permissions::PermissionKinds::deletePerm);
+        if (!result.Succeeded())
+          return false;
       }
-      
-      if (result.Succeeded()) {
-        int delete_count = 0;
-        int disable_count = 0;
-        const size_t count = valid_bp_ids.GetSize();
-        for (size_t i = 0; i < count; ++i) {
-          BreakpointID cur_bp_id = valid_bp_ids.GetBreakpointIDAtIndex(i);
 
-          if (cur_bp_id.GetBreakpointID() != LLDB_INVALID_BREAK_ID) {
-            if (cur_bp_id.GetLocationID() != LLDB_INVALID_BREAK_ID) {
-              Breakpoint *breakpoint =
-                  target.GetBreakpointByID(cur_bp_id.GetBreakpointID()).get();
-              BreakpointLocation *location =
-                  breakpoint->FindLocationByID(cur_bp_id.GetLocationID()).get();
-              // It makes no sense to try to delete individual locations, so we
-              // disable them instead.
-              if (location) {
-                location->SetEnabled(false);
-                ++disable_count;
-              }
-            } else {
-              target.RemoveBreakpointByID(cur_bp_id.GetBreakpointID());
-              ++delete_count;
-            }
+      for (auto breakpoint_sp : breakpoints.Breakpoints()) {
+        if (!breakpoint_sp->IsEnabled() && breakpoint_sp->AllowDelete()) {
+          BreakpointID bp_id(breakpoint_sp->GetID());
+          size_t pos = 0;
+          if (!excluded_bp_ids.FindBreakpointID(bp_id, &pos))
+            valid_bp_ids.AddBreakpointID(breakpoint_sp->GetID());
+        }
+      }
+      if (valid_bp_ids.GetSize() == 0) {
+        result.AppendError("No disabled breakpoints.");
+        return false;
+      }
+    } else {
+      CommandObjectMultiwordBreakpoint::VerifyBreakpointOrLocationIDs(
+          command, &target, result, &valid_bp_ids,
+          BreakpointName::Permissions::PermissionKinds::deletePerm);
+      if (!result.Succeeded())
+        return false;
+    }
+    
+    int delete_count = 0;
+    int disable_count = 0;
+    const size_t count = valid_bp_ids.GetSize();
+    for (size_t i = 0; i < count; ++i) {
+      BreakpointID cur_bp_id = valid_bp_ids.GetBreakpointIDAtIndex(i);
+
+      if (cur_bp_id.GetBreakpointID() != LLDB_INVALID_BREAK_ID) {
+        if (cur_bp_id.GetLocationID() != LLDB_INVALID_BREAK_ID) {
+          Breakpoint *breakpoint =
+              target.GetBreakpointByID(cur_bp_id.GetBreakpointID()).get();
+          BreakpointLocation *location =
+              breakpoint->FindLocationByID(cur_bp_id.GetLocationID()).get();
+          // It makes no sense to try to delete individual locations, so we
+          // disable them instead.
+          if (location) {
+            location->SetEnabled(false);
+            ++disable_count;
           }
+        } else {
+          target.RemoveBreakpointByID(cur_bp_id.GetBreakpointID());
+          ++delete_count;
         }
-        result.AppendMessageWithFormat(
-            "%d breakpoints deleted; %d breakpoint locations disabled.\n",
-            delete_count, disable_count);
-        result.SetStatus(eReturnStatusSuccessFinishNoResult);
       }
     }
+    result.AppendMessageWithFormat(
+        "%d breakpoints deleted; %d breakpoint locations disabled.\n",
+        delete_count, disable_count);
+    result.SetStatus(eReturnStatusSuccessFinishNoResult);
     return result.Succeeded();
   }
 
diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py b/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
@@ -18,7 +18,7 @@ class BreakpointCommandTestCase(TestBase):
 
     @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24528")
     @skipIfReproducer # side_effect bypasses reproducer
-    def not_test_breakpoint_command_sequence(self):
+    def test_breakpoint_command_sequence(self):
         """Test a sequence of breakpoint command add, list, and delete."""
         self.build()
         self.breakpoint_command_sequence()
@@ -302,7 +302,7 @@ def test_breakpoint_delete_disabled(self):
         bp_id_1 = bp_1.GetID()
         bp_id_2 = bp_2.GetID()
         bp_id_3 = bp_3.GetID()
-        
+
         self.runCmd("breakpoint delete --disabled DeleteMeNot")
 
         bp_1 = target.FindBreakpointByID(bp_id_1)
@@ -313,3 +313,21 @@ def test_breakpoint_delete_disabled(self):
 
         bp_3 = target.FindBreakpointByID(bp_id_3)
         self.assertTrue(bp_3.IsValid(), "DeleteMeNot didn't protect disabled breakpoint 3")
+
+        # Reset the first breakpoint, disable it, and do this again with no protected name:
+        bp_1 = target.BreakpointCreateByName("main")
+
+        bp_1.SetEnabled(False)
+
+        bp_id_1 = bp_1.GetID()
+
+        self.runCmd("breakpoint delete --disabled")
+
+        bp_1 = target.FindBreakpointByID(bp_id_1)
+        self.assertFalse(bp_1.IsValid(), "Didn't delete disabled breakpoint 1")
+
+        bp_2 = target.FindBreakpointByID(bp_id_2)
+        self.assertTrue(bp_2.IsValid(), "Deleted enabled breakpoint 2")
+
+        bp_3 = target.FindBreakpointByID(bp_id_3)
+        self.assertFalse(bp_3.IsValid(), "Didn't delete disabled breakpoint 3")
