public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFA v3] enable/disable sub breakpoint range
@ 2017-10-03 10:26 Xavier Roirand
  2017-10-03 14:50 ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Xavier Roirand @ 2017-10-03 10:26 UTC (permalink / raw)
  To: gdb-patches

Allow enabling/disabling breakpoint location range

In some cases, adding one breakpoint corresponds to multiple
places in a program thus leads GDB to define main breakpoint
number and other breakpoints using location number with syntax:

<breakpoint_number>.<location_number>

For example:
Num  Type       Disp Enb  Address    What
1    breakpoint keep y    <MULTIPLE>
1.1                  y    0x080486a2 in void foo<int>()...
1.2                  y    0x080486ca in void foo<double>()...

In this example, main breakpoint is breakpoint 1 where location
breakpoints are 1.1 and 1.2 ones.

This patch allows enable/disable a range of breakpoint locations
using syntax:

<breakpoint_number>.<first_location_number>-<last_location_number>

with inclusive last_location_number.

For instance, if adding a breakpoint to foo() generates 5 breakpoint
locations from 1.1 to 1.5 then it's now possible to enable/disable
only location breakpoint 1.3 to location breakpoint 1.5
(so 1.3, 1.4 and 1.5) using syntax:

enable 1.3-5 or disable 1.3-5

gdb/ChangeLog:

     * breakpoint.c (map_breakpoint_number_range): Create from
     map_breakpoint_numbers refactoring.
     (map_breakpoint_numbers): Refactor by calling
     map_breakpoint_number_range.
     (find_location_by_number): Change parameters from string to
     breakpoint number and location.
     (extract_bp_number_and_location): New function.
     (enable_disable_bp_num_loc): Create from enable/disable_command
     refactoring.
     (enable_disable_command): Create from enable/disable_command
     refactoring.
     (enable_command): Refactor using enable_disable_command.
     (disable_command): Refactor using enable_disable_command.
     * NEWS: Document enable/disable location range feature.

gdb/doc/ChangeLog:

     * gdb.texinfo (Set Breaks): Add documentation for location
     breakpoint range enable/disable action.

gdb/testsuite/ChangeLog:

     * gdb.cp/locbprange.exp: New test scenario.
     * gdb.cp/locbprange.cc: New test.

diff --git a/gdb/NEWS b/gdb/NEWS
index 81c21b8..62d615c 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -65,6 +65,11 @@ QStartupWithShell
  * 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''.
+
  * New commands

  set|show compile-gcc
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 8585f5e..7a9b278 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -104,9 +104,16 @@ static void disable_command (char *, int);

  static void enable_command (char *, int);

+static void map_breakpoint_number_range (std::pair <int, int> 
&bp_num_range,
+                                         gdb::function_view<void 
(breakpoint *)>);
+
  static void map_breakpoint_numbers (const char *,
  				    gdb::function_view<void (breakpoint *)>);

+static void enable_disable_command (char *args, int from_tty, bool enable);
+
+static void enable_disable_command (char *args, int from_tty, bool enable);
+
  static void ignore_command (char *, int);

  static int breakpoint_re_set_one (void *);
@@ -14317,17 +14324,51 @@ ignore_command (char *args, int from_tty)
    if (from_tty)
      printf_filtered ("\n");
  }
-\f
+
+/* 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 <int, int> &bp_num_range,
+                             gdb::function_view<void (breakpoint *)> 
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);
+        }
+    }
+}
+
  /* Call FUNCTION on each of the breakpoints
     whose numbers are given in ARGS.  */

  static void
  map_breakpoint_numbers (const char *args,
-			gdb::function_view<void (breakpoint *)> function)
+                        gdb::function_view<void (breakpoint *)> function)
  {
-  int num;
-  struct breakpoint *b, *tmp;
-
    if (args == 0 || *args == '\0')
      error_no_arg (_("one or more breakpoint numbers"));

@@ -14335,43 +14376,22 @@ map_breakpoint_numbers (const char *args,

    while (!parser.finished ())
      {
-      const char *p = parser.cur_tok ();
-      bool match = false;
+      int num = parser.get_number ();
+      std::pair <int,int> range = std::make_pair (num, num);

-      num = parser.get_number ();
-      if (num == 0)
-	{
-	  warning (_("bad breakpoint number at or near '%s'"), p);
-	}
-      else
-	{
-	  ALL_BREAKPOINTS_SAFE (b, tmp)
-	    if (b->number == num)
-	      {
-		match = true;
-		function (b);
-		break;
-	      }
-	  if (!match)
-	    printf_unfiltered (_("No breakpoint number %d.\n"), num);
-	}
+      map_breakpoint_number_range (range, function);
      }
  }

+/* 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)
        {
@@ -14379,25 +14399,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);

    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
+extract_bp_number_and_location (const std::string &arg,
+                                std::pair <int, int> &bp_num_range,
+                                std::pair <int, int> &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 ();
+      bp_num_range.first = get_number(&ptbf);
+      bp_num_range.second = bp_num_range.first;
+
+      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);
+        }
+      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;
+          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;
+}
+
+/* Enable or disable a breakpoint using BP_NUMB, LOC_NUM and enable.  */
+
+static void
+enable_disable_bp_num_loc (int bp_num, int loc_num, bool enable)
+{
+  struct bp_location *loc = find_location_by_number (bp_num, loc_num);
+  if (loc != NULL)
+    {
+      if (loc->enabled != enable)
+        {
+          loc->enabled = enable;
+          mark_breakpoint_location_modified (loc);
+        }
+      if (target_supports_enable_disable_tracepoint ()
+          && current_trace_status ()->running && loc->owner
+          && is_tracepoint (loc->owner))
+        target_disable_tracepoint (loc);
+    }
+  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.  */
+
+static void
+enable_disable_breakpoint_location_range (int bp_num,
+                                          std::pair <int, int> 
&bp_loc_range,
+                                          bool enable)
+{
+  for (int i = bp_loc_range.first; i <= bp_loc_range.second; i++)
+    enable_disable_bp_num_loc (bp_num, i, enable);
+}

  /* Set ignore-count of breakpoint number BPTNUM to COUNT.
     If from_tty is nonzero, it prints a message to that effect,
@@ -14431,8 +14580,12 @@ 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.  */
+
  static void
-disable_command (char *args, int from_tty)
+enable_disable_command (char *args, int from_tty, bool enable)
  {
    if (args == 0)
      {
@@ -14440,43 +14593,58 @@ disable_command (char *args, int from_tty)

        ALL_BREAKPOINTS (bpt)
  	if (user_breakpoint_p (bpt))
-	  disable_breakpoint (bpt);
+          {
+            if (enable)
+              enable_breakpoint (bpt);
+            else
+              disable_breakpoint (bpt);
+          }
      }
    else
      {
        std::string num = extract_arg (&args);

        while (!num.empty ())
-	{
-	  if (num.find ('.') != std::string::npos)
-	    {
-	      struct bp_location *loc = find_location_by_number (num.c_str ());
-
-	      if (loc)
-		{
-		  if (loc->enabled)
-		    {
-		      loc->enabled = 0;
-		      mark_breakpoint_location_modified (loc);
-		    }
-		  if (target_supports_enable_disable_tracepoint ()
-		      && current_trace_status ()->running && loc->owner
-		      && is_tracepoint (loc->owner))
-		    target_disable_tracepoint (loc);
-		}
-	      update_global_location_list (UGLL_DONT_INSERT);
-	    }
-	  else
-	    map_breakpoint_numbers
-	      (num.c_str (), [&] (breakpoint *b)
-	       {
-		 iterate_over_related_breakpoints (b, disable_breakpoint);
-	       });
-	  num = extract_arg (&args);
-	}
+        {
+          std::pair <int, int> bp_num_range;
+          std::pair <int, int> bp_loc_range;
+
+          int err_ret = extract_bp_number_and_location (num.c_str(),
+                                                        bp_num_range,
+                                                        bp_loc_range);
+          if (!err_ret)
+            {
+              if (bp_loc_range.first == bp_loc_range.second
+                  && bp_loc_range.first == 0)
+                {
+                  /* Handle breakpoint with format x or x-z only.  */
+                  map_breakpoint_number_range (bp_num_range,
+                                               enable ? enable_breakpoint :
+                                                 disable_breakpoint);
+                }
+              else
+                {
+                  /* Handle breakpoint with format is x.y or x.y-z */
+                  enable_disable_breakpoint_location_range
+                    (bp_num_range.first, bp_loc_range, enable);
+                }
+            }
+          num = extract_arg (&args);
+        }
      }
  }

+/* 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
+   extract_bp_number_and_location.  */
+
+static void
+disable_command (char *args, int from_tty)
+{
+  enable_disable_command (args, from_tty, false);
+}
+
  static void
  enable_breakpoint_disp (struct breakpoint *bpt, enum bpdisp disposition,
  			int count)
@@ -14549,52 +14717,13 @@ enable_breakpoint (struct breakpoint *bpt)

  /* 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.  */
+   in stopping the inferior.  ARGS may be any of the form defined in
+   extract_bp_number_and_location.  */

  static void
  enable_command (char *args, int from_tty)
  {
-  if (args == 0)
-    {
-      struct breakpoint *bpt;
-
-      ALL_BREAKPOINTS (bpt)
-	if (user_breakpoint_p (bpt))
-	  enable_breakpoint (bpt);
-    }
-  else
-    {
-      std::string num = extract_arg (&args);
-
-      while (!num.empty ())
-	{
-	  if (num.find ('.') != std::string::npos)
-	    {
-	      struct bp_location *loc = find_location_by_number (num.c_str ());
-
-	      if (loc)
-		{
-		  if (!loc->enabled)
-		    {
-		      loc->enabled = 1;
-		      mark_breakpoint_location_modified (loc);
-		    }
-		  if (target_supports_enable_disable_tracepoint ()
-		      && current_trace_status ()->running && loc->owner
-		      && is_tracepoint (loc->owner))
-		    target_enable_tracepoint (loc);
-		}
-	      update_global_location_list (UGLL_MAY_INSERT);
-	    }
-	  else
-	    map_breakpoint_numbers
-	      (num.c_str (), [&] (breakpoint *b)
-	       {
-		 iterate_over_related_breakpoints (b, enable_breakpoint);
-	       });
-	  num = extract_arg (&args);
-	}
-    }
+  enable_disable_command (args, from_tty, true);
  }

  static void
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 9905ff6..b7fe243 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -3896,13 +3896,20 @@ Num     Type           Disp Enb  Address    What

  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.  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).  Disabling or enabling
-the parent breakpoint (@pxref{Disabling}) affects all of the locations
-that belong to that breakpoint.
+@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},
+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).
+Disabling or enabling the parent breakpoint (@pxref{Disabling}) affects
+all of the locations that belong to that breakpoint.

  @cindex pending breakpoints
  It's quite common to have a breakpoint inside a shared library.
