llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Jordan Rupprecht (rupprecht) <details> <summary>Changes</summary> We check if the next character after `N.` is `*` before we check its length. Using `split` on the string is cleaner and less error prone than using indices with `find` and `substr`. Note: this does not make `N.` mean anything, it just prevents assertion failures. `N.` is treated the same as an unrecognized breakpoint name: ``` (lldb) breakpoint enable 1 1 breakpoints enabled. (lldb) breakpoint enable 1.* 1 breakpoints enabled. (lldb) breakpoint enable 1. 0 breakpoints enabled. (lldb) breakpoint enable xyz 0 breakpoints enabled. ``` Found via LLDB fuzzers. --- Full diff: https://github.com/llvm/llvm-project/pull/87263.diff 2 Files Affected: - (modified) lldb/source/Breakpoint/BreakpointIDList.cpp (+22-26) - (modified) lldb/test/API/functionalities/breakpoint/breakpoint_locations/TestBreakpointLocations.py (+6) ``````````diff diff --git a/lldb/source/Breakpoint/BreakpointIDList.cpp b/lldb/source/Breakpoint/BreakpointIDList.cpp index 851d074e753588..97af1d40eb7a58 100644 --- a/lldb/source/Breakpoint/BreakpointIDList.cpp +++ b/lldb/source/Breakpoint/BreakpointIDList.cpp @@ -16,6 +16,7 @@ #include "lldb/Utility/StreamString.h" #include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/StringRef.h" using namespace lldb; using namespace lldb_private; @@ -111,32 +112,27 @@ llvm::Error BreakpointIDList::FindAndReplaceIDRanges( } else { // See if user has specified id.* llvm::StringRef tmp_str = old_args[i].ref(); - size_t pos = tmp_str.find('.'); - if (pos != llvm::StringRef::npos) { - llvm::StringRef bp_id_str = tmp_str.substr(0, pos); - if (BreakpointID::IsValidIDExpression(bp_id_str) && - tmp_str[pos + 1] == '*' && tmp_str.size() == (pos + 2)) { - - BreakpointSP breakpoint_sp; - auto bp_id = BreakpointID::ParseCanonicalReference(bp_id_str); - if (bp_id) - breakpoint_sp = target->GetBreakpointByID(bp_id->GetBreakpointID()); - if (!breakpoint_sp) { - new_args.Clear(); - return llvm::createStringError( - llvm::inconvertibleErrorCode(), - "'%d' is not a valid breakpoint ID.\n", - bp_id->GetBreakpointID()); - } - const size_t num_locations = breakpoint_sp->GetNumLocations(); - for (size_t j = 0; j < num_locations; ++j) { - BreakpointLocation *bp_loc = - breakpoint_sp->GetLocationAtIndex(j).get(); - StreamString canonical_id_str; - BreakpointID::GetCanonicalReference( - &canonical_id_str, bp_id->GetBreakpointID(), bp_loc->GetID()); - new_args.AppendArgument(canonical_id_str.GetString()); - } + auto [prefix, suffix] = tmp_str.split('.'); + if (suffix == "*" && BreakpointID::IsValidIDExpression(prefix)) { + + BreakpointSP breakpoint_sp; + auto bp_id = BreakpointID::ParseCanonicalReference(prefix); + if (bp_id) + breakpoint_sp = target->GetBreakpointByID(bp_id->GetBreakpointID()); + if (!breakpoint_sp) { + new_args.Clear(); + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "'%d' is not a valid breakpoint ID.\n", + bp_id->GetBreakpointID()); + } + const size_t num_locations = breakpoint_sp->GetNumLocations(); + for (size_t j = 0; j < num_locations; ++j) { + BreakpointLocation *bp_loc = + breakpoint_sp->GetLocationAtIndex(j).get(); + StreamString canonical_id_str; + BreakpointID::GetCanonicalReference( + &canonical_id_str, bp_id->GetBreakpointID(), bp_loc->GetID()); + new_args.AppendArgument(canonical_id_str.GetString()); } } } diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_locations/TestBreakpointLocations.py b/lldb/test/API/functionalities/breakpoint/breakpoint_locations/TestBreakpointLocations.py index 8930bea619bb6e..d87e6275f7b51e 100644 --- a/lldb/test/API/functionalities/breakpoint/breakpoint_locations/TestBreakpointLocations.py +++ b/lldb/test/API/functionalities/breakpoint/breakpoint_locations/TestBreakpointLocations.py @@ -293,6 +293,12 @@ def breakpoint_locations_test(self): startstr="3 breakpoints enabled.", ) + # The 'breakpoint enable 1.' command should not crash. + self.expect( + "breakpoint enable 1.", + startstr="0 breakpoints enabled.", + ) + # The 'breakpoint disable 1.1' command should disable 1 location. self.expect( "breakpoint disable 1.1", `````````` </details> https://github.com/llvm/llvm-project/pull/87263 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
