[RFA 00/10] various frame filter fixes and cleanups

[RFA 00/10] various frame filter fixes and cleanups

[Tom Tromey]

This series fixes a few frame filter bugs.  Some of them have been
reported, but some not.

There are some documentation changes in patch #1 and patch #3.

For the most part I think each patch is reasonably self-explanatory.
However, there are a couple of things to note here:

* You might wonder about the choice of constant in patch #3.  I chose
  32 rather than 16 so as not to conflict with my earlier frame-filter
  patch, in review.

* In some cases writing a test seemed pretty difficult, and also maybe
  low-impact, so I didn't.  This applies to patch #6, #9, and #10.

A future direction for patch #3 would be to add a new "set" command so
that a user can choose "bt elide" as the default.

Tested on the buildbot.

Tom

[RFA 01/10] Rationalize "backtrace" command line parsing

[Tom Tromey]

The backtrace command has peculiar command-line parsing.  In
particular, it splits the command line, then loops over the arguments.
If it sees a word it recognizes, like "full", it effectively drops
this word from the argument vector.  Then, it pastes together the
remaining arguments, passing them on to backtrace_command_1, which in
turn passes the resulting string to parse_and_eval_long.

The documentation doesn't mention the parse_and_eval_long at all, so
it is a bit of a hidden feature that you can "bt 3*2".  The strange
algorithm above also means you can "bt 3 * no-filters 2" and get 6
frames...

This patch changes backtrace's command line parsing to be a bit more
rational.  Now, special words like "full" are only recognized at the
start of the command.

This also updates the documentation to describe the various bt options
individually.

ChangeLog
2017-04-25  Tom Tromey  <tom@tromey.com>

	* stack.c (backtrace_command): Rewrite command line parsing.

doc/ChangeLog
2017-04-25  Tom Tromey  <tom@tromey.com>

	* gdb.texinfo (Backtrace): Describe options individually.
---
 gdb/ChangeLog       |  4 +++
 gdb/doc/ChangeLog   |  4 +++
 gdb/doc/gdb.texinfo | 50 ++++++++++++++-----------------------
 gdb/stack.c         | 71 ++++++++++++++++++++---------------------------------
 4 files changed, 53 insertions(+), 76 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 90ce6ae..48f2063 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2017-04-25  Tom Tromey  <tom@tromey.com>
+
+	* stack.c (backtrace_command): Rewrite command line parsing.
+
 2017-04-25  Yao Qi  <yao.qi@linaro.org>
 
 	* aarch64-tdep.c (aarch64_gdbarch_init): Don't call
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 9be035c..cf2226c 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,7 @@
+2017-04-25  Tom Tromey  <tom@tromey.com>
+
+	* gdb.texinfo (Backtrace): Describe options individually.
+
 2017-04-21  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* gdb.texinfo (GDB/MI Thread Information): Add missing
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index f2e6156..a9f12fd 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -7199,39 +7199,27 @@ frame (frame zero), followed by its caller (frame one), and on up the
 stack.
 
 @anchor{backtrace-command}
-@table @code
 @kindex backtrace
 @kindex bt @r{(@code{backtrace})}
-@item backtrace
-@itemx bt
-Print a backtrace of the entire stack: one line per frame for all
-frames in the stack.
-
-You can stop the backtrace at any time by typing the system interrupt
-character, normally @kbd{Ctrl-c}.
-
-@item backtrace @var{n}
-@itemx bt @var{n}
-Similar, but print only the innermost @var{n} frames.
-
-@item backtrace -@var{n}
-@itemx bt -@var{n}
-Similar, but print only the outermost @var{n} frames.
-
-@item backtrace full
-@itemx bt full
-@itemx bt full @var{n}
-@itemx bt full -@var{n}
-Print the values of the local variables also.  As described above,
-@var{n} specifies the number of frames to print.
-
-@item backtrace no-filters
-@itemx bt no-filters
-@itemx bt no-filters @var{n}
-@itemx bt no-filters -@var{n}
-@itemx bt no-filters full
-@itemx bt no-filters full @var{n}
-@itemx bt no-filters full -@var{n}
+Print a backtrace of the entire stack, use the @code{backtrace}
+command, or its alias @code{bt}.  This command will print one line per
+frame for all frames in the stack.  You can stop the backtrace at any
+time by typing the system interrupt character, normally @kbd{Ctrl-c}.
+@code{backtrace} can accept some arguments:
+
+@table @code
+@item @var{n}
+@itemx @var{n}
+Print only the innermost @var{n} frames.
+
+@item -@var{n}
+@itemx -@var{n}
+Print only the outermost @var{n} frames.
+
+@item full
+Print the values of the local variables also.
+
+@item no-filters
 Do not run Python frame filters on this backtrace.  @xref{Frame
 Filter API}, for more information.  Additionally use @ref{disable
 frame-filter all} to turn off all frame filters.  This is only