diff --git a/gdb/testsuite/gdb.cp/locbprange.cc 
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 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   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 
<http://www.gnu.org/licenses/>.  */
+
+#include <stddef.h>
+
+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;}
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 <http://www.gnu.org/licenses/>.
+
+# 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\]+"
+
+
+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.
+
+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"
+
+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 \]+<MULTIPLE>.*
+2.1\[\t \]+$b21.*
+2.2\[\t \]+$b22.*
+2.3\[\t \]+$b23.*
+2.4\[\t \]+$b24.*"
+
+    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"
+
+# Check that we can enable a breakpoint.
+gdb_test_no_output "enable 1"
+
+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"
+
+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 enable a location breakpoint range.
+gdb_test_no_output "enable 2.2-3"
+
+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 a location breakpoint range reduced
+# to a single location.
+gdb_test_no_output "disable 2.2-2"
+
+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 a location breakpoint range with max >
+# existing breakpoint location.
+gdb_test "disable 2.3-5" "Bad breakpoint location number '$decimal'" \
+    "disable location breakpoint range with max > existing"
+
+gdb_test "info break" [set_info_breakpoint_reply y y y n 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'" \
+    "enable location breakpoint range with max > existing"
+
+gdb_test "info break" [set_info_breakpoint_reply y y y n y y] \
+    "breakpoint info enable 2.3 to 2.5"
+
+# 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"
+
+# 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"
+
+gdb_test "info break" [set_info_breakpoint_reply y y y n y y] \
+    "breakpoint info disable 2.8 to 2.6"

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA v3] enable/disable sub breakpoint range
  2017-10-03 10:26 [RFA v3] enable/disable sub breakpoint range Xavier Roirand
@ 2017-10-03 14:50 ` Eli Zaretskii
  2017-10-03 16:02   ` Pedro Alves
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2017-10-03 14:50 UTC (permalink / raw)
  To: Xavier Roirand; +Cc: gdb-patches

> From: Xavier Roirand <roirand@adacore.com>
> Date: Tue, 3 Oct 2017 12:26:49 +0200
> 
> This patch allows enable/disable a range of breakpoint locations
> using syntax:
> 
> <breakpoint_number>.<first_location_number>-<last_location_number>
> 
> with inclusive last_location_number.
> 
> For instance, if adding a breakpoint to foo() generates 5 breakpoint
> locations from 1.1 to 1.5 then it's now possible to enable/disable
> only location breakpoint 1.3 to location breakpoint 1.5
> (so 1.3, 1.4 and 1.5) using syntax:
> 
> enable 1.3-5 or disable 1.3-5

What if I have, in addition to the 1.1-1.5 breakpoints also
breakpoints 4, 5, and 6 -- how do I disable 1.3, 1.4, 1.5, 4, and 5?
Do I have to say something like "disable 1.3-5.0"?

> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -65,6 +65,11 @@ QStartupWithShell
>   * 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''.
> +

This part is OK.

> +@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},
> +in increasing order, separated by a hyphen, like
> +‘@var{breakpoint-number}.5-7’.
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This should be

  @kbd{@var{breakpoint-number}.@var{location-number1}-@var{location-number2}}

or

  @kbd{@var{breakpoint-number}.5-7}

(In general, if you find yourself using `..', "..", or any similar
quoting in a Texinfo source, you can be sure something's wrong ;-)

> +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.

It is always best to avoid passive tense, as using that tends to
produce unnecessarily complicated and cumbersome text.  Suggest to
reword:

  If you use such a range of location numbers, @value{GDBN} will act
  on all the breakpoints in that range.

> +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).
> +Disabling or enabling the parent breakpoint (@pxref{Disabling}) affects
> +all of the locations that belong to that breakpoint.

This is just a complicated way of saying that the 'delete' command
doesn't support location-ranges, right?  If so, why not just say so?

Thanks.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA v3] enable/disable sub breakpoint range
  2017-10-03 14:50 ` Eli Zaretskii
@ 2017-10-03 16:02   ` Pedro Alves
  2017-10-03 16:05     ` Pedro Alves
                       ` (4 more replies)
  0 siblings, 5 replies; 19+ messages in thread
From: Pedro Alves @ 2017-10-03 16:02 UTC (permalink / raw)
  To: Eli Zaretskii, Xavier Roirand; +Cc: gdb-patches

On 10/03/2017 03:50 PM, Eli Zaretskii wrote:
>> From: Xavier Roirand <roirand@adacore.com>
>> Date: Tue, 3 Oct 2017 12:26:49 +0200
>>
>> This patch allows enable/disable a range of breakpoint locations
>> using syntax:
>>
>> <breakpoint_number>.<first_location_number>-<last_location_number>
>>
>> with inclusive last_location_number.
>>
>> For instance, if adding a breakpoint to foo() generates 5 breakpoint
>> locations from 1.1 to 1.5 then it's now possible to enable/disable
>> only location breakpoint 1.3 to location breakpoint 1.5
>> (so 1.3, 1.4 and 1.5) using syntax:
>>
>> enable 1.3-5 or disable 1.3-5
> 
> What if I have, in addition to the 1.1-1.5 breakpoints also
> breakpoints 4, 5, and 6 -- how do I disable 1.3, 1.4, 1.5, 4, and 5?
> Do I have to say something like "disable 1.3-5.0"?

I hope not.  Supposedly you could do it with:

  (gdb) delete 1.3 1.4 1.5 4 5

or:

  (gdb) delete 1-5 4-5

though I haven't looked at the implementation in detail.
I definitely thing the above is what we should aim for though.
It's very much what we support for thread ID lists, at least
(expect the support for thread ID star ranges).

I'm wondering whether it wouldn't be better to expand this section
of the manual:

 @cindex breakpoint ranges
 @cindex breakpoint lists
 @cindex ranges of breakpoints
 @cindex lists of breakpoints
 Some @value{GDBN} commands accept a space-separated list of breakpoints
 on which to operate.  A list element can be either a single breakpoint number,
 like @samp{5}, or a range of such numbers, like @samp{5-7}.
 When a breakpoint list is given to a command, all breakpoints in that list
 are operated on.

To describe locations as well.  Similarly to how we describe
"thread ID lists", where we have:

 @anchor{thread ID lists}
 @cindex thread ID lists
 Some commands accept a space-separated @dfn{thread ID list} as
 argument.  A list element can be:

 @enumerate
 @item
 A thread ID as shown in the first field of the @samp{info threads}
 display, with or without an inferior qualifier.  E.g., @samp{2.1} or
 @samp{1}.

 @item
 A range of thread numbers, again with or without an inferior
 qualifier, as in @var{inf}.@var{thr1}-@var{thr2} or
 @var{thr1}-@var{thr2}.  E.g., @samp{1.2-4} or @samp{2-4}.

 @item
 All threads of an inferior, specified with a star wildcard, with or
 without an inferior qualifier, as in @var{inf}.@code{*} (e.g.,
 @samp{1.*}) or @code{*}.  The former refers to all threads of the
 given inferior, and the latter form without an inferior qualifier
 refers to all threads of the current inferior.

 @end enumerate

 For example, if the current inferior is 1, and inferior 7 has one
 thread with ID 7.1, the thread list @samp{1 2-3 4.5 6.7-9 7.*}
 includes threads 1 to 3 of inferior 1, thread 5 of inferior 4, threads
 7 to 9 of inferior 6 and all threads of inferior 7.  That is, in
 expanded qualified form, the same as @samp{1.1 1.2 1.3 4.5 6.7 6.8 6.9
 7.1}.

Then commands that accept a thread ID list xref here.

We'd do the same to breakpoint commands, i.e., commands that take
an breakpoint/location list would xref the description of breakpoint
lists.

See commit 5d5658a1d3c3 ("Per-inferior/Inferior-qualified thread IDs")
for how that looked like before support for '*' ranges was added.

(And now I wonder whether it'd make sense to model the breakpoint
number parsing on a simplified version of the thread ID number
parsing.  See gdb/tid-parse.h / tid_range_parser.)

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA v3] enable/disable sub breakpoint range
  2017-10-03 16:02   ` Pedro Alves
@ 2017-10-03 16:05     ` Pedro Alves
  2017-10-03 16:06     ` Xavier Roirand
                       ` (3 subsequent siblings)
  4 siblings, 0 replies; 19+ messages in thread
From: Pedro Alves @ 2017-10-03 16:05 UTC (permalink / raw)
  To: Eli Zaretskii, Xavier Roirand; +Cc: gdb-patches

On 10/03/2017 05:02 PM, Pedro Alves wrote:
>> > 
>> > What if I have, in addition to the 1.1-1.5 breakpoints also
>> > breakpoints 4, 5, and 6 -- how do I disable 1.3, 1.4, 1.5, 4, and 5?
>> > Do I have to say something like "disable 1.3-5.0"?
> I hope not.  Supposedly you could do it with:
> 
>   (gdb) delete 1.3 1.4 1.5 4 5
> 
> or:
> 
>   (gdb) delete 1-5 4-5

Sorry, typo above in the last line.

I meant either:
 (gdb) delete 1.3 1.4 1.5 4 5
or:
 (gdb) delete 1.3-5 4-5

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA v3] enable/disable sub breakpoint range
  2017-10-03 16:02   ` Pedro Alves
  2017-10-03 16:05     ` Pedro Alves
@ 2017-10-03 16:06     ` Xavier Roirand
  2017-10-03 16:34     ` Eli Zaretskii
                       ` (2 subsequent siblings)
  4 siblings, 0 replies; 19+ messages in thread
From: Xavier Roirand @ 2017-10-03 16:06 UTC (permalink / raw)
  To: gdb-patches

Le 10/3/17 à 6:02 PM, Pedro Alves a écrit :
> On 10/03/2017 03:50 PM, Eli Zaretskii wrote:
>>> From: Xavier Roirand <roirand@adacore.com>
>>> Date: Tue, 3 Oct 2017 12:26:49 +0200
>>>
>>> This patch allows enable/disable a range of breakpoint locations
>>> using syntax:
>>>
>>> <breakpoint_number>.<first_location_number>-<last_location_number>
>>>
>>> with inclusive last_location_number.
>>>
>>> For instance, if adding a breakpoint to foo() generates 5 breakpoint
>>> locations from 1.1 to 1.5 then it's now possible to enable/disable
>>> only location breakpoint 1.3 to location breakpoint 1.5
>>> (so 1.3, 1.4 and 1.5) using syntax:
>>>
>>> enable 1.3-5 or disable 1.3-5
>>
>> What if I have, in addition to the 1.1-1.5 breakpoints also
>> breakpoints 4, 5, and 6 -- how do I disable 1.3, 1.4, 1.5, 4, and 5?
>> Do I have to say something like "disable 1.3-5.0"?
> 
> I hope not.  Supposedly you could do it with:
> 
>    (gdb) delete 1.3 1.4 1.5 4 5
> 
> or:
> 
>    (gdb) delete 1-5 4-5
> 

Yes that's how it is supposed to be used.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA v3] enable/disable sub breakpoint range
  2017-10-03 16:02   ` Pedro Alves
  2017-10-03 16:05     ` Pedro Alves
  2017-10-03 16:06     ` Xavier Roirand
@ 2017-10-03 16:34     ` Eli Zaretskii
  2017-10-03 16:40       ` Pedro Alves
  2017-10-06  8:54     ` Xavier Roirand
  2017-10-20 12:17     ` Xavier Roirand
  4 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2017-10-03 16:34 UTC (permalink / raw)
  To: Pedro Alves; +Cc: roirand, gdb-patches

> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Tue, 3 Oct 2017 17:02:17 +0100
> 
> > What if I have, in addition to the 1.1-1.5 breakpoints also
> > breakpoints 4, 5, and 6 -- how do I disable 1.3, 1.4, 1.5, 4, and 5?
> > Do I have to say something like "disable 1.3-5.0"?
> 
> I hope not.  Supposedly you could do it with:
> 
>   (gdb) delete 1.3 1.4 1.5 4 5

This basically means I need to give up the ranges?  Too bad.

> I'm wondering whether it wouldn't be better to expand this section
> of the manual:
> 
>  @cindex breakpoint ranges
>  @cindex breakpoint lists
>  @cindex ranges of breakpoints
>  @cindex lists of breakpoints
>  Some @value{GDBN} commands accept a space-separated list of breakpoints
>  on which to operate.  A list element can be either a single breakpoint number,
>  like @samp{5}, or a range of such numbers, like @samp{5-7}.
>  When a breakpoint list is given to a command, all breakpoints in that list
>  are operated on.

Yes, I think so.  With a cross-reference to the "Set Breaks" section,
where the .loc thing is explained.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA v3] enable/disable sub breakpoint range
  2017-10-03 16:34     ` Eli Zaretskii
@ 2017-10-03 16:40       ` Pedro Alves
  2017-10-03 17:00         ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Pedro Alves @ 2017-10-03 16:40 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: roirand, gdb-patches


On 10/03/2017 05:33 PM, Eli Zaretskii wrote:
>> Cc: gdb-patches@sourceware.org
>> From: Pedro Alves <palves@redhat.com>
>> Date: Tue, 3 Oct 2017 17:02:17 +0100
>>
>>> What if I have, in addition to the 1.1-1.5 breakpoints also
>>> breakpoints 4, 5, and 6 -- how do I disable 1.3, 1.4, 1.5, 4, and 5?
>>> Do I have to say something like "disable 1.3-5.0"?
>>
>> I hope not.  Supposedly you could do it with:
>>
>>   (gdb) delete 1.3 1.4 1.5 4 5
> 
> This basically means I need to give up the ranges?  Too bad.

The second suggestion I gave uses equivalent ranges:

 (gdb) delete 1.3-5 4-5

I don't understand why you say you have to give them up.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA v3] enable/disable sub breakpoint range
  2017-10-03 16:40       ` Pedro Alves
@ 2017-10-03 17:00         ` Eli Zaretskii
  2017-10-03 17:15           ` Pedro Alves
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2017-10-03 17:00 UTC (permalink / raw)
  To: Pedro Alves; +Cc: roirand, gdb-patches

> Cc: roirand@adacore.com, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Tue, 3 Oct 2017 17:40:45 +0100
> 
>  (gdb) delete 1.3-5 4-5
> 
> I don't understand why you say you have to give them up.

Because I now need to break a single range into several ranges.

Wouldn't it be better to use 1.3-1.5 for the subranges, and then we
could still say 1.3-5 to mean all the breakpoints between 1.3 and 5?

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA v3] enable/disable sub breakpoint range
  2017-10-03 17:00         ` Eli Zaretskii
@ 2017-10-03 17:15           ` Pedro Alves
  2017-10-03 18:32             ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Pedro Alves @ 2017-10-03 17:15 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: roirand, gdb-patches

