Re: [PATCH] Teach -data-list-register-values to not include unavailable registers

[Yao Qi <yao at codesourcery dot com>]

>> +  ** The command -data-list-register-values now accepts an optional
>> +     "--skip-unavailable" option.  When used, only available registers
>> +     are displayed.                           ^^^^^^^^^^^^^^
> 
> "only the available"
> 

Fixed.

On 06/08/2013 03:58 AM, Pedro Alves wrote:
>> "are to be returned".  And why do you need the first part of that
>> >sentence, about the "context of trace frames"?  Does it add anything
>> >to the description?
> Not really.  Meanwhile, we've added uses of <unavailable> outside trace
> frames, so this it's really better to remove mention of trace frames.

OK, the "context of trace frames" is removed.  Here is the updated
patch.

-- 
Yao (éå)

gdb/doc:

2013-06-08  Pedro Alves  <pedro@codesourcery.com>

	* gdb.texinfo (GDB/MI Data Manipulation)
	<-data-list-register-values>: Document the --skip-unavailable
	option.

gdb:

2013-06-08  Pedro Alves  <pedro@codesourcery.com>
	    Yao Qi  <yao@codesourcery.com>

	* NEWS: Mention the new option '--skip-unavailable' of command
	-data-list-register-values.
	* mi/mi-main.c (mi_cmd_data_list_register_values): Accept the
	--skip-unavailable option.  Adjust to use output_register.
	(output_register): Add new 'skip_unavailable' parameter.
	Handle it.

gdb/testsuite:

2013-06-08  Yao Qi  <yao@codesourcery.com>

	* gdb.trace/mi-trace-unavailable.exp: Set tracepoint on 'foo'
	and set an action.
	(test_trace_unavailable): Test command -data-list-register-values
	in the context of traceframe and with option --skip-unavailable.
	* gdb.trace/trace-unavailable.c (foo): New.
	(main): Call it.
	* gdb.mi/gdb2549.exp: Update matching pattern.
---
 gdb/NEWS                                         |    4 ++
 gdb/doc/gdb.texinfo                              |    7 ++-
 gdb/mi/mi-main.c                                 |   59 +++++++++++++++++-----
 gdb/testsuite/gdb.mi/gdb2549.exp                 |    2 +-
 gdb/testsuite/gdb.trace/mi-trace-unavailable.exp |   36 +++++++++++++
 gdb/testsuite/gdb.trace/trace-unavailable.c      |    5 ++
 6 files changed, 97 insertions(+), 16 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index eea9917..ed1c32f 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -76,6 +76,10 @@ show range-stepping
 
   ** The new command -dprintf-insert sets a dynamic printf breakpoint.
 
+  ** The command -data-list-register-values now accepts an optional
+     "--skip-unavailable" option.  When used, only the available registers
+     are displayed.
+
 * New system-wide configuration scripts
   A GDB installation now provides scripts suitable for use as system-wide
   configuration scripts for the following systems:
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index e6ec4ff..d25cdae 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -32726,13 +32726,16 @@ For the PPC MBX board:
 @subsubheading Synopsis
 
 @smallexample
- -data-list-register-values @var{fmt} [ ( @var{regno} )*]
+ -data-list-register-values
+    [ @code{--skip-unavailable} ] @var{fmt} [ ( @var{regno} )*]
 @end smallexample
 
 Display the registers' contents.  @var{fmt} is the format according to
 which the registers' contents are to be returned, followed by an optional
 list of numbers specifying the registers to display.  A missing list of
-numbers indicates that the contents of all the registers must be returned.
+numbers indicates that the contents of all the registers must be
+returned.  The @code{--skip-unavailable} option indicates that only
+the available registers are to be returned.
 
 Allowed formats for @var{fmt} are:
 
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 956fc5c..127f6ff 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -103,7 +103,8 @@ static void mi_execute_async_cli_command (char *cli_command,
 					  char **argv, int argc);
 static int register_changed_p (int regnum, struct regcache *,
 			       struct regcache *);
