Author: Jim Ingham Date: 2021-07-27T13:38:09-07:00 New Revision: 0018c7123be3e090ba546fb730ed316fa2567655
URL: https://github.com/llvm/llvm-project/commit/0018c7123be3e090ba546fb730ed316fa2567655 DIFF: https://github.com/llvm/llvm-project/commit/0018c7123be3e090ba546fb730ed316fa2567655.diff LOG: Fix "break delete --disabled" with no arguments. The code that figured out which breakpoints to delete was supposed to set the result status if it found breakpoints, and then the code that actually deleted them checked that the result's status was set. The code for "break delete --disabled" failed to set the status if no "protected" breakpoints were provided. This was a confusing way to implement this, so I reworked it with early returns so it was less error prone, and added a test case for the no arguments case. Differential Revision: https://reviews.llvm.org/D106623 Added: Modified: lldb/source/Commands/CommandObjectBreakpoint.cpp lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py Removed: ################################################################################ diff --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp b/lldb/source/Commands/CommandObjectBreakpoint.cpp index ba66a12267191..722d5c4d8f472 100644 --- a/lldb/source/Commands/CommandObjectBreakpoint.cpp +++ b/lldb/source/Commands/CommandObjectBreakpoint.cpp @@ -1458,6 +1458,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( @@ -1471,67 +1472,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 index d7872ff71ce75..5f10cd7514516 100644 --- a/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py +++ b/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py @@ -313,3 +313,22 @@ 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") + _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