diff --git a/gdb/stack.c b/gdb/stack.c
index 7f8a51c..c2fc3f1 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1868,65 +1868,46 @@ backtrace_command_1 (char *count_exp, int show_locals, int no_filters,
 static void
 backtrace_command (char *arg, int from_tty)
 {
-  struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
-  int fulltrace_arg = -1, arglen = 0, argc = 0, no_filters  = -1;
-  int user_arg = 0;
+  bool fulltrace = false;
+  bool filters = true;
 
   if (arg)
     {
-      char **argv;
-      int i;
+      bool done = false;
 
-      argv = gdb_buildargv (arg);
-      make_cleanup_freeargv (argv);
-      argc = 0;
-      for (i = 0; argv[i]; i++)
+      while (!done)
 	{
 	  unsigned int j;
+	  char *this_arg = extract_arg (&arg);
 
-	  for (j = 0; j < strlen (argv[i]); j++)
-	    argv[i][j] = TOLOWER (argv[i][j]);
+	  if (this_arg == NULL)
+	    break;
+
+	  struct cleanup *arg_cleanup = make_cleanup (xfree, this_arg);
+
+	  for (j = 0; j < strlen (this_arg); j++)
+	    this_arg[j] = TOLOWER (this_arg[j]);
 
-	  if (no_filters < 0 && subset_compare (argv[i], "no-filters"))
-	    no_filters = argc;
+	  if (subset_compare (this_arg, "no-filters"))
+	    filters = false;
+	  else if (subset_compare (this_arg, "full"))
+	    fulltrace = true;
 	  else
 	    {
-	      if (fulltrace_arg < 0 && subset_compare (argv[i], "full"))
-		fulltrace_arg = argc;
-	      else
-		{
-		  user_arg++;
-		  arglen += strlen (argv[i]);
-		}
+	      /* Not a recognized argument, so stop.  */
+	      done = true;
 	    }
-	  argc++;
-	}
-      arglen += user_arg;
-      if (fulltrace_arg >= 0 || no_filters >= 0)
-	{
-	  if (arglen > 0)
-	    {
-	      arg = (char *) xmalloc (arglen + 1);
-	      make_cleanup (xfree, arg);
-	      arg[0] = 0;
-	      for (i = 0; i < argc; i++)
-		{
-		  if (i != fulltrace_arg && i != no_filters)
-		    {
-		      strcat (arg, argv[i]);
-		      strcat (arg, " ");
-		    }
-		}
-	    }
-	  else
-	    arg = NULL;
+
+	  do_cleanups (arg_cleanup);
 	}
-    }
 
-  backtrace_command_1 (arg, fulltrace_arg >= 0 /* show_locals */,
-		       no_filters >= 0 /* no frame-filters */, from_tty);
+      arg = skip_spaces (arg);
+      if (*arg == '\0')
+	arg = NULL;
+    }
 
-  do_cleanups (old_chain);
+  backtrace_command_1 (arg, fulltrace /* show_locals */,
+		       !filters /* no frame-filters */, from_tty);
 }
 
 /* Iterate over the local variables of a block B, calling CB with
-- 
2.9.3

Re: [RFA 01/10] Rationalize "backtrace" command line parsing

[Eli Zaretskii]

> From: Tom Tromey <tom@tromey.com>
> Cc: Tom Tromey <tom@tromey.com>
> Date: Tue, 25 Apr 2017 13:41:04 -0600
> 
> The backtrace command has peculiar command-line parsing.  In
> particular, it splits the command line, then loops over the arguments.
> If it sees a word it recognizes, like "full", it effectively drops
> this word from the argument vector.  Then, it pastes together the
> remaining arguments, passing them on to backtrace_command_1, which in
> turn passes the resulting string to parse_and_eval_long.
> 
> The documentation doesn't mention the parse_and_eval_long at all, so
> it is a bit of a hidden feature that you can "bt 3*2".  The strange
> algorithm above also means you can "bt 3 * no-filters 2" and get 6
> frames...
> 
> This patch changes backtrace's command line parsing to be a bit more
> rational.  Now, special words like "full" are only recognized at the
> start of the command.

Do we really want to make backward-incompatible changes in such a
veteran command?

> +@code{backtrace} can accept some arguments:

This sentence will look erroneous in print, because it starts with a
lower-case letter.  Maybe use "The @code{backtrace} command can ..."
instead.

Also, you've removed the @table and the corresponding @item, so now
the description text describes something that was not yet called out:

  @node Backtrace
  @section Backtraces

  @cindex traceback
  @cindex call stack traces
  A backtrace is a summary of how your program got where it is.  It shows one
  line per frame, for many frames, starting with the currently executing
  frame (frame zero), followed by its caller (frame one), and on up the
  stack.

  @anchor{backtrace-command}
  @kindex backtrace
  @kindex bt @r{(@code{backtrace})}
  Print a backtrace of the entire stack, use the @code{backtrace}
  command, or its alias @code{bt}.  This command will print one line ...

Since @kindex leaves no traces in the manual, I think this will look
weird, don't you agree?

Thanks.

Re: [RFA 01/10] Rationalize "backtrace" command line parsing

[Pedro Alves]

I noticed now that this series is still pending.

On 04/25/2017 08:41 PM, Tom Tromey wrote:
> The backtrace command has peculiar command-line parsing.  In
> particular, it splits the command line, then loops over the arguments.
> If it sees a word it recognizes, like "full", it effectively drops
> this word from the argument vector.  Then, it pastes together the
> remaining arguments, passing them on to backtrace_command_1, which in
> turn passes the resulting string to parse_and_eval_long.
> 
> The documentation doesn't mention the parse_and_eval_long at all, so
> it is a bit of a hidden feature that you can "bt 3*2".  The strange
> algorithm above also means you can "bt 3 * no-filters 2" and get 6
> frames...
> 

Funny.

> This patch changes backtrace's command line parsing to be a bit more
> rational.  Now, special words like "full" are only recognized at the
> start of the command.
> 
> This also updates the documentation to describe the various bt options
> individually.
> 
> ChangeLog
> 2017-04-25  Tom Tromey  <tom@tromey.com>
> 
> 	* stack.c (backtrace_command): Rewrite command line parsing.

LGTM.

It might be good to clarify "help bt", to show something around:

  Usage: backtrace [QUALIFIERS]... COUNT


I'd support deprecating the existing non-hyphenated qualifiers,
and start supporting hyphenated options:

  bt -full -no-filters -whatever-new-option -- COUNT

Thanks,
Pedro Alves

Re: [RFA 01/10] Rationalize "backtrace" command line parsing

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> It might be good to clarify "help bt", to show something around:
Pedro>   Usage: backtrace [QUALIFIERS]... COUNT

How about the appended?

Tom

commit 46ab61faf8412739124639aac528d2b7df387632
Author: Tom Tromey <tom@tromey.com>
Date:   Sun Jul 9 16:31:11 2017 -0600

    Improve "backtrace" help text
    
    This improves help text in stack.c in two ways.  First, it removes
    trailing newlines from various help strings.  I think these are never
    needed.  Second, it adds a "Usage" line to the "backtrace" text, as
    suggested by Pedro.
    
    ChangeLog
    2017-07-09  Tom Tromey  <tom@tromey.com>
    
            * stack.c (_initialize_stack): Remove trailing newlines from help
            text.  Add "Usage" line to "backtrace" help.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 210bebc..73b463e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2017-07-09  Tom Tromey  <tom@tromey.com>
+
+	* stack.c (_initialize_stack): Remove trailing newlines from help
+	text.  Add "Usage" line to "backtrace" help.
+
 2017-04-25  Tom Tromey  <tom@tromey.com>
 
 	PR python/16486:
diff --git a/gdb/stack.c b/gdb/stack.c
index 4f6b117..5df25c4 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -2589,22 +2589,23 @@ This is useful in command scripts."));
 Select and print a stack frame.\nWith no argument, \
 print the selected stack frame.  (See also \"info frame\").\n\
 An argument specifies the frame to select.\n\
-It can be a stack frame number or the address of the frame.\n"));
+It can be a stack frame number or the address of the frame."));
 
   add_com_alias ("f", "frame", class_stack, 1);
 
   add_com_suppress_notification ("select-frame", class_stack, select_frame_command, _("\
 Select a stack frame without printing anything.\n\
 An argument specifies the frame to select.\n\
-It can be a stack frame number or the address of the frame.\n"),
+It can be a stack frame number or the address of the frame."),
 		 &cli_suppress_notification.user_selected_context);
 
   add_com ("backtrace", class_stack, backtrace_command, _("\
 Print backtrace of all stack frames, or innermost COUNT frames.\n\
-With a negative argument, print outermost -COUNT frames.\nUse of the \
-'full' qualifier also prints the values of the local variables.\n\
+Usage: backtrace [QUALIFIERS]... [COUNT]\n\
+With a negative argument, print outermost -COUNT frames.\n\
+Use of the 'full' qualifier also prints the values of the local variables.\n\
 Use of the 'no-filters' qualifier prohibits frame filters from executing\n\
-on this backtrace.\n"));
+on this backtrace."));
   add_com_alias ("bt", "backtrace", class_stack, 0);
 
   add_com_alias ("where", "backtrace", class_alias, 0);
@@ -2622,7 +2623,7 @@ on this backtrace.\n"));
   if (dbx_commands)
     add_com ("func", class_stack, func_command, _("\
 Select the stack frame that contains <func>.\n\
-Usage: func <name>\n"));
+Usage: func <name>"));
 
   add_setshow_enum_cmd ("frame-arguments", class_stack,
 			print_frame_arguments_choices, &print_frame_arguments,

Re: [RFA 01/10] Rationalize "backtrace" command line parsing

[Eli Zaretskii]

> From: Tom Tromey <tom@tromey.com>
> Cc: Tom Tromey <tom@tromey.com>,  gdb-patches@sourceware.org
> Date: Sun, 09 Jul 2017 16:34:21 -0600
> 
> >>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> It might be good to clarify "help bt", to show something around:
> Pedro>   Usage: backtrace [QUALIFIERS]... COUNT
> 
> How about the appended?
> 
> Tom
> 
> commit 46ab61faf8412739124639aac528d2b7df387632
> Author: Tom Tromey <tom@tromey.com>
> Date:   Sun Jul 9 16:31:11 2017 -0600
> 
>     Improve "backtrace" help text
>     
>     This improves help text in stack.c in two ways.  First, it removes
>     trailing newlines from various help strings.  I think these are never
>     needed.  Second, it adds a "Usage" line to the "backtrace" text, as
>     suggested by Pedro.
>     
>     ChangeLog
>     2017-07-09  Tom Tromey  <tom@tromey.com>
>     
>             * stack.c (_initialize_stack): Remove trailing newlines from help
>             text.  Add "Usage" line to "backtrace" help.

Fine with me, thanks.

Re: [RFA 01/10] Rationalize "backtrace" command line parsing

[Pedro Alves]

On 07/09/2017 11:34 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> It might be good to clarify "help bt", to show something around:
> Pedro>   Usage: backtrace [QUALIFIERS]... COUNT
> 
> How about the appended?

Thanks, LGTM, with a minor nit:

> +Usage: backtrace [QUALIFIERS]... [COUNT]\n\

I should have suggested "[QUALIFIER]..." (singular).
The "..." already indicates you can have multiple instances.

The output of 

  $ grep -rn "]\.\.\." -rn

seems to agree.

and so does "ls --help", etc.

Thanks,
Pedro Alves

[RFA 04/10] Remove EXT_LANG_BT_COMPLETED

[Tom Tromey]

While looking at the frame filter code, I noticed that
EXT_LANG_BT_COMPLETED is not really needed.  Semantically there is no
difference between the "completed" and "ok" results.  So, this patch
removes this constant.

ChangeLog
2017-04-25  Tom Tromey  <tom@tromey.com>

	* python/py-framefilter.c (py_print_frame): Return
	EXT_LANG_BT_OK.
	(gdbpy_apply_frame_filter): Update comment.
	* extension.h (enum ext_lang_bt_status) <EXT_LANG_BT_COMPLETED>:
	Remove.
	<EXT_LANG_BT_NO_FILTERS>: Change value.
---
 gdb/ChangeLog               | 9 +++++++++
 gdb/extension.h             | 6 +-----
 gdb/python/py-framefilter.c | 8 ++++----
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f68e32c..176332a 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,14 @@
 2017-04-25  Tom Tromey  <tom@tromey.com>
 
+	* python/py-framefilter.c (py_print_frame): Return
+	EXT_LANG_BT_OK.
+	(gdbpy_apply_frame_filter): Update comment.
+	* extension.h (enum ext_lang_bt_status) <EXT_LANG_BT_COMPLETED>:
+	Remove.
+	<EXT_LANG_BT_NO_FILTERS>: Change value.
+
+2017-04-25  Tom Tromey  <tom@tromey.com>
+
 	PR backtrace/15582:
 	* stack.c (backtrace_command): Parse "elide" argument.
 	* python/py-framefilter.c (py_print_frame): Handle PRINT_ELIDE.
diff --git a/gdb/extension.h b/gdb/extension.h
index 7c35502..59205e1 100644
--- a/gdb/extension.h
+++ b/gdb/extension.h
@@ -76,13 +76,9 @@ enum ext_lang_bt_status
        succeeded.  */
     EXT_LANG_BT_OK = 1,
 
-    /* Return when the frame filter process is complete, and all
-       operations have succeeded.  */
-    EXT_LANG_BT_COMPLETED = 2,
-
     /* Return when the frame filter process is complete, but there
        were no filter registered and enabled to process.  */
-    EXT_LANG_BT_NO_FILTERS = 3
+    EXT_LANG_BT_NO_FILTERS = 2
   };
 
 /* Flags to pass to apply_extlang_frame_filter.  */
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index f0474a9..2bbbfbe 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -916,7 +916,7 @@ py_print_args (PyObject *filter,
     containing all the frames level that have already been printed.
     If a frame level has been printed, do not print it again (in the
     case of elided frames).  Returns EXT_LANG_BT_ERROR on error, with any
-    GDB exceptions converted to a Python exception, or EXT_LANG_BT_COMPLETED
+    GDB exceptions converted to a Python exception, or EXT_LANG_BT_OK
     on success.  It can also throw an exception RETURN_QUIT.  */
 
 static enum ext_lang_bt_status
@@ -969,7 +969,7 @@ py_print_frame (PyObject *filter, int flags,
       if (py_mi_print_variables (filter, out, &opts,
 				 args_type, frame) == EXT_LANG_BT_ERROR)
 	return EXT_LANG_BT_ERROR;
-      return EXT_LANG_BT_COMPLETED;
+      return EXT_LANG_BT_OK;
     }
 
   gdb::optional<ui_out_emit_tuple> tuple;
@@ -1270,7 +1270,7 @@ py_print_frame (PyObject *filter, int flags,
 	}
     }
 