On 10/03/2017 06:00 PM, Eli Zaretskii wrote:
>> Cc: roirand@adacore.com, gdb-patches@sourceware.org
>> From: Pedro Alves <palves@redhat.com>
>> Date: Tue, 3 Oct 2017 17:40:45 +0100
>>
>>  (gdb) delete 1.3-5 4-5
>>
>> I don't understand why you say you have to give them up.
> 
> Because I now need to break a single range into several ranges.
> 
> Wouldn't it be better to use 1.3-1.5 for the subranges, and then we
> could still say 1.3-5 to mean all the breakpoints between 1.3 and 5?

IMO, I'd rather not treat treat locations as decimals.
If we supported that, then we'd naturally want to support 1.3-4.5 as 
well as meaning all locations from 1.3 to 4.5.  I don't think
that that's going to be a normal use case at all.  Usually you'll
set a breakpoint, and then look at the list of created locations
and disable a few, all for the same breakpoint.

Also, I think it'd be very confusing if we broke from how
we treat sub ranges in other commands (such as thread ids).

And also, if we ever come up with some object as is more than
2 levels deep, like e.g., A.B.C, then I think range specifiers like 
A.B.C1-C2 is much more friendly to type (and easier to implement)
than A.B.C1-A.B.C2.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA v3] enable/disable sub breakpoint range
  2017-10-03 17:15           ` Pedro Alves
@ 2017-10-03 18:32             ` Eli Zaretskii
  0 siblings, 0 replies; 19+ messages in thread
From: Eli Zaretskii @ 2017-10-03 18:32 UTC (permalink / raw)
  To: Pedro Alves; +Cc: roirand, gdb-patches

> Cc: roirand@adacore.com, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Tue, 3 Oct 2017 18:15:39 +0100
> 
> > Wouldn't it be better to use 1.3-1.5 for the subranges, and then we
> > could still say 1.3-5 to mean all the breakpoints between 1.3 and 5?
> 
> IMO, I'd rather not treat treat locations as decimals.

Fair enough.  Let me just say that, as a user, the notion that 1.3-5
actually 'expands" into "1.3 1.4 1.5" is extremely surprising, as
that's not what I'm used to in other similar situations.  But maybe
I'm just the odd one out.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA v3] enable/disable sub breakpoint range
  2017-10-03 16:02   ` Pedro Alves
                       ` (2 preceding siblings ...)
  2017-10-03 16:34     ` Eli Zaretskii
@ 2017-10-06  8:54     ` Xavier Roirand
  2017-10-16 22:21       ` Philippe Waroquiers
  2017-10-20 12:17     ` Xavier Roirand
  4 siblings, 1 reply; 19+ messages in thread
From: Xavier Roirand @ 2017-10-06  8:54 UTC (permalink / raw)
  To: gdb-patches

Le 10/3/17 à 6:02 PM, Pedro Alves a écrit :
> We'd do the same to breakpoint commands, i.e., commands that take
> an breakpoint/location list would xref the description of breakpoint
> lists.
> 
> See commit 5d5658a1d3c3 ("Per-inferior/Inferior-qualified thread IDs")
> for how that looked like before support for '*' ranges was added.
> 
> (And now I wonder whether it'd make sense to model the breakpoint
> number parsing on a simplified version of the thread ID number
> parsing.  See gdb/tid-parse.h / tid_range_parser.)
> 
> Thanks,
> Pedro Alves >

We have several ways for achieving this, especially when taking the 
C++ization of the code in account. What do you think would be the best 
approach:

- commit patch I've done as a first step, then propose a new patch 
including '.*' support ?

- write a breakpoint location range parser similar to the tid (for 
thread) one in pure C style including the '.*' support ? In that case 
what about the C++ization ? Would it be done in the future ?

- Change the patch I've proposed to integrate '.*' support in the new 
C++ style function I wrote ?

Something else ?

Regards.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA v3] enable/disable sub breakpoint range
  2017-10-06  8:54     ` Xavier Roirand
@ 2017-10-16 22:21       ` Philippe Waroquiers
  0 siblings, 0 replies; 19+ messages in thread
From: Philippe Waroquiers @ 2017-10-16 22:21 UTC (permalink / raw)
  To: Xavier Roirand, gdb-patches