-static void output_register (struct frame_info *, int regnum, int format);
+static void output_register (struct frame_info *, int regnum, int format,
+			     int skip_unavailable);
 
 /* Command implementations.  FIXME: Is this libgdb?  No.  This is the MI
    layer that calls libgdb.  Any operation used in the below should be
@@ -1073,6 +1074,17 @@ mi_cmd_data_list_register_values (char *command, char **argv, int argc)
   int regnum, numregs, format;
   int i;
   struct cleanup *list_cleanup;
+  int skip_unavailable = 0;
+  int oind = 0;
+  enum opt
+  {
+    SKIP_UNAVAILABLE,
+  };
+  static const struct mi_opt opts[] =
+    {
+      {"-skip-unavailable", SKIP_UNAVAILABLE, 0},
+      { 0, 0, 0 }
+    };
 
   /* Note that the test for a valid register must include checking the
      gdbarch_register_name because gdbarch_num_regs may be allocated
@@ -1081,11 +1093,28 @@ mi_cmd_data_list_register_values (char *command, char **argv, int argc)
      will change depending upon the particular processor being
      debugged.  */
 
-  if (argc == 0)
+  while (1)
+    {
+      char *oarg;
+      int opt = mi_getopt ("-data-list-register-values", argc, argv,
+			   opts, &oind, &oarg);
+
+      if (opt < 0)
+	break;
+      switch ((enum opt) opt)
+	{
+	case SKIP_UNAVAILABLE:
+	  skip_unavailable = 1;
+	  break;
+	}
+    }
+
+  if (argc - oind < 1)
     error (_("-data-list-register-values: Usage: "
-	     "-data-list-register-values <format> [<regnum1>...<regnumN>]"));
+	     "-data-list-register-values [--skip-unavailable] <format>"
+	     " [<regnum1>...<regnumN>]"));
 
-  format = (int) argv[0][0];
+  format = (int) argv[oind][0];
 
   frame = get_selected_frame (NULL);
   gdbarch = get_frame_arch (frame);
@@ -1093,7 +1122,7 @@ mi_cmd_data_list_register_values (char *command, char **argv, int argc)
 
   list_cleanup = make_cleanup_ui_out_list_begin_end (uiout, "register-values");
 
-  if (argc == 1)
+  if (argc - oind == 1)
     {
       /* No args, beside the format: do all the regs.  */
       for (regnum = 0;
@@ -1104,12 +1133,12 @@ mi_cmd_data_list_register_values (char *command, char **argv, int argc)
 	      || *(gdbarch_register_name (gdbarch, regnum)) == '\0')
 	    continue;
 
-	  output_register (frame, regnum, format);
+	  output_register (frame, regnum, format, skip_unavailable);
 	}
     }
 
   /* Else, list of register #s, just do listed regs.  */
-  for (i = 1; i < argc; i++)
+  for (i = 1 + oind; i < argc; i++)
     {
       regnum = atoi (argv[i]);
 
@@ -1117,31 +1146,35 @@ mi_cmd_data_list_register_values (char *command, char **argv, int argc)
 	  && regnum < numregs
 	  && gdbarch_register_name (gdbarch, regnum) != NULL
 	  && *gdbarch_register_name (gdbarch, regnum) != '\000')
-	output_register (frame, regnum, format);
+	output_register (frame, regnum, format, skip_unavailable);
       else
 	error (_("bad register number"));
     }
   do_cleanups (list_cleanup);
 }
 
-/* Output register REGNUM's contents in the desired FORMAT.  */
+/* Output one register REGNUM's contents in the desired FORMAT.  If
+   SKIP_UNAVAILABLE is true, skip the register if it is
+   unavailable.  */
 
 static void