-  return EXT_LANG_BT_COMPLETED;
+  return EXT_LANG_BT_OK;
 }
 
 /* Helper function to initiate frame filter invocation at starting
@@ -1328,7 +1328,7 @@ bootstrap_python_frame_filters (struct frame_info *frame,
     format, OUT is the output stream to print.  FRAME_LOW is the
     beginning of the slice of frames to print, and FRAME_HIGH is the
     upper limit of the frames to count.  Returns EXT_LANG_BT_ERROR on error,
-    or EXT_LANG_BT_COMPLETED on success.  */
+    or EXT_LANG_BT_OK on success.  */
 
 enum ext_lang_bt_status
 gdbpy_apply_frame_filter (const struct extension_language_defn *extlang,
-- 
2.9.3

Re: [RFA 04/10] Remove EXT_LANG_BT_COMPLETED

[Phil Muldoon]

On 25/04/17 20:41, Tom Tromey wrote:
> While looking at the frame filter code, I noticed that
> EXT_LANG_BT_COMPLETED is not really needed.  Semantically there is no
> difference between the "completed" and "ok" results.  So, this patch
> removes this constant.
> 
> ChangeLog
> 2017-04-25  Tom Tromey  <tom@tromey.com>
> 
> 	* python/py-framefilter.c (py_print_frame): Return
> 	EXT_LANG_BT_OK.
> 	(gdbpy_apply_frame_filter): Update comment.
> 	* extension.h (enum ext_lang_bt_status) <EXT_LANG_BT_COMPLETED>:
> 	Remove.
> 	<EXT_LANG_BT_NO_FILTERS>: Change value.

I wish I could remember why I added this. OTOH it could have been a relic even from the initial code submission.

Anyway, patch LGTM.

Cheers,

Phil

Re: [RFA 04/10] Remove EXT_LANG_BT_COMPLETED

[Pedro Alves]

On 04/28/2017 03:52 PM, Phil Muldoon wrote:
> On 25/04/17 20:41, Tom Tromey wrote:
>> While looking at the frame filter code, I noticed that
>> EXT_LANG_BT_COMPLETED is not really needed.  Semantically there is no
>> difference between the "completed" and "ok" results.  So, this patch
>> removes this constant.
>>
>> ChangeLog
>> 2017-04-25  Tom Tromey  <tom@tromey.com>
>>
>> 	* python/py-framefilter.c (py_print_frame): Return
>> 	EXT_LANG_BT_OK.
>> 	(gdbpy_apply_frame_filter): Update comment.
>> 	* extension.h (enum ext_lang_bt_status) <EXT_LANG_BT_COMPLETED>:
>> 	Remove.
>> 	<EXT_LANG_BT_NO_FILTERS>: Change value.
> 
> I wish I could remember why I added this. OTOH it could have been a relic even from the initial code submission.
> 
> Anyway, patch LGTM.

LGTM too.

Thanks,
Pedro Alves

[RFA 06/10] Allow C-c to work in backtrace in more cases

[Tom Tromey]

PR cli/17716 notes that it is difficult to C-c (or "q" at a pagination
prompt) while backtracing using a frame filter.  One reason for this
is that many places in py-framefilter.c use RETURN_MASK_ALL in a
try/catch.

This patch changes these spots to use RETURN_MASK_ERROR instead.  This
is safe to do because this entire file is exception safe now.

ChangeLog
2017-04-25  Tom Tromey  <tom@tromey.com>

	PR cli/17716:
	* python/py-framefilter.c (py_print_type, py_print_value)
	(enumerate_args, py_print_args, gdbpy_apply_frame_filter): Use
	RETURN_MASK_ERROR.
---
 gdb/ChangeLog               |  7 +++++++
 gdb/python/py-framefilter.c | 22 +++++++++++-----------
 2 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 6b0c584..d0e0be4 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,12 @@
 2017-04-25  Tom Tromey  <tom@tromey.com>
 
+	PR cli/17716:
+	* python/py-framefilter.c (py_print_type, py_print_value)
+	(enumerate_args, py_print_args, gdbpy_apply_frame_filter): Use
+	RETURN_MASK_ERROR.
+
+2017-04-25  Tom Tromey  <tom@tromey.com>
+
 	* python/py-framefilter.c (enumerate_args): Use
 	gdb::unique_xmalloc_ptr.
 
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index 2ae2158..e0a7bd0 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -215,7 +215,7 @@ py_print_type (struct ui_out *out, struct value *val)
       type_print (value_type (val), "", &stb, -1);
       out->field_stream ("type", stb);
     }
