[RFC] new setting to not-print frame arguments values

[RFC] new setting to not-print frame arguments values

[Joel Brobecker]

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

Hello,

Context:
--------

As a user, I had often wished that there would be a way to print
backtraces without printing any argument, almost like the way we
print a frame for a function that does not have any debug info.
This is because the argument values tend to clutter the part of
the backtrace that I am really interested in.

But recently, a bug that we fixed causing arguments to be printed
correctly instead of not printed at all, had the unexpected side-effect
of dramatically slowing down the printing of each frame in a backtrace.

The slow-down is due to the encoding used for Ada, which requires
what we call "parallel lookups", which are lookups that scan through
the entire application. This is because we are forced by non-dwarf
debugging info (stabs) to use parallel types to help us determine
certain characteristics that can not be encoded otherwise. This is
hardly noticeable on small projects, but quite annoying on large
scale projects.

These lookups are only used for non-scalar types, such as records
and arrays. So, since we cannot do without the parallel lookups
for now, the following idea emerged.

Proposal:
---------

We would like to propose the inclusion of a new setting:

    (gdb) set print frame-arguments (all|scalars|none)

The effect would be:

  (a) set print frame-arguments all
      print all argument values. Same behavior as before.


  (b) set print frame-arguments scalars
      print the argument value only if the argument is a scalar.

  (c) set print frame-arguments none
      Print no argument value at all.

When the argument value is not printed, its value is substituted
with "..." to give the user a clue that the value has intentionally
not been printed.

For our needs, AdaCore will set the default "print frame-arguments"
to "scalars", but we may here want to preserve the default behavior,
and thus use "all" by default.

This is what this patch implements. Testcase and documentation
are still missing, but will be provided later if the idea is accepted.

The testcase will be in C, but for now, with Ada, here is the output
we get for each value of this new switch:

    (gdb) set print frame-arguments all 
    (gdb) frame
    #1  0x0804954f in pck.call_me (int=1, flt=2.0, bln=true, chr=106 'j', 
        sad=(system.address) 0xbfac2ae4, rec=(a => 3, b => 7)) at pck.adb:17
    17            Break_Me;

    (gdb) set print frame-arguments scalars 
    (gdb) frame
    #1  0x0804954f in pck.call_me (int=1, flt=2.0, bln=true, chr=106 'j', 
        sad=(system.address) 0xbfac2ae4, rec=...) at pck.adb:17
    17            Break_Me;

    (gdb) set print frame-arguments none    
    (gdb) frame
    #1  0x0804954f in pck.call_me (int=..., flt=..., bln=..., chr=..., 
        sad=..., rec=...) at pck.adb:17
    17            Break_Me;


2007-11-06  Joel Brobecker  <brobecker@adacore.com>

        * stack.c (print_frame_arguments_choices): New static global.
        (print_frame_arguments): Likewise.
        (print_this_frame_argument_p): New function.
        (print_frame_args): Print the argument value only when appropriate.
        (_initialize_task): Add new "set/show print frame-arguments" command.

Tested on x86-linux. No regression.

Any comments? Let's be bold: Ok to checkin? ;-)

Thanks,
-- 
Joel

