[patch 3/3] [RFC] Add the watch-range command

[Thiago Jung Bauermann <bauerman at br dot ibm dot com>]

Hi,

This is the polemic part. :-)

Adds the watch-range command which was already discussed. The code is
unchanged from the last review.

Opinions? IMHO the issue is whether the commands

watch *((char *) addr)@len

and

watch {char[len]} addr

are hard for the user to come up with and remember (I mention them on
the manual in my previous patch in this series).
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center


2010-11-23  Sergio Durigan Junior  <sergiodj@linux.vnet.ibm.com>
	    Thiago Jung Bauermann  <bauerman@br.ibm.com>

	Add the watch-range command.

gdb/
	*NEWS: Mention the watch-range command.
	Create "New commands" section and populate it based on other
	NEWS items.
	* breakpoint.c (print_one_detail_ranged_watchpoint): Check for
	b->exp_string_reparse and exit early.
	(print_recreate_ranged_watchpoint): New function.
	(ranged_watchpoint_breakpoint_ops) <print_recreate>: Initialize
	with print_recreate_ranged_watchpoint.
	(watch_range_command_1, watch_range_command)
	(awatch_range_command, rwatch_range_command): New functions.
	(_initialize_breakpoint): Register watch-range, awatch-range and
	rwatch-range commands.
	* findcmd.c (parse_addr_range): New function factored out of
	parse_find_args.
	(parse_find_args): Call `parse_addr_range'.
	* value.h (parse_addr_range): Declare.

gdb/doc/
	* gdb.texinfo (PowerPC Embedded): Document the watch-range command.


diff --git a/gdb/NEWS b/gdb/NEWS
index 0ac97b5..0b0c042 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -94,9 +94,10 @@
   section in the user manual for more details.
 
 * GDB now supports ranged watchpoints, which stop the inferior when it
-  accesses any address within a specified memory range.  The watchpoint
-  is hardware-accelerated on some targets (currently only when locally
-  debugging programs on PowerPC BookE processors running a Linux
+  accesses any address within a specified memory range.  See the
+  documentation on the watch-range command for more information.  The
+  watchpoint is hardware-accelerated on some targets (currently only when
+  locally debugging programs on PowerPC BookE processors running a Linux
   kernel version 2.6.34 or later).
 
 * New features in the GDB remote stub, GDBserver
@@ -111,6 +112,24 @@
 
 * Guile support was removed.
 
+* New commands
+
+watch-range START_ADDR,+LENGTH | START_ADDR, END_ADDR
+rwatch-range START_ADDR,+LENGTH | START_ADDR, END_ADDR
+awatch-range START_ADDR,+LENGTH | START_ADDR, END_ADDR
+  Set a hardware watchpoint for an address range.
+  The watchpoint will stop execution of your program whenever the inferior
+  writes, reads, or accesses (respectively for watch-range, awatch-range
+  and rwatch-range) any address within the specified range.
+
+set directories PATH
+  It is like the "dir" command except that it replaces the
+  source path list instead of augmenting it.
+
+info pretty-printers
+enable pretty-printer
+disable pretty-printer
+
 * Changed commands
 
 watch EXPRESSION mask MASK_VALUE
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 5f4d472..40ee951 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -8194,6 +8194,11 @@ extra_resources_needed_ranged_watchpoint (const struct breakpoint *b)
 static void
 print_one_detail_ranged_watchpoint (const struct breakpoint *b, struct ui_out *uiout)
 {
+  /* If there's a separate expression for reparsing, then exp_string is already
+     a nice text set by watch_range_command_1 and was printed earlier.  */
+  if (b->exp_string_reparse)
+    return;
+
   gdb_assert (b->loc);
 
   ui_out_text (uiout, "\tmemory range: ");
@@ -8239,6 +8244,49 @@ print_mention_ranged_watchpoint (struct breakpoint *b)
   do_cleanups (ui_out_chain);
 }
 
+static void
+print_recreate_ranged_watchpoint (struct breakpoint *b, struct ui_file *fp)
+{
+  switch (b->type)
+    {
+    case bp_watchpoint:
+    case bp_hardware_watchpoint:
+      if (b->exp_string_reparse)
+	fprintf_unfiltered (fp, "watch-range");
+      else
+	fprintf_unfiltered (fp, "watch");
+      break;
+    case bp_read_watchpoint:
+      if (b->exp_string_reparse)
+	fprintf_unfiltered (fp, "rwatch-range");
+      else
+	fprintf_unfiltered (fp, "rwatch");
+      break;
+    case bp_access_watchpoint:
+      if (b->exp_string_reparse)
+	fprintf_unfiltered (fp, "awatch-range");
+      else
+	fprintf_unfiltered (fp, "awatch");
+      break;
+    default:
+      internal_error (__FILE__, __LINE__,
+		      _("Invalid hardware watchpoint type."));
+    }
+
+  if (b->exp_string_reparse)
+    {
+      char start_addr[40], length[40];
+
+      /* watch_range_command_1 creates the following expression to represent
+	 internally a ranged watchpoint.  */
+      sscanf (b->exp_string_reparse, "{char[ %39s ]} %39s", length, start_addr);
+
+      fprintf_unfiltered (fp, " %s, +%s", start_addr, length);
+    }
+  else
+    fprintf_unfiltered (fp, " %s", b->exp_string);
+}
+
 /* The breakpoint_ops structure to be used in ranged hardware watchpoints.  */
 
 static struct breakpoint_ops ranged_watchpoint_breakpoint_ops =
@@ -8252,7 +8300,7 @@ static struct breakpoint_ops ranged_watchpoint_breakpoint_ops =
   NULL, /* print_one */
   print_one_detail_ranged_watchpoint,
   print_mention_ranged_watchpoint,
-  NULL  /* print_recreate */
+  print_recreate_ranged_watchpoint
 };
 
 /* Implement the "insert" breakpoint_ops method for
@@ -8948,6 +8996,98 @@ awatch_command (char *arg, int from_tty)
 {
   watch_maybe_just_location (arg, hw_access, from_tty);
 }
+
+static void
+watch_range_command_1 (char *arg, enum bptype type, int from_tty)
+{
+  char *exp_string, *string_p;
+  struct gdbarch *gdbarch = get_current_arch ();
+  int wp_count, other_type_used, can_use_wp, mem_cnt, pc = 0;
+  CORE_ADDR start_addr;
+  ULONGEST length;
+  struct breakpoint *b;
+  struct expression *exp;
+  struct symtab_and_line sal;
+  struct value *val;
+  struct cleanup *cleanups;
+
+  /* Check if we need hardware watchpoints, and if we have enough
+     of them available.  */
+
+  if (!can_use_hw_watchpoints && type != bp_hardware_watchpoint)
+    error (_("\
+Need watchpoint hardware for read and access watchpoints, but cannot use it\n\
+(see set can-use-hw-watchpoints)."));
+
+  mem_cnt = target_hw_point_extra_slot_count (HW_POINT_RANGED_WATCH) + 1;
+  wp_count = hw_watchpoint_used_count (type, &other_type_used);
+  can_use_wp = target_can_use_hardware_watchpoint (type, wp_count + mem_cnt,
+						   other_type_used);
+  if (can_use_wp <= 0 || !can_use_hw_watchpoints)
+    {
+      if (type == bp_hardware_watchpoint)
+	/* Change the type of breakpoint to an ordinary watchpoint if a
+	   hardware watchpoint could not be set.  */
+	type = bp_watchpoint;
+      else
+	error (_("Not enough available hardware watchpoints (need %d)."),
+	       mem_cnt);
+    }
+
+  parse_addr_range (&arg, &start_addr, &length);
+
+  /* We need spaces around the brackets in the expression below so that
+     print_it_recreate_ranged_watchpoint can use scanf on it.  */
+  exp_string = string_p = xstrprintf ("{char[ %s ]} %s", pulongest (length),
+				      paddress (gdbarch, start_addr));
+  exp = parse_exp_1 (&string_p, 0, 0);
+  fetch_subexp_value (exp, &pc, &val, NULL, NULL);
+  if (val != NULL)
+    release_value (val);
+  cleanups = make_cleanup (xfree, exp_string);
+
+  init_sal (&sal);		/* initialize to zeroes */
+  sal.pspace = current_program_space;
+
+  /* Now set up the breakpoint.  */
+  b = set_raw_breakpoint (gdbarch, sal, type);
+  set_breakpoint_count (breakpoint_count + 1);
+  b->number = breakpoint_count;
+  b->thread = -1;
+  b->disposition = disp_donttouch;
+  b->exp = exp;
+  b->exp_string_reparse = exp_string;
+  b->exp_string = xstrprintf (_("range [%s, %s]"),
+			      paddress (gdbarch, start_addr),
+			      paddress (gdbarch, start_addr + length - 1));
+  b->ops = &ranged_watchpoint_breakpoint_ops;
+  b->val = val;
+  b->val_valid = 1;
+  b->watchpoint_frame = null_frame_id;
+
+  mention (b);
+  update_global_location_list (1);
+
+  discard_cleanups (cleanups);
+}
+
+static void
+watch_range_command (char *arg, int from_tty)
+{
+  watch_range_command_1 (arg, bp_hardware_watchpoint, from_tty);
+}
+
+static void
+awatch_range_command (char *arg, int from_tty)
+{
+  watch_range_command_1 (arg, bp_access_watchpoint, from_tty);
+}
+
+static void
+rwatch_range_command (char *arg, int from_tty)
+{
+  watch_range_command_1 (arg, bp_read_watchpoint, from_tty);
+}
 
 
 /* Helper routines for the until_command routine in infcmd.c.  Here
@@ -12689,7 +12829,44 @@ inferior in all-stop mode, gdb behaves as if always-inserted mode is off."),
 			   &show_always_inserted_mode,
 			   &breakpoint_set_cmdlist,
 			   &breakpoint_show_cmdlist);
-  
+
+  c = add_com ("watch-range", class_breakpoint, watch_range_command, _("\
+Set a hardware watchpoint for an address range.\n\
+The address range should be specified in one of the following formats:\n\
+\n\
+   start-address, end-address\n\
+   start-address, +length\n\
+\n\
+The watchpoint will stop execution of the inferior whenever it writes\n\
+to any address within the [start-address, end-address] range\n\
+(including start-address and end-address)."));
+  set_cmd_completer (c, expression_completer);
+
+  c = add_com ("awatch-range", class_breakpoint, awatch_range_command, _("\
+Set an access hardware watchpoint for an address range.\n\
+The address range should be specified in one of the following formats:\n\
+\n\
+   start-address, end-address\n\
+   start-address, +length\n\
+\n\
+The watchpoint will stop execution of the inferior whenever it accesses\n\
+(reads from or writes to) any address within the\n\
+[start-address, end-address] range (including start-address\n\
+and end-address)."));
+  set_cmd_completer (c, expression_completer);
+
+  c = add_com ("rwatch-range", class_breakpoint, rwatch_range_command, _("\
+Set a read hardware watchpoint for an address range.\n\
+The address range should be specified in one of the following formats:\n\
+\n\
+   start-address, end-address\n\
+   start-address, +length\n\
+\n\
+The watchpoint will stop execution of the inferior whenever it reads\n\
+from any address within the [start-address, end-address] range\n\
+(including start-address and end-address)."));
+  set_cmd_completer (c, expression_completer);
+
   automatic_hardware_breakpoints = 1;
 
   observer_attach_about_to_proceed (breakpoint_about_to_proceed);
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 97ec8d5..824f4c8 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -18720,19 +18720,26 @@ the @code{watch} command (@pxref{Set Watchpoints}), as in:
 @end smallexample
 
 A @dfn{ranged watchpoint} watches a contiguous range of addresses.
-@value{GDBN} automatically creates a ranged watchpoint when asked to watch
-an array or struct of known size and there are enough hardware registers
-available.  You can create an artificial array to watch an arbitrary memory
-region using one of the following commands (@pxref{Expressions}):
-
-@smallexample
-(@value{GDBP}) watch *((char *) @var{ADDRESS})@@@var{LENGTH}
-(@value{GDBP}) watch @{char[@var{LENGTH}]@} @var{ADDRESS}
-@end smallexample
+To set a ranged watchpoint in @value{GDBN}, use the @code{watch-range} command.
+In addition, @value{GDBN} automatically creates a ranged watchpoint when asked
+to watch an array or struct of known size and there are enough hardware
+registers available.
 
 @value{GDBN} provides the following PowerPC-specific commands:
 
 @table @code
+@kindex watch-range
+@item watch-range @var{start-address}, +@var{length}
+@itemx watch-range @var{start-address}, @var{end-address}
+@item rwatch-range @var{start-address}, +@var{length}
+@itemx rwatch-range @var{start-address}, @var{end-address}
+@item awatch-range @var{start-address}, +@var{length}
+@itemx awatch-range @var{start-address}, @var{end-address}
+Set a hardware watchpoint for an address range.
+The watchpoint will stop execution of your program whenever the inferior
+writes, reads, or accesses (respectively for watch-range, awatch-range
+and rwatch-range) any address within the specified range.
+
 @kindex set powerpc
 @item set powerpc soft-float
 @itemx show powerpc soft-float
diff --git a/gdb/findcmd.c b/gdb/findcmd.c
index ac63a9e..34b0dfe 100644
--- a/gdb/findcmd.c
+++ b/gdb/findcmd.c
@@ -45,6 +45,79 @@ put_bits (bfd_uint64_t data, char *buf, int bits, bfd_boolean big_p)
     }
 }
 
+/* Reads an address range, in one of the following formats:
+
+   start-address, end-address
+   start-address, +length
+
+   ARGS will be set to the first character after the end-address or length,
+   or if that character is a comma, the character following it.  If a parser
+   error occurs, an exception is thrown and none of the arguments is
+   touched.  Returns 1 on success, or 0 if an empty address range was given.  */
+
+int
+parse_addr_range (char **args, CORE_ADDR *start_addrp,
+		  ULONGEST *address_range_lenp)
+{
+  char *s = *args;
+  CORE_ADDR start_addr;
+  ULONGEST address_range_len;
+  struct value *v;
+
+  v = parse_to_comma_and_eval (&s);
+  start_addr = value_as_address (v);
+
+  if (*s == ',')
+    ++s;
+  while (isspace (*s))
+    ++s;
+
+  if (*s == '+')
+    {
+      LONGEST len;
+
+      ++s;
+      v = parse_to_comma_and_eval (&s);
+      len = value_as_long (v);
+      if (len == 0)
+	{
+	  printf_filtered (_("Empty address range.\n"));
+	  return 0;
+	}
+      if (len < 0)
+	error (_("Invalid (negative) length of the address range."));
+      /* Watch for overflows.  */
+      if (len > CORE_ADDR_MAX
+	  || (start_addr + len - 1) < start_addr)
+	error (_("Address range too large."));
+      address_range_len = len;
+    }
+  else
+    {
+      CORE_ADDR end_addr;
+
+      v = parse_to_comma_and_eval (&s);
+      end_addr = value_as_address (v);
+      if (start_addr > end_addr)
+	error (_("Invalid address range, end preceeds start."));
+      address_range_len = end_addr - start_addr + 1;
+      /* We don't support searching all of memory
+	 (i.e. start=0, end = 0xff..ff).
+	 Bail out to avoid overflows later on.  */
+      if (address_range_len == 0)
+	error (_("Overflow in address range computation, choose smaller range."));
+    }
+
+  if (*s == ',')
+    ++s;
+
+  *args = s;
+  *start_addrp = start_addr;
+  *address_range_lenp = address_range_len;
+
+  return 1;
+}
+
 /* Subroutine of find_command to simplify it.
    Parse the arguments of the "find" command.  */
 
@@ -114,53 +187,8 @@ parse_find_args (char *args, ULONGEST *max_countp,
     }
 
   /* Get the search range.  */
-
-  v = parse_to_comma_and_eval (&s);
-  start_addr = value_as_address (v);
-
-  if (*s == ',')
-    ++s;
-  while (isspace (*s))
-    ++s;
-
-  if (*s == '+')
-    {
-      LONGEST len;
-
-      ++s;
-      v = parse_to_comma_and_eval (&s);
-      len = value_as_long (v);
-      if (len == 0)
-	{
-	  printf_filtered (_("Empty search range.\n"));
-	  return;
-	}
-      if (len < 0)
-	error (_("Invalid length."));
-      /* Watch for overflows.  */
-      if (len > CORE_ADDR_MAX
-	  || (start_addr + len - 1) < start_addr)
-	error (_("Search space too large."));
-      search_space_len = len;
-    }
-  else
-    {
-      CORE_ADDR end_addr;
-
-      v = parse_to_comma_and_eval (&s);
-      end_addr = value_as_address (v);
-      if (start_addr > end_addr)
-	error (_("Invalid search space, end preceeds start."));
-      search_space_len = end_addr - start_addr + 1;
-      /* We don't support searching all of memory
-	 (i.e. start=0, end = 0xff..ff).
-	 Bail to avoid overflows later on.  */
-      if (search_space_len == 0)
-	error (_("Overflow in address range computation, choose smaller range."));
-    }
-
-  if (*s == ',')
-    ++s;
+  if (!parse_addr_range (&s, &start_addr, &search_space_len))
+    return;
 
   /* Fetch the search string.  */
 
diff --git a/gdb/value.h b/gdb/value.h
index ef2cb4f..cc3bf90 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -560,6 +560,9 @@ extern CORE_ADDR parse_and_eval_address_1 (char **expptr);
 
 extern LONGEST parse_and_eval_long (char *exp);
 
+int parse_addr_range (char **args, CORE_ADDR *start_addrp,
+		      ULONGEST *search_space_lenp);
+
 extern void unop_promote (const struct language_defn *language,
 			  struct gdbarch *gdbarch,
 			  struct value **arg1);