On Fri, 2017-10-06 at 10:54 +0200, Xavier Roirand wrote:
> Le 10/3/17 à 6:02 PM, Pedro Alves a écrit :
> > We'd do the same to breakpoint commands, i.e., commands that take
> > an breakpoint/location list would xref the description of
> > breakpoint
> > lists.
> > 
> > See commit 5d5658a1d3c3 ("Per-inferior/Inferior-qualified thread
> > IDs")
> > for how that looked like before support for '*' ranges was added.
> > 
> > (And now I wonder whether it'd make sense to model the breakpoint
> > number parsing on a simplified version of the thread ID number
> > parsing.  See gdb/tid-parse.h / tid_range_parser.)
> > 
> > Thanks,
> > Pedro Alves >
> 
> We have several ways for achieving this, especially when taking the 
> C++ization of the code in account. What do you think would be the
> best 
> approach:
> 
> - commit patch I've done as a first step, then propose a new patch 
> including '.*' support ?
> 
> - write a breakpoint location range parser similar to the tid (for 
> thread) one in pure C style including the '.*' support ? In that
> case 
> what about the C++ization ? Would it be done in the future ?
> 
> - Change the patch I've proposed to integrate '.*' support in the
> new 
> C++ style function I wrote ?
> 
> Something else ?
IMO, if there are a number of breakpoint related commands that benefits
from breakpoints range and/or breakpoints location range, a C++ parser
inspired from tid parser looks a good approach.

Today, however, it looks like only the enable/disable commands
are accepting a bp loc.

We might envisage to convert other commands to work on bp loc
rather than on bp, but that looks like a much bigger work.
For example, 'commands' today associates the list of commands to
run with the bp, and not with each bp loc.
Similarly, 'enable once' is on the bp, as the hit count is maintained
per bp, and not per bp loc. 
Same for 'condition', 'delete', 'enable count/delete/once'

So, with the current state of the breakpoints related commands,
IIUC, only enable/disable are ready to work with bp loc.

When another breakpoint related command is converted to work on bp
loc, then a "general bp set/bp range/bp loc/bp loc range' parser would
be interesting.

In the meantime, it looks to me that 'perfect is the enemy of good'
and that the current V3 patch looks good enough for the moment.

My 2 cents ...

Philippe
Note: at my day job, I am a (paying) Adacore customer, and I suggested
to Adacore this enhancement of enable/disable to support bp loc ranges.
So, the above 2 cents opinion that the patch is good enough might be
somewhat biased.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA v3] enable/disable sub breakpoint range
  2017-10-03 16:02   ` Pedro Alves
                       ` (3 preceding siblings ...)
  2017-10-06  8:54     ` Xavier Roirand
@ 2017-10-20 12:17     ` Xavier Roirand
  2017-10-20 14:41       ` Pedro Alves
  4 siblings, 1 reply; 19+ messages in thread
From: Xavier Roirand @ 2017-10-20 12:17 UTC (permalink / raw)
  To: gdb-patches

Hello,

Le 10/3/17 à 6:02 PM, Pedro Alves a écrit :
> On 10/03/2017 03:50 PM, Eli Zaretskii wrote:
>>> From: Xavier Roirand <roirand@adacore.com>
> 
> I'm wondering whether it wouldn't be better to expand this section
> of the manual:
> 
>   @cindex breakpoint ranges
>   @cindex breakpoint lists
>   @cindex ranges of breakpoints
>   @cindex lists of breakpoints
>   Some @value{GDBN} commands accept a space-separated list of breakpoints
>   on which to operate.  A list element can be either a single breakpoint number,
>   like @samp{5}, or a range of such numbers, like @samp{5-7}.
>   When a breakpoint list is given to a command, all breakpoints in that list
>   are operated on.
> 
> To describe locations as well.  Similarly to how we describe
> "thread ID lists", where we have:
> 
>   @anchor{thread ID lists}
>   @cindex thread ID lists
>   Some commands accept a space-separated @dfn{thread ID list} as
>   argument.  A list element can be:
> 
>   @enumerate
>   @item
>   A thread ID as shown in the first field of the @samp{info threads}
>   display, with or without an inferior qualifier.  E.g., @samp{2.1} or
>   @samp{1}.
> 
>   @item
>   A range of thread numbers, again with or without an inferior
>   qualifier, as in @var{inf}.@var{thr1}-@var{thr2} or
>   @var{thr1}-@var{thr2}.  E.g., @samp{1.2-4} or @samp{2-4}.
> 
>   @item
>   All threads of an inferior, specified with a star wildcard, with or
>   without an inferior qualifier, as in @var{inf}.@code{*} (e.g.,
>   @samp{1.*}) or @code{*}.  The former refers to all threads of the
>   given inferior, and the latter form without an inferior qualifier
>   refers to all threads of the current inferior.
> 
>   @end enumerate
> 
>   For example, if the current inferior is 1, and inferior 7 has one
>   thread with ID 7.1, the thread list @samp{1 2-3 4.5 6.7-9 7.*}
>   includes threads 1 to 3 of inferior 1, thread 5 of inferior 4, threads
>   7 to 9 of inferior 6 and all threads of inferior 7.  That is, in
>   expanded qualified form, the same as @samp{1.1 1.2 1.3 4.5 6.7 6.8 6.9
>   7.1}.
> 
> Then commands that accept a thread ID list xref here.
> 
> We'd do the same to breakpoint commands, i.e., commands that take
> an breakpoint/location list would xref the description of breakpoint
> lists.
> 
> See commit 5d5658a1d3c3 ("Per-inferior/Inferior-qualified thread IDs")
> for how that looked like before support for '*' ranges was added.
> 
> (And now I wonder whether it'd make sense to model the breakpoint
> number parsing on a simplified version of the thread ID number
> parsing.  See gdb/tid-parse.h / tid_range_parser.)
> 

Unfortunately I don't have enough time to work on a simplified version 
of the thread id number parsing in order to provide this enable/disable 
sub range feature. What I can add in my current patch is the support for 
.* notation if you think this is something really useful. Let me know 
what do you think so I can propose a new patch.

Regards.

> Thanks,
> Pedro Alves
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA v3] enable/disable sub breakpoint range
  2017-10-20 12:17     ` Xavier Roirand
@ 2017-10-20 14:41       ` Pedro Alves
  2017-10-20 14:58         ` Pedro Alves
  0 siblings, 1 reply; 19+ messages in thread
From: Pedro Alves @ 2017-10-20 14:41 UTC (permalink / raw)
  To: Xavier Roirand, gdb-patches

On 10/20/2017 01:17 PM, Xavier Roirand wrote:
> Hello,
> 
> Le 10/3/17 à 6:02 PM, Pedro Alves a écrit :
>> On 10/03/2017 03:50 PM, Eli Zaretskii wrote:
>>>> From: Xavier Roirand <roirand@adacore.com>
>>
>> I'm wondering whether it wouldn't be better to expand this section
>> of the manual:
>>
>>   @cindex breakpoint ranges
>>   @cindex breakpoint lists
>>   @cindex ranges of breakpoints
>>   @cindex lists of breakpoints
>>   Some @value{GDBN} commands accept a space-separated list of breakpoints
>>   on which to operate.  A list element can be either a single
>> breakpoint number,
>>   like @samp{5}, or a range of such numbers, like @samp{5-7}.
>>   When a breakpoint list is given to a command, all breakpoints in
>> that list
>>   are operated on.
>>
>> To describe locations as well.  Similarly to how we describe
>> "thread ID lists", where we have:
>>
>>   @anchor{thread ID lists}
>>   @cindex thread ID lists
>>   Some commands accept a space-separated @dfn{thread ID list} as
>>   argument.  A list element can be:
>>
>>   @enumerate
>>   @item
>>   A thread ID as shown in the first field of the @samp{info threads}
>>   display, with or without an inferior qualifier.  E.g., @samp{2.1} or
>>   @samp{1}.
>>
>>   @item
>>   A range of thread numbers, again with or without an inferior
>>   qualifier, as in @var{inf}.@var{thr1}-@var{thr2} or
>>   @var{thr1}-@var{thr2}.  E.g., @samp{1.2-4} or @samp{2-4}.
>>
>>   @item
>>   All threads of an inferior, specified with a star wildcard, with or
>>   without an inferior qualifier, as in @var{inf}.@code{*} (e.g.,
>>   @samp{1.*}) or @code{*}.  The former refers to all threads of the
>>   given inferior, and the latter form without an inferior qualifier
>>   refers to all threads of the current inferior.
>>
>>   @end enumerate
>>
>>   For example, if the current inferior is 1, and inferior 7 has one
>>   thread with ID 7.1, the thread list @samp{1 2-3 4.5 6.7-9 7.*}
>>   includes threads 1 to 3 of inferior 1, thread 5 of inferior 4, threads
>>   7 to 9 of inferior 6 and all threads of inferior 7.  That is, in
>>   expanded qualified form, the same as @samp{1.1 1.2 1.3 4.5 6.7 6.8 6.9
>>   7.1}.
>>
>> Then commands that accept a thread ID list xref here.
>>
>> We'd do the same to breakpoint commands, i.e., commands that take
>> an breakpoint/location list would xref the description of breakpoint
>> lists.
>>
>> See commit 5d5658a1d3c3 ("Per-inferior/Inferior-qualified thread IDs")
>> for how that looked like before support for '*' ranges was added.
>>
>> (And now I wonder whether it'd make sense to model the breakpoint
>> number parsing on a simplified version of the thread ID number
>> parsing.  See gdb/tid-parse.h / tid_range_parser.)
>>
> 
> Unfortunately I don't have enough time to work on a simplified version
> of the thread id number parsing in order to provide this enable/disable
> sub range feature. What I can add in my current patch is the support for
> .* notation if you think this is something really useful. Let me know
> what do you think so I can propose a new patch.

Eh, I never suggested to add the '*' parsing.  Above I explicitly
pointed at the commit _before_ that, even, so you could see how the
documentation looked like then.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA v3] enable/disable sub breakpoint range
  2017-10-20 14:41       ` Pedro Alves
@ 2017-10-20 14:58         ` Pedro Alves
  2017-10-23 10:07           ` [RFA v4] " Xavier Roirand
  0 siblings, 1 reply; 19+ messages in thread
From: Pedro Alves @ 2017-10-20 14:58 UTC (permalink / raw)
  To: Xavier Roirand, gdb-patches

On 10/20/2017 03:41 PM, Pedro Alves wrote:
> On 10/20/2017 01:17 PM, Xavier Roirand wrote:
>> Hello,
>>
>> Le 10/3/17 à 6:02 PM, Pedro Alves a écrit :
>>> On 10/03/2017 03:50 PM, Eli Zaretskii wrote:
>>>>> From: Xavier Roirand <roirand@adacore.com>
>>>
>>> I'm wondering whether it wouldn't be better to expand this section
>>> of the manual:
>>>
>>>   @cindex breakpoint ranges
>>>   @cindex breakpoint lists
>>>   @cindex ranges of breakpoints
>>>   @cindex lists of breakpoints
>>>   Some @value{GDBN} commands accept a space-separated list of breakpoints
>>>   on which to operate.  A list element can be either a single
>>> breakpoint number,
>>>   like @samp{5}, or a range of such numbers, like @samp{5-7}.
>>>   When a breakpoint list is given to a command, all breakpoints in
>>> that list
>>>   are operated on.
>>>
>>> To describe locations as well.  Similarly to how we describe
>>> "thread ID lists", where we have:
>>>
>>>   @anchor{thread ID lists}
>>>   @cindex thread ID lists
>>>   Some commands accept a space-separated @dfn{thread ID list} as
>>>   argument.  A list element can be:
>>>
>>>   @enumerate
>>>   @item
>>>   A thread ID as shown in the first field of the @samp{info threads}
>>>   display, with or without an inferior qualifier.  E.g., @samp{2.1} or
>>>   @samp{1}.
>>>
>>>   @item
>>>   A range of thread numbers, again with or without an inferior
>>>   qualifier, as in @var{inf}.@var{thr1}-@var{thr2} or
>>>   @var{thr1}-@var{thr2}.  E.g., @samp{1.2-4} or @samp{2-4}.
>>>
>>>   @item
>>>   All threads of an inferior, specified with a star wildcard, with or
>>>   without an inferior qualifier, as in @var{inf}.@code{*} (e.g.,
>>>   @samp{1.*}) or @code{*}.  The former refers to all threads of the
>>>   given inferior, and the latter form without an inferior qualifier
>>>   refers to all threads of the current inferior.
>>>
>>>   @end enumerate
>>>
>>>   For example, if the current inferior is 1, and inferior 7 has one
>>>   thread with ID 7.1, the thread list @samp{1 2-3 4.5 6.7-9 7.*}
>>>   includes threads 1 to 3 of inferior 1, thread 5 of inferior 4, threads
>>>   7 to 9 of inferior 6 and all threads of inferior 7.  That is, in
>>>   expanded qualified form, the same as @samp{1.1 1.2 1.3 4.5 6.7 6.8 6.9
>>>   7.1}.
>>>
>>> Then commands that accept a thread ID list xref here.
>>>
>>> We'd do the same to breakpoint commands, i.e., commands that take
>>> an breakpoint/location list would xref the description of breakpoint
>>> lists.
>>>
>>> See commit 5d5658a1d3c3 ("Per-inferior/Inferior-qualified thread IDs")
>>> for how that looked like before support for '*' ranges was added.
>>>
>>> (And now I wonder whether it'd make sense to model the breakpoint
>>> number parsing on a simplified version of the thread ID number
>>> parsing.  See gdb/tid-parse.h / tid_range_parser.)
>>>
>>
>> Unfortunately I don't have enough time to work on a simplified version
>> of the thread id number parsing in order to provide this enable/disable
>> sub range feature. What I can add in my current patch is the support for
>> .* notation if you think this is something really useful. Let me know
>> what do you think so I can propose a new patch.
> 
> Eh, I never suggested to add the '*' parsing.  Above I explicitly
> pointed at the commit _before_ that, even, so you could see how the
> documentation looked like then.

I'm having trouble applying your patch locally.  Saving your email
as an mbox file and then using 'git am' gets me:

$ git am /tmp/bps.mbox
Applying: enable/disable sub breakpoint range
/home/pedro/gdb/mygit/src/.git/rebase-apply/patch:25: trailing whitespace.
static void map_breakpoint_number_range (std::pair <int, int>
fatal: corrupt patch at line 26
Patch failed at 0001 enable/disable sub breakpoint range
The copy of the patch that failed is found in:
   /home/pedro/gdb/mygit/src/.git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

I tried fixing it manually, but failed.  I also tried creating
a patch from the email's rendered content but failed to apply
too.  :-/

Could you resend with git send-email?  That would be best.
Or, as an attachment?
(In any case, if rebased to current master too then
that'd be appreciated.)

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFA v4] enable/disable sub breakpoint range
  2017-10-20 14:58         ` Pedro Alves
