Index: include/lldb/Breakpoint/BreakpointID.h =================================================================== --- include/lldb/Breakpoint/BreakpointID.h +++ include/lldb/Breakpoint/BreakpointID.h @@ -17,6 +17,8 @@ #include "lldb/lldb-private.h" +#include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/Optional.h" #include "llvm/ADT/StringRef.h" namespace lldb_private { @@ -49,11 +51,9 @@ void GetDescription(Stream *s, lldb::DescriptionLevel level); - static bool IsRangeIdentifier(const char *str); - - static bool IsValidIDExpression(const char *str); - - static const char *g_range_specifiers[]; + static bool IsRangeIdentifier(llvm::StringRef str); + static bool IsValidIDExpression(llvm::StringRef str); + static llvm::ArrayRef GetRangeSpecifiers(); //------------------------------------------------------------------ /// Takes an input string containing the description of a breakpoint or @@ -71,9 +71,8 @@ /// \b true if the call was able to extract a breakpoint location from the /// string. \b false otherwise. //------------------------------------------------------------------ - static bool ParseCanonicalReference(const char *input, - lldb::break_id_t *break_id, - lldb::break_id_t *break_loc_id); + static llvm::Optional + ParseCanonicalReference(llvm::StringRef input); //------------------------------------------------------------------ /// Takes an input string and checks to see whether it is a breakpoint name. Index: include/lldb/Breakpoint/BreakpointIDList.h =================================================================== --- include/lldb/Breakpoint/BreakpointIDList.h +++ include/lldb/Breakpoint/BreakpointIDList.h @@ -12,6 +12,7 @@ // C Includes // C++ Includes +#include #include // Other libraries and framework includes @@ -28,6 +29,7 @@ class BreakpointIDList { public: + // TODO: Convert this class to StringRef. typedef std::vector BreakpointIDArray; BreakpointIDList(); @@ -46,6 +48,7 @@ bool AddBreakpointID(const char *bp_id); + // TODO: This should take a const BreakpointID. bool FindBreakpointID(BreakpointID &bp_id, size_t *position) const; bool FindBreakpointID(const char *bp_id, size_t *position) const; @@ -53,9 +56,11 @@ void InsertStringArray(const char **string_array, size_t array_size, CommandReturnObject &result); - static bool StringContainsIDRangeExpression(const char *in_string, - size_t *range_start_len, - size_t *range_end_pos); + // Returns a pair consisting of the beginning and end of a breakpoint + // ID range expression. If the input string is not a valid specification, + // returns an empty pair. + static std::pair + SplitIDRangeExpression(llvm::StringRef in_string); static void FindAndReplaceIDRanges(Args &old_args, Target *target, bool allow_locations, Index: source/Breakpoint/BreakpointID.cpp =================================================================== --- source/Breakpoint/BreakpointID.cpp +++ source/Breakpoint/BreakpointID.cpp @@ -26,8 +26,7 @@ BreakpointID::~BreakpointID() = default; -const char *BreakpointID::g_range_specifiers[] = {"-", "to", "To", "TO", - nullptr}; +static llvm::StringRef g_range_specifiers[] = {"-", "to", "To", "TO"}; // Tells whether or not STR is valid to use between two strings representing // breakpoint IDs, to @@ -35,25 +34,21 @@ // function so that we can // easily change or add to the format for specifying ID ranges at a later date. -bool BreakpointID::IsRangeIdentifier(const char *str) { - int specifier_count = 0; - for (int i = 0; g_range_specifiers[i] != nullptr; ++i) - ++specifier_count; - - for (int i = 0; i < specifier_count; ++i) { - if (strcmp(g_range_specifiers[i], str) == 0) +bool BreakpointID::IsRangeIdentifier(llvm::StringRef str) { + for (auto spec : g_range_specifiers) { + if (spec == str) return true; } return false; } -bool BreakpointID::IsValidIDExpression(const char *str) { - break_id_t bp_id; - break_id_t loc_id; - BreakpointID::ParseCanonicalReference(str, &bp_id, &loc_id); +bool BreakpointID::IsValidIDExpression(llvm::StringRef str) { + return BreakpointID::ParseCanonicalReference(str).hasValue(); +} - return (bp_id != LLDB_INVALID_BREAK_ID); +llvm::ArrayRef BreakpointID::GetRangeSpecifiers() { + return g_range_specifiers; } void BreakpointID::GetDescription(Stream *s, lldb::DescriptionLevel level) { @@ -78,33 +73,29 @@ s->Printf("%i.%i", bp_id, loc_id); } -bool BreakpointID::ParseCanonicalReference(const char *input, - break_id_t *break_id_ptr, - break_id_t *break_loc_id_ptr) { - *break_id_ptr = LLDB_INVALID_BREAK_ID; - *break_loc_id_ptr = LLDB_INVALID_BREAK_ID; +llvm::Optional +BreakpointID::ParseCanonicalReference(llvm::StringRef input) { + break_id_t bp_id; + break_id_t loc_id = LLDB_INVALID_BREAK_ID; - if (input == nullptr || *input == '\0') - return false; + if (input.empty()) + return llvm::None; - const char *format = "%i%n.%i%n"; - int chars_consumed_1 = 0; - int chars_consumed_2 = 0; - int n_items_parsed = ::sscanf( - input, format, - break_id_ptr, // %i parse the breakpoint ID - &chars_consumed_1, // %n gets the number of characters parsed so far - break_loc_id_ptr, // %i parse the breakpoint location ID - &chars_consumed_2); // %n gets the number of characters parsed so far - - if ((n_items_parsed == 1 && input[chars_consumed_1] == '\0') || - (n_items_parsed == 2 && input[chars_consumed_2] == '\0')) - return true; - - // Badly formatted canonical reference. - *break_id_ptr = LLDB_INVALID_BREAK_ID; - *break_loc_id_ptr = LLDB_INVALID_BREAK_ID; - return false; + // If it doesn't start with an integer, it's not valid. + if (input.consumeInteger(0, bp_id)) + return llvm::None; + + // period is optional, but if it exists, it must be followed by a number. + if (input.consume_front(".")) { + if (input.consumeInteger(0, loc_id)) + return llvm::None; + } + + // And at the end, the entire string must have been consumed. + if (!input.empty()) + return llvm::None; + + return BreakpointID(bp_id, loc_id); } bool BreakpointID::StringIsBreakpointName(llvm::StringRef str, Error &error) { Index: source/Breakpoint/BreakpointIDList.cpp =================================================================== --- source/Breakpoint/BreakpointIDList.cpp +++ source/Breakpoint/BreakpointIDList.cpp @@ -57,19 +57,11 @@ } bool BreakpointIDList::AddBreakpointID(const char *bp_id_str) { - BreakpointID temp_bp_id; - break_id_t bp_id; - break_id_t loc_id; - - bool success = - BreakpointID::ParseCanonicalReference(bp_id_str, &bp_id, &loc_id); - - if (success) { - temp_bp_id.SetID(bp_id, loc_id); - m_breakpoint_ids.push_back(temp_bp_id); - } + auto bp_id = BreakpointID::ParseCanonicalReference(bp_id_str); + if (!bp_id.hasValue()) + return false; - return success; + m_breakpoint_ids.push_back(*bp_id); } bool BreakpointIDList::FindBreakpointID(BreakpointID &bp_id, @@ -88,15 +80,11 @@ bool BreakpointIDList::FindBreakpointID(const char *bp_id_str, size_t *position) const { - BreakpointID temp_bp_id; - break_id_t bp_id; - break_id_t loc_id; - - if (BreakpointID::ParseCanonicalReference(bp_id_str, &bp_id, &loc_id)) { - temp_bp_id.SetID(bp_id, loc_id); - return FindBreakpointID(temp_bp_id, position); - } else + auto bp_id = BreakpointID::ParseCanonicalReference(bp_id_str); + if (!bp_id.hasValue()) return false; + + return FindBreakpointID(*bp_id, position); } void BreakpointIDList::InsertStringArray(const char **string_array, @@ -106,20 +94,14 @@ return; for (uint32_t i = 0; i < array_size; ++i) { - break_id_t bp_id; - break_id_t loc_id; - - if (BreakpointID::ParseCanonicalReference(string_array[i], &bp_id, - &loc_id)) { - if (bp_id != LLDB_INVALID_BREAK_ID) { - BreakpointID temp_bp_id(bp_id, loc_id); - m_breakpoint_ids.push_back(temp_bp_id); - } else { - result.AppendErrorWithFormat("'%s' is not a valid breakpoint ID.\n", - string_array[i]); - result.SetStatus(eReturnStatusFailed); - return; - } + auto bp_id = BreakpointID::ParseCanonicalReference(string_array[i]); + if (bp_id.hasValue()) { + m_breakpoint_ids.push_back(*bp_id); + } else { + result.AppendErrorWithFormat("'%s' is not a valid breakpoint ID.\n", + string_array[i]); + result.SetStatus(eReturnStatusFailed); + return; } } result.SetStatus(eReturnStatusSuccessFinishNoResult); @@ -142,9 +124,9 @@ bool allow_locations, CommandReturnObject &result, Args &new_args) { - std::string range_start; - const char *range_end; - const char *current_arg; + llvm::StringRef range_from; + llvm::StringRef range_to; + llvm::StringRef current_arg; const size_t num_old_args = old_args.GetArgumentCount(); std::set names_found; @@ -152,24 +134,22 @@ bool is_range = false; current_arg = old_args.GetArgumentAtIndex(i); - if (!allow_locations && strchr(current_arg, '.') != nullptr) { + if (!allow_locations && current_arg.contains('.')) { result.AppendErrorWithFormat( - "Breakpoint locations not allowed, saw location: %s.", current_arg); + "Breakpoint locations not allowed, saw location: %s.", + current_arg.str().c_str()); new_args.Clear(); return; } - size_t range_start_len = 0; - size_t range_end_pos = 0; + llvm::StringRef range_expr; Error error; - if (BreakpointIDList::StringContainsIDRangeExpression( - current_arg, &range_start_len, &range_end_pos)) { + std::tie(range_from, range_to) = + BreakpointIDList::SplitIDRangeExpression(current_arg); + if (!range_from.empty() && !range_to.empty()) { is_range = true; - range_start.assign(current_arg, range_start_len); - range_end = current_arg + range_end_pos; - } else if (BreakpointID::StringIsBreakpointName( - llvm::StringRef(current_arg), error)) { + } else if (BreakpointID::StringIsBreakpointName(current_arg, error)) { if (!error.Success()) { new_args.Clear(); result.AppendError(error.AsCString()); @@ -183,24 +163,23 @@ BreakpointID::IsValidIDExpression(current_arg) && BreakpointID::IsValidIDExpression( old_args.GetArgumentAtIndex(i + 2))) { - range_start.assign(current_arg); - range_end = old_args.GetArgumentAtIndex(i + 2); + range_from = current_arg; + range_to = old_args.GetArgumentAtIndex(i + 2); is_range = true; i = i + 2; } else { // See if user has specified id.* - std::string tmp_str = old_args.GetArgumentAtIndex(i); + llvm::StringRef tmp_str = old_args.GetArgumentAtIndex(i); size_t pos = tmp_str.find('.'); - if (pos != std::string::npos) { - std::string bp_id_str = tmp_str.substr(0, pos); - if (BreakpointID::IsValidIDExpression(bp_id_str.c_str()) && - tmp_str[pos + 1] == '*' && tmp_str.length() == (pos + 2)) { - break_id_t bp_id; - break_id_t bp_loc_id; - - BreakpointID::ParseCanonicalReference(bp_id_str.c_str(), &bp_id, - &bp_loc_id); - BreakpointSP breakpoint_sp = target->GetBreakpointByID(bp_id); + 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.hasValue()) + breakpoint_sp = target->GetBreakpointByID(bp_id->GetBreakpointID()); if (!breakpoint_sp) { new_args.Clear(); result.AppendErrorWithFormat("'%d' is not a valid breakpoint ID.\n", @@ -213,127 +192,120 @@ BreakpointLocation *bp_loc = breakpoint_sp->GetLocationAtIndex(j).get(); StreamString canonical_id_str; - BreakpointID::GetCanonicalReference(&canonical_id_str, bp_id, - bp_loc->GetID()); + BreakpointID::GetCanonicalReference( + &canonical_id_str, bp_id->GetBreakpointID(), bp_loc->GetID()); new_args.AppendArgument(canonical_id_str.GetString()); } } } } - if (is_range) { - break_id_t start_bp_id; - break_id_t end_bp_id; - break_id_t start_loc_id; - break_id_t end_loc_id; + if (!is_range) { + new_args.AppendArgument(current_arg); + continue; + } - BreakpointID::ParseCanonicalReference(range_start.c_str(), &start_bp_id, - &start_loc_id); - BreakpointID::ParseCanonicalReference(range_end, &end_bp_id, &end_loc_id); + auto start_bp = BreakpointID::ParseCanonicalReference(range_from); + auto end_bp = BreakpointID::ParseCanonicalReference(range_to); - if ((start_bp_id == LLDB_INVALID_BREAK_ID) || - (!target->GetBreakpointByID(start_bp_id))) { - new_args.Clear(); - result.AppendErrorWithFormat("'%s' is not a valid breakpoint ID.\n", - range_start.c_str()); - result.SetStatus(eReturnStatusFailed); - return; - } + if (!start_bp.hasValue() || + !target->GetBreakpointByID(start_bp->GetBreakpointID())) { + new_args.Clear(); + result.AppendErrorWithFormat("'%s' is not a valid breakpoint ID.\n", + range_from.str().c_str()); + result.SetStatus(eReturnStatusFailed); + return; + } - if ((end_bp_id == LLDB_INVALID_BREAK_ID) || - (!target->GetBreakpointByID(end_bp_id))) { - new_args.Clear(); - result.AppendErrorWithFormat("'%s' is not a valid breakpoint ID.\n", - range_end); - result.SetStatus(eReturnStatusFailed); - return; - } + if (!end_bp.hasValue() || + !target->GetBreakpointByID(end_bp->GetBreakpointID())) { + new_args.Clear(); + result.AppendErrorWithFormat("'%s' is not a valid breakpoint ID.\n", + range_to.str().c_str()); + result.SetStatus(eReturnStatusFailed); + return; + } + break_id_t start_bp_id = start_bp->GetBreakpointID(); + break_id_t start_loc_id = start_bp->GetLocationID(); + break_id_t end_bp_id = end_bp->GetBreakpointID(); + break_id_t end_loc_id = end_bp->GetLocationID(); + if ((start_bp_id == LLDB_INVALID_BREAK_ID) && + (end_bp_id != LLDB_INVALID_BREAK_ID) || + ((start_loc_id != LLDB_INVALID_BREAK_ID) && + (end_loc_id == LLDB_INVALID_BREAK_ID))) { + new_args.Clear(); + result.AppendErrorWithFormat("Invalid breakpoint id range: Either " + "both ends of range must specify" + " a breakpoint location, or neither can " + "specify a breakpoint location.\n"); + result.SetStatus(eReturnStatusFailed); + return; + } + + // We have valid range starting & ending breakpoint IDs. Go through all + // the breakpoints in the target and find all the breakpoints that fit + // into this range, and add them to new_args. - if (((start_loc_id == LLDB_INVALID_BREAK_ID) && - (end_loc_id != LLDB_INVALID_BREAK_ID)) || - ((start_loc_id != LLDB_INVALID_BREAK_ID) && - (end_loc_id == LLDB_INVALID_BREAK_ID))) { + // Next check to see if we have location id's. If so, make sure the + // start_bp_id and end_bp_id are for the same breakpoint; otherwise we + // have an illegal range: breakpoint id ranges that specify bp locations + // are NOT allowed to cross major bp id numbers. + + if ((start_loc_id != LLDB_INVALID_BREAK_ID) || + (end_loc_id != LLDB_INVALID_BREAK_ID)) { + if (start_bp_id != end_bp_id) { new_args.Clear(); - result.AppendErrorWithFormat("Invalid breakpoint id range: Either " - "both ends of range must specify" - " a breakpoint location, or neither can " - "specify a breakpoint location.\n"); + result.AppendErrorWithFormat( + "Invalid range: Ranges that specify particular breakpoint " + "locations" + " must be within the same major breakpoint; you specified two" + " different major breakpoints, %d and %d.\n", + start_bp_id, end_bp_id); result.SetStatus(eReturnStatusFailed); return; } + } - // We have valid range starting & ending breakpoint IDs. Go through all - // the breakpoints in the - // target and find all the breakpoints that fit into this range, and add - // them to new_args. - - // Next check to see if we have location id's. If so, make sure the - // start_bp_id and end_bp_id are - // for the same breakpoint; otherwise we have an illegal range: breakpoint - // id ranges that specify - // bp locations are NOT allowed to cross major bp id numbers. - - if ((start_loc_id != LLDB_INVALID_BREAK_ID) || - (end_loc_id != LLDB_INVALID_BREAK_ID)) { - if (start_bp_id != end_bp_id) { - new_args.Clear(); - result.AppendErrorWithFormat( - "Invalid range: Ranges that specify particular breakpoint " - "locations" - " must be within the same major breakpoint; you specified two" - " different major breakpoints, %d and %d.\n", - start_bp_id, end_bp_id); - result.SetStatus(eReturnStatusFailed); - return; - } - } - - const BreakpointList &breakpoints = target->GetBreakpointList(); - const size_t num_breakpoints = breakpoints.GetSize(); - for (size_t j = 0; j < num_breakpoints; ++j) { - Breakpoint *breakpoint = breakpoints.GetBreakpointAtIndex(j).get(); - break_id_t cur_bp_id = breakpoint->GetID(); + const BreakpointList &breakpoints = target->GetBreakpointList(); + const size_t num_breakpoints = breakpoints.GetSize(); + for (size_t j = 0; j < num_breakpoints; ++j) { + Breakpoint *breakpoint = breakpoints.GetBreakpointAtIndex(j).get(); + break_id_t cur_bp_id = breakpoint->GetID(); - if ((cur_bp_id < start_bp_id) || (cur_bp_id > end_bp_id)) - continue; + if ((cur_bp_id < start_bp_id) || (cur_bp_id > end_bp_id)) + continue; - const size_t num_locations = breakpoint->GetNumLocations(); + const size_t num_locations = breakpoint->GetNumLocations(); - if ((cur_bp_id == start_bp_id) && - (start_loc_id != LLDB_INVALID_BREAK_ID)) { - for (size_t k = 0; k < num_locations; ++k) { - BreakpointLocation *bp_loc = - breakpoint->GetLocationAtIndex(k).get(); - if ((bp_loc->GetID() >= start_loc_id) && - (bp_loc->GetID() <= end_loc_id)) { - StreamString canonical_id_str; - BreakpointID::GetCanonicalReference(&canonical_id_str, cur_bp_id, - bp_loc->GetID()); - new_args.AppendArgument(canonical_id_str.GetString()); - } + if ((cur_bp_id == start_bp_id) && + (start_loc_id != LLDB_INVALID_BREAK_ID)) { + for (size_t k = 0; k < num_locations; ++k) { + BreakpointLocation *bp_loc = breakpoint->GetLocationAtIndex(k).get(); + if ((bp_loc->GetID() >= start_loc_id) && + (bp_loc->GetID() <= end_loc_id)) { + StreamString canonical_id_str; + BreakpointID::GetCanonicalReference(&canonical_id_str, cur_bp_id, + bp_loc->GetID()); + new_args.AppendArgument(canonical_id_str.GetString()); } - } else if ((cur_bp_id == end_bp_id) && - (end_loc_id != LLDB_INVALID_BREAK_ID)) { - for (size_t k = 0; k < num_locations; ++k) { - BreakpointLocation *bp_loc = - breakpoint->GetLocationAtIndex(k).get(); - if (bp_loc->GetID() <= end_loc_id) { - StreamString canonical_id_str; - BreakpointID::GetCanonicalReference(&canonical_id_str, cur_bp_id, - bp_loc->GetID()); - new_args.AppendArgument(canonical_id_str.GetString()); - } + } + } else if ((cur_bp_id == end_bp_id) && + (end_loc_id != LLDB_INVALID_BREAK_ID)) { + for (size_t k = 0; k < num_locations; ++k) { + BreakpointLocation *bp_loc = breakpoint->GetLocationAtIndex(k).get(); + if (bp_loc->GetID() <= end_loc_id) { + StreamString canonical_id_str; + BreakpointID::GetCanonicalReference(&canonical_id_str, cur_bp_id, + bp_loc->GetID()); + new_args.AppendArgument(canonical_id_str.GetString()); } - } else { - StreamString canonical_id_str; - BreakpointID::GetCanonicalReference(&canonical_id_str, cur_bp_id, - LLDB_INVALID_BREAK_ID); - new_args.AppendArgument(canonical_id_str.GetString()); } + } else { + StreamString canonical_id_str; + BreakpointID::GetCanonicalReference(&canonical_id_str, cur_bp_id, + LLDB_INVALID_BREAK_ID); + new_args.AppendArgument(canonical_id_str.GetString()); } - } else // else is_range was false - { - new_args.AppendArgument(llvm::StringRef::withNullAsEmpty(current_arg)); } } @@ -354,45 +326,22 @@ result.SetStatus(eReturnStatusSuccessFinishNoResult); } -bool BreakpointIDList::StringContainsIDRangeExpression(const char *in_string, - size_t *range_start_len, - size_t *range_end_pos) { - bool is_range_expression = false; - std::string arg_str = in_string; - std::string::size_type idx; - std::string::size_type start_pos = 0; - - *range_start_len = 0; - *range_end_pos = 0; - - int specifiers_size = 0; - for (int i = 0; BreakpointID::g_range_specifiers[i] != nullptr; ++i) - ++specifiers_size; - - for (int i = 0; i < specifiers_size && !is_range_expression; ++i) { - const char *specifier_str = BreakpointID::g_range_specifiers[i]; - size_t len = strlen(specifier_str); - idx = arg_str.find(BreakpointID::g_range_specifiers[i]); - if (idx != std::string::npos) { - *range_start_len = idx - start_pos; - std::string start_str = arg_str.substr(start_pos, *range_start_len); - if (idx + len < arg_str.length()) { - *range_end_pos = idx + len; - std::string end_str = arg_str.substr(*range_end_pos); - if (BreakpointID::IsValidIDExpression(start_str.c_str()) && - BreakpointID::IsValidIDExpression(end_str.c_str())) { - is_range_expression = true; - //*range_start = start_str; - //*range_end = end_str; - } - } - } - } +std::pair +BreakpointIDList::SplitIDRangeExpression(llvm::StringRef in_string) { + for (auto specifier_str : BreakpointID::GetRangeSpecifiers()) { + size_t idx = in_string.find(specifier_str); + if (idx == llvm::StringRef::npos) + continue; + llvm::StringRef right1 = in_string.drop_front(idx); + + llvm::StringRef from = in_string.take_front(idx); + llvm::StringRef to = right1.drop_front(specifier_str.size()); - if (!is_range_expression) { - *range_start_len = 0; - *range_end_pos = 0; + if (BreakpointID::IsValidIDExpression(from) && + BreakpointID::IsValidIDExpression(to)) { + return std::make_pair(from, to); + } } - return is_range_expression; + return std::pair(); }