-  CATCH (except, RETURN_MASK_ALL)
+  CATCH (except, RETURN_MASK_ERROR)
     {
       gdbpy_convert_exception (except);
       return EXT_LANG_BT_ERROR;
@@ -255,7 +255,7 @@ py_print_value (struct ui_out *out, struct value *val,
 	{
 	  type = check_typedef (value_type (val));
 	}
-      CATCH (except, RETURN_MASK_ALL)
+      CATCH (except, RETURN_MASK_ERROR)
 	{
 	  gdbpy_convert_exception (except);
 	  return EXT_LANG_BT_ERROR;
@@ -282,7 +282,7 @@ py_print_value (struct ui_out *out, struct value *val,
 	  common_val_print (val, &stb, indent, opts, language);
 	  out->field_stream ("value", stb);
 	}
-      CATCH (except, RETURN_MASK_ALL)
+      CATCH (except, RETURN_MASK_ERROR)
 	{
 	  gdbpy_convert_exception (except);
 	  return EXT_LANG_BT_ERROR;
@@ -497,7 +497,7 @@ enumerate_args (PyObject *iter,
     {
       annotate_frame_args ();
     }
-  CATCH (except, RETURN_MASK_ALL)
+  CATCH (except, RETURN_MASK_ERROR)
     {
       gdbpy_convert_exception (except);
       return EXT_LANG_BT_ERROR;
@@ -553,7 +553,7 @@ enumerate_args (PyObject *iter,
 	    {
 	      read_frame_arg (sym, frame, &arg, &entryarg);
 	    }
-	  CATCH (except, RETURN_MASK_ALL)
+	  CATCH (except, RETURN_MASK_ERROR)
 	    {
 	      gdbpy_convert_exception (except);
 	      return EXT_LANG_BT_ERROR;
@@ -586,7 +586,7 @@ enumerate_args (PyObject *iter,
 		      out->text (", ");
 		      out->wrap_hint ("    ");
 		    }
-		  CATCH (except, RETURN_MASK_ALL)
+		  CATCH (except, RETURN_MASK_ERROR)
 		    {
 		      gdbpy_convert_exception (except);
 		      return EXT_LANG_BT_ERROR;
@@ -622,7 +622,7 @@ enumerate_args (PyObject *iter,
 	    {
 	      out->text (", ");
 	    }
-	  CATCH (except, RETURN_MASK_ALL)
+	  CATCH (except, RETURN_MASK_ERROR)
 	    {
 	      gdbpy_convert_exception (except);
 	      return EXT_LANG_BT_ERROR;
@@ -636,7 +636,7 @@ enumerate_args (PyObject *iter,
 	{
 	  annotate_arg_end ();
 	}
-      CATCH (except, RETURN_MASK_ALL)
+      CATCH (except, RETURN_MASK_ERROR)
 	{
 	  gdbpy_convert_exception (except);
 	  return EXT_LANG_BT_ERROR;
@@ -867,7 +867,7 @@ py_print_args (PyObject *filter,
       if (! out->is_mi_like_p ())
 	out->text (" (");
     }
-  CATCH (except, RETURN_MASK_ALL)
+  CATCH (except, RETURN_MASK_ERROR)
     {
       gdbpy_convert_exception (except);
       return EXT_LANG_BT_ERROR;
@@ -884,7 +884,7 @@ py_print_args (PyObject *filter,
       if (! out->is_mi_like_p ())
 	out->text (")");
     }
-  CATCH (except, RETURN_MASK_ALL)
+  CATCH (except, RETURN_MASK_ERROR)
     {
       gdbpy_convert_exception (except);
       return EXT_LANG_BT_ERROR;
@@ -1336,7 +1336,7 @@ gdbpy_apply_frame_filter (const struct extension_language_defn *extlang,
     {
       gdbarch = get_frame_arch (frame);
     }
-  CATCH (except, RETURN_MASK_ALL)
+  CATCH (except, RETURN_MASK_ERROR)
     {
       /* Let gdb try to print the stack trace.  */
       return EXT_LANG_BT_NO_FILTERS;
-- 
2.9.3

Re: [RFA 06/10] Allow C-c to work in backtrace in more cases

[Phil Muldoon]

On 25/04/17 20:41, Tom Tromey wrote:
> PR cli/17716 notes that it is difficult to C-c (or "q" at a pagination
> prompt) while backtracing using a frame filter.  One reason for this
> is that many places in py-framefilter.c use RETURN_MASK_ALL in a
> try/catch.
> 
> This patch changes these spots to use RETURN_MASK_ERROR instead.  This
> is safe to do because this entire file is exception safe now.
> 
> ChangeLog
> 2017-04-25  Tom Tromey  <tom@tromey.com>
> 
> 	PR cli/17716:
> 	* python/py-framefilter.c (py_print_type, py_print_value)
> 	(enumerate_args, py_print_args, gdbpy_apply_frame_filter): Use
> 	RETURN_MASK_ERROR.

Ugh that was a simple mistake from when I submitted the patch and thanks for cleaning that up.

Patch LGTM

Cheers,

Phil

Re: [RFA 06/10] Allow C-c to work in backtrace in more cases

[Pedro Alves]

On 04/25/2017 08:41 PM, Tom Tromey wrote:
> PR cli/17716 notes that it is difficult to C-c (or "q" at a pagination
> prompt) while backtracing using a frame filter.  One reason for this
> is that many places in py-framefilter.c use RETURN_MASK_ALL in a
> try/catch.
> 
> This patch changes these spots to use RETURN_MASK_ERROR instead.  This
> is safe to do because this entire file is exception safe now.

Are any of these returning back to the Python runtime?  If so, then
we can't let C++ exceptions cross it.

Thanks,
Pedro Alves

[RFA 10/10] Call wrap_hint in one more spot in py-framefilter.c

[Tom Tromey]

PR python/16486 notes that "bt" output is still wrapped differently
when a frame filter is in use.  This patch brings it a bit closer by
adding one more wrap_hint call, in a place where stack.c does this as
well.

ChangeLog
2017-04-25  Tom Tromey  <tom@tromey.com>

	PR python/16486:
	* python/py-framefilter.c (py_print_args): Call wrap_hint.
---
 gdb/ChangeLog               | 5 +++++
 gdb/python/py-framefilter.c | 1 +
 2 files changed, 6 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b6c3f40..406b561 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2017-04-25  Tom Tromey  <tom@tromey.com>
 
+	PR python/16486:
+	* python/py-framefilter.c (py_print_args): Call wrap_hint.
+
+2017-04-25  Tom Tromey  <tom@tromey.com>
+
 	* python/py-framefilter.c (py_print_single_arg): Return
 	EXT_LANG_BT_ERROR from catch.
 
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index 73f2a31..7114030 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -864,6 +864,7 @@ py_print_args (PyObject *filter,
 
   TRY
     {
+      out->wrap_hint ("   ");
       annotate_frame_args ();
       if (! out->is_mi_like_p ())
 	out->text (" (");
-- 
2.9.3

Re: [RFA 10/10] Call wrap_hint in one more spot in py-framefilter.c

[Phil Muldoon]

On 25/04/17 20:41, Tom Tromey wrote:
> PR python/16486 notes that "bt" output is still wrapped differently
> when a frame filter is in use.  This patch brings it a bit closer by
> adding one more wrap_hint call, in a place where stack.c does this as
> well.
> 
> ChangeLog
> 2017-04-25  Tom Tromey  <tom@tromey.com>
> 
> 	PR python/16486:
> 	* python/py-framefilter.c (py_print_args): Call wrap_hint.

Thanks for catching this. LGTM

Cheers

Phil

Re: [RFA 10/10] Call wrap_hint in one more spot in py-framefilter.c

[Pedro Alves]

On 04/28/2017 04:09 PM, Phil Muldoon wrote:
> On 25/04/17 20:41, Tom Tromey wrote:
>> PR python/16486 notes that "bt" output is still wrapped differently
>> when a frame filter is in use.  This patch brings it a bit closer by
>> adding one more wrap_hint call, in a place where stack.c does this as
>> well.
>>
>> ChangeLog
>> 2017-04-25  Tom Tromey  <tom@tromey.com>
>>
>> 	PR python/16486:
>> 	* python/py-framefilter.c (py_print_args): Call wrap_hint.
> 
> Thanks for catching this. LGTM

To me too.

Thanks,
Pedro Alves

[RFA 09/10] Return EXT_LANG_BT_ERROR in one more spot in py-framefilter.c

[Tom Tromey]

While reading py-framefilter.c, I found one spot where an exception
could be caught but then not be turned into EXT_LANG_BT_ERROR.  This
patch fixes this spot.

ChangeLog
2017-04-25  Tom Tromey  <tom@tromey.com>

	* python/py-framefilter.c (py_print_single_arg): Return
	EXT_LANG_BT_ERROR from catch.
---
 gdb/ChangeLog               | 5 +++++
 gdb/python/py-framefilter.c | 1 +
 2 files changed, 6 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index db6dccf..b6c3f40 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2017-04-25  Tom Tromey  <tom@tromey.com>
 
+	* python/py-framefilter.c (py_print_single_arg): Return
+	EXT_LANG_BT_ERROR from catch.
+
+2017-04-25  Tom Tromey  <tom@tromey.com>
+
 	PR backtrace/15584:
 	* stack.c (backtrace_command_1): Move some code into no-filters
 	"if".
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index 76e637c..73f2a31 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -457,6 +457,7 @@ py_print_single_arg (struct ui_out *out,
   CATCH (except, RETURN_MASK_ERROR)
     {
       gdbpy_convert_exception (except);
+      retval = EXT_LANG_BT_ERROR;
     }
   END_CATCH
 
-- 
2.9.3

Re: [RFA 09/10] Return EXT_LANG_BT_ERROR in one more spot in py-framefilter.c

[Phil Muldoon]

On 25/04/17 20:41, Tom Tromey wrote:
> While reading py-framefilter.c, I found one spot where an exception
> could be caught but then not be turned into EXT_LANG_BT_ERROR.  This
> patch fixes this spot.
> 
> ChangeLog
> 2017-04-25  Tom Tromey  <tom@tromey.com>
> 
> 	* python/py-framefilter.c (py_print_single_arg): Return
> 	EXT_LANG_BT_ERROR from catch.

Patch LGTM/

Cheers

Phil

Re: [RFA 09/10] Return EXT_LANG_BT_ERROR in one more spot in py-framefilter.c

[Pedro Alves]

On 04/25/2017 08:41 PM, Tom Tromey wrote:
> While reading py-framefilter.c, I found one spot where an exception
> could be caught but then not be turned into EXT_LANG_BT_ERROR.  This
> patch fixes this spot.

Eek.  LGTM.

I wonder whether we could wrap the TRY/CATCHes in something
that would do this exception handling systematically/automatically.
Maybe:

template<typename Func>
enum ext_lang_bt_status success
try_py (Func &&f)
{
  TRY
    {
      f ();
    }
  CATCH (except, RETURN_MASK_ALL)
    {
      gdbpy_convert_exception (except);
      return EXT_LANG_BT_ERROR;
    }
  END_CATCH  

  return EXT_LANG_BT_OK;
}

Used like:

 return try_py ([] 
   {
     // old body of TRY goes here.
   });

Thanks,
Pedro Alves

Re: [RFA 09/10] Return EXT_LANG_BT_ERROR in one more spot in py-framefilter.c

[Pedro Alves]

On 06/27/2017 06:31 PM, Pedro Alves wrote:
> On 04/25/2017 08:41 PM, Tom Tromey wrote:
>> While reading py-framefilter.c, I found one spot where an exception
>> could be caught but then not be turned into EXT_LANG_BT_ERROR.  This
>> patch fixes this spot.
> 
> Eek.  LGTM.
> 
> I wonder whether we could wrap the TRY/CATCHes in something
> that would do this exception handling systematically/automatically.
> Maybe:
> 
> template<typename Func>
> enum ext_lang_bt_status success
> try_py (Func &&f)
> {
>   TRY
>     {
>       f ();
>     }
>   CATCH (except, RETURN_MASK_ALL)
>     {
>       gdbpy_convert_exception (except);
>       return EXT_LANG_BT_ERROR;
>     }
>   END_CATCH  
> 
>   return EXT_LANG_BT_OK;
> }
> 
> Used like:
> 
>  return try_py ([] 
>    {
>      // old body of TRY goes here.
>    });

I gave this a quick try, but it looked ugly, not much
of an improvement.  Maybe with statement expressions
or macros it'd look better...

The way to go is actually probably to follow up on the idea
of patch #6, and actually just remove the TRY/CATCHes,
letting the exceptions propagate and catch them somewhere
higher up the chain.  Other than marshalling C++ exceptions near
Python/ext_lang entry point code, is there another reason we need
all the TRY/CATCHes?  [assuming local resource management has
been RAII-fied.]

Thanks,
Pedro Alves

[RFA 07/10] Throw a "quit" on a KeyboardException in py-framefilter.c

[Tom Tromey]

If a C-c comes while the Python code for a frame filter is running, it
will be turned into a Python KeyboardException.  It seems good for
this to be treated like a GDB quit, so this patch changes
py-framefilter.c to notice this situation and call throw_quit in this
case.

ChangeLog
2017-04-25  Tom Tromey  <tom@tromey.com>

	* python/py-framefilter.c (throw_quit_or_print_exception): New
	function.
	(gdbpy_apply_frame_filter): Use it.

testsuite/ChangeLog
2017-04-25  Tom Tromey  <tom@tromey.com>

	* gdb.python/py-framefilter.exp: Add test for KeyboardInterrupt.
	* gdb.python/py-framefilter.py (name_error): New global.
	(ErrorInName.function): Use name_error.
---
 gdb/ChangeLog                               |  6 ++++++
 gdb/python/py-framefilter.c                 | 21 ++++++++++++++++++---
 gdb/testsuite/ChangeLog                     |  6 ++++++
 gdb/testsuite/gdb.python/py-framefilter.exp | 10 ++++++++++
 gdb/testsuite/gdb.python/py-framefilter.py  |  6 +++++-
 5 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index d0e0be4..0bf5386 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2017-04-25  Tom Tromey  <tom@tromey.com>
 
+	* python/py-framefilter.c (throw_quit_or_print_exception): New
+	function.
+	(gdbpy_apply_frame_filter): Use it.
+
+2017-04-25  Tom Tromey  <tom@tromey.com>
+
 	PR cli/17716:
 	* python/py-framefilter.c (py_print_type, py_print_value)
 	(enumerate_args, py_print_args, gdbpy_apply_frame_filter): Use
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index e0a7bd0..76e637c 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -1305,6 +1305,21 @@ bootstrap_python_frame_filters (struct frame_info *frame,
     return iterable.release ();
 }
 
+/* A helper function that will either print an exception or, if it is
+   a KeyboardException, throw a quit.  This can only be called when
+   the Python exception is set.  */
+
+static void
+throw_quit_or_print_exception ()
+{
+  if (PyErr_ExceptionMatches (PyExc_KeyboardInterrupt))
+    {
+      PyErr_Clear ();
+      throw_quit ("Quit");
+    }
+  gdbpy_print_stack ();
+}
+
 /*  This is the only publicly exported function in this file.  FRAME
     is the source frame to start frame-filter invocation.  FLAGS is an
     integer holding the flags for printing.  The following elements of
@@ -1363,7 +1378,7 @@ gdbpy_apply_frame_filter (const struct extension_language_defn *extlang,
 	 initialization error.  This return code will trigger a
 	 default backtrace.  */
 
-      gdbpy_print_stack ();
+      throw_quit_or_print_exception ();
       return EXT_LANG_BT_NO_FILTERS;
     }
 
@@ -1386,7 +1401,7 @@ gdbpy_apply_frame_filter (const struct extension_language_defn *extlang,
 	{
 	  if (PyErr_Occurred ())
 	    {
-	      gdbpy_print_stack ();
+	      throw_quit_or_print_exception ();
 	      return EXT_LANG_BT_ERROR;
 	    }
 	  break;
@@ -1398,7 +1413,7 @@ gdbpy_apply_frame_filter (const struct extension_language_defn *extlang,
       /* Do not exit on error printing a single frame.  Print the
 	 error and continue with other frames.  */
       if (success == EXT_LANG_BT_ERROR)
-	gdbpy_print_stack ();
+	throw_quit_or_print_exception ();
     }
 
   return success;
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index f9f48c5..6f6bbce 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,11 @@
 2017-04-25  Tom Tromey  <tom@tromey.com>
 
+	* gdb.python/py-framefilter.exp: Add test for KeyboardInterrupt.
+	* gdb.python/py-framefilter.py (name_error): New global.
+	(ErrorInName.function): Use name_error.
+
+2017-04-25  Tom Tromey  <tom@tromey.com>
+
 	PR backtrace/15582:
 	* gdb.python/py-framefilter.exp: Add "bt elide" test.
 
diff --git a/gdb/testsuite/gdb.python/py-framefilter.exp b/gdb/testsuite/gdb.python/py-framefilter.exp
index 80aa495..1a0572d 100644
--- a/gdb/testsuite/gdb.python/py-framefilter.exp
+++ b/gdb/testsuite/gdb.python/py-framefilter.exp
@@ -213,6 +213,16 @@ gdb_test_multiple "bt 1" $test {
     }
 }
 
+# Now verify that we can see a quit.
+gdb_test_no_output "python name_error = KeyboardInterrupt" \
+    "Change ErrorFilter to throw KeyboardInterrupt"
+set test "bt 1 with KeyboardInterrupt"
+gdb_test_multiple "bt 1" $test {
+    -re "Quit" {
+	pass $test
+    }
+}
+
 # Test with no debuginfo
 
 # We cannot use prepare_for_testing as we have to set the safe-patch
diff --git a/gdb/testsuite/gdb.python/py-framefilter.py b/gdb/testsuite/gdb.python/py-framefilter.py
index edd0e00..b579ec3 100644
--- a/gdb/testsuite/gdb.python/py-framefilter.py
+++ b/gdb/testsuite/gdb.python/py-framefilter.py
@@ -134,13 +134,17 @@ class FrameElider ():
     def filter (self, frame_iter):
         return ElidingIterator (frame_iter)
 
+# This is here so the test can change the kind of error that is
+# thrown.
+name_error = RuntimeError
+
 # A simple decorator that gives an error when computing the function.
 class ErrorInName(FrameDecorator):
     def __init__(self, frame):
         FrameDecorator.__init__(self, frame)
 
     def function(self):
-        raise RuntimeError('whoops')
+        raise name_error('whoops')
 
 # A filter that supplies buggy frames.  Disabled by default.
 class ErrorFilter():
-- 
2.9.3

Re: [RFA 07/10] Throw a "quit" on a KeyboardException in py-framefilter.c

[Phil Muldoon]

On 25/04/17 20:41, Tom Tromey wrote:
> If a C-c comes while the Python code for a frame filter is running, it
> will be turned into a Python KeyboardException.  It seems good for
> this to be treated like a GDB quit, so this patch changes
> py-framefilter.c to notice this situation and call throw_quit in this
> case.

Patch LGTM (it took me awhile to understand the name_error = RuntimeError|KeyboardInterrupt and then throwing name_error. Clever!)

Cheers

Phil

Re: [RFA 07/10] Throw a "quit" on a KeyboardException in py-framefilter.c

[Pedro Alves]

On 04/25/2017 08:41 PM, Tom Tromey wrote:
> If a C-c comes while the Python code for a frame filter is running, it
> will be turned into a Python KeyboardException.  It seems good for
> this to be treated like a GDB quit, so this patch changes
> py-framefilter.c to notice this situation and call throw_quit in this
> case.

LGTM.

Thanks,
Pedro Alves

[RFA 05/10] Avoid manual resource management in py-framefilter.c

[Tom Tromey]

This patch removes the last bit of manual resource management from
py-framefilter.c.  This will be useful in the next patch.

ChangeLog
2017-04-25  Tom Tromey  <tom@tromey.com>

	* python/py-framefilter.c (enumerate_args): Use
	gdb::unique_xmalloc_ptr.
---
 gdb/ChangeLog               |  5 +++++
 gdb/python/py-framefilter.c | 20 +++++---------------
 2 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 176332a..6b0c584 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2017-04-25  Tom Tromey  <tom@tromey.com>
 
+	* python/py-framefilter.c (enumerate_args): Use
+	gdb::unique_xmalloc_ptr.
+
+2017-04-25  Tom Tromey  <tom@tromey.com>
+
 	* python/py-framefilter.c (py_print_frame): Return
 	EXT_LANG_BT_OK.
 	(gdbpy_apply_frame_filter): Update comment.
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index 2bbbfbe..2ae2158 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -560,6 +560,9 @@ enumerate_args (PyObject *iter,
 	    }
 	  END_CATCH
 
+	  gdb::unique_xmalloc_ptr<char> arg_holder (arg.error);
+	  gdb::unique_xmalloc_ptr<char> entry_holder (entryarg.error);
+
 	  /* The object has not provided a value, so this is a frame
 	     argument to be read by GDB.  In this case we have to
 	     account for entry-values.  */
@@ -571,11 +574,7 @@ enumerate_args (PyObject *iter,
 				       args_type,
 				       print_args_field,
 				       NULL) == EXT_LANG_BT_ERROR)
-		{
-		  xfree (arg.error);
-		  xfree (entryarg.error);
-		  return EXT_LANG_BT_ERROR;
-		}
+		return EXT_LANG_BT_ERROR;
 	    }
 
 	  if (entryarg.entry_kind != print_entry_values_no)
@@ -589,8 +588,6 @@ enumerate_args (PyObject *iter,
 		    }
 		  CATCH (except, RETURN_MASK_ALL)
 		    {
-		      xfree (arg.error);
-		      xfree (entryarg.error);
 		      gdbpy_convert_exception (except);
 		      return EXT_LANG_BT_ERROR;
 		    }
@@ -600,15 +597,8 @@ enumerate_args (PyObject *iter,
 	      if (py_print_single_arg (out, NULL, &entryarg, NULL, &opts,
 				       args_type, print_args_field, NULL)
 		  == EXT_LANG_BT_ERROR)
-		{
-		  xfree (arg.error);
-		  xfree (entryarg.error);
-		  return EXT_LANG_BT_ERROR;
-		}
+		return EXT_LANG_BT_ERROR;
 	    }
-
-	  xfree (arg.error);
-	  xfree (entryarg.error);
 	}
       else
 	{
-- 
2.9.3

Re: [RFA 05/10] Avoid manual resource management in py-framefilter.c

[Phil Muldoon]

On 25/04/17 20:41, Tom Tromey wrote:
> This patch removes the last bit of manual resource management from
> py-framefilter.c.  This will be useful in the next patch.
> 
> ChangeLog
> 2017-04-25  Tom Tromey  <tom@tromey.com>
> 
> 	* python/py-framefilter.c (enumerate_args): Use
> 	gdb::unique_xmalloc_ptr.

LGTM (and much cleaner). Thanks for doing this and the original automated resource management and reference counting from previous patches.

Cheers

Phil

Re: [RFA 05/10] Avoid manual resource management in py-framefilter.c

[Pedro Alves]

On 04/25/2017 08:41 PM, Tom Tromey wrote:
> This patch removes the last bit of manual resource management from
> py-framefilter.c.  This will be useful in the next patch.
> 
> ChangeLog
> 2017-04-25  Tom Tromey  <tom@tromey.com>
> 
> 	* python/py-framefilter.c (enumerate_args): Use
> 	gdb::unique_xmalloc_ptr.

OK.

Thanks,
Pedro Alves

[RFA 03/10] Allow elision of some filtered frames

[Tom Tromey]

When a frame filter elides some frames, they are still printed by
"bt", indented a few spaces.  PR backtrace/15582 notes that it would
be nice for users if elided frames could simply be dropped.  This
patch adds this capability.

ChangeLog
2017-04-25  Tom Tromey  <tom@tromey.com>

	PR backtrace/15582:
	* stack.c (backtrace_command): Parse "elide" argument.
	* python/py-framefilter.c (py_print_frame): Handle PRINT_ELIDE.
	* extension.h (enum frame_filter_flags) <PRINT_ELIDE>: New
	constant.

doc/ChangeLog
2017-04-25  Tom Tromey  <tom@tromey.com>

	PR backtrace/15582:
	* gdb.texinfo (Backtrace): Mention "elide" argument.

testsuite/ChangeLog
2017-04-25  Tom Tromey  <tom@tromey.com>

	PR backtrace/15582:
	* gdb.python/py-framefilter.exp: Add "bt elide" test.
---
 gdb/ChangeLog                               |  8 +++++
 gdb/doc/ChangeLog                           |  5 +++
 gdb/doc/gdb.texinfo                         |  6 ++++
 gdb/extension.h                             |  3 ++
 gdb/python/py-framefilter.c                 | 50 ++++++++++++++---------------
 gdb/stack.c                                 |  2 ++
 gdb/testsuite/ChangeLog                     |  5 +++
 gdb/testsuite/gdb.python/py-framefilter.exp |  3 ++
 8 files changed, 57 insertions(+), 25 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ab9b432..f68e32c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,13 @@
 2017-04-25  Tom Tromey  <tom@tromey.com>
 
+	PR backtrace/15582:
+	* stack.c (backtrace_command): Parse "elide" argument.
+	* python/py-framefilter.c (py_print_frame): Handle PRINT_ELIDE.
+	* extension.h (enum frame_filter_flags) <PRINT_ELIDE>: New
+	constant.
+
+2017-04-25  Tom Tromey  <tom@tromey.com>
+
 	* stack.c (backtrace_command_1): Remove "show_locals" parameter,
 	add "flags".
 	(backtrace_command): Remove "fulltrace", add "flags".
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index cf2226c..1cdc809 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,5 +1,10 @@
 2017-04-25  Tom Tromey  <tom@tromey.com>
 
+	PR backtrace/15582:
+	* gdb.texinfo (Backtrace): Mention "elide" argument.
+
+2017-04-25  Tom Tromey  <tom@tromey.com>
+
 	* gdb.texinfo (Backtrace): Describe options individually.
 
 2017-04-21  Simon Marchi  <simon.marchi@ericsson.com>
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index a9f12fd..c30b7b5 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -7225,6 +7225,12 @@ Filter API}, for more information.  Additionally use @ref{disable
 frame-filter all} to turn off all frame filters.  This is only
 relevant when @value{GDBN} has been configured with @code{Python}
 support.
+
+@item backtrace elide
+A Python frame filter might decide to ``elide'' some frames.  Normally
+such elided frames are still printed, but they are indented relative
+to the filtered frames that cause them to be elided.  The @code{elide}
+option causes elided frames to not be printed at all.
 @end table
 
 @kindex where
diff --git a/gdb/extension.h b/gdb/extension.h
index 2c79411..7c35502 100644
--- a/gdb/extension.h
+++ b/gdb/extension.h
@@ -100,6 +100,9 @@ enum frame_filter_flags
 
     /* Set this flag if frame locals are to be printed.  */
     PRINT_LOCALS = 8,
+
+    /* Set this flag if elided frames should not be printed.  */
+    PRINT_ELIDE = 32,
   };
 
 /* A choice of the different frame argument printing strategies that
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index 75b055c..f0474a9 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -1238,37 +1238,37 @@ py_print_frame (PyObject *filter, int flags,
 	return EXT_LANG_BT_ERROR;
     }
 
-  {
-    /* Finally recursively print elided frames, if any.  */
-    gdbpy_ref<> elided (get_py_iter_from_func (filter, "elided"));
-    if (elided == NULL)
-      return EXT_LANG_BT_ERROR;
+  if ((flags & PRINT_ELIDE) == 0)
+    {
+      /* Finally recursively print elided frames, if any.  */
+      gdbpy_ref<> elided (get_py_iter_from_func (filter, "elided"));
+      if (elided == NULL)
+	return EXT_LANG_BT_ERROR;
 
-    if (elided != Py_None)
-      {
-	PyObject *item;
+      if (elided != Py_None)
+	{
+	  PyObject *item;
 
-	ui_out_emit_list inner_list_emiter (out, "children");
+	  ui_out_emit_list inner_list_emiter (out, "children");
 
-	if (! out->is_mi_like_p ())
-	  indent++;
+	  if (! out->is_mi_like_p ())
+	    indent++;
 
-	while ((item = PyIter_Next (elided.get ())))
-	  {
-	    gdbpy_ref<> item_ref (item);
+	  while ((item = PyIter_Next (elided.get ())))
+	    {
+	      gdbpy_ref<> item_ref (item);
 
-	    enum ext_lang_bt_status success = py_print_frame (item, flags,
-							      args_type, out,
-							      indent,
-							      levels_printed);
+	      enum ext_lang_bt_status success
+		= py_print_frame (item, flags, args_type, out, indent,
+				  levels_printed);
 
-	    if (success == EXT_LANG_BT_ERROR)
-	      return EXT_LANG_BT_ERROR;
-	  }
-	if (item == NULL && PyErr_Occurred ())
-	  return EXT_LANG_BT_ERROR;
-      }
-  }
+	      if (success == EXT_LANG_BT_ERROR)
+		return EXT_LANG_BT_ERROR;
+	    }
+	  if (item == NULL && PyErr_Occurred ())
+	    return EXT_LANG_BT_ERROR;
+	}
+    }
 
   return EXT_LANG_BT_COMPLETED;
 }
diff --git a/gdb/stack.c b/gdb/stack.c
index b320c93..77e8c9b 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1890,6 +1890,8 @@ backtrace_command (char *arg, int from_tty)
 	    filters = false;
 	  else if (subset_compare (this_arg, "full"))
 	    flags |= PRINT_LOCALS;
+	  else if (subset_compare (this_arg, "elide"))
+	    flags |= PRINT_ELIDE;
 	  else
 	    {
 	      /* Not a recognized argument, so stop.  */
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index c4d5b79..f9f48c5 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2017-04-25  Tom Tromey  <tom@tromey.com>
+
+	PR backtrace/15582:
+	* gdb.python/py-framefilter.exp: Add "bt elide" test.
+
 2017-04-19  Pedro Alves  <palves@redhat.com>
 
 	* gdb.threads/threadapply.exp (kill_and_remove_inferior): New
diff --git a/gdb/testsuite/gdb.python/py-framefilter.exp b/gdb/testsuite/gdb.python/py-framefilter.exp
index bbec48d..80aa495 100644
--- a/gdb/testsuite/gdb.python/py-framefilter.exp
+++ b/gdb/testsuite/gdb.python/py-framefilter.exp
@@ -157,6 +157,9 @@ gdb_test "bt no-filter full" \
 gdb_test "bt full" \
     ".*#0.*end_func.*str = $hex \"The End\".*st2 = $hex \"Is Near\".*b = 12.*c = 5.*#1.*in funca \\(\\).*#2.*in funcb \\(j=10\\).*bar = \{a = 42, b = 84\}.*#22.*in func1 \\(\\).*#23.*in func2 \\(f=3\\).*elided = $hex \"Elided frame\".*fb = \{nothing = $hex \"Elided Foo Bar\", f = 84, s = 38\}.*bf = $hex.*" \
     "bt full with Reverse disabled"
+gdb_test "bt full elide" \
+    ".*#0.*end_func.*str = $hex \"The End\".*st2 = $hex \"Is Near\".*b = 12.*c = 5.*#1.*in funca \\(\\).*#2.*in funcb \\(j=10\\).*bar = \{a = 42, b = 84\}.*#22.*in func1 \\(\\)\[^#\]*#24.*in func3 \\(i=3\\).*" \
+    "bt full elide with Reverse disabled"
 
 # Test set print frame-arguments
 # none
-- 
2.9.3

Re: [RFA 03/10] Allow elision of some filtered frames

[Eli Zaretskii]

> From: Tom Tromey <tom@tromey.com>
> Cc: Tom Tromey <tom@tromey.com>
> Date: Tue, 25 Apr 2017 13:41:06 -0600
> 
> When a frame filter elides some frames, they are still printed by
> "bt", indented a few spaces.  PR backtrace/15582 notes that it would
> be nice for users if elided frames could simply be dropped.  This
> patch adds this capability.

OK for the documentation part.

Does this warrant a NEWS entry?

Thanks.

Re: [RFA 03/10] Allow elision of some filtered frames

[Phil Muldoon]

On 25/04/17 20:41, Tom Tromey wrote:
> When a frame filter elides some frames, they are still printed by
> "bt", indented a few spaces.  PR backtrace/15582 notes that it would
> be nice for users if elided frames could simply be dropped.  This
> patch adds this capability.
> 
> ChangeLog
> 2017-04-25  Tom Tromey  <tom@tromey.com>
> 
> 	PR backtrace/15582:
> 	* stack.c (backtrace_command): Parse "elide" argument.
> 	* python/py-framefilter.c (py_print_frame): Handle PRINT_ELIDE.
> 	* extension.h (enum frame_filter_flags) <PRINT_ELIDE>: New
> 	constant.
> 
> doc/ChangeLog
> 2017-04-25  Tom Tromey  <tom@tromey.com>
> 
> 	PR backtrace/15582:
> 	* gdb.texinfo (Backtrace): Mention "elide" argument.

I would have like to have seen these elide/display decisions (whether to show or not to show, whether to show with indention, etc) be made in the frame decorator itself. The decision whether a frame is actually elided or not is made there and it would allowed more flexibility if elided frames were to be displayed on a frame by frame basis. But I'm not sure if this could lead to confusing output. Anyway, it does not matter to much at this point as your patch adds the equivalent of a global override and this other, more intricate functionality, can be added later. I'm curious what you think though.

Patch LGTM.

Cheers

Phil

Re: [RFA 03/10] Allow elision of some filtered frames

[Pedro Alves]

On 04/25/2017 08:41 PM, Tom Tromey wrote:
> When a frame filter elides some frames, they are still printed by
> "bt", indented a few spaces.  PR backtrace/15582 notes that it would
> be nice for users if elided frames could simply be dropped.  This
> patch adds this capability.
> 

So "bt elide" means "elide the elided frames", not "show me the
elided frames too".  It's fine with me, though I mildly wonder whether
users will be confused by the "double negative".

> diff --git a/gdb/extension.h b/gdb/extension.h
> index 2c79411..7c35502 100644
> --- a/gdb/extension.h
> +++ b/gdb/extension.h
> @@ -100,6 +100,9 @@ enum frame_filter_flags
>  
>      /* Set this flag if frame locals are to be printed.  */
>      PRINT_LOCALS = 8,
> +
> +    /* Set this flag if elided frames should not be printed.  */
> +    PRINT_ELIDE = 32,

Looks like 16 was elided.  :-)

LGTM.

Thanks,
Pedro Alves

Re: [RFA 03/10] Allow elision of some filtered frames

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

>> PRINT_LOCALS = 8,
>> +
>> +    /* Set this flag if elided frames should not be printed.  */
>> +    PRINT_ELIDE = 32,

Pedro> Looks like 16 was elided.  :-)

I had used it in a different patch (which is also still pending).

Tom

[RFA 02/10] Change backtrace_command_1 calling to use flags

[Tom Tromey]

The next patch will add more flags to backtrace_command_1; and rather
than add another boolean argument, this patch changes it to accept a
flags value.

ChangeLog
2017-04-25  Tom Tromey  <tom@tromey.com>

	* stack.c (backtrace_command_1): Remove "show_locals" parameter,
	add "flags".
	(backtrace_command): Remove "fulltrace", add "flags".
---
 gdb/ChangeLog |  6 ++++++
 gdb/stack.c   | 15 ++++++---------
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 48f2063..ab9b432 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2017-04-25  Tom Tromey  <tom@tromey.com>
 
+	* stack.c (backtrace_command_1): Remove "show_locals" parameter,
+	add "flags".
+	(backtrace_command): Remove "fulltrace", add "flags".
+
+2017-04-25  Tom Tromey  <tom@tromey.com>
+
 	* stack.c (backtrace_command): Rewrite command line parsing.
 
 2017-04-25  Yao Qi  <yao.qi@linaro.org>
diff --git a/gdb/stack.c b/gdb/stack.c
index c2fc3f1..b320c93 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1714,7 +1714,7 @@ frame_info (char *addr_exp, int from_tty)
    frames.  */
 
 static void
-backtrace_command_1 (char *count_exp, int show_locals, int no_filters,
+backtrace_command_1 (char *count_exp, int flags, int no_filters,
 		     int from_tty)
 {
   struct frame_info *fi;
@@ -1795,11 +1795,9 @@ backtrace_command_1 (char *count_exp, int show_locals, int no_filters,
 
   if (! no_filters)
     {
-      int flags = PRINT_LEVEL | PRINT_FRAME_INFO | PRINT_ARGS;
       enum ext_lang_frame_args arg_type;
 
-      if (show_locals)
-	flags |= PRINT_LOCALS;
+      flags |= PRINT_LEVEL | PRINT_FRAME_INFO | PRINT_ARGS;
 
       if (!strcmp (print_frame_arguments, "scalars"))
 	arg_type = CLI_SCALAR_VALUES;
@@ -1827,7 +1825,7 @@ backtrace_command_1 (char *count_exp, int show_locals, int no_filters,
 	     the frame->prev field gets set to NULL in that case).  */
 
 	  print_frame_info (fi, 1, LOCATION, 1, 0);
-	  if (show_locals)
+	  if ((flags & PRINT_LOCALS) != 0)
 	    {
 	      struct frame_id frame_id = get_frame_id (fi);
 
@@ -1868,7 +1866,7 @@ backtrace_command_1 (char *count_exp, int show_locals, int no_filters,
 static void
 backtrace_command (char *arg, int from_tty)
 {
-  bool fulltrace = false;
+  int flags = 0;
   bool filters = true;
 
   if (arg)
@@ -1891,7 +1889,7 @@ backtrace_command (char *arg, int from_tty)
 	  if (subset_compare (this_arg, "no-filters"))
 	    filters = false;
 	  else if (subset_compare (this_arg, "full"))
-	    fulltrace = true;
+	    flags |= PRINT_LOCALS;
 	  else
 	    {
 	      /* Not a recognized argument, so stop.  */
@@ -1906,8 +1904,7 @@ backtrace_command (char *arg, int from_tty)
 	arg = NULL;
     }
 
-  backtrace_command_1 (arg, fulltrace /* show_locals */,
-		       !filters /* no frame-filters */, from_tty);
+  backtrace_command_1 (arg, flags, !filters /* no frame-filters */, from_tty);
 }
 
 /* Iterate over the local variables of a block B, calling CB with
-- 
2.9.3

Re: [RFA 02/10] Change backtrace_command_1 calling to use flags

[Phil Muldoon]

On 25/04/17 20:41, Tom Tromey wrote:
> The next patch will add more flags to backtrace_command_1; and rather
> than add another boolean argument, this patch changes it to accept a
> flags value.

Patch LGTM.

Cheers,

Phil

Re: [RFA 02/10] Change backtrace_command_1 calling to use flags

[Pedro Alves]

On 04/25/2017 08:41 PM, Tom Tromey wrote:

> @@ -1891,7 +1889,7 @@ backtrace_command (char *arg, int from_tty)
>  	  if (subset_compare (this_arg, "no-filters"))
>  	    filters = false;
>  	  else if (subset_compare (this_arg, "full"))
> -	    fulltrace = true;
> +	    flags |= PRINT_LOCALS;
>  	  else
>  	    {
>  	      /* Not a recognized argument, so stop.  */
> @@ -1906,8 +1904,7 @@ backtrace_command (char *arg, int from_tty)
>  	arg = NULL;
>      }
>  
> -  backtrace_command_1 (arg, fulltrace /* show_locals */,
> -		       !filters /* no frame-filters */, from_tty);
> +  backtrace_command_1 (arg, flags, !filters /* no frame-filters */, from_tty);
>  }
>  

Looks fine.

(We should really make enum frame_filter_flags be an enum-flags.)

Thanks,
Pedro Alves

Re: [RFA 02/10] Change backtrace_command_1 calling to use flags

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> (We should really make enum frame_filter_flags be an enum-flags.)

I wrote a patch for this on my "bt N" branch, after Sergio requested the
same thing, but I neglected to send it.

I've appended it.  As it is based on a different series (that also
touches the enum), it won't apply directly as part of this one.

Tom

commit a6fe3b8ebb23001f9bdf1149a8b8e186b1121ed8
Author: Tom Tromey <tom@tromey.com>
Date:   Tue Apr 25 22:33:50 2017 -0600

    Change frame_filter_flags to use DEF_ENUM_FLAGS_TYPE
    
    This changes frame_filter_flags to use DEF_ENUM_FLAGS_TYPE, and
    updates all the uses.  It also changes the enum constants to use <<,
    as suggested by Sergio.
    
    ChangeLog
    2017-04-25  Tom Tromey  <tom@tromey.com>
    
            * stack.c (backtrace_command_1): Update.
            * python/python-internal.h (gdbpy_apply_frame_filter): Change type
            of "flags".
            * python/py-framefilter.c (py_print_frame)
            (gdbpy_apply_frame_filter): Change type of "flags".
            * mi/mi-cmd-stack.c (mi_apply_ext_lang_frame_filter): Change type
            of "flags".
            (mi_cmd_stack_list_frames, mi_cmd_stack_list_locals)
            (mi_cmd_stack_list_args, mi_cmd_stack_list_variables): Update.
            * extension.h (enum frame_filter_flag): Rename from
            frame_filter_flags.
            (frame_filter_flags): Define using DEF_ENUM_FLAGS_TYPE.
            (apply_ext_lang_frame_filter): Change type of "flags".
            * extension.c (apply_ext_lang_frame_filter): Change type of
            "flags".
            * extension-priv.h (struct extension_language_ops)
            <apply_frame_filter>: Change type of "flags".

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7571fa9..35a1e81 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,23 @@
+2017-04-25  Tom Tromey  <tom@tromey.com>
+
+	* stack.c (backtrace_command_1): Update.
+	* python/python-internal.h (gdbpy_apply_frame_filter): Change type
+	of "flags".
+	* python/py-framefilter.c (py_print_frame)
+	(gdbpy_apply_frame_filter): Change type of "flags".
+	* mi/mi-cmd-stack.c (mi_apply_ext_lang_frame_filter): Change type
+	of "flags".
+	(mi_cmd_stack_list_frames, mi_cmd_stack_list_locals)
+	(mi_cmd_stack_list_args, mi_cmd_stack_list_variables): Update.
+	* extension.h (enum frame_filter_flag): Rename from
+	frame_filter_flags.
+	(frame_filter_flags): Define using DEF_ENUM_FLAGS_TYPE.
+	(apply_ext_lang_frame_filter): Change type of "flags".
+	* extension.c (apply_ext_lang_frame_filter): Change type of
+	"flags".
+	* extension-priv.h (struct extension_language_ops)
+	<apply_frame_filter>: Change type of "flags".
+
 2017-04-22  Tom Tromey  <tom@tromey.com>
 
 	PR python/16497:
diff --git a/gdb/extension-priv.h b/gdb/extension-priv.h
index f77f088..05bb850 100644
--- a/gdb/extension-priv.h
+++ b/gdb/extension-priv.h
@@ -202,7 +202,8 @@ struct extension_language_ops
      or SCR_BT_COMPLETED on success.  */
   enum ext_lang_bt_status (*apply_frame_filter)
     (const struct extension_language_defn *,
-     struct frame_info *frame, int flags, enum ext_lang_frame_args args_type,
+     struct frame_info *frame, frame_filter_flags flags,
+     enum ext_lang_frame_args args_type,
      struct ui_out *out, int frame_low, int frame_high);
 
   /* Update values held by the extension language when OBJFILE is discarded.
diff --git a/gdb/extension.c b/gdb/extension.c
index cfbae2c..4a7bc6e 100644
--- a/gdb/extension.c
+++ b/gdb/extension.c
@@ -553,7 +553,8 @@ apply_ext_lang_val_pretty_printer (struct type *type,
    rather than trying filters in other extension languages.  */
 
 enum ext_lang_bt_status
-apply_ext_lang_frame_filter (struct frame_info *frame, int flags,
+apply_ext_lang_frame_filter (struct frame_info *frame,
+			     frame_filter_flags flags,
 			     enum ext_lang_frame_args args_type,
 			     struct ui_out *out,
 			     int frame_low, int frame_high)
diff --git a/gdb/extension.h b/gdb/extension.h
index cda2ebf..a9e3c85 100644
--- a/gdb/extension.h
+++ b/gdb/extension.h
@@ -87,24 +87,26 @@ enum ext_lang_bt_status
 
 /* Flags to pass to apply_extlang_frame_filter.  */
 
-enum frame_filter_flags
+enum frame_filter_flag
   {
     /* Set this flag if frame level is to be printed.  */
-    PRINT_LEVEL = 1,
+    PRINT_LEVEL = 1 << 0,
 
     /* Set this flag if frame information is to be printed.  */
-    PRINT_FRAME_INFO = 2,
+    PRINT_FRAME_INFO = 1 << 1,
 
     /* Set this flag if frame arguments are to be printed.  */
-    PRINT_ARGS = 4,
+    PRINT_ARGS = 1 << 2,
 
     /* Set this flag if frame locals are to be printed.  */
-    PRINT_LOCALS = 8,
+    PRINT_LOCALS = 1 << 3,
 
     /* Set this flag if a "More frames" message is to be printed.  */
-    PRINT_MORE_FRAMES = 16,
+    PRINT_MORE_FRAMES = 1 << 4,
   };
 
+DEF_ENUM_FLAGS_TYPE (enum frame_filter_flag, frame_filter_flags);
+
 /* A choice of the different frame argument printing strategies that
    can occur in different cases of frame filter instantiation.  */
 
@@ -235,7 +237,8 @@ extern int apply_ext_lang_val_pretty_printer
    const struct language_defn *language);
 
 extern enum ext_lang_bt_status apply_ext_lang_frame_filter
-  (struct frame_info *frame, int flags, enum ext_lang_frame_args args_type,
+  (struct frame_info *frame, frame_filter_flags flags,
+   enum ext_lang_frame_args args_type,
    struct ui_out *out, int frame_low, int frame_high);
 
 extern void preserve_ext_lang_values (struct objfile *, htab_t copied_types);
diff --git a/gdb/mi/mi-cmd-stack.c b/gdb/mi/mi-cmd-stack.c
index 6250b75..4b9230e 100644
--- a/gdb/mi/mi-cmd-stack.c
+++ b/gdb/mi/mi-cmd-stack.c
@@ -57,7 +57,8 @@ mi_cmd_enable_frame_filters (const char *command, char **argv, int argc)
 /* Like apply_ext_lang_frame_filter, but take a print_values */
 
 static enum ext_lang_bt_status
-mi_apply_ext_lang_frame_filter (struct frame_info *frame, int flags,
+mi_apply_ext_lang_frame_filter (struct frame_info *frame,
+				frame_filter_flags flags,
 				enum print_values print_values,
 				struct ui_out *out,
 				int frame_low, int frame_high)
@@ -146,7 +147,7 @@ mi_cmd_stack_list_frames (const char *command, char **argv, int argc)
 
   if (! raw_arg && frame_filters)
     {
-      int flags = PRINT_LEVEL | PRINT_FRAME_INFO;
+      frame_filter_flags flags = PRINT_LEVEL | PRINT_FRAME_INFO;
       int py_frame_low = frame_low;
 
       /* We cannot pass -1 to frame_low, as that would signify a
@@ -262,7 +263,7 @@ mi_cmd_stack_list_locals (const char *command, char **argv, int argc)
 
    if (! raw_arg && frame_filters)
      {
-       int flags = PRINT_LEVEL | PRINT_LOCALS;
+       frame_filter_flags flags = PRINT_LEVEL | PRINT_LOCALS;
 
        result = mi_apply_ext_lang_frame_filter (frame, flags, print_value,
 						current_uiout, 0, 0);
@@ -359,7 +360,7 @@ mi_cmd_stack_list_args (const char *command, char **argv, int argc)
 
   if (! raw_arg && frame_filters)
     {
-      int flags = PRINT_LEVEL | PRINT_ARGS;
+      frame_filter_flags flags = PRINT_LEVEL | PRINT_ARGS;
       int py_frame_low = frame_low;
 
       /* We cannot pass -1 to frame_low, as that would signify a
@@ -451,7 +452,7 @@ mi_cmd_stack_list_variables (const char *command, char **argv, int argc)
 
    if (! raw_arg && frame_filters)
      {
-       int flags = PRINT_LEVEL | PRINT_ARGS | PRINT_LOCALS;
+       frame_filter_flags flags = PRINT_LEVEL | PRINT_ARGS | PRINT_LOCALS;
 
        result = mi_apply_ext_lang_frame_filter (frame, flags,
 						print_value,
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index 9eef81b..22bf3b0 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -920,7 +920,7 @@ py_print_args (PyObject *filter,
     on success.  It can also throw an exception RETURN_QUIT.  */
 
 static enum ext_lang_bt_status
-py_print_frame (PyObject *filter, int flags,
+py_print_frame (PyObject *filter, frame_filter_flags flags,
 		enum ext_lang_frame_args args_type,
 		struct ui_out *out, int indent, htab_t levels_printed)
 {
@@ -1332,7 +1332,7 @@ bootstrap_python_frame_filters (struct frame_info *frame,
 
 enum ext_lang_bt_status
 gdbpy_apply_frame_filter (const struct extension_language_defn *extlang,
-			  struct frame_info *frame, int flags,
+			  struct frame_info *frame, frame_filter_flags flags,
 			  enum ext_lang_frame_args args_type,
 			  struct ui_out *out, int frame_low, int frame_high)
 {
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index e84c8d2..b6cca7c 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -453,7 +453,8 @@ extern enum ext_lang_rc gdbpy_apply_val_pretty_printer
    const struct language_defn *language);
 extern enum ext_lang_bt_status gdbpy_apply_frame_filter
   (const struct extension_language_defn *,
-   struct frame_info *frame, int flags, enum ext_lang_frame_args args_type,
+   struct frame_info *frame, frame_filter_flags flags,
+   enum ext_lang_frame_args args_type,
    struct ui_out *out, int frame_low, int frame_high);
 extern void gdbpy_preserve_values (const struct extension_language_defn *,
 				   struct objfile *objfile,
diff --git a/gdb/stack.c b/gdb/stack.c
index 37e8767..7493eb1 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1797,7 +1797,7 @@ backtrace_command_1 (char *count_exp, int show_locals, int no_filters,
 
   if (! no_filters)
     {
-      int flags = PRINT_LEVEL | PRINT_FRAME_INFO | PRINT_ARGS;
+      frame_filter_flags flags = PRINT_LEVEL | PRINT_FRAME_INFO | PRINT_ARGS;
       enum ext_lang_frame_args arg_type;
 
       if (show_locals)

Re: [RFA 02/10] Change backtrace_command_1 calling to use flags

[Pedro Alves]

On 07/09/2017 10:58 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> (We should really make enum frame_filter_flags be an enum-flags.)
> 
> I wrote a patch for this on my "bt N" branch, after Sergio requested the
> same thing, but I neglected to send it.
> 
> I've appended it.  As it is based on a different series (that also
> touches the enum), it won't apply directly as part of this one.
> 

Thanks, looks fine to me.

Thanks,
Pedro Alves

Re: [RFA 02/10] Change backtrace_command_1 calling to use flags

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

>> I've appended it.  As it is based on a different series (that also
>> touches the enum), it won't apply directly as part of this one.

Pedro> Thanks, looks fine to me.

Thanks.  This is on the "bt N" branch, so it will go in whenever that
patch is approved.

Tom

[RFA 08/10] Move some code later in backtrace_command_1

[Tom Tromey]

PR backtrace/15584 notes that some code in backtrace_command_1 is not
useful when frame filters are in use.  This patch moves this code into
the no-frame-filters "if".  This also removes the unused local
"trailing_level", which I noticed while moving the code around.

ChangeLog
2017-04-25  Tom Tromey  <tom@tromey.com>

	PR backtrace/15584:
	* stack.c (backtrace_command_1): Move some code into no-filters
	"if".
---
 gdb/ChangeLog |   6 ++++
 gdb/stack.c   | 105 +++++++++++++++++++++++++++++-----------------------------
 2 files changed, 59 insertions(+), 52 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0bf5386..db6dccf 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2017-04-25  Tom Tromey  <tom@tromey.com>
 
+	PR backtrace/15584:
+	* stack.c (backtrace_command_1): Move some code into no-filters
+	"if".
+
+2017-04-25  Tom Tromey  <tom@tromey.com>
+
 	* python/py-framefilter.c (throw_quit_or_print_exception): New
 	function.
 	(gdbpy_apply_frame_filter): Use it.
diff --git a/gdb/stack.c b/gdb/stack.c
index 77e8c9b..4f6b117 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1720,49 +1720,17 @@ backtrace_command_1 (char *count_exp, int flags, int no_filters,
   struct frame_info *fi;
   int count;
   int i;
-  struct frame_info *trailing;
-  int trailing_level, py_start = 0, py_end = 0;
+  int py_start = 0, py_end = 0;
   enum ext_lang_bt_status result = EXT_LANG_BT_ERROR;
 
   if (!target_has_stack)
     error (_("No stack."));
 
-  /* The following code must do two things.  First, it must set the
-     variable TRAILING to the frame from which we should start
-     printing.  Second, it must set the variable count to the number
-     of frames which we should print, or -1 if all of them.  */
-  trailing = get_current_frame ();
-
-  trailing_level = 0;
   if (count_exp)
     {
       count = parse_and_eval_long (count_exp);
       if (count < 0)
-	{
-	  struct frame_info *current;
-
-	  py_start = count;
-	  count = -count;
-
-	  current = trailing;
-	  while (current && count--)
-	    {
-	      QUIT;
-	      current = get_prev_frame (current);
-	    }
-
-	  /* Will stop when CURRENT reaches the top of the stack.
-	     TRAILING will be COUNT below it.  */
-	  while (current)
-	    {
-	      QUIT;
-	      trailing = get_prev_frame (trailing);
-	      current = get_prev_frame (current);
-	      trailing_level++;
-	    }
-
-	  count = -1;
-	}
+	py_start = count;
       else
 	{
 	  py_start = 0;
@@ -1775,24 +1743,6 @@ backtrace_command_1 (char *count_exp, int flags, int no_filters,
       count = -1;
     }
 
-  if (info_verbose)
-    {
-      /* Read in symbols for all of the frames.  Need to do this in a
-         separate pass so that "Reading in symbols for xxx" messages
-         don't screw up the appearance of the backtrace.  Also if
-         people have strong opinions against reading symbols for
-         backtrace this may have to be an option.  */
-      i = count;
-      for (fi = trailing; fi != NULL && i--; fi = get_prev_frame (fi))
-	{
-	  CORE_ADDR pc;
-
-	  QUIT;
-	  pc = get_frame_address_in_block (fi);
-	  expand_symtab_containing_pc (pc, find_pc_mapped_section (pc));
-	}
-    }
-
   if (! no_filters)
     {
       enum ext_lang_frame_args arg_type;
@@ -1815,6 +1765,57 @@ backtrace_command_1 (char *count_exp, int flags, int no_filters,
      "no-filters" has been specified from the command.  */
   if (no_filters ||  result == EXT_LANG_BT_NO_FILTERS)
     {
+      struct frame_info *trailing;
+
+      /* The following code must do two things.  First, it must set the
+	 variable TRAILING to the frame from which we should start
+	 printing.  Second, it must set the variable count to the number
+	 of frames which we should print, or -1 if all of them.  */
+      trailing = get_current_frame ();
+
+      if (count_exp != NULL && count < 0)
+	{
+	  struct frame_info *current;
+
+	  count = -count;
+
+	  current = trailing;
+	  while (current && count--)
+	    {
+	      QUIT;
+	      current = get_prev_frame (current);
+	    }
+
+	  /* Will stop when CURRENT reaches the top of the stack.
+	     TRAILING will be COUNT below it.  */
+	  while (current)
+	    {
+	      QUIT;
+	      trailing = get_prev_frame (trailing);
+	      current = get_prev_frame (current);
+	    }
+
+	  count = -1;
+	}
+
+      if (info_verbose)
+	{
+	  /* Read in symbols for all of the frames.  Need to do this in a
+	     separate pass so that "Reading in symbols for xxx" messages
+	     don't screw up the appearance of the backtrace.  Also if
+	     people have strong opinions against reading symbols for
+	     backtrace this may have to be an option.  */
+	  i = count;
+	  for (fi = trailing; fi != NULL && i--; fi = get_prev_frame (fi))
+	    {
+	      CORE_ADDR pc;
+
+	      QUIT;
+	      pc = get_frame_address_in_block (fi);
+	      expand_symtab_containing_pc (pc, find_pc_mapped_section (pc));
+	    }
+	}
+
       for (i = 0, fi = trailing; fi && count--; i++, fi = get_prev_frame (fi))
 	{
 	  QUIT;
-- 
2.9.3

Re: [RFA 08/10] Move some code later in backtrace_command_1

[Phil Muldoon]

On 25/04/17 20:41, Tom Tromey wrote:
> PR backtrace/15584 notes that some code in backtrace_command_1 is not
> useful when frame filters are in use.  This patch moves this code into
> the no-frame-filters "if".  This also removes the unused local
> "trailing_level", which I noticed while moving the code around.
> 
> ChangeLog
> 2017-04-25  Tom Tromey  <tom@tromey.com>
> 
> 	PR backtrace/15584:
> 	* stack.c (backtrace_command_1): Move some code into no-filters
> 	"if".

Reads much better now, thanks. LGTM

Cheers

Phil

Re: [RFA 08/10] Move some code later in backtrace_command_1

[Pedro Alves]

On 04/25/2017 08:41 PM, Tom Tromey wrote:
> PR backtrace/15584 notes that some code in backtrace_command_1 is not
> useful when frame filters are in use.  

Doesn't the "info verbose on" bit affect frame filters too?
Particularly the:

"Need to do this in a
 separate pass so that "Reading in symbols for xxx" messages
 don't screw up the appearance of the backtrace."

... remark.

Not sure I fully understand whether that "info verbose" code
makes sense nowadays, though.  It doesn't seem to be documented
in the manual.

In any case, if you need that, then I guess it'd be done
somewhere inside the frame filters code, I suppose?

With that in mind, this LGTM.

Thanks,
Pedro Alves

Re: [RFA 00/10] various frame filter fixes and cleanups

[Phil Muldoon]

On 25/04/17 20:41, Tom Tromey wrote:
> This series fixes a few frame filter bugs.  Some of them have been
> reported, but some not.
> 
> There are some documentation changes in patch #1 and patch #3.
> 
> For the most part I think each patch is reasonably self-explanatory.
> However, there are a couple of things to note here:
> 
> * You might wonder about the choice of constant in patch #3.  I chose
>   32 rather than 16 so as not to conflict with my earlier frame-filter
>   patch, in review.
> 
> * In some cases writing a test seemed pretty difficult, and also maybe
>   low-impact, so I didn't.  This applies to patch #6, #9, and #10.
> 
> A future direction for patch #3 would be to add a new "set" command so
> that a user can choose "bt elide" as the default.
> 
> Tested on the buildbot.

I've looked through the patches and nothing stands out (beyond the minor nits pointed out already).  I'd like to make the case that this be an inclusion for GDB 8. It's a series of very nice bugfixes for some long standing issues. It might be too late in the day for this, though. What does everyone else think?

Cheers

Phil

Re: [RFA 00/10] various frame filter fixes and cleanups

[Tom Tromey]

> This series fixes a few frame filter bugs.  Some of them have been
> reported, but some not.

Ping for this series.

Tom