@ 2017-10-23 10:07           ` Xavier Roirand
  2017-10-23 10:25             ` Pedro Alves
  0 siblings, 1 reply; 19+ messages in thread
From: Xavier Roirand @ 2017-10-23 10:07 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches

Hello,

Seems that my v3 patch sent in my previous email was not merging fine so 
here's a rebased version. Hope it will work better.

-----

     Allow enabling/disabling breakpoint location range

     In some cases, adding one breakpoint corresponds to multiple
     places in a program thus leads GDB to define main breakpoint
     number and other breakpoints using location number with syntax:

     <breakpoint_number>.<location_number>

     For example:
     Num  Type       Disp Enb  Address    What
     1    breakpoint keep y    <MULTIPLE>
     1.1                  y    0x080486a2 in void foo<int>()...
     1.2                  y    0x080486ca in void foo<double>()...

     In this example, main breakpoint is breakpoint 1 where location
     breakpoints are 1.1 and 1.2 ones.

     This patch allows enable/disable a range of breakpoint locations
     using syntax:

     <breakpoint_number>.<first_location_number>-<last_location_number>

     with inclusive last_location_number.

     For instance, if adding a breakpoint to foo() generates 5 breakpoint
     locations from 1.1 to 1.5 then it's now possible to enable/disable
     only location breakpoint 1.3 to location breakpoint 1.5
     (so 1.3, 1.4 and 1.5) using syntax:

     enable 1.3-5 or disable 1.3-5

     gdb/ChangeLog:

         * breakpoint.c (map_breakpoint_number_range): Create from
         map_breakpoint_numbers refactoring.
         (map_breakpoint_numbers): Refactor by calling
         map_breakpoint_number_range.
         (find_location_by_number): Change parameters from string to
         breakpoint number and location.
         (extract_bp_number_and_location): New function.
         (enable_disable_bp_num_loc): Create from enable/disable_command
         refactoring.
         (enable_disable_command): Create from enable/disable_command
         refactoring.
         (enable_command): Refactor using enable_disable_command.
         (disable_command): Refactor using enable_disable_command.
         * NEWS: Document enable/disable location range feature.

     gdb/doc/ChangeLog:

         * gdb.texinfo (Set Breaks): Add documentation for location
         breakpoint range enable/disable action.

     gdb/testsuite/ChangeLog:

         * gdb.cp/locbprange.exp: New test scenario.
         * gdb.cp/locbprange.cc: New test.

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''.
+
  * 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 <int, int> 
&bp_num_range,
+                                         gdb::function_view<void 
(breakpoint *)>);
+
  static void map_breakpoint_numbers (const char *,
  				    gdb::function_view<void (breakpoint *)>);

+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 *);
@@ -14160,17 +14169,51 @@ ignore_command (char *args, int from_tty)
    if (from_tty)
      printf_filtered ("\n");
  }
-\f
+
+/* 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 <int, int> &bp_num_range,
+                             gdb::function_view<void (breakpoint *)> 
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);
+        }
+    }
+}
+
  /* Call FUNCTION on each of the breakpoints
     whose numbers are given in ARGS.  */

  static void
  map_breakpoint_numbers (const char *args,
-			gdb::function_view<void (breakpoint *)> function)
+                        gdb::function_view<void (breakpoint *)> function)
  {
-  int num;
-  struct breakpoint *b, *tmp;
-
    if (args == 0 || *args == '\0')
      error_no_arg (_("one or more breakpoint numbers"));

@@ -14178,43 +14221,22 @@ map_breakpoint_numbers (const char *args,

    while (!parser.finished ())
      {
-      const char *p = parser.cur_tok ();
-      bool match = false;
+      int num = parser.get_number ();
+      std::pair <int,int> range = std::make_pair (num, num);

-      num = parser.get_number ();
-      if (num == 0)
-	{
-	  warning (_("bad breakpoint number at or near '%s'"), p);
-	}
-      else
-	{
-	  ALL_BREAKPOINTS_SAFE (b, tmp)
-	    if (b->number == num)
-	      {
-		match = true;
-		function (b);
-		break;
-	      }
-	  if (!match)
-	    printf_unfiltered (_("No breakpoint number %d.\n"), num);
-	}
+      map_breakpoint_number_range (range, function);
      }
  }

+/* 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);

    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
+extract_bp_number_and_location (const std::string &arg,
+                                std::pair <int, int> &bp_num_range,
+                                std::pair <int, int> &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 ();
+      bp_num_range.first = get_number(&ptbf);
+      bp_num_range.second = bp_num_range.first;
+
+      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);
+        }
+      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;
+          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;
+}
+
+/* Enable or disable a breakpoint using BP_NUMB, LOC_NUM and enable.  */
+
+static void
+enable_disable_bp_num_loc (int bp_num, int loc_num, bool enable)
+{
+  struct bp_location *loc = find_location_by_number (bp_num, loc_num);
+  if (loc != NULL)
+    {
+      if (loc->enabled != enable)
+        {
+          loc->enabled = enable;
+          mark_breakpoint_location_modified (loc);
+        }
+      if (target_supports_enable_disable_tracepoint ()
+          && current_trace_status ()->running && loc->owner
+          && is_tracepoint (loc->owner))
+        target_disable_tracepoint (loc);
+    }
+  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.  */
+
+static void
+enable_disable_breakpoint_location_range (int bp_num,
+                                          std::pair <int, int> 
&bp_loc_range,
+                                          bool enable)
+{
+  for (int i = bp_loc_range.first; i <= bp_loc_range.second; i++)
+    enable_disable_bp_num_loc (bp_num, i, enable);
+}

  /* Set ignore-count of breakpoint number BPTNUM to COUNT.
     If from_tty is nonzero, it prints a message to that effect,
@@ -14274,8 +14425,12 @@ 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.  */
+
  static void
-disable_command (const char *args, int from_tty)
+enable_disable_command (char *args, int from_tty, bool enable)
  {
    if (args == 0)
      {
@@ -14283,43 +14438,58 @@ disable_command (const char *args, int from_tty)

        ALL_BREAKPOINTS (bpt)
  	if (user_breakpoint_p (bpt))
-	  disable_breakpoint (bpt);
+          {
+            if (enable)
+              enable_breakpoint (bpt);
+            else
+              disable_breakpoint (bpt);
+          }
      }
    else
      {
        std::string num = extract_arg (&args);

        while (!num.empty ())
-	{
-	  if (num.find ('.') != std::string::npos)
-	    {
-	      struct bp_location *loc = find_location_by_number (num.c_str ());
-
-	      if (loc)
-		{
-		  if (loc->enabled)
-		    {
-		      loc->enabled = 0;
-		      mark_breakpoint_location_modified (loc);
-		    }
-		  if (target_supports_enable_disable_tracepoint ()
-		      && current_trace_status ()->running && loc->owner
-		      && is_tracepoint (loc->owner))
-		    target_disable_tracepoint (loc);
-		}
-	      update_global_location_list (UGLL_DONT_INSERT);
-	    }
-	  else
-	    map_breakpoint_numbers
-	      (num.c_str (), [&] (breakpoint *b)
-	       {
-		 iterate_over_related_breakpoints (b, disable_breakpoint);
-	       });
-	  num = extract_arg (&args);
-	}
+        {
+          std::pair <int, int> bp_num_range;
+          std::pair <int, int> bp_loc_range;
+
+          int err_ret = extract_bp_number_and_location (num.c_str(),
+                                                        bp_num_range,
+                                                        bp_loc_range);
+          if (!err_ret)
+            {
+              if (bp_loc_range.first == bp_loc_range.second
+                  && bp_loc_range.first == 0)
+                {
+                  /* Handle breakpoint with format x or x-z only.  */
+                  map_breakpoint_number_range (bp_num_range,
+                                               enable ? enable_breakpoint :
+                                                 disable_breakpoint);
+                }
+              else
+                {
+                  /* Handle breakpoint with format is x.y or x.y-z */
+                  enable_disable_breakpoint_location_range
+                    (bp_num_range.first, bp_loc_range, enable);
+                }
+            }
+          num = extract_arg (&args);
+        }
      }
  }

+/* 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
+   extract_bp_number_and_location.  */
+
+static void
+disable_command (char *args, int from_tty)
+{
+  enable_disable_command (args, from_tty, false);
+}
+
  static void
  enable_breakpoint_disp (struct breakpoint *bpt, enum bpdisp disposition,
  			int count)
@@ -14392,52 +14562,13 @@ enable_breakpoint (struct breakpoint *bpt)

  /* 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.  */
+   in stopping the inferior.  ARGS may be any of the form defined in
+   extract_bp_number_and_location.  */

  static void
  enable_command (const char *args, int from_tty)
  {
-  if (args == 0)
-    {
-      struct breakpoint *bpt;
-
-      ALL_BREAKPOINTS (bpt)
-	if (user_breakpoint_p (bpt))
-	  enable_breakpoint (bpt);
-    }
-  else
-    {
-      std::string num = extract_arg (&args);
-
-      while (!num.empty ())
-	{
-	  if (num.find ('.') != std::string::npos)
-	    {
-	      struct bp_location *loc = find_location_by_number (num.c_str ());
-
-	      if (loc)
-		{
-		  if (!loc->enabled)
-		    {
-		      loc->enabled = 1;
-		      mark_breakpoint_location_modified (loc);
-		    }
-		  if (target_supports_enable_disable_tracepoint ()
-		      && current_trace_status ()->running && loc->owner
-		      && is_tracepoint (loc->owner))
-		    target_enable_tracepoint (loc);
-		}
-	      update_global_location_list (UGLL_MAY_INSERT);
-	    }
-	  else
-	    map_breakpoint_numbers
-	      (num.c_str (), [&] (breakpoint *b)
-	       {
-		 iterate_over_related_breakpoints (b, enable_breakpoint);
-	       });
-	  num = extract_arg (&args);
-	}
-    }
+  enable_disable_command (args, from_tty, true);
  }

  static void
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index bfeb7a9..b1346d9 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -3929,13 +3929,20 @@ Num     Type           Disp Enb  Address    What

  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.  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).  Disabling or enabling
-the parent breakpoint (@pxref{Disabling}) affects all of the locations
-that belong to that breakpoint.
+@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},
+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).
+Disabling or enabling the parent breakpoint (@pxref{Disabling}) affects
+all of the locations that belong to that breakpoint.

  @cindex pending breakpoints
  It's quite common to have a breakpoint inside a shared library.