[-- Attachment #2: stack.c.diff --]
[-- Type: text/plain, Size: 4185 bytes --]

Index: stack.c
===================================================================
RCS file: /cvs/src/src/gdb/stack.c,v
retrieving revision 1.156
diff -u -p -r1.156 stack.c
--- stack.c	2 Nov 2007 14:47:28 -0000	1.156
+++ stack.c	8 Nov 2007 01:00:11 -0000
@@ -50,6 +50,13 @@
 
 void (*deprecated_selected_frame_level_changed_hook) (int);
 
+/* The possible choices of "set print frame-arguments, and the value
+   of this setting.  */
+
+static const char *print_frame_arguments_choices[] =
+  {"all", "scalars", "none", NULL};
+static const char *print_frame_arguments = "all";
+
 /* Prototypes for local functions. */
 
 static void print_frame_local_vars (struct frame_info *, int,
@@ -148,6 +155,44 @@ print_frame_nameless_args (struct frame_
     }
 }
 
+/* Return non-zero if the debugger should print the value of the provided
+   symbol parameter (SYM).  */
+
+static int
+print_this_frame_argument_p (struct symbol *sym)
+{
+  struct type *type;
+  
+  /* If the user asked to print no argument at all, then obviously
+     do not print this argument.  */
+
+  if (strcmp (print_frame_arguments, "none") == 0)
+    return 0;
+
+  /* If the user asked to print all arguments, then we should print
+     that one.  */
+
+  if (strcmp (print_frame_arguments, "all") == 0)
+    return 1;
+
+  /* The user asked to print only the scalar arguments, so do not
+     print the non-scalar ones.  */
+
+  type = CHECK_TYPEDEF (SYMBOL_TYPE (sym));
+  switch (TYPE_CODE (type))
+    {
+      case TYPE_CODE_ARRAY:
+      case TYPE_CODE_STRUCT:
+      case TYPE_CODE_UNION:
+      case TYPE_CODE_SET:
+      case TYPE_CODE_STRING:
+      case TYPE_CODE_BITSTRING:
+        return 0;
+      default:
+        return 1;
+    }
+}
+
 /* Print the arguments of frame FRAME on STREAM, given the function
    FUNC running in that frame (as a symbol), where NUM is the number
    of arguments according to the stack frame (or -1 if the number of
@@ -304,23 +349,30 @@ print_frame_args (struct symbol *func, s
 	  annotate_arg_name_end ();
 	  ui_out_text (uiout, "=");
 
-	  /* Avoid value_print because it will deref ref parameters.
-	     We just want to print their addresses.  Print ??? for
-	     args whose address we do not know.  We pass 2 as
-	     "recurse" to val_print because our standard indentation
-	     here is 4 spaces, and val_print indents 2 for each
-	     recurse.  */
-	  val = read_var_value (sym, frame);
-
-	  annotate_arg_value (val == NULL ? NULL : value_type (val));
+          if (print_this_frame_argument_p (sym))
+            {
+	      /* Avoid value_print because it will deref ref parameters.
+		 We just want to print their addresses.  Print ??? for
+		 args whose address we do not know.  We pass 2 as
+		 "recurse" to val_print because our standard indentation
+		 here is 4 spaces, and val_print indents 2 for each
+		 recurse.  */
+	      val = read_var_value (sym, frame);
+
+	      annotate_arg_value (val == NULL ? NULL : value_type (val));
+
+	      if (val)
+	        {
+		  common_val_print (val, stb->stream, 0, 0, 2,
+				    Val_no_prettyprint);
+		  ui_out_field_stream (uiout, "value", stb);
+	        }
+	      else
+		ui_out_text (uiout, "???");
+            }
+          else
+            ui_out_text (uiout, "...");
 
-	  if (val)
-	    {
-	      common_val_print (val, stb->stream, 0, 0, 2, Val_no_prettyprint);
-	      ui_out_field_stream (uiout, "value", stb);
-	    }
-	  else
-	    ui_out_text (uiout, "???");
 
 	  /* Invoke ui_out_tuple_end.  */
 	  do_cleanups (list_chain);
@@ -2039,6 +2091,12 @@ Usage: func <name>\n"));
   add_info ("catch", catch_info,
 	    _("Exceptions that can be caught in the current stack frame."));
 
+  add_setshow_enum_cmd ("frame-arguments", class_stack,
+			print_frame_arguments_choices, &print_frame_arguments,
+                        _("Set printing of non-scalar frame arguments"),
+                        _("Show printing of non-scalar frame arguments"),
+                        NULL, NULL, NULL, &setprintlist, &showprintlist);
+
 #if 0
   add_cmd ("backtrace-limit", class_stack, set_backtrace_limit_command, _(\
 "Specify maximum number of frames for \"backtrace\" to print by default."),

Re: [RFC] new setting to not-print frame arguments values

[Eli Zaretskii]

> Date: Wed, 7 Nov 2007 17:19:40 -0800
> From: Joel Brobecker <brobecker@adacore.com>
> 
> We would like to propose the inclusion of a new setting:
> 
>     (gdb) set print frame-arguments (all|scalars|none)
> 
> The effect would be:
> 
>   (a) set print frame-arguments all
>       print all argument values. Same behavior as before.
> 
> 
>   (b) set print frame-arguments scalars
>       print the argument value only if the argument is a scalar.
> 
>   (c) set print frame-arguments none
>       Print no argument value at all.
> 
> When the argument value is not printed, its value is substituted
> with "..." to give the user a clue that the value has intentionally
> not been printed.

I'm okay with the idea.

Re: [RFC] new setting to not-print frame arguments values

[Daniel Jacobowitz]

On Thu, Nov 08, 2007 at 06:17:03AM +0200, Eli Zaretskii wrote:
> I'm okay with the idea.

Me too.  Something looked odd with your indentation in
print_frame_args, but otherwise the patch looks OK, too.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC] new setting to not-print frame arguments values

[Joel Brobecker]

> > I'm okay with the idea.
> 
> Me too.  Something looked odd with your indentation in
> print_frame_args, but otherwise the patch looks OK, too.

Thanks everyone for the feedback. The oddness, I believe, comes from
the ($@^$#%) tabs - The extra leading character in the diff sometimes
makes it look odd. However, there was one place at the end of the patch
where I forgot to convert spaces into tabs, so I fixed it.

The patch is now in. Documentation and testcase coming up shortly.

-- 
Joel

PS: God, I hate tabs...