From patchwork Thu Oct 26 13:10:59 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 23848 Received: (qmail 83716 invoked by alias); 26 Oct 2017 13:11:10 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 80535 invoked by uid 89); 26 Oct 2017 13:11:08 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=ws, disposition, thx, dash X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 26 Oct 2017 13:11:03 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id CE7BA5DA01; Thu, 26 Oct 2017 13:11:01 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com CE7BA5DA01 Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=palves@redhat.com Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 982FD80952; Thu, 26 Oct 2017 13:11:00 +0000 (UTC) Subject: Re: [RFA v4] enable/disable sub breakpoint range To: Xavier Roirand , gdb-patches@sourceware.org References: <83lgks1e1h.fsf@gnu.org> <57a9cdc7-a5fc-0852-8ebe-c7c32c78d475@adacore.com> <77911433-028a-ee7c-e617-eb1fd84f65c0@redhat.com> <17fbf187-6f3f-25c5-b590-50d133c50395@adacore.com> From: Pedro Alves Message-ID: <23d405d7-2b28-ccbd-6a28-20409e64f262@redhat.com> Date: Thu, 26 Oct 2017 14:10:59 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <17fbf187-6f3f-25c5-b590-50d133c50395@adacore.com> Hi Xavier, On 10/23/2017 12:07 PM, Xavier Roirand wrote: > You're right, I was not using the right command to produce the patch and > it did not work on my side either :( I've generated a new version below > and tested it successfully without the previous issue. In case of > copy/paste issue, I've attached it as well. > Thx Thanks. I got it, and have been playing with it. I noticed a few things that would need fixing. I started playing with one of the suggestions I'd be giving to see if it'd really work (get_number -> get_number_trailer), and ended up fixing the other issues I found too. Looks like you didn't address Eli's comments on the manual parts between v3 -> v4. I've done that too. At the bottom I've pasted a diff of my changes on top of your patch. I'll squash that in and post a v5 in a moment. Below I've pointed out some of the issues I found, FYI and for reference. Thanks again for the patch. > diff --git a/gdb/NEWS b/gdb/NEWS > index fbf5591..76d7cec 100644 > --- a/gdb/NEWS > +++ b/gdb/NEWS > @@ -75,6 +75,11 @@ QSetWorkingDir > * The "maintenance selftest" command now takes an optional argument to > filter the tests to be run. > > +* Breakpoint commands accept location ranges. > + > +The breakpoint commands ``enable'', and ``disable'' now accept a > +location range of breakpoints, e.g. ``1.3-5''. I think we should say "range of breakpoint locations". Here and throughout. > + > * New commands > > set|show cwd > diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c > index 32ceea7..c5a84eb 100644 > --- a/gdb/breakpoint.c > +++ b/gdb/breakpoint.c > @@ -93,9 +93,18 @@ enum exception_event_kind > > /* Prototypes for local functions. */ > > +static void disable_command (char *, int); > + > +static void enable_command (char *, int); > + > +static void map_breakpoint_number_range (std::pair > &bp_num_range, > + gdb::function_view (breakpoint *)>); > + These forward declarations are unnecessary. > static void map_breakpoint_numbers (const char *, > gdb::function_view); > > +static void enable_disable_command (char *args, int from_tty, bool > enable); > + And so is this one. These new command-related functions actually have the wrong prototype for master. Should take "const char *" instead of "char *". > static void ignore_command (char *, int); > > static void breakpoint_re_set_default (struct breakpoint *); > @@ -14160,17 +14169,51 @@ ignore_command (char *args, int from_tty) > if (from_tty) > printf_filtered ("\n"); > } > - > + > +/* Call FUNCTION on each of the breakpoints present in range defined by > + BP_NUM_RANGE as pair of integer in which BP_NUM_RANGE.FIRST is the > start > + of the breakpoint number range and BP_NUM_RANGE.SECOND is the end of > + the breakpoint number range. > + If BP_NUM_RANGE.FIRST == BP_NUM_RANGE.SECOND then the > + range is just a single breakpoint number. */ > + > +static void > +map_breakpoint_number_range (std::pair &bp_num_range, > + gdb::function_view > function) > +{ > + if (bp_num_range.first == 0) > + { > + warning (_("bad breakpoint number at or near '%d'"), > + bp_num_range.first); > + } > + else > + { > + struct breakpoint *b, *tmp; > + > + for (int i = bp_num_range.first; i <= bp_num_range.second; i++) > + { > + bool match = false; > + > + ALL_BREAKPOINTS_SAFE (b, tmp) > + if (b->number == i) > + { > + match = true; > + function (b); > + break; > + } > + if (!match) > + printf_unfiltered (_("No breakpoint number %d.\n"), i); > + } > + } > +} There's a lot of tab/space mixup in your patch. Please configure your editor to preserve tabs. I've fixed it all locally. > +/* Return the breakpoint location structure corresponding to the > + BP_NUM and LOC_NUM values. */ > + > static struct bp_location * > -find_location_by_number (const char *number) > +find_location_by_number (int bp_num, int loc_num) > { > - const char *p1; > - int bp_num; > - int loc_num; > struct breakpoint *b; > struct bp_location *loc; > > - p1 = number; > - bp_num = get_number_trailer (&p1, '.'); > - if (bp_num == 0 || p1[0] != '.') > - error (_("Bad breakpoint number '%s'"), number); > - > ALL_BREAKPOINTS (b) > if (b->number == bp_num) > { > @@ -14222,25 +14244,154 @@ find_location_by_number (const char *number) > } > > if (!b || b->number != bp_num) > - error (_("Bad breakpoint number '%s'"), number); > + error (_("Bad breakpoint number '%d'"), bp_num); > > - /* Skip the dot. */ > - ++p1; > - const char *save = p1; > - loc_num = get_number (&p1); > if (loc_num == 0) > - error (_("Bad breakpoint location number '%s'"), number); > + error (_("Bad breakpoint location number '%d'"), loc_num); > > --loc_num; > loc = b->loc; > for (;loc_num && loc; --loc_num, loc = loc->next) > ; > if (!loc) > - error (_("Bad breakpoint location number '%s'"), save); > + error (_("Bad breakpoint location number '%d'"), loc_num); This introduces a regression in the error string: Before: (gdb) disable 1.2 Bad breakpoint location number '2' (gdb) disable 1.3 Bad breakpoint location number '3' After: (gdb) disable 1.2 Bad breakpoint location number '0' (gdb) disable 1.3 Bad breakpoint location number '1' In the quoted hunk above, note how loc_num was decremented before and in the for loop. The new test exposes this, testsuite/gdb.log shows: disable 2.3-5 Bad breakpoint location number '0' (gdb) PASS: gdb.cp/locbprange.exp: disable location breakpoint range with max > existing Note the '0'. It was still a PASS because the test uses $decimal: +gdb_test "disable 2.3-5" "Bad breakpoint location number '$decimal'" \ + "disable location breakpoint range with max > existing" Those '$decimal's should instead be replaced by the right numbers. > > return loc; > } > > +/* Extract the breakpoint range defined by ARG. Return the start of > + the breakpoint range defined by BP_NUM_RANGE.FIRST and > + BP_LOC_RANGE.FIRST and the end of the breakpoint range defined by > + BP_NUM_RANGE.second and BP_LOC_RANGE.SECOND. > + > + The range may be any of the following form: > + > + x where x is breakpoint number. > + x-y where x and y are breakpoint numbers range. > + x.y where x is breakpoint number and z a location number. > + x.y-z where x is breakpoint number and y and z a location number > + range. */ > + > +static int This returns '1' on error and '0' on success. I think it'd be more natural if this returned bool/true/false. I.e. reverse the logic. (Though it seems strange that there's one code path that warns while all others error. I'll fix that as a follow up.) > +extract_bp_number_and_location (const std::string &arg, > + std::pair &bp_num_range, > + std::pair &bp_loc_range) > +{ > + std::size_t dot = arg.find ("."); > + > + if (dot != std::string::npos) > + { > + /* Handle x.y and x.y-z cases. */ > + std::size_t dash; > + std::string bp_loc; > + > + if (arg.length () == dot + 1 || dot == 0) > + error (_("bad breakpoint number at or near: '%s'"), arg.c_str ()); > + > + dash = arg.find ("-", dot + 1); > + > + bp_loc = arg.substr (dot + 1); > + const char *ptbf = arg.substr (0, dot).c_str (); This is undefined behavior. The 'arg.substr (0, dot)' parts returns a std::string by value (a deep copy of the substring), and that std::string temporary is going to die at the end of the expression (at the ';'). This means that that '.c_str()' gives you a pointer to the internal raw string of the temporary that is about to die. I.e., 'ptbf' is dangling after this statement. You could avoid that by splitting the statement in two parts: std::string sub = arg.substr (0, dot); const char *ptbf = sub.c_str (); ... which would make the 'ptbf' pointer valid for as long as 'sub' is (as long as you don't modify the string). However... > + bp_num_range.first = get_number(&ptbf); > + bp_num_range.second = bp_num_range.first; ... we can use get_number_trailer instead which completely avoids the need for the string copies. This occurs several times in this function. > + > + if (bp_num_range.first == 0) > + error (_("Bad breakpoint number '%s'"), arg.c_str ()); > + > + if (dash != std::string::npos) > + { > + /* bp_loc is range (x-z). */ > + if (arg.length () == dash + 1) > + error (_("bad breakpoint number at or near: '%s'"), > arg.c_str ()); > + dash = bp_loc.find ("-"); > + const char *ptlf = bp_loc.substr (0, dash).c_str (); > + bp_loc_range.first = get_number(&ptlf); > + const char *ptls= bp_loc.substr (dash + 1).c_str (); > + bp_loc_range.second = get_number(&ptls); Missing space before '=' while at it. > + } > + else > + { > + /* bp_loc is single value. */ > + const char *ptls= bp_loc.c_str (); Ditto. > + bp_loc_range.second = get_number(&ptls); Missing space before '('. > + bp_loc_range.first = bp_loc_range.second; > + if (bp_loc_range.first == 0) > + { > + warning (_("bad breakpoint number at or near '%s'"), > arg.c_str ()); > + return 1; > + } > + } > + } > + else > + { > + /* Handle x and x-y cases. */ > + std::size_t dash; > + > + dash = arg.find ("-"); > + bp_loc_range.first = 0; > + bp_loc_range.second = 0; > + if (dash != std::string::npos) > + { > + if (arg.length () == dash + 1 || dash == 0) > + error (_("bad breakpoint number at or near: '%s'"), > arg.c_str ()); > + > + const char *ptlf = arg.substr (0, dash).c_str (); > + bp_num_range.first = get_number (&ptlf); > + const char *ptls= arg.substr (dash + 1).c_str (); > + bp_num_range.second = get_number (&ptls); > + } > + else > + { > + const char * ptlf = arg.c_str (); > + bp_num_range.first = get_number (&ptlf); > + bp_num_range.second = bp_num_range.first; > + if (bp_num_range.first == 0) > + { > + warning (_("bad breakpoint number at or near '%s'"), > arg.c_str ()); > + return 1; > + } > + } > + } > + > + if (bp_num_range.first == 0 || bp_num_range.second == 0) > + error (_("bad breakpoint number at or near: '%s'"), arg.c_str ()); > + > + return 0; > +} > + > b/gdb/testsuite/gdb.cp/locbprange.cc > new file mode 100644 > index 0000000..ff44b50 > --- /dev/null > +++ b/gdb/testsuite/gdb.cp/locbprange.cc > @@ -0,0 +1,57 @@ > + > +#include > + > +class foo > + { > + public: > + static int overload (void); > + static int overload (char); > + static int overload (int); > + static int overload (double); > + }; > + > +void marker1() > + { > + } > + > +int main () > + { > + foo::overload (); > + foo::overload (111); > + foo::overload ('h'); > + foo::overload (3.14); > + > + marker1 (); // marker1-returns-here > + > + return 0; > + } > + > +/* Some functions to test overloading by varying one argument type. */ > + > +int foo::overload (void) > + { > + return 1; > + } > +int foo::overload (char arg) > + { > + arg = 0; > + return 2; > + } > +int foo::overload (int arg) { arg = 0; return 3;} > +int foo::overload (double arg) { arg = 0; return 4;} We follow GNU coding conventions/standards in the tests too unless the test requires otherwise. (We have old tests that predate that rule, though.) > diff --git a/gdb/testsuite/gdb.cp/locbprange.exp > b/gdb/testsuite/gdb.cp/locbprange.exp > new file mode 100644 > index 0000000..2a13791 > --- /dev/null > +++ b/gdb/testsuite/gdb.cp/locbprange.exp > @@ -0,0 +1,160 @@ > +# Copyright 2017 Free Software Foundation, Inc. > + > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see . > + > +# This file is part of the gdb testsuite Missing period. > + > +# Tests for breakpoint location range enable/disable commands. > + > +set ws "\[\r\n\t \]+" > +set nl "\[\r\n\]+" These are not used. > + > + > +if { [skip_cplus_tests] } { continue } > + > +standard_testfile .cc > + > +if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug > c++}]} { > + return -1 > +} > + > +# Set it up at a breakpoint so we can play with the variable values. We're not playing with variables. > + > +if ![runto 'marker1'] then { > + perror "couldn't run to marker1" > + continue > +} > + > +# Prevent symbol on address 0x0 being printed. > +gdb_test_no_output "set print symbol off" Not necessary for this testscase. > + > +gdb_test "up" ".*main.*" "up from marker1" Not necessary either. > + > +# Returns a buffer corresponding to what gdb replies when > +# asking for 'info breakpoint'. The parameters are all the > +# existing breakpoints enabled/disable value: 'n' or 'y'. > + > +proc set_info_breakpoint_reply {b1 b2 b21 b22 b23 b24} { I think I can see where the "set" comes from, though I think "get_info..." or "make_info" instead of "set_info..." would read a bit less surprising. > + > + set buf "Num Type\[ \]+Disp Enb Address\[ \]+What.* > +1\[\t \]+breakpoint keep $b1.* in marker1\\(\\) at .* > +\[\t \]+breakpoint already hit 1 time.* > +2\[\t \]+breakpoint\[\t \]+keep $b2\[\t \]+.* > +2.1\[\t \]+$b21.* > +2.2\[\t \]+$b22.* > +2.3\[\t \]+$b23.* > +2.4\[\t \]+$b24.*" Use multi_line. Can use something like the $ws variable here. > + > + return $buf > +} > + > +gdb_test "break foo::overload" \ > + "Breakpoint \[0-9\]+ at $hex: foo::overload. .4 locations." \ > + "set breakpoint at overload" > + > +gdb_test "info break" [set_info_breakpoint_reply y y y y y y] \ > + "breakpoint info" > + > +# Check that we can disable a breakpoint. > +gdb_test_no_output "disable 1" > + > +gdb_test "info break" [set_info_breakpoint_reply n y y y y y] \ > + "breakpoint info disable bkpt 1" This "disable" -> "check with 'info break'" pattern is repeated several times. We can move that to a helper procedure. > +gdb_test "info break" [set_info_breakpoint_reply y y y y y y] \ > + "breakpoint info enaable bkpt 2.2 to 2.3" ... which eliminates this "enabble" typo above. > +# Check that disabling an reverse location breakpoint range does > +# not work. > +gdb_test_no_output "disable 2.3-2" I think this should complain loudly instead of silently, but I'll leave that for a follow up patch. > + > +gdb_test "info break" [set_info_breakpoint_reply y y y n y y] \ > + "breakpoint info disable 2.3 to 2.3" > + > +# Check that disabling an unvalid location breakpoint range does > +# not cause unexpected behavior. > +gdb_test "disable 2.6-7" "Bad breakpoint location number '$decimal'" \ > + "disable an unvalid location breakpoint range" > + > +gdb_test "info break" [set_info_breakpoint_reply y y y n y y] \ > + "breakpoint info disable 2.6 to 2.7" > + > +# Check that disabling an invalid location breakpoint range does not > +# cause trouble. > +gdb_test_no_output "disable 2.8-6" Ditto. > + > +gdb_test "info break" [set_info_breakpoint_reply y y y n y y] \ > + "breakpoint info disable 2.8 to 2.6" Follows the diff of my changes that apply on top of your patch. (This is a "git diff -w" because there were a good number of whitespace fixes.) diff --git c/gdb/doc/gdb.texinfo w/gdb/doc/gdb.texinfo index b1346d9..29d4789 100644 --- c/gdb/doc/gdb.texinfo +++ w/gdb/doc/gdb.texinfo @@ -3927,20 +3927,15 @@ Num Type Disp Enb Address What 1.2 y 0x080486ca in void foo() at t.cc:8 @end smallexample -Each location can be individually enabled or disabled by passing +You cannot delete the individual locations from a breakpoint. However, +each location can be individually enabled or disabled by passing @var{breakpoint-number}.@var{location-number} as argument to the @code{enable} and @code{disable} commands. It's also possible to -@code{enable} and @code{disable} range of @var{location-number} -breakpoints using a @var{breakpoint-number} and two @var{location-number}, +@code{enable} and @code{disable} a range of @var{location-number} +locations using a @var{breakpoint-number} and two @var{location-number}s, in increasing order, separated by a hyphen, like -‘@var{breakpoint-number}.5-7’. -In this case, when a @var{location-number} range is given to this -command, all breakpoints belonging to this @var{breakpoint-number} -and inside that range are operated on. -Note that you cannot delete the individual locations from the list, -you can only delete the entire list of locations that belong to their -parent breakpoint (with the @kbd{delete @var{num}} command, where -@var{num} is the number of the parent breakpoint, 1 in the above example). +@kbd{@var{breakpoint-number}.@var{location-number1}-@var{location-number2}}, +in which case @value{GDBN} acts on all the locations in the range (inclusive). Disabling or enabling the parent breakpoint (@pxref{Disabling}) affects all of the locations that belong to that breakpoint. diff --git c/gdb/NEWS w/gdb/NEWS index 76d7cec..aadb7a3 100644 --- c/gdb/NEWS +++ w/gdb/NEWS @@ -75,10 +75,8 @@ QSetWorkingDir * The "maintenance selftest" command now takes an optional argument to filter the tests to be run. -* Breakpoint commands accept location ranges. - -The breakpoint commands ``enable'', and ``disable'' now accept a -location range of breakpoints, e.g. ``1.3-5''. +* The "enable", and "disable" commands now accept a range of + breakpoint locations, e.g. "enable 1.3-5". * New commands diff --git c/gdb/breakpoint.c w/gdb/breakpoint.c index b0b16c7..2e43013 100644 --- c/gdb/breakpoint.c +++ w/gdb/breakpoint.c @@ -93,18 +93,9 @@ enum exception_event_kind /* Prototypes for local functions. */ -static void disable_command (char *, int); - -static void enable_command (char *, int); - -static void map_breakpoint_number_range (std::pair &bp_num_range, - gdb::function_view); - static void map_breakpoint_numbers (const char *, gdb::function_view); -static void enable_disable_command (char *args, int from_tty, bool enable); - static void ignore_command (char *, int); static void breakpoint_re_set_default (struct breakpoint *); @@ -14169,15 +14160,12 @@ ignore_command (char *args, int from_tty) printf_filtered ("\n"); } -/* Call FUNCTION on each of the breakpoints present in range defined by - BP_NUM_RANGE as pair of integer in which BP_NUM_RANGE.FIRST is the start - of the breakpoint number range and BP_NUM_RANGE.SECOND is the end of - the breakpoint number range. - If BP_NUM_RANGE.FIRST == BP_NUM_RANGE.SECOND then the - range is just a single breakpoint number. */ + +/* Call FUNCTION on each of the breakpoints with numbers in the range + defined by BP_NUM_RANGE (an inclusive range). */ static void -map_breakpoint_number_range (std::pair &bp_num_range, +map_breakpoint_number_range (std::pair bp_num_range, gdb::function_view function) { if (bp_num_range.first == 0) @@ -14206,14 +14194,14 @@ map_breakpoint_number_range (std::pair &bp_num_range, } } -/* Call FUNCTION on each of the breakpoints - whose numbers are given in ARGS. */ +/* Call FUNCTION on each of the breakpoints whose numbers are given in + ARGS. */ static void map_breakpoint_numbers (const char *args, gdb::function_view function) { - if (args == 0 || *args == '\0') + if (args == NULL || *args == '\0') error_no_arg (_("one or more breakpoint numbers")); number_or_range_parser parser (args); @@ -14221,9 +14209,7 @@ map_breakpoint_numbers (const char *args, while (!parser.finished ()) { int num = parser.get_number (); - std::pair range = std::make_pair (num, num); - - map_breakpoint_number_range (range, function); + map_breakpoint_number_range (std::make_pair (num, num), function); } } @@ -14234,7 +14220,6 @@ static struct bp_location * find_location_by_number (int bp_num, int loc_num) { struct breakpoint *b; - struct bp_location *loc; ALL_BREAKPOINTS (b) if (b->number == bp_num) @@ -14248,117 +14233,114 @@ find_location_by_number (int bp_num, int loc_num) if (loc_num == 0) error (_("Bad breakpoint location number '%d'"), loc_num); - --loc_num; - loc = b->loc; - for (;loc_num && loc; --loc_num, loc = loc->next) - ; - if (!loc) - error (_("Bad breakpoint location number '%d'"), loc_num); - + int n = 0; + for (bp_location *loc = b->loc; loc != NULL; loc = loc->next) + if (++n == loc_num) return loc; + + error (_("Bad breakpoint location number '%d'"), loc_num); } -/* Extract the breakpoint range defined by ARG. Return the start of - the breakpoint range defined by BP_NUM_RANGE.FIRST and - BP_LOC_RANGE.FIRST and the end of the breakpoint range defined by - BP_NUM_RANGE.second and BP_LOC_RANGE.SECOND. +/* Extract the breakpoint/location range specified by ARG. Returns + the breakpoint range in BP_NUM_RANGE, and the location range in + BP_LOC_RANGE. - The range may be any of the following form: + ARG may be in any of the following forms: - x where x is breakpoint number. - x-y where x and y are breakpoint numbers range. - x.y where x is breakpoint number and z a location number. - x.y-z where x is breakpoint number and y and z a location number - range. */ + x where 'x' is a breakpoint number. + x-y where 'x' and 'y' specify a breakpoint numbers range. + x.y where 'x' is a breakpoint number and 'z' a location number. + x.y-z where 'x' is a breakpoint number and 'y' and 'z' specify a + location number range. +*/ -static int +static bool extract_bp_number_and_location (const std::string &arg, std::pair &bp_num_range, std::pair &bp_loc_range) { - std::size_t dot = arg.find ("."); + std::string::size_type dot = arg.find ('.'); if (dot != std::string::npos) { - /* Handle x.y and x.y-z cases. */ - std::size_t dash; - std::string bp_loc; + /* Handle 'x.y' and 'x.y-z' cases. */ if (arg.length () == dot + 1 || dot == 0) error (_("bad breakpoint number at or near: '%s'"), arg.c_str ()); - dash = arg.find ("-", dot + 1); - - bp_loc = arg.substr (dot + 1); - const char *ptbf = arg.substr (0, dot).c_str (); - bp_num_range.first = get_number(&ptbf); - bp_num_range.second = bp_num_range.first; - - if (bp_num_range.first == 0) + const char *ptb = arg.c_str (); + int bp_num = get_number_trailer (&ptb, '.'); + if (bp_num == 0) error (_("Bad breakpoint number '%s'"), arg.c_str ()); + bp_num_range.first = bp_num; + bp_num_range.second = bp_num; + + const char *bp_loc = &arg[dot + 1]; + std::string::size_type dash = arg.find ('-', dot + 1); if (dash != std::string::npos) { - /* bp_loc is range (x-z). */ + /* bp_loc is a range (x-z). */ if (arg.length () == dash + 1) - error (_("bad breakpoint number at or near: '%s'"), arg.c_str ()); - dash = bp_loc.find ("-"); - const char *ptlf = bp_loc.substr (0, dash).c_str (); - bp_loc_range.first = get_number(&ptlf); - const char *ptls= bp_loc.substr (dash + 1).c_str (); - bp_loc_range.second = get_number(&ptls); + error (_("bad breakpoint number at or near: '%s'"), bp_loc); + + const char *ptlf = bp_loc; + bp_loc_range.first = get_number_trailer (&ptlf, '-'); + + const char *ptls = &arg[dash + 1]; + bp_loc_range.second = get_number_trailer (&ptls, '\0'); } else { - /* bp_loc is single value. */ - const char *ptls= bp_loc.c_str (); - bp_loc_range.second = get_number(&ptls); - bp_loc_range.first = bp_loc_range.second; + /* bp_loc is a single value. */ + const char *ptls = bp_loc; + bp_loc_range.first = get_number_trailer (&ptls, '\0'); if (bp_loc_range.first == 0) { warning (_("bad breakpoint number at or near '%s'"), arg.c_str ()); - return 1; + return false; } + bp_loc_range.second = bp_loc_range.first; } } else { /* Handle x and x-y cases. */ - std::size_t dash; - - dash = arg.find ("-"); - bp_loc_range.first = 0; - bp_loc_range.second = 0; + std::string::size_type dash = arg.find ('-'); if (dash != std::string::npos) { if (arg.length () == dash + 1 || dash == 0) error (_("bad breakpoint number at or near: '%s'"), arg.c_str ()); - const char *ptlf = arg.substr (0, dash).c_str (); - bp_num_range.first = get_number (&ptlf); - const char *ptls= arg.substr (dash + 1).c_str (); - bp_num_range.second = get_number (&ptls); + const char *ptf = arg.c_str (); + bp_num_range.first = get_number_trailer (&ptf, '-'); + + const char *pts = &arg[dash + 1]; + bp_num_range.second = get_number_trailer (&pts, '\0'); } else { - const char * ptlf = arg.c_str (); - bp_num_range.first = get_number (&ptlf); - bp_num_range.second = bp_num_range.first; + const char *ptf = arg.c_str (); + bp_num_range.first = get_number (&ptf); if (bp_num_range.first == 0) { warning (_("bad breakpoint number at or near '%s'"), arg.c_str ()); - return 1; + return false; } + bp_num_range.second = bp_num_range.first; } + bp_loc_range.first = 0; + bp_loc_range.second = 0; } if (bp_num_range.first == 0 || bp_num_range.second == 0) error (_("bad breakpoint number at or near: '%s'"), arg.c_str ()); - return 0; + return true; } -/* Enable or disable a breakpoint using BP_NUMB, LOC_NUM and enable. */ +/* Enable or disable a breakpoint location BP_NUM.LOC_NUM. ENABLE + specifies whether to enable or disable. */ static void enable_disable_bp_num_loc (int bp_num, int loc_num, bool enable) @@ -14379,9 +14361,11 @@ enable_disable_bp_num_loc (int bp_num, int loc_num, bool enable) update_global_location_list (UGLL_DONT_INSERT); } -/* Enable or disable a breakpoint location range. It uses BP_NUM, - BP_LOC_RANGE.FIRST for the start of the range, BP_LOC_RANGE.SECOND for - the end of the range and enable. */ +/* Enable or disable a range of breakpoint locations. BP_NUM is the + number of the breakpoint, and BP_LOC_RANGE specifies the + (inclusive) range of location numbers of that breakpoint to + enable/disable. ENABLE specifies whether to enable or disable the + location. */ static void enable_disable_breakpoint_location_range (int bp_num, @@ -14424,12 +14408,13 @@ disable_breakpoint (struct breakpoint *bpt) observer_notify_breakpoint_modified (bpt); } -/* Enable or disable breakpoint defined in ARGS. Breakpoint may be - any of the form defined in extract_bp_number_and_location. - ENABLE enable or disable breakpoint. */ +/* Enable or disable the breakpoint(s) or breakpoint location(s) + specified in ARGS. ARGS may be in any of the formats handled by + extract_bp_number_and_location. ENABLE specifies whether to enable + or disable the breakpoints/locations. */ static void -enable_disable_command (char *args, int from_tty, bool enable) +enable_disable_command (const char *args, int from_tty, bool enable) { if (args == 0) { @@ -14450,25 +14435,23 @@ enable_disable_command (char *args, int from_tty, bool enable) while (!num.empty ()) { - std::pair bp_num_range; - std::pair bp_loc_range; + std::pair bp_num_range, bp_loc_range; - int err_ret = extract_bp_number_and_location (num.c_str(), - bp_num_range, - bp_loc_range); - if (!err_ret) + if (extract_bp_number_and_location (num, bp_num_range, bp_loc_range)) { if (bp_loc_range.first == bp_loc_range.second && bp_loc_range.first == 0) { - /* Handle breakpoint with format x or x-z only. */ + /* Handle breakpoint ids with formats 'x' or 'x-z'. */ map_breakpoint_number_range (bp_num_range, - enable ? enable_breakpoint : - disable_breakpoint); + enable + ? enable_breakpoint + : disable_breakpoint); } else { - /* Handle breakpoint with format is x.y or x.y-z */ + /* Handle breakpoint ids with formats 'x.y' or + 'x.y-z'. */ enable_disable_breakpoint_location_range (bp_num_range.first, bp_loc_range, enable); } @@ -14478,13 +14461,13 @@ enable_disable_command (char *args, int from_tty, bool enable) } } -/* The disable command disables the specified breakpoints (or all defined - breakpoints) so they once stop be effective in stopping - the inferior. ARGS may be any of the form defined in +/* The disable command disables the specified breakpoints/locations + (or all defined breakpoints) so they're no longer effective in + stopping the inferior. ARGS may be in any of the forms defined in extract_bp_number_and_location. */ static void -disable_command (char *args, int from_tty) +disable_command (const char *args, int from_tty) { enable_disable_command (args, from_tty, false); } @@ -14559,10 +14542,10 @@ enable_breakpoint (struct breakpoint *bpt) enable_breakpoint_disp (bpt, bpt->disposition, 0); } -/* The enable command enables the specified breakpoints (or all defined - breakpoints) so they once again become (or continue to be) effective - in stopping the inferior. ARGS may be any of the form defined in - extract_bp_number_and_location. */ +/* The enable command enables the specified breakpoints/locations (or + all defined breakpoints) so they once again become (or continue to + be) effective in stopping the inferior. ARGS may be in any of the + forms defined in extract_bp_number_and_location. */ static void enable_command (const char *args, int from_tty) diff --git c/gdb/testsuite/gdb.cp/locbprange.cc w/gdb/testsuite/gdb.cp/locbprange.cc index ff44b50..caae259 100644 --- c/gdb/testsuite/gdb.cp/locbprange.cc +++ w/gdb/testsuite/gdb.cp/locbprange.cc @@ -20,38 +20,49 @@ class foo { public: - static int overload (void); - static int overload (char); - static int overload (int); - static int overload (double); + static void overload (void); + static void overload (char); + static void overload (int); + static void overload (double); }; -void marker1() +void +marker () { } -int main () +int +main () { foo::overload (); foo::overload (111); foo::overload ('h'); foo::overload (3.14); - marker1 (); // marker1-returns-here + marker (); return 0; } -/* Some functions to test overloading by varying one argument type. */ +/* Some functions to test overloading by varying one argument + type. */ -int foo::overload (void) +void +foo::overload () { - return 1; } -int foo::overload (char arg) + +void +foo::overload (char arg) +{ +} + +void +foo::overload (int arg) +{ +} + +void +foo::overload (double arg) { - arg = 0; - return 2; } -int foo::overload (int arg) { arg = 0; return 3;} -int foo::overload (double arg) { arg = 0; return 4;} diff --git c/gdb/testsuite/gdb.cp/locbprange.exp w/gdb/testsuite/gdb.cp/locbprange.exp index 2a13791..0631b9f 100644 --- c/gdb/testsuite/gdb.cp/locbprange.exp +++ w/gdb/testsuite/gdb.cp/locbprange.exp @@ -13,13 +13,9 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -# This file is part of the gdb testsuite - -# Tests for breakpoint location range enable/disable commands. - -set ws "\[\r\n\t \]+" -set nl "\[\r\n\]+" +# This file is part of the gdb testsuite. +# Test the enable/disable commands with breakpoint location ranges. if { [skip_cplus_tests] } { continue } @@ -31,130 +27,104 @@ if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} { # Set it up at a breakpoint so we can play with the variable values. -if ![runto 'marker1'] then { - perror "couldn't run to marker1" - continue +if ![runto 'marker'] then { + fail "run to marker" + return -1 } -# Prevent symbol on address 0x0 being printed. -gdb_test_no_output "set print symbol off" - -gdb_test "up" ".*main.*" "up from marker1" - -# Returns a buffer corresponding to what gdb replies when -# asking for 'info breakpoint'. The parameters are all the -# existing breakpoints enabled/disable value: 'n' or 'y'. - -proc set_info_breakpoint_reply {b1 b2 b21 b22 b23 b24} { - - set buf "Num Type\[ \]+Disp Enb Address\[ \]+What.* -1\[\t \]+breakpoint keep $b1.* in marker1\\(\\) at .* -\[\t \]+breakpoint already hit 1 time.* -2\[\t \]+breakpoint\[\t \]+keep $b2\[\t \]+.* -2.1\[\t \]+$b21.* -2.2\[\t \]+$b22.* -2.3\[\t \]+$b23.* -2.4\[\t \]+$b24.*" - - return $buf +# Returns a buffer corresponding to what GDB replies when asking for +# 'info breakpoint'. The parameters are all the existing breakpoints +# enabled/disable value: 'n' or 'y'. + +proc make_info_breakpoint_reply_re {b1 b2 b21 b22 b23 b24} { + set ws "\[\t \]+" + return [multi_line \ + "Num Type${ws}Disp Enb Address${ws}What.*" \ + "1${ws}breakpoint keep ${b1}${ws}.* in marker\\(\\) at .*" \ + "${ws}breakpoint already hit 1 time.*" \ + "2${ws}breakpoint${ws}keep${ws}${b2}${ws}.*" \ + "2.1${ws}${b21}.*" \ + "2.2${ws}${b22}.*" \ + "2.3${ws}${b23}.*" \ + "2.4${ws}${b24}.*" \ + ] } gdb_test "break foo::overload" \ "Breakpoint \[0-9\]+ at $hex: foo::overload. .4 locations." \ "set breakpoint at overload" -gdb_test "info break" [set_info_breakpoint_reply y y y y y y] \ +gdb_test "info break" [make_info_breakpoint_reply_re y y y y y y] \ "breakpoint info" -# Check that we can disable a breakpoint. -gdb_test_no_output "disable 1" - -gdb_test "info break" [set_info_breakpoint_reply n y y y y y] \ - "breakpoint info disable bkpt 1" - -# Check that we can enable a breakpoint. -gdb_test_no_output "enable 1" +# Test the enable/disable commands, and check the enable/disable state +# of the breakpoints/locations in the "info break" output. CMD is the +# actual disable/enable command. The bNN parameters are the same as +# make_info_breakpoint_reply_re's. +proc test_enable_disable {cmd b1 b2 b21 b22 b23 b24} { + gdb_test_no_output $cmd -gdb_test "info break" [set_info_breakpoint_reply y y y y y y] \ - "breakpoint info enable bkpt 1" - -# Check that we can disable a breakpoint containing location breakpoints. -gdb_test_no_output "disable 2" - -gdb_test "info break" [set_info_breakpoint_reply y n y y y y] \ - "breakpoint info disable bkpt 2" - -# Check that we can enable a breakpoint containing location breakpoints. -gdb_test_no_output "enable 2" - -gdb_test "info break" [set_info_breakpoint_reply y y y y y y] \ - "breakpoint info enable bkpt 2" - -# Check that we can disable a single location breakpoint. -gdb_test_no_output "disable 2.2" - -gdb_test "info break" [set_info_breakpoint_reply y y y n y y] \ - "breakpoint info disable bkpt 2.2" - -# Check that we can enable a single location breakpoint. -gdb_test_no_output "enable 2.2" - -gdb_test "info break" [set_info_breakpoint_reply y y y y y y] \ - "breakpoint info enable bkpt 2.2" - -# Check that we can disable a location breakpoint range. -gdb_test_no_output "disable 2.2-3" + set re [make_info_breakpoint_reply_re $b1 $b2 $b21 $b22 $b23 $b24] + gdb_test "info break" $re "breakpoint info $cmd" +} -gdb_test "info break" [set_info_breakpoint_reply y y y n n y] \ - "breakpoint info disable bkpt 2.2 to 2.3" +# Check that we can disable/enable a breakpoint with a single +# location. +test_enable_disable "disable 1" n y y y y y +test_enable_disable "enable 1" y y y y y y -# Check that we can enable a location breakpoint range. -gdb_test_no_output "enable 2.2-3" +# Check that we can disable/disable a breakpoint with multiple +# locations. +test_enable_disable "disable 2" y n y y y y +test_enable_disable "enable 2" y y y y y y -gdb_test "info break" [set_info_breakpoint_reply y y y y y y] \ - "breakpoint info enaable bkpt 2.2 to 2.3" +# Check that we can disable/enable a single location breakpoint. +test_enable_disable "disable 2.2" y y y n y y +test_enable_disable "enable 2.2" y y y y y y -# Check that we can disable a location breakpoint range reduced -# to a single location. -gdb_test_no_output "disable 2.2-2" +# Check that we can disable/enable a range of breakpoint locations. +test_enable_disable "disable 2.2-3" y y y n n y +test_enable_disable "enable 2.2-3" y y y y y y -gdb_test "info break" [set_info_breakpoint_reply y y y n y y] \ - "breakpoint info disable 2.2 to 2.2" +# Check that we can disable/enable a breakpoint location range with +# START==END. +test_enable_disable "disable 2.2-2" y y y n y y +test_enable_disable "enable 2.2-2" y y y y y y # Check that we can disable a location breakpoint range with max > # existing breakpoint location. -gdb_test "disable 2.3-5" "Bad breakpoint location number '$decimal'" \ +gdb_test "disable 2.3-5" "Bad breakpoint location number '5'" \ "disable location breakpoint range with max > existing" -gdb_test "info break" [set_info_breakpoint_reply y y y n n n] \ +gdb_test "info break" [make_info_breakpoint_reply_re y y y y n n] \ "breakpoint info disable 2.3 to 2.5" # Check that we can enable a location breakpoint range with max > # existing breakpoint location. -gdb_test "enable 2.3-5" "Bad breakpoint location number '$decimal'" \ +gdb_test "enable 2.3-5" "Bad breakpoint location number '5'" \ "enable location breakpoint range with max > existing" -gdb_test "info break" [set_info_breakpoint_reply y y y n y y] \ +gdb_test "info break" [make_info_breakpoint_reply_re y y y y y y] \ "breakpoint info enable 2.3 to 2.5" -# Check that disabling an reverse location breakpoint range does -# not work. +# Check that disabling an reverse location breakpoint range does not +# work. gdb_test_no_output "disable 2.3-2" -gdb_test "info break" [set_info_breakpoint_reply y y y n y y] \ - "breakpoint info disable 2.3 to 2.3" +gdb_test "info break" [make_info_breakpoint_reply_re y y y y y y] \ + "breakpoint info disable 2.3-2" -# Check that disabling an unvalid location breakpoint range does -# not cause unexpected behavior. -gdb_test "disable 2.6-7" "Bad breakpoint location number '$decimal'" \ +# Check that disabling an invalid breakpoint location range does not +# cause unexpected behavior. +gdb_test "disable 2.6-7" "Bad breakpoint location number '6'" \ "disable an unvalid location breakpoint range" -gdb_test "info break" [set_info_breakpoint_reply y y y n y y] \ - "breakpoint info disable 2.6 to 2.7" +gdb_test "info break" [make_info_breakpoint_reply_re y y y y y y] \ + "breakpoint info disable 2.6-7" -# Check that disabling an invalid location breakpoint range does not +# Check that disabling an invalid breakpoint location range does not # cause trouble. gdb_test_no_output "disable 2.8-6" -gdb_test "info break" [set_info_breakpoint_reply y y y n y y] \ - "breakpoint info disable 2.8 to 2.6" +gdb_test "info break" [make_info_breakpoint_reply_re y y y y y y] \ + "breakpoint info disable 2.8-6"