diff --git a/gdb/testsuite/gdb.cp/locbprange.cc 
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 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   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 
<http://www.gnu.org/licenses/>.  */
+
+#include <stddef.h>
+
+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;}
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 <http://www.gnu.org/licenses/>.
+
+# 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\]+"
+
+
+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.
+
+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"
+
+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 \]+<MULTIPLE>.*
+2.1\[\t \]+$b21.*
+2.2\[\t \]+$b22.*
+2.3\[\t \]+$b23.*
+2.4\[\t \]+$b24.*"
+
+    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"
+
+# Check that we can enable a breakpoint.
+gdb_test_no_output "enable 1"
+
+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"
+
+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 enable a location breakpoint range.
+gdb_test_no_output "enable 2.2-3"
+
+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 a location breakpoint range reduced
+# to a single location.
+gdb_test_no_output "disable 2.2-2"
+
+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 a location breakpoint range with max >
+# existing breakpoint location.
+gdb_test "disable 2.3-5" "Bad breakpoint location number '$decimal'" \
+    "disable location breakpoint range with max > existing"
+
+gdb_test "info break" [set_info_breakpoint_reply y y y n 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'" \
+    "enable location breakpoint range with max > existing"
+
+gdb_test "info break" [set_info_breakpoint_reply y y y n y y] \
+    "breakpoint info enable 2.3 to 2.5"
+
+# 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"
+
+# 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"
+
+gdb_test "info break" [set_info_breakpoint_reply y y y n y y] \
+    "breakpoint info disable 2.8 to 2.6"

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA v4] enable/disable sub breakpoint range
  2017-10-23 10:07           ` [RFA v4] " Xavier Roirand
@ 2017-10-23 10:25             ` Pedro Alves
  2017-10-23 11:07               ` Xavier Roirand
  0 siblings, 1 reply; 19+ messages in thread
From: Pedro Alves @ 2017-10-23 10:25 UTC (permalink / raw)
  To: Xavier Roirand, gdb-patches

On 10/23/2017 11:07 AM, Xavier Roirand wrote:
> Hello,
> 
> Seems that my v3 patch sent in my previous email was not merging fine so
> here's a rebased version. Hope it will work better.
> 

Hmm, I still get (against current master / f6af9f3428fa):

$ patch --dry-run -i patch.diff -p1
checking file gdb/NEWS
checking file gdb/breakpoint.c
Hunk #1 FAILED at 93.
Hunk #2 FAILED at 14160.
Hunk #3 FAILED at 14178.
Hunk #4 FAILED at 14222.
Hunk #6 FAILED at 14287.
Hunk #7 FAILED at 14396.
6 out of 7 hunks FAILED
checking file gdb/doc/gdb.texinfo
checking file gdb/testsuite/gdb.cp/locbprange.cc
checking file gdb/testsuite/gdb.cp/locbprange.exp

Not sure why...  Does it work for you if you copy paste it from your
mail client?


There's a leading indentation in all lines in the commit log below which
suggests to me that you copy/pasted the patch from the output
of "git show"?  A better way is to use "git format-patch -1" instead.
That makes it possible to use "git am" to import a patch.
Please can you try that, and send the result as an attachment.

[Please also look into git send-email for the future.  It should
make things smoother.  Joel uses that so I assume it wouldn't be
a problem on your server's end.]

> -----
> 
>     Allow enabling/disabling breakpoint location range
> 
>     In some cases, adding one breakpoint corresponds to multiple
>     places in a program thus leads GDB to define main breakpoint
>     number and other breakpoints using location number with syntax:
> 

...

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA v4] enable/disable sub breakpoint range
  2017-10-23 10:25             ` Pedro Alves
@ 2017-10-23 11:07               ` Xavier Roirand
  2017-10-26 13:11                 ` Pedro Alves
  0 siblings, 1 reply; 19+ messages in thread
From: Xavier Roirand @ 2017-10-23 11:07 UTC (permalink / raw)
  To: Pedro Alves, gdb-patches

[-- Attachment #1: Type: text/plain, Size: 26284 bytes --]

Pedro,

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

-----

 From 90ca30fc493a950fa4c9f9b413f6360bff0a96c5 Mon Sep 17 00:00:00 2001
From: Xavier Roirand <roirand@adacore.com>
Date: Mon, 2 Oct 2017 11:54:30 +0200
Subject: [PATCH] Allow enabling/disabling breakpoint location range

In some cases, adding one breakpoint corresponds to multiple
places in a program thus leads GDB to define main breakpoint
number and other breakpoints using location number with syntax:

<breakpoint_number>.<location_number>

For example:
Num  Type       Disp Enb  Address    What
1    breakpoint keep y    <MULTIPLE>
1.1                  y    0x080486a2 in void foo<int>()...
1.2                  y    0x080486ca in void foo<double>()...

In this example, main breakpoint is breakpoint 1 where location
breakpoints are 1.1 and 1.2 ones.

This patch allows enable/disable a range of breakpoint locations
using syntax:

<breakpoint_number>.<first_location_number>-<last_location_number>

with inclusive last_location_number.

For instance, if adding a breakpoint to foo() generates 5 breakpoint
locations from 1.1 to 1.5 then it's now possible to enable/disable
only location breakpoint 1.3 to location breakpoint 1.5
(so 1.3, 1.4 and 1.5) using syntax:

enable 1.3-5 or disable 1.3-5

gdb/ChangeLog:

     * breakpoint.c (map_breakpoint_number_range): Create from
     map_breakpoint_numbers refactoring.
     (map_breakpoint_numbers): Refactor by calling
     map_breakpoint_number_range.
     (find_location_by_number): Change parameters from string to
     breakpoint number and location.
     (extract_bp_number_and_location): New function.
     (enable_disable_bp_num_loc): Create from enable/disable_command
     refactoring.
     (enable_disable_command): Create from enable/disable_command
     refactoring.
     (enable_command): Refactor using enable_disable_command.
     (disable_command): Refactor using enable_disable_command.
     * NEWS: Document enable/disable location range feature.

gdb/doc/ChangeLog:

     * gdb.texinfo (Set Breaks): Add documentation for location
     breakpoint range enable/disable action.

gdb/testsuite/ChangeLog:

     * gdb.cp/locbprange.exp: New test scenario.
     * gdb.cp/locbprange.cc: New test.
---
  gdb/NEWS                            |   5 +
  gdb/breakpoint.c                    | 353 
++++++++++++++++++++++++------------
  gdb/doc/gdb.texinfo                 |  21 ++-
  gdb/testsuite/gdb.cp/locbprange.cc  |  57 ++++++
  gdb/testsuite/gdb.cp/locbprange.exp | 160 ++++++++++++++++
  5 files changed, 478 insertions(+), 118 deletions(-)
  create mode 100644 gdb/testsuite/gdb.cp/locbprange.cc
  create mode 100644 gdb/testsuite/gdb.cp/locbprange.exp

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''.
+
  * 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 <int, int> 
&bp_num_range,
+                                         gdb::function_view<void 
(breakpoint *)>);
+
  static void map_breakpoint_numbers (const char *,
  				    gdb::function_view<void (breakpoint *)>);

+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 *);
@@ -14160,17 +14169,51 @@ ignore_command (char *args, int from_tty)
    if (from_tty)
      printf_filtered ("\n");
  }