-output_register (struct frame_info *frame, int regnum, int format)
+output_register (struct frame_info *frame, int regnum, int format,
+		 int skip_unavailable)
 {
   struct gdbarch *gdbarch = get_frame_arch (frame);
   struct ui_out *uiout = current_uiout;
-  struct value *val;
+  struct value *val = get_frame_register_value (frame, regnum);
   struct cleanup *tuple_cleanup;
 
+  if (skip_unavailable && !value_entirely_available (val))
+    return;
+
   tuple_cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
   ui_out_field_int (uiout, "number", regnum);
 
   if (format == 'N')
     format = 0;
 
-  val = get_frame_register_value (frame, regnum);
-
   if (value_optimized_out (val))
     error (_("Optimized out"));
 
diff --git a/gdb/testsuite/gdb.mi/gdb2549.exp b/gdb/testsuite/gdb.mi/gdb2549.exp
index 48c3bb3..5c1fede 100644
--- a/gdb/testsuite/gdb.mi/gdb2549.exp
+++ b/gdb/testsuite/gdb.mi/gdb2549.exp
@@ -40,7 +40,7 @@ if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {deb
 proc register_tests_no_exec { } {
 	# Test the generic IDT chip.
 	mi_gdb_test "111-data-list-register-values" \
-		".*111\\^error,msg=\"-data-list-register-values: Usage: -data-list-register-values <format> \\\[<regnum1>...<regnumN>\\\]\"" \
+	    ".*111\\^error,msg=\"-data-list-register-values: Usage: -data-list-register-values \\\[--skip-unavailable\\\] <format> \\\[<regnum1>...<regnumN>\\\]\"" \
 		"wrong arguments"
 
 	mi_gdb_test "111-data-list-register-values x" \
diff --git a/gdb/testsuite/gdb.trace/mi-trace-unavailable.exp b/gdb/testsuite/gdb.trace/mi-trace-unavailable.exp
index 3098f9a..42f6e32 100644
--- a/gdb/testsuite/gdb.trace/mi-trace-unavailable.exp
+++ b/gdb/testsuite/gdb.trace/mi-trace-unavailable.exp
@@ -52,6 +52,14 @@ mi_gdb_test "-break-insert -a bar" \
 mi_gdb_test "-break-commands 3 \"collect array\" \"collect j\" \"end\" " \
     {\^done} "set action"
 
+mi_gdb_test "-break-insert -a foo" \
+    "\\^done,bkpt=\{number=\"${decimal}\",type=\"tracepoint\".*\"\}" \
+    "insert tracepoint on foo"
+
+# Collect 'main' to make sure no registers are collected except PC.
+mi_gdb_test "-break-commands 4 \"collect main\" \"end\" " \
+    {\^done} "set action on tracepoint 4"
+
 mi_gdb_test "-trace-start" {.*\^done} "trace start"
 mi_send_resuming_command "exec-continue" "continuing to marker"
 mi_expect_stop \
@@ -96,6 +104,34 @@ proc test_trace_unavailable { data_source } {
 	    ".*\\^done,variables=\\\[\{name=\"j\",arg=\"1\",type=\"int\",value=\"4\"\},\{name=\"s\",arg=\"1\",type=\"char \\\*\",value=\"<unavailable>\"\},\{name=\"array\",type=\"unsigned char \\\[2\\\]\"\},\{name=\"i\",type=\"int\",value=\"<unavailable>\"\}\\\]" \
 	    "-stack-list-variables --simple-values"
 
+	mi_gdb_test "-trace-find frame-number 1" \
+	    ".*\\^done,found=\"1\",tracepoint=\"${decimal}\",traceframe=\"1\",frame=\{.*" \
+	    "-trace-find frame-number 1"
+
+	set pcnum 0
+	if [is_amd64_regs_target] {
+	    set pcnum 16
+	} elseif [is_x86_like_target] {
+	    set pcnum 8
+	} else {
+	    # Other ports support tracepoint should define the number
+	    # of its own pc register.
+	}
+
+	if { $pcnum != 0 } {
+	    global hex
+	    # Test that register 0 and PC are displayed, and register
+	    # 0 is unavailable.
+	    mi_gdb_test "-data-list-register-values x 0 ${pcnum}" \
+		".*\\^done,register-values=\\\[\{number=\"0\",value=\"<unavailable>\"\},\{number=\"${pcnum}\",value=\"${hex}\"\}\\\]" \
+		"-data-list-register-values x"
+
+	    # Test that only available register PC is displayed.
+	    mi_gdb_test "-data-list-register-values --skip-unavailable x 0 ${pcnum}" \
+		".*\\^done,register-values=\\\[\{number=\"${pcnum}\",value=\"${hex}\"\}\\\]" \
+		"-data-list-register-values --skip-unavailable x"
+	}
+
 	# Don't issue command '-trace-find none' to return from
 	# tfind mode (examining trace frames) on purpose, in order
 	# to test that GDB is able to clear its tracing-related local state
diff --git a/gdb/testsuite/gdb.trace/trace-unavailable.c b/gdb/testsuite/gdb.trace/trace-unavailable.c
index 9b3e230..407c069 100644
--- a/gdb/testsuite/gdb.trace/trace-unavailable.c
+++ b/gdb/testsuite/gdb.trace/trace-unavailable.c
@@ -26,6 +26,10 @@ bar (int j, char *s)
 }
 
 static void
+foo (void)
+{}
+
+static void
 marker (void)
 {}
 
@@ -35,6 +39,7 @@ main (void)
   char s[4];
 
   bar (4, s);
+  foo ();
   marker ();
   return 0;
 }
-- 
1.7.7.6