-\f
+
+/* 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 <int, int> &bp_num_range,
+                             gdb::function_view<void (breakpoint *)> 
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);
+        }
+    }
+}
+
  /* Call FUNCTION on each of the breakpoints
     whose numbers are given in ARGS.  */

  static void
  map_breakpoint_numbers (const char *args,
-			gdb::function_view<void (breakpoint *)> function)
+                        gdb::function_view<void (breakpoint *)> function)
  {
-  int num;
-  struct breakpoint *b, *tmp;
-
    if (args == 0 || *args == '\0')
      error_no_arg (_("one or more breakpoint numbers"));

@@ -14178,43 +14221,22 @@ map_breakpoint_numbers (const char *args,

    while (!parser.finished ())
      {
-      const char *p = parser.cur_tok ();
-      bool match = false;
+      int num = parser.get_number ();
+      std::pair <int,int> range = std::make_pair (num, num);

-      num = parser.get_number ();
-      if (num == 0)
-	{
-	  warning (_("bad breakpoint number at or near '%s'"), p);
-	}
-      else
-	{
-	  ALL_BREAKPOINTS_SAFE (b, tmp)
-	    if (b->number == num)
-	      {
-		match = true;
-		function (b);
-		break;
-	      }
-	  if (!match)
-	    printf_unfiltered (_("No breakpoint number %d.\n"), num);
-	}
+      map_breakpoint_number_range (range, function);
      }
  }

+/* 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);

    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
+extract_bp_number_and_location (const std::string &arg,
+                                std::pair <int, int> &bp_num_range,
+                                std::pair <int, int> &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 ();
+      bp_num_range.first = get_number(&ptbf);
+      bp_num_range.second = bp_num_range.first;
+
+      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);
+        }
+      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;
+          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;
+}
+
+/* Enable or disable a breakpoint using BP_NUMB, LOC_NUM and enable.  */
+
+static void
+enable_disable_bp_num_loc (int bp_num, int loc_num, bool enable)
+{
+  struct bp_location *loc = find_location_by_number (bp_num, loc_num);
+  if (loc != NULL)
+    {
+      if (loc->enabled != enable)
+        {
+          loc->enabled = enable;
+          mark_breakpoint_location_modified (loc);
+        }
+      if (target_supports_enable_disable_tracepoint ()
+          && current_trace_status ()->running && loc->owner
+          && is_tracepoint (loc->owner))
+        target_disable_tracepoint (loc);
+    }
+  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.  */
+
+static void
+enable_disable_breakpoint_location_range (int bp_num,
+                                          std::pair <int, int> 
&bp_loc_range,
+                                          bool enable)
+{
+  for (int i = bp_loc_range.first; i <= bp_loc_range.second; i++)
+    enable_disable_bp_num_loc (bp_num, i, enable);
+}

  /* Set ignore-count of breakpoint number BPTNUM to COUNT.
     If from_tty is nonzero, it prints a message to that effect,
@@ -14274,8 +14425,12 @@ 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.  */
+
  static void
-disable_command (const char *args, int from_tty)
+enable_disable_command (char *args, int from_tty, bool enable)
  {
    if (args == 0)
      {
@@ -14283,43 +14438,58 @@ disable_command (const char *args, int from_tty)

        ALL_BREAKPOINTS (bpt)
  	if (user_breakpoint_p (bpt))
-	  disable_breakpoint (bpt);
+          {
+            if (enable)
+              enable_breakpoint (bpt);
+            else
+              disable_breakpoint (bpt);
+          }
      }
    else
      {
        std::string num = extract_arg (&args);

        while (!num.empty ())
-	{
-	  if (num.find ('.') != std::string::npos)
-	    {
-	      struct bp_location *loc = find_location_by_number (num.c_str ());
-
-	      if (loc)
-		{
-		  if (loc->enabled)
-		    {
-		      loc->enabled = 0;
-		      mark_breakpoint_location_modified (loc);
-		    }
-		  if (target_supports_enable_disable_tracepoint ()
-		      && current_trace_status ()->running && loc->owner
-		      && is_tracepoint (loc->owner))
-		    target_disable_tracepoint (loc);
-		}
-	      update_global_location_list (UGLL_DONT_INSERT);
-	    }
-	  else
-	    map_breakpoint_numbers
-	      (num.c_str (), [&] (breakpoint *b)
-	       {
-		 iterate_over_related_breakpoints (b, disable_breakpoint);
-	       });
-	  num = extract_arg (&args);
-	}
+        {
+          std::pair <int, int> bp_num_range;
+          std::pair <int, int> bp_loc_range;
+
+          int err_ret = extract_bp_number_and_location (num.c_str(),
+                                                        bp_num_range,
+                                                        bp_loc_range);
+          if (!err_ret)
+            {
+              if (bp_loc_range.first == bp_loc_range.second
+                  && bp_loc_range.first == 0)
+                {
+                  /* Handle breakpoint with format x or x-z only.  */
+                  map_breakpoint_number_range (bp_num_range,
+                                               enable ? enable_breakpoint :
+                                                 disable_breakpoint);
+                }
+              else
+                {
+                  /* Handle breakpoint with format is x.y or x.y-z */
+                  enable_disable_breakpoint_location_range
+                    (bp_num_range.first, bp_loc_range, enable);
+                }
+            }
+          num = extract_arg (&args);
+        }
      }
  }

+/* 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
+   extract_bp_number_and_location.  */
+
+static void
+disable_command (char *args, int from_tty)
+{
+  enable_disable_command (args, from_tty, false);
+}
+
  static void
  enable_breakpoint_disp (struct breakpoint *bpt, enum bpdisp disposition,
  			int count)
@@ -14392,52 +14562,13 @@ enable_breakpoint (struct breakpoint *bpt)

  /* 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.  */
+   in stopping the inferior.  ARGS may be any of the form defined in
+   extract_bp_number_and_location.  */

  static void
  enable_command (const char *args, int from_tty)
  {
-  if (args == 0)
-    {
-      struct breakpoint *bpt;
-
-      ALL_BREAKPOINTS (bpt)
-	if (user_breakpoint_p (bpt))
-	  enable_breakpoint (bpt);
-    }
-  else
-    {
-      std::string num = extract_arg (&args);
-
-      while (!num.empty ())
-	{
-	  if (num.find ('.') != std::string::npos)
-	    {
-	      struct bp_location *loc = find_location_by_number (num.c_str ());
-
-	      if (loc)
-		{
-		  if (!loc->enabled)
-		    {
-		      loc->enabled = 1;
-		      mark_breakpoint_location_modified (loc);
-		    }
-		  if (target_supports_enable_disable_tracepoint ()
-		      && current_trace_status ()->running && loc->owner
-		      && is_tracepoint (loc->owner))
-		    target_enable_tracepoint (loc);
-		}
-	      update_global_location_list (UGLL_MAY_INSERT);
-	    }
-	  else
-	    map_breakpoint_numbers
-	      (num.c_str (), [&] (breakpoint *b)
-	       {
-		 iterate_over_related_breakpoints (b, enable_breakpoint);
-	       });
-	  num = extract_arg (&args);
-	}
-    }
+  enable_disable_command (args, from_tty, true);
  }

  static void
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index bfeb7a9..b1346d9 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -3929,13 +3929,20 @@ Num     Type           Disp Enb  Address    What

  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.  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).  Disabling or enabling
-the parent breakpoint (@pxref{Disabling}) affects all of the locations
-that belong to that breakpoint.
+@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},
+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).
+Disabling or enabling the parent breakpoint (@pxref{Disabling}) affects
+all of the locations that belong to that breakpoint.

  @cindex pending breakpoints
  It's quite common to have a breakpoint inside a shared library.
diff --git a/gdb/testsuite/gdb.cp/locbprange.cc 
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 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   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 
<http://www.gnu.org/licenses/>.  */
+
+#include <stddef.h>
+
+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;}
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 <http://www.gnu.org/licenses/>.
+
+# 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\]+"
+
+
+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.
+
+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"
+
+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 \]+<MULTIPLE>.*
+2.1\[\t \]+$b21.*
+2.2\[\t \]+$b22.*
+2.3\[\t \]+$b23.*
+2.4\[\t \]+$b24.*"
+
+    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"
+
+# Check that we can enable a breakpoint.
+gdb_test_no_output "enable 1"
+
+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"
+
+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 enable a location breakpoint range.
+gdb_test_no_output "enable 2.2-3"
+
+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 a location breakpoint range reduced
+# to a single location.
+gdb_test_no_output "disable 2.2-2"
+
+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 a location breakpoint range with max >
+# existing breakpoint location.
+gdb_test "disable 2.3-5" "Bad breakpoint location number '$decimal'" \
+    "disable location breakpoint range with max > existing"
+
+gdb_test "info break" [set_info_breakpoint_reply y y y n 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'" \
+    "enable location breakpoint range with max > existing"
+
+gdb_test "info break" [set_info_breakpoint_reply y y y n y y] \
+    "breakpoint info enable 2.3 to 2.5"
+
+# 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"
+
+# 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"
+
+gdb_test "info break" [set_info_breakpoint_reply y y y n y y] \
+    "breakpoint info disable 2.8 to 2.6"
-- 
2.7.4


[-- Attachment #2: 0001-Allow-enabling-disabling-breakpoint-location-range.patch --]
[-- Type: text/plain, Size: 25940 bytes --]

From 90ca30fc493a950fa4c9f9b413f6360bff0a96c5 Mon Sep 17 00:00:00 2001
From: Xavier Roirand <roirand@adacore.com>
Date: Mon, 2 Oct 2017 11:54:30 +0200
Subject: [PATCH] Allow enabling/disabling breakpoint location range

In some cases, adding one breakpoint corresponds to multiple
places in a program thus leads GDB to define main breakpoint
number and other breakpoints using location number with syntax:

<breakpoint_number>.<location_number>

For example:
Num  Type       Disp Enb  Address    What
1    breakpoint keep y    <MULTIPLE>
1.1                  y    0x080486a2 in void foo<int>()...
1.2                  y    0x080486ca in void foo<double>()...

In this example, main breakpoint is breakpoint 1 where location
breakpoints are 1.1 and 1.2 ones.

This patch allows enable/disable a range of breakpoint locations
using syntax:

<breakpoint_number>.<first_location_number>-<last_location_number>

with inclusive last_location_number.

For instance, if adding a breakpoint to foo() generates 5 breakpoint
locations from 1.1 to 1.5 then it's now possible to enable/disable
only location breakpoint 1.3 to location breakpoint 1.5
(so 1.3, 1.4 and 1.5) using syntax:

enable 1.3-5 or disable 1.3-5

gdb/ChangeLog:

    * breakpoint.c (map_breakpoint_number_range): Create from
    map_breakpoint_numbers refactoring.
    (map_breakpoint_numbers): Refactor by calling
    map_breakpoint_number_range.
    (find_location_by_number): Change parameters from string to
    breakpoint number and location.
    (extract_bp_number_and_location): New function.
    (enable_disable_bp_num_loc): Create from enable/disable_command
    refactoring.
    (enable_disable_command): Create from enable/disable_command
    refactoring.
    (enable_command): Refactor using enable_disable_command.
    (disable_command): Refactor using enable_disable_command.
    * NEWS: Document enable/disable location range feature.

gdb/doc/ChangeLog:

    * gdb.texinfo (Set Breaks): Add documentation for location
    breakpoint range enable/disable action.

gdb/testsuite/ChangeLog:

    * gdb.cp/locbprange.exp: New test scenario.
    * gdb.cp/locbprange.cc: New test.
---
 gdb/NEWS                            |   5 +
 gdb/breakpoint.c                    | 353 ++++++++++++++++++++++++------------
 gdb/doc/gdb.texinfo                 |  21 ++-
 gdb/testsuite/gdb.cp/locbprange.cc  |  57 ++++++
 gdb/testsuite/gdb.cp/locbprange.exp | 160 ++++++++++++++++
 5 files changed, 478 insertions(+), 118 deletions(-)
 create mode 100644 gdb/testsuite/gdb.cp/locbprange.cc
 create mode 100644 gdb/testsuite/gdb.cp/locbprange.exp

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''.
+
 * 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 <int, int> &bp_num_range,
+                                         gdb::function_view<void (breakpoint *)>);
+
 static void map_breakpoint_numbers (const char *,
 				    gdb::function_view<void (breakpoint *)>);
 
+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 *);
@@ -14160,17 +14169,51 @@ ignore_command (char *args, int from_tty)
   if (from_tty)
     printf_filtered ("\n");
 }
-\f
+
+/* 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 <int, int> &bp_num_range,
+                             gdb::function_view<void (breakpoint *)> 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);
+        }
+    }
+}
+
 /* Call FUNCTION on each of the breakpoints
    whose numbers are given in ARGS.  */
 
 static void
 map_breakpoint_numbers (const char *args,
-			gdb::function_view<void (breakpoint *)> function)
+                        gdb::function_view<void (breakpoint *)> function)
 {
-  int num;
-  struct breakpoint *b, *tmp;
-
   if (args == 0 || *args == '\0')
     error_no_arg (_("one or more breakpoint numbers"));
 
@@ -14178,43 +14221,22 @@ map_breakpoint_numbers (const char *args,
 
   while (!parser.finished ())
     {
-      const char *p = parser.cur_tok ();
-      bool match = false;
+      int num = parser.get_number ();
+      std::pair <int,int> range = std::make_pair (num, num);
 
-      num = parser.get_number ();
-      if (num == 0)
-	{
-	  warning (_("bad breakpoint number at or near '%s'"), p);
-	}
-      else
-	{
-	  ALL_BREAKPOINTS_SAFE (b, tmp)
-	    if (b->number == num)
-	      {
-		match = true;
-		function (b);
-		break;
-	      }
-	  if (!match)
-	    printf_unfiltered (_("No breakpoint number %d.\n"), num);
-	}
+      map_breakpoint_number_range (range, function);
     }
 }
 
+/* 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);
     
   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
+extract_bp_number_and_location (const std::string &arg,
+                                std::pair <int, int> &bp_num_range,
+                                std::pair <int, int> &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 ();
+      bp_num_range.first = get_number(&ptbf);
+      bp_num_range.second = bp_num_range.first;
+
+      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);
+        }
+      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;
+          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;
+}
+
+/* Enable or disable a breakpoint using BP_NUMB, LOC_NUM and enable.  */
+
+static void
+enable_disable_bp_num_loc (int bp_num, int loc_num, bool enable)
+{
+  struct bp_location *loc = find_location_by_number (bp_num, loc_num);
+  if (loc != NULL)
+    {
+      if (loc->enabled != enable)
+        {
+          loc->enabled = enable;
+          mark_breakpoint_location_modified (loc);
+        }
+      if (target_supports_enable_disable_tracepoint ()
+          && current_trace_status ()->running && loc->owner
+          && is_tracepoint (loc->owner))
+        target_disable_tracepoint (loc);
+    }
+  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.  */
+
+static void
+enable_disable_breakpoint_location_range (int bp_num,
+                                          std::pair <int, int> &bp_loc_range,
+                                          bool enable)
+{
+  for (int i = bp_loc_range.first; i <= bp_loc_range.second; i++)
+    enable_disable_bp_num_loc (bp_num, i, enable);
+}
 
 /* Set ignore-count of breakpoint number BPTNUM to COUNT.
    If from_tty is nonzero, it prints a message to that effect,
@@ -14274,8 +14425,12 @@ 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.  */
+
 static void
-disable_command (const char *args, int from_tty)
+enable_disable_command (char *args, int from_tty, bool enable)
 {
   if (args == 0)
     {
@@ -14283,43 +14438,58 @@ disable_command (const char *args, int from_tty)
 
       ALL_BREAKPOINTS (bpt)
 	if (user_breakpoint_p (bpt))
-	  disable_breakpoint (bpt);
+          {
+            if (enable)
+              enable_breakpoint (bpt);
+            else
+              disable_breakpoint (bpt);
+          }
     }
   else
     {
       std::string num = extract_arg (&args);
 
       while (!num.empty ())
-	{
-	  if (num.find ('.') != std::string::npos)
-	    {
-	      struct bp_location *loc = find_location_by_number (num.c_str ());
-
-	      if (loc)
-		{
-		  if (loc->enabled)
-		    {
-		      loc->enabled = 0;
-		      mark_breakpoint_location_modified (loc);
-		    }
-		  if (target_supports_enable_disable_tracepoint ()
-		      && current_trace_status ()->running && loc->owner
-		      && is_tracepoint (loc->owner))
-		    target_disable_tracepoint (loc);
-		}
-	      update_global_location_list (UGLL_DONT_INSERT);
-	    }
-	  else
-	    map_breakpoint_numbers
-	      (num.c_str (), [&] (breakpoint *b)
-	       {
-		 iterate_over_related_breakpoints (b, disable_breakpoint);
-	       });
-	  num = extract_arg (&args);
-	}
+        {
+          std::pair <int, int> bp_num_range;
+          std::pair <int, int> bp_loc_range;
+
+          int err_ret = extract_bp_number_and_location (num.c_str(),
+                                                        bp_num_range,
+                                                        bp_loc_range);
+          if (!err_ret)
+            {
+              if (bp_loc_range.first == bp_loc_range.second
+                  && bp_loc_range.first == 0)
+                {
+                  /* Handle breakpoint with format x or x-z only.  */
+                  map_breakpoint_number_range (bp_num_range,
+                                               enable ? enable_breakpoint :
+                                                 disable_breakpoint);
+                }
+              else
+                {
+                  /* Handle breakpoint with format is x.y or x.y-z */
+                  enable_disable_breakpoint_location_range
+                    (bp_num_range.first, bp_loc_range, enable);
+                }
+            }
+          num = extract_arg (&args);
+        }
     }
 }
 
+/* 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
+   extract_bp_number_and_location.  */
+
+static void
+disable_command (char *args, int from_tty)
+{
+  enable_disable_command (args, from_tty, false);
+}
+
 static void
 enable_breakpoint_disp (struct breakpoint *bpt, enum bpdisp disposition,
 			int count)
@@ -14392,52 +14562,13 @@ enable_breakpoint (struct breakpoint *bpt)
 
 /* 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.  */
+   in stopping the inferior.  ARGS may be any of the form defined in
+   extract_bp_number_and_location.  */
 
 static void
 enable_command (const char *args, int from_tty)
 {
-  if (args == 0)
-    {
-      struct breakpoint *bpt;
-
-      ALL_BREAKPOINTS (bpt)
-	if (user_breakpoint_p (bpt))
-	  enable_breakpoint (bpt);
-    }
-  else
-    {
-      std::string num = extract_arg (&args);
-
-      while (!num.empty ())
-	{
-	  if (num.find ('.') != std::string::npos)
-	    {
-	      struct bp_location *loc = find_location_by_number (num.c_str ());
-
-	      if (loc)
-		{
-		  if (!loc->enabled)
-		    {
-		      loc->enabled = 1;
-		      mark_breakpoint_location_modified (loc);
-		    }
-		  if (target_supports_enable_disable_tracepoint ()
-		      && current_trace_status ()->running && loc->owner
-		      && is_tracepoint (loc->owner))
-		    target_enable_tracepoint (loc);
-		}
-	      update_global_location_list (UGLL_MAY_INSERT);
-	    }
-	  else
-	    map_breakpoint_numbers
-	      (num.c_str (), [&] (breakpoint *b)
-	       {
-		 iterate_over_related_breakpoints (b, enable_breakpoint);
-	       });
-	  num = extract_arg (&args);
-	}
-    }
+  enable_disable_command (args, from_tty, true);
 }
 
 static void
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index bfeb7a9..b1346d9 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -3929,13 +3929,20 @@ Num     Type           Disp Enb  Address    What
 
 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.  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).  Disabling or enabling
-the parent breakpoint (@pxref{Disabling}) affects all of the locations
-that belong to that breakpoint.
+@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},
+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).
+Disabling or enabling the parent breakpoint (@pxref{Disabling}) affects
+all of the locations that belong to that breakpoint.
 
 @cindex pending breakpoints
 It's quite common to have a breakpoint inside a shared library.
diff --git a/gdb/testsuite/gdb.cp/locbprange.cc 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 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   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 <http://www.gnu.org/licenses/>.  */
+
+#include <stddef.h>
+
+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;}
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 <http://www.gnu.org/licenses/>.
+
+# 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\]+"
+
+
+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.
+
+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"
+
+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 \]+<MULTIPLE>.*
+2.1\[\t \]+$b21.*
+2.2\[\t \]+$b22.*
+2.3\[\t \]+$b23.*
+2.4\[\t \]+$b24.*"
+
+    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"
+
+# Check that we can enable a breakpoint.
+gdb_test_no_output "enable 1"
+
+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"
+
+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 enable a location breakpoint range.
+gdb_test_no_output "enable 2.2-3"
+
+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 a location breakpoint range reduced
+# to a single location.
+gdb_test_no_output "disable 2.2-2"
+
+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 a location breakpoint range with max >
+# existing breakpoint location.
+gdb_test "disable 2.3-5" "Bad breakpoint location number '$decimal'" \
+    "disable location breakpoint range with max > existing"
+
+gdb_test "info break" [set_info_breakpoint_reply y y y n 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'" \
+    "enable location breakpoint range with max > existing"
+
+gdb_test "info break" [set_info_breakpoint_reply y y y n y y] \
+    "breakpoint info enable 2.3 to 2.5"
+
+# 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"
+
+# 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"
+
+gdb_test "info break" [set_info_breakpoint_reply y y y n y y] \
+    "breakpoint info disable 2.8 to 2.6"
-- 
2.7.4


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFA v4] enable/disable sub breakpoint range
  2017-10-23 11:07               ` Xavier Roirand
@ 2017-10-26 13:11                 ` Pedro Alves
  0 siblings, 0 replies; 19+ messages in thread
From: Pedro Alves @ 2017-10-26 13:11 UTC (permalink / raw)
  To: Xavier Roirand, gdb-patches

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 <int, int>
> &bp_num_range,
> +                                         gdb::function_view<void
> (breakpoint *)>);
> +

These forward declarations are unnecessary.

>  static void map_breakpoint_numbers (const char *,
>                      gdb::function_view<void (breakpoint *)>);
> 
> +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");
>  }
> -\f
> +
> +/* 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 <int, int> &bp_num_range,
> +                             gdb::function_view<void (breakpoint *)>
> 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 <int, int> &bp_num_range,
> +                                std::pair <int, int> &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 <stddef.h>
> +
> +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 <http://www.gnu.org/licenses/>.
> +
> +# 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 \]+<MULTIPLE>.*
> +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<double>() 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 <int, int> &bp_num_range,
-                                         gdb::function_view<void (breakpoint *)>);
-
 static void map_breakpoint_numbers (const char *,
 				    gdb::function_view<void (breakpoint *)>);
 
-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");
 }
 \f
-/* 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 <int, int> &bp_num_range,
+map_breakpoint_number_range (std::pair<int, int> bp_num_range,
 			     gdb::function_view<void (breakpoint *)> function)
 {
   if (bp_num_range.first == 0)
@@ -14206,14 +14194,14 @@ map_breakpoint_number_range (std::pair <int, int> &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<void (breakpoint *)> 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 <int,int> 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<int, int> &bp_num_range,
 				std::pair<int, int> &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 <int, int> bp_num_range;
-          std::pair <int, int> bp_loc_range;
+	  std::pair<int, int> 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 <http://www.gnu.org/licenses/>.
 
-# 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 \]+<MULTIPLE>.*
-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}<MULTIPLE>.*" \
+		"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"

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2017-10-26 13:11 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-03 10:26 [RFA v3] enable/disable sub breakpoint range Xavier Roirand
2017-10-03 14:50 ` Eli Zaretskii
2017-10-03 16:02   ` Pedro Alves
2017-10-03 16:05     ` Pedro Alves
2017-10-03 16:06     ` Xavier Roirand
2017-10-03 16:34     ` Eli Zaretskii
2017-10-03 16:40       ` Pedro Alves
2017-10-03 17:00         ` Eli Zaretskii
2017-10-03 17:15           ` Pedro Alves
2017-10-03 18:32             ` Eli Zaretskii
2017-10-06  8:54     ` Xavier Roirand
2017-10-16 22:21       ` Philippe Waroquiers
2017-10-20 12:17     ` Xavier Roirand
2017-10-20 14:41       ` Pedro Alves
2017-10-20 14:58         ` Pedro Alves
2017-10-23 10:07           ` [RFA v4] " Xavier Roirand
2017-10-23 10:25             ` Pedro Alves
2017-10-23 11:07               ` Xavier Roirand
2017-10-26 13:11                 ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).