public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFA v3 09/13] Return EXT_LANG_BT_ERROR in one more spot in py-framefilter.c
  2018-03-23 20:55 [RFA v3 00/13] various frame filter fixes and cleanups Tom Tromey
  2018-03-23 20:55 ` [RFA v3 07/13] Throw a "quit" on a KeyboardException in py-framefilter.c Tom Tromey
@ 2018-03-23 20:55 ` Tom Tromey
  2018-03-23 20:55 ` [RFA v3 05/13] Avoid manual resource management " Tom Tromey
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2018-03-23 20:55 UTC (permalink / raw)
  To: gdb-patches; +Cc: 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.

gdb/ChangeLog
2018-03-23  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/python/py-framefilter.c b/gdb/python/py-framefilter.c
index 0662e68941..6186ffd274 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.13.6

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

* [RFA v3 01/13] Rationalize "backtrace" command line parsing
  2018-03-23 20:55 [RFA v3 00/13] various frame filter fixes and cleanups Tom Tromey
                   ` (10 preceding siblings ...)
  2018-03-23 20:55 ` [RFA v3 11/13] Improve "backtrace" help text Tom Tromey
@ 2018-03-23 20:55 ` Tom Tromey
  2018-03-24  6:31   ` Eli Zaretskii
  2018-03-23 20:55 ` [RFA v3 10/13] Call wrap_hint in one more spot in py-framefilter.c Tom Tromey
  2018-03-24 11:42 ` [RFA v3 00/13] various frame filter fixes and cleanups Pedro Alves
  13 siblings, 1 reply; 26+ messages in thread
From: Tom Tromey @ 2018-03-23 20:55 UTC (permalink / raw)
  To: gdb-patches; +Cc: 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.

gdb/ChangeLog
2018-03-23  Tom Tromey  <tom@tromey.com>

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

gdb/doc/ChangeLog
2018-03-23  Tom Tromey  <tom@tromey.com>

	* gdb.texinfo (Backtrace): Describe options individually.
---
 gdb/ChangeLog       |  4 ++++
 gdb/doc/ChangeLog   |  4 ++++
 gdb/doc/gdb.texinfo | 54 ++++++++++++++++++++--------------------------
 gdb/stack.c         | 62 +++++++++++++++++------------------------------------
 4 files changed, 51 insertions(+), 73 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 74e0fdb4a4..b48dd4ed4b 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -7307,39 +7307,31 @@ 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 frames in the stack.  By default, all stack frames are
+printed.  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, where @var{n} is a positive
+number.
+
+@item -@var{n}
+@itemx -@var{n}
+Print only the outermost @var{n} frames, where @var{n} is a positive
+number.
+
+@item full
+Print the values of the local variables also.  This can be combined
+with a number to limit the number of frames shown.
+
+@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 aad8fcd987..13af6594a9 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1850,61 +1850,39 @@ backtrace_command_1 (const char *count_exp, int show_locals, int no_filters,
 static void
 backtrace_command (const char *arg, int from_tty)
 {
-  int fulltrace_arg = -1, arglen = 0, argc = 0, no_filters  = -1;
-  int user_arg = 0;
+  bool fulltrace = false;
+  bool filters = true;
 
-  std::string reconstructed_arg;
   if (arg)
     {
-      char **argv;
-      int i;
+      bool done = false;
 
-      gdb_argv built_argv (arg);
-      argv = built_argv.get ();
-      argc = 0;
-      for (i = 0; argv[i]; i++)
+      while (!done)
 	{
-	  unsigned int j;
+	  const char *save_arg = arg;
+	  std::string this_arg = extract_arg (&arg);
 
-	  for (j = 0; j < strlen (argv[i]); j++)
-	    argv[i][j] = TOLOWER (argv[i][j]);
+	  if (this_arg.empty ())
+	    break;
 
-	  if (no_filters < 0 && subset_compare (argv[i], "no-filters"))
-	    no_filters = argc;
+	  if (subset_compare (this_arg.c_str (), "no-filters"))
+	    filters = false;
+	  else if (subset_compare (this_arg.c_str (), "full"))
+	    fulltrace = true;
 	  else
 	    {
-	      if (fulltrace_arg < 0 && subset_compare (argv[i], "full"))
-		fulltrace_arg = argc;
-	      else
-		{
-		  user_arg++;
-		  arglen += strlen (argv[i]);
-		}
-	    }
-	  argc++;
-	}
-      arglen += user_arg;
-      if (fulltrace_arg >= 0 || no_filters >= 0)
-	{
-	  if (arglen > 0)
-	    {
-	      for (i = 0; i < argc; i++)
-		{
-		  if (i != fulltrace_arg && i != no_filters)
-		    {
-		      reconstructed_arg += argv[i];
-		      reconstructed_arg += " ";
-		    }
-		}
-	      arg = reconstructed_arg.c_str ();
+	      /* Not a recognized argument, so stop.  */
+	      arg = save_arg;
+	      done = true;
 	    }
-	  else
-	    arg = NULL;
 	}
+
+      if (*arg == '\0')
+	arg = NULL;
     }
 
-  backtrace_command_1 (arg, fulltrace_arg >= 0 /* show_locals */,
-		       no_filters >= 0 /* no frame-filters */, from_tty);
+  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.13.6

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

* [RFA v3 03/13] Allow hiding of some filtered frames
  2018-03-23 20:55 [RFA v3 00/13] various frame filter fixes and cleanups Tom Tromey
                   ` (3 preceding siblings ...)
  2018-03-23 20:55 ` [RFA v3 06/13] Allow C-c to work in backtrace in more cases Tom Tromey
@ 2018-03-23 20:55 ` Tom Tromey
  2018-03-24  6:32   ` Eli Zaretskii
  2018-03-23 20:55 ` [RFA v3 12/13] Simplify exception handling in py-framefilter.c Tom Tromey
                   ` (8 subsequent siblings)
  13 siblings, 1 reply; 26+ messages in thread
From: Tom Tromey @ 2018-03-23 20:55 UTC (permalink / raw)
  To: gdb-patches; +Cc: 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.

gdb/ChangeLog
2018-03-23  Tom Tromey  <tom@tromey.com>

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

gdb/doc/ChangeLog
2018-03-23  Tom Tromey  <tom@tromey.com>

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

gdb/testsuite/ChangeLog
2018-03-23  Tom Tromey  <tom@tromey.com>

	PR backtrace/15582:
	* gdb.python/py-framefilter.exp: Add "bt hide" 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/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index b48dd4ed4b..cf9282911a 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -7337,6 +7337,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 hide
+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{hide}
+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 943792db29..a97292875a 100644
--- a/gdb/extension.h
+++ b/gdb/extension.h
@@ -103,6 +103,9 @@ enum frame_filter_flag
 
     /* Set this flag if a "More frames" message is to be printed.  */
     PRINT_MORE_FRAMES = 1 << 4,
+
+    /* Set this flag if elided frames should not be printed.  */
+    PRINT_HIDE = 1 << 5,
   };
 
 DEF_ENUM_FLAGS_TYPE (enum frame_filter_flag, frame_filter_flags);
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index 99d47f247a..aae8d5decf 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -1238,37 +1238,37 @@ py_print_frame (PyObject *filter, frame_filter_flags 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_HIDE) == 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 76695ff7e6..8e8fe12109 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1867,6 +1867,8 @@ backtrace_command (const char *arg, int from_tty)
 	    filters = false;
 	  else if (subset_compare (this_arg.c_str (), "full"))
 	    flags |= PRINT_LOCALS;
+	  else if (subset_compare (this_arg.c_str (), "hide"))
+	    flags |= PRINT_HIDE;
 	  else
 	    {
 	      /* Not a recognized argument, so stop.  */
diff --git a/gdb/testsuite/gdb.python/py-framefilter.exp b/gdb/testsuite/gdb.python/py-framefilter.exp
index 7e02e9d701..cc31dd5893 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 hide" \
+    ".*#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 hide with Reverse disabled"
 
 # Test set print frame-arguments
 # none
-- 
2.13.6

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

* [RFA v3 05/13] Avoid manual resource management in py-framefilter.c
  2018-03-23 20:55 [RFA v3 00/13] various frame filter fixes and cleanups Tom Tromey
  2018-03-23 20:55 ` [RFA v3 07/13] Throw a "quit" on a KeyboardException in py-framefilter.c Tom Tromey
  2018-03-23 20:55 ` [RFA v3 09/13] Return EXT_LANG_BT_ERROR in one more spot " Tom Tromey
@ 2018-03-23 20:55 ` Tom Tromey
  2018-03-23 20:55 ` [RFA v3 06/13] Allow C-c to work in backtrace in more cases Tom Tromey
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2018-03-23 20:55 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

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

gdb/ChangeLog
2018-03-23  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/python/py-framefilter.c b/gdb/python/py-framefilter.c
index 1dc15ae14e..dcac42df8e 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.13.6

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

* [RFA v3 04/13] Remove EXT_LANG_BT_COMPLETED
  2018-03-23 20:55 [RFA v3 00/13] various frame filter fixes and cleanups Tom Tromey
                   ` (7 preceding siblings ...)
  2018-03-23 20:55 ` [RFA v3 13/13] Remove verbose code from backtrace command Tom Tromey
@ 2018-03-23 20:55 ` Tom Tromey
  2018-03-23 20:55 ` [RFA v3 08/13] Move some code later in backtrace_command_1 Tom Tromey
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2018-03-23 20:55 UTC (permalink / raw)
  To: gdb-patches; +Cc: 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.

gdb/ChangeLog
2018-03-23  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/extension.h b/gdb/extension.h
index a97292875a..c675ae2446 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 aae8d5decf..1dc15ae14e 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, frame_filter_flags 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, frame_filter_flags 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.13.6

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

* [RFA v3 12/13] Simplify exception handling in py-framefilter.c
  2018-03-23 20:55 [RFA v3 00/13] various frame filter fixes and cleanups Tom Tromey
                   ` (4 preceding siblings ...)
  2018-03-23 20:55 ` [RFA v3 03/13] Allow hiding of some filtered frames Tom Tromey
@ 2018-03-23 20:55 ` Tom Tromey
  2018-03-23 20:55 ` [RFA v3 02/13] Change backtrace_command_1 calling to use flags Tom Tromey
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2018-03-23 20:55 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This patch changes py-framefilter.c as suggested by Pedro in:
https://sourceware.org/ml/gdb-patches/2017-06/msg00748.html

In particular, gdb exceptions are now caught at the outermost layer,
rather than in each particular function.  This simplifies much of the
code.

gdb/ChangeLog
2018-03-23  Tom Tromey  <tom@tromey.com>

	* python/py-framefilter.c (py_print_type): Don't catch
	exceptions.  Return void.
	(py_print_value): Likewise.
	(py_print_single_arg): Likewise.
	(enumerate_args): Don't catch exceptions.
	(py_print_args): Likewise.
	(py_print_frame): Likewise.
	(gdbpy_apply_frame_filter): Catch exceptions here.
---
 gdb/ChangeLog               |  11 +
 gdb/python/py-framefilter.c | 550 +++++++++++++-------------------------------
 2 files changed, 167 insertions(+), 394 deletions(-)

diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index eb4f9c6adf..698fd87f4b 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -199,30 +199,16 @@ mi_should_print (struct symbol *sym, enum mi_print_types type)
 /* Helper function which outputs a type name extracted from VAL to a
    "type" field in the output stream OUT.  OUT is the ui-out structure
    the type name will be output too, and VAL is the value that the
-   type will be extracted from.  Returns EXT_LANG_BT_ERROR on error, with
-   any GDB exceptions converted to a Python exception, or EXT_LANG_BT_OK on
-   success.  */
+   type will be extracted from.  */
 
-static enum ext_lang_bt_status
+static void
 py_print_type (struct ui_out *out, struct value *val)
 {
+  check_typedef (value_type (val));
 
-  TRY
-    {
-      check_typedef (value_type (val));
-
-      string_file stb;
-      type_print (value_type (val), "", &stb, -1);
-      out->field_stream ("type", stb);
-    }
-  CATCH (except, RETURN_MASK_ERROR)
-    {
-      gdbpy_convert_exception (except);
-      return EXT_LANG_BT_ERROR;
-    }
-  END_CATCH
-
-  return EXT_LANG_BT_OK;
+  string_file stb;
+  type_print (value_type (val), "", &stb, -1);
+  out->field_stream ("type", stb);
 }
 
 /* Helper function which outputs a value to an output field in a
@@ -230,11 +216,9 @@ py_print_type (struct ui_out *out, struct value *val)
    VAL is the value that will be printed, OPTS contains the value
    printing options, ARGS_TYPE is an enumerator describing the
    argument format, and LANGUAGE is the language_defn that the value
-   will be printed with.  Returns EXT_LANG_BT_ERROR on error, with any GDB
-   exceptions converted to a Python exception, or EXT_LANG_BT_OK on
-   success. */
+   will be printed with.  */
 
-static enum ext_lang_bt_status
+static void
 py_print_value (struct ui_out *out, struct value *val,
 		const struct value_print_options *opts,
 		int indent,
@@ -249,18 +233,7 @@ py_print_value (struct ui_out *out, struct value *val,
   if (args_type == MI_PRINT_SIMPLE_VALUES
       || args_type == MI_PRINT_ALL_VALUES)
     {
-      struct type *type = NULL;
-
-      TRY
-	{
-	  type = check_typedef (value_type (val));
-	}
-      CATCH (except, RETURN_MASK_ERROR)
-	{
-	  gdbpy_convert_exception (except);
-	  return EXT_LANG_BT_ERROR;
-	}
-      END_CATCH
+      struct type *type = check_typedef (value_type (val));
 
       if (args_type == MI_PRINT_ALL_VALUES)
 	should_print = 1;
@@ -275,22 +248,11 @@ py_print_value (struct ui_out *out, struct value *val,
 
   if (should_print)
     {
-      TRY
-	{
-	  string_file stb;
+      string_file stb;
 
-	  common_val_print (val, &stb, indent, opts, language);
-	  out->field_stream ("value", stb);
-	}
-      CATCH (except, RETURN_MASK_ERROR)
-	{
-	  gdbpy_convert_exception (except);
-	  return EXT_LANG_BT_ERROR;
-	}
-      END_CATCH
+      common_val_print (val, &stb, indent, opts, language);
+      out->field_stream ("value", stb);
     }
-
-  return EXT_LANG_BT_OK;
 }
 
 /* Helper function to call a Python method and extract an iterator
@@ -338,10 +300,9 @@ get_py_iter_from_func (PyObject *filter, const char *func)
     ARGS_TYPE is an enumerator describing the argument format,
     PRINT_ARGS_FIELD is a flag which indicates if we output "ARGS=1"
     in MI output in commands where both arguments and locals are
-    printed.  Returns EXT_LANG_BT_ERROR on error, with any GDB exceptions
-    converted to a Python exception, or EXT_LANG_BT_OK on success.  */
+    printed.  */
 
-static enum ext_lang_bt_status
+static void
 py_print_single_arg (struct ui_out *out,
 		     const char *sym_name,
 		     struct frame_arg *fa,
@@ -352,116 +313,97 @@ py_print_single_arg (struct ui_out *out,
 		     const struct language_defn *language)
 {
   struct value *val;
-  enum ext_lang_bt_status retval = EXT_LANG_BT_OK;
 
   if (fa != NULL)
     {
       if (fa->val == NULL && fa->error == NULL)
-	return EXT_LANG_BT_OK;
+	return;
       language = language_def (SYMBOL_LANGUAGE (fa->sym));
       val = fa->val;
     }
   else
     val = fv;
 
-  TRY
-    {
-      gdb::optional<ui_out_emit_tuple> maybe_tuple;
+  gdb::optional<ui_out_emit_tuple> maybe_tuple;
 
-      /*  MI has varying rules for tuples, but generally if there is only
+  /*  MI has varying rules for tuples, but generally if there is only
       one element in each item in the list, do not start a tuple.  The
       exception is -stack-list-variables which emits an ARGS="1" field
       if the value is a frame argument.  This is denoted in this
       function with PRINT_ARGS_FIELD which is flag from the caller to
       emit the ARGS field.  */
-      if (out->is_mi_like_p ())
-	{
-	  if (print_args_field || args_type != NO_VALUES)
-	    maybe_tuple.emplace (out, nullptr);
-	}
+  if (out->is_mi_like_p ())
+    {
+      if (print_args_field || args_type != NO_VALUES)
+	maybe_tuple.emplace (out, nullptr);
+    }
 
-      annotate_arg_begin ();
+  annotate_arg_begin ();
+
+  /* If frame argument is populated, check for entry-values and the
+     entry value options.  */
+  if (fa != NULL)
+    {
+      string_file stb;
 
-      /* If frame argument is populated, check for entry-values and the
-	 entry value options.  */
-      if (fa != NULL)
+      fprintf_symbol_filtered (&stb, SYMBOL_PRINT_NAME (fa->sym),
+			       SYMBOL_LANGUAGE (fa->sym),
+			       DMGL_PARAMS | DMGL_ANSI);
+      if (fa->entry_kind == print_entry_values_compact)
 	{
-	  string_file stb;
+	  stb.puts ("=");
 
 	  fprintf_symbol_filtered (&stb, SYMBOL_PRINT_NAME (fa->sym),
 				   SYMBOL_LANGUAGE (fa->sym),
 				   DMGL_PARAMS | DMGL_ANSI);
-	  if (fa->entry_kind == print_entry_values_compact)
-	    {
-	      stb.puts ("=");
-
-	      fprintf_symbol_filtered (&stb, SYMBOL_PRINT_NAME (fa->sym),
-				       SYMBOL_LANGUAGE (fa->sym),
-				       DMGL_PARAMS | DMGL_ANSI);
-	    }
-	  if (fa->entry_kind == print_entry_values_only
-	      || fa->entry_kind == print_entry_values_compact)
-	    stb.puts ("@entry");
-	  out->field_stream ("name", stb);
 	}
-      else
-	/* Otherwise, just output the name.  */
-	out->field_string ("name", sym_name);
+      if (fa->entry_kind == print_entry_values_only
+	  || fa->entry_kind == print_entry_values_compact)
+	stb.puts ("@entry");
+      out->field_stream ("name", stb);
+    }
+  else
+    /* Otherwise, just output the name.  */
+    out->field_string ("name", sym_name);
 
-      annotate_arg_name_end ();
+  annotate_arg_name_end ();
 
-      if (! out->is_mi_like_p ())
-	out->text ("=");
+  if (! out->is_mi_like_p ())
+    out->text ("=");
 
-      if (print_args_field)
-	out->field_int ("arg", 1);
+  if (print_args_field)
+    out->field_int ("arg", 1);
 
-      /* For MI print the type, but only for simple values.  This seems
-	 weird, but this is how MI choose to format the various output
-	 types.  */
-      if (args_type == MI_PRINT_SIMPLE_VALUES && val != NULL)
-	{
-	  if (py_print_type (out, val) == EXT_LANG_BT_ERROR)
-	    retval = EXT_LANG_BT_ERROR;
-	}
+  /* For MI print the type, but only for simple values.  This seems
+     weird, but this is how MI choose to format the various output
+     types.  */
+  if (args_type == MI_PRINT_SIMPLE_VALUES && val != NULL)
+    py_print_type (out, val);
 
-      if (retval != EXT_LANG_BT_ERROR)
-	{
-	  if (val != NULL)
-	    annotate_arg_value (value_type (val));
+  if (val != NULL)
+    annotate_arg_value (value_type (val));
 
-	  /* If the output is to the CLI, and the user option "set print
-	     frame-arguments" is set to none, just output "...".  */
-	  if (! out->is_mi_like_p () && args_type == NO_VALUES)
-	    out->field_string ("value", "...");
-	  else
+  /* If the output is to the CLI, and the user option "set print
+     frame-arguments" is set to none, just output "...".  */
+  if (! out->is_mi_like_p () && args_type == NO_VALUES)
+    out->field_string ("value", "...");
+  else
+    {
+      /* Otherwise, print the value for both MI and the CLI, except
+	 for the case of MI_PRINT_NO_VALUES.  */
+      if (args_type != NO_VALUES)
+	{
+	  if (val == NULL)
 	    {
-	      /* Otherwise, print the value for both MI and the CLI, except
-		 for the case of MI_PRINT_NO_VALUES.  */
-	      if (args_type != NO_VALUES)
-		{
-		  if (val == NULL)
-		    {
-		      gdb_assert (fa != NULL && fa->error != NULL);
-		      out->field_fmt ("value",
-					_("<error reading variable: %s>"),
-					fa->error);
-		    }
-		  else if (py_print_value (out, val, opts, 0, args_type, language)
-			   == EXT_LANG_BT_ERROR)
-		    retval = EXT_LANG_BT_ERROR;
-		}
+	      gdb_assert (fa != NULL && fa->error != NULL);
+	      out->field_fmt ("value",
+			      _("<error reading variable: %s>"),
+			      fa->error);
 	    }
+	  else
+	    py_print_value (out, val, opts, 0, args_type, language);
 	}
     }
-  CATCH (except, RETURN_MASK_ERROR)
-    {
-      gdbpy_convert_exception (except);
-      retval = EXT_LANG_BT_ERROR;
-    }
-  END_CATCH
-
-  return retval;
 }
 
 /* Helper function to loop over frame arguments provided by the
@@ -494,16 +436,7 @@ enumerate_args (PyObject *iter,
 
   opts.deref_ref = 1;
 
-  TRY
-    {
-      annotate_frame_args ();
-    }
-  CATCH (except, RETURN_MASK_ERROR)
-    {
-      gdbpy_convert_exception (except);
-      return EXT_LANG_BT_ERROR;
-    }
-  END_CATCH
+  annotate_frame_args ();
 
   /*  Collect the first argument outside of the loop, so output of
       commas in the argument output is correct.  At the end of the
@@ -550,16 +483,7 @@ enumerate_args (PyObject *iter,
 	      return EXT_LANG_BT_ERROR;
 	    }
 
-	  TRY
-	    {
-	      read_frame_arg (sym, frame, &arg, &entryarg);
-	    }
-	  CATCH (except, RETURN_MASK_ERROR)
-	    {
-	      gdbpy_convert_exception (except);
-	      return EXT_LANG_BT_ERROR;
-	    }
-	  END_CATCH
+	  read_frame_arg (sym, frame, &arg, &entryarg);
 
 	  gdb::unique_xmalloc_ptr<char> arg_holder (arg.error);
 	  gdb::unique_xmalloc_ptr<char> entry_holder (entryarg.error);
@@ -570,47 +494,32 @@ enumerate_args (PyObject *iter,
 
 	  if (arg.entry_kind != print_entry_values_only)
 	    {
-	      if (py_print_single_arg (out, NULL, &arg,
-				       NULL, &opts,
-				       args_type,
-				       print_args_field,
-				       NULL) == EXT_LANG_BT_ERROR)
-		return EXT_LANG_BT_ERROR;
+	      py_print_single_arg (out, NULL, &arg,
+				   NULL, &opts,
+				   args_type,
+				   print_args_field,
+				   NULL);
 	    }
 
 	  if (entryarg.entry_kind != print_entry_values_no)
 	    {
 	      if (arg.entry_kind != print_entry_values_only)
 		{
-		  TRY
-		    {
-		      out->text (", ");
-		      out->wrap_hint ("    ");
-		    }
-		  CATCH (except, RETURN_MASK_ERROR)
-		    {
-		      gdbpy_convert_exception (except);
-		      return EXT_LANG_BT_ERROR;
-		    }
-		  END_CATCH
+		  out->text (", ");
+		  out->wrap_hint ("    ");
 		}
 
-	      if (py_print_single_arg (out, NULL, &entryarg, NULL, &opts,
-				       args_type, print_args_field, NULL)
-		  == EXT_LANG_BT_ERROR)
-		return EXT_LANG_BT_ERROR;
+	      py_print_single_arg (out, NULL, &entryarg, NULL, &opts,
+				   args_type, print_args_field, NULL);
 	    }
 	}
       else
 	{
 	  /* If the object has provided a value, we just print that.  */
 	  if (val != NULL)
-	    {
-	      if (py_print_single_arg (out, sym_name.get (), NULL, val, &opts,
-				       args_type, print_args_field,
-				       language) == EXT_LANG_BT_ERROR)
-		return EXT_LANG_BT_ERROR;
-	    }
+	    py_print_single_arg (out, sym_name.get (), NULL, val, &opts,
+				 args_type, print_args_field,
+				 language);
 	}
 
       /* Collect the next item from the iterator.  If
@@ -618,31 +527,11 @@ enumerate_args (PyObject *iter,
 	 comma.  */
       item.reset (PyIter_Next (iter));
       if (item != NULL)
-	{
-	  TRY
-	    {
-	      out->text (", ");
-	    }
-	  CATCH (except, RETURN_MASK_ERROR)
-	    {
-	      gdbpy_convert_exception (except);
-	      return EXT_LANG_BT_ERROR;
-	    }
-	  END_CATCH
-	}
+	out->text (", ");
       else if (PyErr_Occurred ())
 	return EXT_LANG_BT_ERROR;
 
-      TRY
-	{
-	  annotate_arg_end ();
-	}
-      CATCH (except, RETURN_MASK_ERROR)
-	{
-	  gdbpy_convert_exception (except);
-	  return EXT_LANG_BT_ERROR;
-	}
-      END_CATCH
+      annotate_arg_end ();
     }
 
   return EXT_LANG_BT_OK;
@@ -703,18 +592,7 @@ enumerate_locals (PyObject *iter,
 
       /* If the object did not provide a value, read it.  */
       if (val == NULL)
-	{
-	  TRY
-	    {
-	      val = read_var_value (sym, sym_block, frame);
-	    }
-	  CATCH (except, RETURN_MASK_ERROR)
-	    {
-	      gdbpy_convert_exception (except);
-	      return EXT_LANG_BT_ERROR;
-	    }
-	  END_CATCH
-	}
+	val = read_var_value (sym, sym_block, frame);
 
       /* With PRINT_NO_VALUES, MI does not emit a tuple normally as
 	 each output contains only one field.  The exception is
@@ -724,31 +602,19 @@ enumerate_locals (PyObject *iter,
 	  if (print_args_field || args_type != NO_VALUES)
 	    tuple.emplace (out, nullptr);
 	}
-      TRY
+      if (! out->is_mi_like_p ())
 	{
-	  if (! out->is_mi_like_p ())
-	    {
-	      /* If the output is not MI we indent locals.  */
-	      out->spaces (local_indent);
-	    }
+	  /* If the output is not MI we indent locals.  */
+	  out->spaces (local_indent);
+	}
 
-	  out->field_string ("name", sym_name.get ());
+      out->field_string ("name", sym_name.get ());
 
-	  if (! out->is_mi_like_p ())
-	    out->text (" = ");
-	}
-      CATCH (except, RETURN_MASK_ERROR)
-	{
-	  gdbpy_convert_exception (except);
-	  return EXT_LANG_BT_ERROR;
-	}
-      END_CATCH
+      if (! out->is_mi_like_p ())
+	out->text (" = ");
 
       if (args_type == MI_PRINT_SIMPLE_VALUES)
-	{
-	  if (py_print_type (out, val) == EXT_LANG_BT_ERROR)
-	    return EXT_LANG_BT_ERROR;
-	}
+	py_print_type (out, val);
 
       /* CLI always prints values for locals.  MI uses the
 	 simple/no/all system.  */
@@ -756,30 +622,17 @@ enumerate_locals (PyObject *iter,
 	{
 	  int val_indent = (indent + 1) * 4;
 
-	  if (py_print_value (out, val, &opts, val_indent, args_type,
-			      language) == EXT_LANG_BT_ERROR)
-	    return EXT_LANG_BT_ERROR;
+	  py_print_value (out, val, &opts, val_indent, args_type,
+			  language);
 	}
       else
 	{
 	  if (args_type != NO_VALUES)
-	    {
-	      if (py_print_value (out, val, &opts, 0, args_type,
-				  language) == EXT_LANG_BT_ERROR)
-		return EXT_LANG_BT_ERROR;
-	    }
+	    py_print_value (out, val, &opts, 0, args_type,
+			    language);
 	}
 
-      TRY
-	{
-	  out->text ("\n");
-	}
-      CATCH (except, RETURN_MASK_ERROR)
-	{
-	  gdbpy_convert_exception (except);
-	  return EXT_LANG_BT_ERROR;
-	}
-      END_CATCH
+      out->text ("\n");
     }
 
   if (!PyErr_Occurred ())
@@ -862,36 +715,18 @@ py_print_args (PyObject *filter,
 
   ui_out_emit_list list_emitter (out, "args");
 
-  TRY
-    {
-      out->wrap_hint ("   ");
-      annotate_frame_args ();
-      if (! out->is_mi_like_p ())
-	out->text (" (");
-    }
-  CATCH (except, RETURN_MASK_ERROR)
-    {
-      gdbpy_convert_exception (except);
-      return EXT_LANG_BT_ERROR;
-    }
-  END_CATCH
+  out->wrap_hint ("   ");
+  annotate_frame_args ();
+  if (! out->is_mi_like_p ())
+    out->text (" (");
 
   if (args_iter != Py_None
       && (enumerate_args (args_iter.get (), out, args_type, 0, frame)
 	  == EXT_LANG_BT_ERROR))
     return EXT_LANG_BT_ERROR;
 
-  TRY
-    {
-      if (! out->is_mi_like_p ())
-	out->text (")");
-    }
-  CATCH (except, RETURN_MASK_ERROR)
-    {
-      gdbpy_convert_exception (except);
-      return EXT_LANG_BT_ERROR;
-    }
-  END_CATCH
+  if (! out->is_mi_like_p ())
+    out->text (")");
 
   return EXT_LANG_BT_OK;
 }
@@ -944,16 +779,7 @@ py_print_frame (PyObject *filter, frame_filter_flags flags,
   if (frame == NULL)
     return EXT_LANG_BT_ERROR;
 
-  TRY
-    {
-      gdbarch = get_frame_arch (frame);
-    }
-  CATCH (except, RETURN_MASK_ERROR)
-    {
-      gdbpy_convert_exception (except);
-      return EXT_LANG_BT_ERROR;
-    }
-  END_CATCH
+  gdbarch = get_frame_arch (frame);
 
   /* stack-list-variables.  */
   if (print_locals && print_args && ! print_frame_info)
@@ -976,18 +802,7 @@ py_print_frame (PyObject *filter, frame_filter_flags flags,
       /* Elided frames are also printed with this function (recursively)
 	 and are printed with indention.  */
       if (indent > 0)
-	{
-	  TRY
-	    {
-	      out->spaces (indent * 4);
-	    }
-	  CATCH (except, RETURN_MASK_ERROR)
-	    {
-	      gdbpy_convert_exception (except);
-	      return EXT_LANG_BT_ERROR;
-	    }
-	  END_CATCH
-	}
+	out->spaces (indent * 4);
 
       /* The address is required for frame annotations, and also for
 	 address printing.  */
@@ -1017,32 +832,24 @@ py_print_frame (PyObject *filter, frame_filter_flags flags,
 
       slot = (struct frame_info **) htab_find_slot (levels_printed,
 						    frame, INSERT);
-      TRY
-	{
-	  level = frame_relative_level (frame);
-
-	  /* Check if this frame has already been printed (there are cases
-	     where elided synthetic dummy-frames have to 'borrow' the frame
-	     architecture from the eliding frame.  If that is the case, do
-	     not print 'level', but print spaces.  */
-	  if (*slot == frame)
-	    out->field_skip ("level");
-	  else
-	    {
-	      *slot = frame;
-	      annotate_frame_begin (print_level ? level : 0,
-				    gdbarch, address);
-	      out->text ("#");
-	      out->field_fmt_int (2, ui_left, "level",
-				    level);
-	    }
-	}
-      CATCH (except, RETURN_MASK_ERROR)
+
+      level = frame_relative_level (frame);
+
+      /* Check if this frame has already been printed (there are cases
+	 where elided synthetic dummy-frames have to 'borrow' the frame
+	 architecture from the eliding frame.  If that is the case, do
+	 not print 'level', but print spaces.  */
+      if (*slot == frame)
+	out->field_skip ("level");
+      else
 	{
-	  gdbpy_convert_exception (except);
-	  return EXT_LANG_BT_ERROR;
+	  *slot = frame;
+	  annotate_frame_begin (print_level ? level : 0,
+				gdbarch, address);
+	  out->text ("#");
+	  out->field_fmt_int (2, ui_left, "level",
+			      level);
 	}
-      END_CATCH
     }
 
   if (print_frame_info)
@@ -1051,19 +858,10 @@ py_print_frame (PyObject *filter, frame_filter_flags flags,
 	 print nothing.  */
       if (opts.addressprint && has_addr)
 	{
-	  TRY
-	    {
-	      annotate_frame_address ();
-	      out->field_core_addr ("addr", gdbarch, address);
-	      annotate_frame_address_end ();
-	      out->text (" in ");
-	    }
-	  CATCH (except, RETURN_MASK_ERROR)
-	    {
-	      gdbpy_convert_exception (except);
-	      return EXT_LANG_BT_ERROR;
-	    }
-	  END_CATCH
+	  annotate_frame_address ();
+	  out->field_core_addr ("addr", gdbarch, address);
+	  annotate_frame_address_end ();
+	  out->text (" in ");
 	}
 
       /* Print frame function name.  */
@@ -1104,20 +902,11 @@ py_print_frame (PyObject *filter, frame_filter_flags flags,
 	      return EXT_LANG_BT_ERROR;
 	    }
 
-	  TRY
-	    {
-	      annotate_frame_function_name ();
-	      if (function == NULL)
-		out->field_skip ("func");
-	      else
-		out->field_string ("func", function);
-	    }
-	  CATCH (except, RETURN_MASK_ERROR)
-	    {
-	      gdbpy_convert_exception (except);
-	      return EXT_LANG_BT_ERROR;
-	    }
-	  END_CATCH
+	  annotate_frame_function_name ();
+	  if (function == NULL)
+	    out->field_skip ("func");
+	  else
+	    out->field_string ("func", function);
 	}
     }
 
@@ -1133,16 +922,7 @@ py_print_frame (PyObject *filter, frame_filter_flags flags,
   /* File name/source/line number information.  */
   if (print_frame_info)
     {
-      TRY
-	{
-	  annotate_frame_source_begin ();
-	}
-      CATCH (except, RETURN_MASK_ERROR)
-	{
-	  gdbpy_convert_exception (except);
-	  return EXT_LANG_BT_ERROR;
-	}
-      END_CATCH
+      annotate_frame_source_begin ();
 
       if (PyObject_HasAttrString (filter, "filename"))
 	{
@@ -1159,20 +939,11 @@ py_print_frame (PyObject *filter, frame_filter_flags flags,
 	      if (filename == NULL)
 		return EXT_LANG_BT_ERROR;
 
-	      TRY
-		{
-		  out->wrap_hint ("   ");
-		  out->text (" at ");
-		  annotate_frame_source_file ();
-		  out->field_string ("file", filename.get ());
-		  annotate_frame_source_file_end ();
-		}
-	      CATCH (except, RETURN_MASK_ERROR)
-		{
-		  gdbpy_convert_exception (except);
-		  return EXT_LANG_BT_ERROR;
-		}
-	      END_CATCH
+	      out->wrap_hint ("   ");
+	      out->text (" at ");
+	      annotate_frame_source_file ();
+	      out->field_string ("file", filename.get ());
+	      annotate_frame_source_file_end ();
 	    }
 	}
 
@@ -1190,18 +961,9 @@ py_print_frame (PyObject *filter, frame_filter_flags flags,
 	      if (PyErr_Occurred ())
 		return EXT_LANG_BT_ERROR;
 
-	      TRY
-		{
-		  out->text (":");
-		  annotate_frame_source_line ();
-		  out->field_int ("line", line);
-		}
-	      CATCH (except, RETURN_MASK_ERROR)
-		{
-		  gdbpy_convert_exception (except);
-		  return EXT_LANG_BT_ERROR;
-		}
-	      END_CATCH
+	      out->text (":");
+	      annotate_frame_source_line ();
+	      out->field_int ("line", line);
 	    }
 	}
     }
@@ -1210,17 +972,8 @@ py_print_frame (PyObject *filter, frame_filter_flags flags,
      elided frames, so if MI output detected do not send newline.  */
   if (! out->is_mi_like_p ())
     {
-      TRY
-	{
-	  annotate_frame_end ();
-	  out->text ("\n");
-	}
-      CATCH (except, RETURN_MASK_ERROR)
-	{
-	  gdbpy_convert_exception (except);
-	  return EXT_LANG_BT_ERROR;
-	}
-      END_CATCH
+      annotate_frame_end ();
+      out->text ("\n");
     }
 
   if (print_locals)
@@ -1434,8 +1187,17 @@ gdbpy_apply_frame_filter (const struct extension_language_defn *extlang,
 	    }
 	}
 
-      success = py_print_frame (item.get (), flags, args_type, out, 0,
-				levels_printed.get ());
+      TRY
+	{
+	  success = py_print_frame (item.get (), flags, args_type, out, 0,
+				    levels_printed.get ());
+	}
+      CATCH (except, RETURN_MASK_ERROR)
+	{
+	  gdbpy_convert_exception (except);
+	  success = EXT_LANG_BT_ERROR;
+	}
+      END_CATCH
 
       /* Do not exit on error printing a single frame.  Print the
 	 error and continue with other frames.  */
-- 
2.13.6

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

* [RFA v3 07/13] Throw a "quit" on a KeyboardException in py-framefilter.c
  2018-03-23 20:55 [RFA v3 00/13] various frame filter fixes and cleanups Tom Tromey
@ 2018-03-23 20:55 ` Tom Tromey
  2018-03-24 11:41   ` Pedro Alves
  2018-03-23 20:55 ` [RFA v3 09/13] Return EXT_LANG_BT_ERROR in one more spot " Tom Tromey
                   ` (12 subsequent siblings)
  13 siblings, 1 reply; 26+ messages in thread
From: Tom Tromey @ 2018-03-23 20:55 UTC (permalink / raw)
  To: gdb-patches; +Cc: 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.

gdb/ChangeLog
2018-03-23  Tom Tromey  <tom@tromey.com>

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

gdb/testsuite/ChangeLog
2018-03-23  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/python/py-framefilter.c b/gdb/python/py-framefilter.c
index 28d5c37b25..0662e68941 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
@@ -1375,7 +1390,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;
     }
 
@@ -1398,7 +1413,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;
@@ -1423,7 +1438,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/gdb.python/py-framefilter.exp b/gdb/testsuite/gdb.python/py-framefilter.exp
index cc31dd5893..e28cc3bd4c 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 0c3a3a91b6..46f752274e 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.13.6

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

* [RFA v3 13/13] Remove verbose code from backtrace command
  2018-03-23 20:55 [RFA v3 00/13] various frame filter fixes and cleanups Tom Tromey
                   ` (6 preceding siblings ...)
  2018-03-23 20:55 ` [RFA v3 02/13] Change backtrace_command_1 calling to use flags Tom Tromey
@ 2018-03-23 20:55 ` Tom Tromey
  2018-03-23 20:55 ` [RFA v3 04/13] Remove EXT_LANG_BT_COMPLETED Tom Tromey
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2018-03-23 20:55 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

In https://sourceware.org/ml/gdb-patches/2017-06/msg00741.html,
Pedro asks:

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

The answer is that yes, it could.  However, it's not completely
effective, because the C code can't guess how many frames might need
to be unwound to satisfy the request -- a frame filter will request as
many frames as it needs.

Also, I tried removing this code from backtrace, and I think the
result is better without it.  In particular, now the expansion line
occurs just before the frame that caused the expansion, like:

    (gdb) bt no-filters
    #0  0x00007ffff576cecd in poll () from /lib64/libc.so.6
    Reading in symbols for ../../binutils-gdb/gdb/event-loop.c...done.
    #1  0x00000000007ecc33 in gdb_wait_for_event (block=1)
	at ../../binutils-gdb/gdb/event-loop.c:772
    #2  0x00000000007ec006 in gdb_do_one_event ()
	at ../../binutils-gdb/gdb/event-loop.c:347
    #3  0x00000000007ec03e in start_event_loop ()
	at ../../binutils-gdb/gdb/event-loop.c:371
    Reading in symbols for ../../binutils-gdb/gdb/main.c...done.
    #4  0x000000000086693d in captured_command_loop (
	Reading in symbols for ../../binutils-gdb/gdb/exceptions.c...done.
    data=0x0) at ../../binutils-gdb/gdb/main.c:325

So, I am proposing this patch to simply remove this code.

gdb/ChangeLog
2018-03-23  Tom Tromey  <tom@tromey.com>

	* stack.c (backtrace_command_1): Remove verbose code.
---
 gdb/ChangeLog |  4 ++++
 gdb/stack.c   | 18 ------------------
 2 files changed, 4 insertions(+), 18 deletions(-)

diff --git a/gdb/stack.c b/gdb/stack.c
index 427b182c7f..9fdc9eece2 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1780,24 +1780,6 @@ backtrace_command_1 (const char *count_exp, frame_filter_flags flags,
 	  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.13.6

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

* [RFA v3 06/13] Allow C-c to work in backtrace in more cases
  2018-03-23 20:55 [RFA v3 00/13] various frame filter fixes and cleanups Tom Tromey
                   ` (2 preceding siblings ...)
  2018-03-23 20:55 ` [RFA v3 05/13] Avoid manual resource management " Tom Tromey
@ 2018-03-23 20:55 ` Tom Tromey
  2018-03-23 20:55 ` [RFA v3 03/13] Allow hiding of some filtered frames Tom Tromey
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2018-03-23 20:55 UTC (permalink / raw)
  To: gdb-patches; +Cc: 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.

gdb/ChangeLog
2018-03-23  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/python/py-framefilter.c b/gdb/python/py-framefilter.c
index dcac42df8e..28d5c37b25 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.13.6

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

* [RFA v3 08/13] Move some code later in backtrace_command_1
  2018-03-23 20:55 [RFA v3 00/13] various frame filter fixes and cleanups Tom Tromey
                   ` (8 preceding siblings ...)
  2018-03-23 20:55 ` [RFA v3 04/13] Remove EXT_LANG_BT_COMPLETED Tom Tromey
@ 2018-03-23 20:55 ` Tom Tromey
  2018-03-23 20:55 ` [RFA v3 11/13] Improve "backtrace" help text Tom Tromey
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2018-03-23 20:55 UTC (permalink / raw)
  To: gdb-patches; +Cc: 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.

gdb/ChangeLog
2018-03-23  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/stack.c b/gdb/stack.c
index 8e8fe12109..16f7931be1 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1698,49 +1698,17 @@ backtrace_command_1 (const char *count_exp, frame_filter_flags flags,
   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;
@@ -1755,24 +1723,6 @@ backtrace_command_1 (const char *count_exp, frame_filter_flags flags,
       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;
@@ -1797,6 +1747,57 @@ backtrace_command_1 (const char *count_exp, frame_filter_flags flags,
      "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.13.6

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

* [RFA v3 00/13] various frame filter fixes and cleanups
@ 2018-03-23 20:55 Tom Tromey
  2018-03-23 20:55 ` [RFA v3 07/13] Throw a "quit" on a KeyboardException in py-framefilter.c Tom Tromey
                   ` (13 more replies)
  0 siblings, 14 replies; 26+ messages in thread
From: Tom Tromey @ 2018-03-23 20:55 UTC (permalink / raw)
  To: gdb-patches

This is version 3 of a series that I last sent last August:

https://sourceware.org/ml/gdb-patches/2017-08/msg00277.html

Most of the patches were already approved, though of course it doesn't
hurt to check again.  At least patch #1 needed some reworking in order
to be rebased on top of the changes that have been made since August.

This version addresses most of the review comments from version 2.
Also, in response to Pedro's feedback about the choice of "elide" as
the option name, I've changed the new command to be "bt hide".  I
think this should be unambiguous.

I have not addressed Phil's comments:

https://sourceware.org/ml/gdb-patches/2017-08/msg00360.html
https://sourceware.org/ml/gdb-patches/2017-08/msg00359.html

One reason is that I agree with Pedro's comment here:

https://sourceware.org/ml/gdb-patches/2017-08/msg00297.html

Another reason is that I think that the two ideas can live together,
if necessary: I'd still appreciate a way to hide frames from the
command line, overriding whatever individual frame filters think is
appropriate.  Similarly, adding this feature to frame filters would
not impact this series -- it can always still be done after this.

Regression tested by the buildbot.

Tom

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

* [RFA v3 10/13] Call wrap_hint in one more spot in py-framefilter.c
  2018-03-23 20:55 [RFA v3 00/13] various frame filter fixes and cleanups Tom Tromey
                   ` (11 preceding siblings ...)
  2018-03-23 20:55 ` [RFA v3 01/13] Rationalize "backtrace" command line parsing Tom Tromey
@ 2018-03-23 20:55 ` Tom Tromey
  2018-03-24 11:42 ` [RFA v3 00/13] various frame filter fixes and cleanups Pedro Alves
  13 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2018-03-23 20:55 UTC (permalink / raw)
  To: gdb-patches; +Cc: 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.

gdb/ChangeLog
2018-03-23  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/python/py-framefilter.c b/gdb/python/py-framefilter.c
index 6186ffd274..eb4f9c6adf 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.13.6

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

* [RFA v3 02/13] Change backtrace_command_1 calling to use flags
  2018-03-23 20:55 [RFA v3 00/13] various frame filter fixes and cleanups Tom Tromey
                   ` (5 preceding siblings ...)
  2018-03-23 20:55 ` [RFA v3 12/13] Simplify exception handling in py-framefilter.c Tom Tromey
@ 2018-03-23 20:55 ` Tom Tromey
  2018-03-23 20:55 ` [RFA v3 13/13] Remove verbose code from backtrace command Tom Tromey
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2018-03-23 20:55 UTC (permalink / raw)
  To: gdb-patches; +Cc: 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.

gdb/ChangeLog
2018-03-23  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   | 17 +++++++----------
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/gdb/stack.c b/gdb/stack.c
index 13af6594a9..76695ff7e6 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1692,8 +1692,8 @@ info_frame_command (const char *addr_exp, int from_tty)
    frames.  */
 
 static void
-backtrace_command_1 (const char *count_exp, int show_locals, int no_filters,
-		     int from_tty)
+backtrace_command_1 (const char *count_exp, frame_filter_flags flags,
+		     int no_filters, int from_tty)
 {
   struct frame_info *fi;
   int count;
@@ -1775,11 +1775,9 @@ backtrace_command_1 (const char *count_exp, int show_locals, int no_filters,
 
   if (! no_filters)
     {
-      frame_filter_flags 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 (from_tty)
 	flags |= PRINT_MORE_FRAMES;
 
@@ -1809,7 +1807,7 @@ backtrace_command_1 (const 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);
 
@@ -1850,8 +1848,8 @@ backtrace_command_1 (const char *count_exp, int show_locals, int no_filters,
 static void
 backtrace_command (const char *arg, int from_tty)
 {
-  bool fulltrace = false;
   bool filters = true;
+  frame_filter_flags flags = 0;
 
   if (arg)
     {
@@ -1868,7 +1866,7 @@ backtrace_command (const char *arg, int from_tty)
 	  if (subset_compare (this_arg.c_str (), "no-filters"))
 	    filters = false;
 	  else if (subset_compare (this_arg.c_str (), "full"))
-	    fulltrace = true;
+	    flags |= PRINT_LOCALS;
 	  else
 	    {
 	      /* Not a recognized argument, so stop.  */
@@ -1881,8 +1879,7 @@ backtrace_command (const 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.13.6

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

* [RFA v3 11/13] Improve "backtrace" help text
  2018-03-23 20:55 [RFA v3 00/13] various frame filter fixes and cleanups Tom Tromey
                   ` (9 preceding siblings ...)
  2018-03-23 20:55 ` [RFA v3 08/13] Move some code later in backtrace_command_1 Tom Tromey
@ 2018-03-23 20:55 ` Tom Tromey
  2018-03-23 20:55 ` [RFA v3 01/13] Rationalize "backtrace" command line parsing Tom Tromey
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2018-03-23 20:55 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

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.

gdb/ChangeLog
2018-03-23  Tom Tromey  <tom@tromey.com>

	* stack.c (_initialize_stack): Remove trailing newlines from help
	text.  Add "Usage" line to "backtrace" help.
---
 gdb/ChangeLog |  5 +++++
 gdb/stack.c   | 13 +++++++------
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/gdb/stack.c b/gdb/stack.c
index 16f7931be1..427b182c7f 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -2554,22 +2554,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);
@@ -2587,7 +2588,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,
-- 
2.13.6

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

* Re: [RFA v3 01/13] Rationalize "backtrace" command line parsing
  2018-03-23 20:55 ` [RFA v3 01/13] Rationalize "backtrace" command line parsing Tom Tromey
@ 2018-03-24  6:31   ` Eli Zaretskii
  2018-03-25 16:50     ` Tom Tromey
  0 siblings, 1 reply; 26+ messages in thread
From: Eli Zaretskii @ 2018-03-24  6:31 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

> From: Tom Tromey <tom@tromey.com>
> Cc: Tom Tromey <tom@tromey.com>
> Date: Fri, 23 Mar 2018 14:55:00 -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.
> 
> This also updates the documentation to describe the various bt options
> individually.
> 
> gdb/ChangeLog
> 2018-03-23  Tom Tromey  <tom@tromey.com>
> 
> 	* stack.c (backtrace_command): Rewrite command line parsing.
> 
> gdb/doc/ChangeLog
> 2018-03-23  Tom Tromey  <tom@tromey.com>
> 
> 	* gdb.texinfo (Backtrace): Describe options individually.
> ---
>  gdb/ChangeLog       |  4 ++++
>  gdb/doc/ChangeLog   |  4 ++++
>  gdb/doc/gdb.texinfo | 54 ++++++++++++++++++++--------------------------
>  gdb/stack.c         | 62 +++++++++++++++++------------------------------------
>  4 files changed, 51 insertions(+), 73 deletions(-)
> 
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 74e0fdb4a4..b48dd4ed4b 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -7307,39 +7307,31 @@ 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}

Is it wise to delete the @table?  We always describe commands in that
format, AFAIR.

> +Print a backtrace of the entire stack, use the @code{backtrace}
> +command, or its alias @code{bt}.  This command will print one line per

I gues you meant "To print a backtrace of the entire stack ...",
because otherwise the first sentence reads weirdly.

Otherwise, the documentation part is OK.  Thanks.

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

* Re: [RFA v3 03/13] Allow hiding of some filtered frames
  2018-03-23 20:55 ` [RFA v3 03/13] Allow hiding of some filtered frames Tom Tromey
@ 2018-03-24  6:32   ` Eli Zaretskii
  0 siblings, 0 replies; 26+ messages in thread
From: Eli Zaretskii @ 2018-03-24  6:32 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches, tom

> From: Tom Tromey <tom@tromey.com>
> Cc: Tom Tromey <tom@tromey.com>
> Date: Fri, 23 Mar 2018 14:55:02 -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.
> 
> gdb/ChangeLog
> 2018-03-23  Tom Tromey  <tom@tromey.com>
> 
> 	PR backtrace/15582:
> 	* stack.c (backtrace_command): Parse "hide" argument.
> 	* python/py-framefilter.c (py_print_frame): Handle PRINT_HIDE.
> 	* extension.h (enum frame_filter_flags) <PRINT_HIDE>: New
> 	constant.
> 
> gdb/doc/ChangeLog
> 2018-03-23  Tom Tromey  <tom@tromey.com>
> 
> 	PR backtrace/15582:
> 	* gdb.texinfo (Backtrace): Mention "hide" argument.
> 
> gdb/testsuite/ChangeLog
> 2018-03-23  Tom Tromey  <tom@tromey.com>
> 
> 	PR backtrace/15582:
> 	* gdb.python/py-framefilter.exp: Add "bt hide" test.

OK for the documentation part.

Thanks.

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

* Re: [RFA v3 07/13] Throw a "quit" on a KeyboardException in py-framefilter.c
  2018-03-23 20:55 ` [RFA v3 07/13] Throw a "quit" on a KeyboardException in py-framefilter.c Tom Tromey
@ 2018-03-24 11:41   ` Pedro Alves
  2018-03-25 16:37     ` Tom Tromey
  0 siblings, 1 reply; 26+ messages in thread
From: Pedro Alves @ 2018-03-24 11:41 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

On 03/23/2018 08:55 PM, Tom Tromey wrote:

> +set test "bt 1 with KeyboardInterrupt"
> +gdb_test_multiple "bt 1" $test {
> +    -re "Quit" {
> +	pass $test
> +    }
> +}

What's the gdb output in gdb.log in this case?  Is there a GDB prompt
involved?  

I'm wondering whether this is racy as is.  Does e.g., the test pass
with "make check-read1"?

Or, are we leaving a gdb prompt in the expect buffer unprocessed?

Thanks,
Pedro Alves

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

* Re: [RFA v3 00/13] various frame filter fixes and cleanups
  2018-03-23 20:55 [RFA v3 00/13] various frame filter fixes and cleanups Tom Tromey
                   ` (12 preceding siblings ...)
  2018-03-23 20:55 ` [RFA v3 10/13] Call wrap_hint in one more spot in py-framefilter.c Tom Tromey
@ 2018-03-24 11:42 ` Pedro Alves
  2018-03-27  4:01   ` Tom Tromey
  13 siblings, 1 reply; 26+ messages in thread
From: Pedro Alves @ 2018-03-24 11:42 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

On 03/23/2018 08:54 PM, Tom Tromey wrote:
> This is version 3 of a series that I last sent last August:
> 
> https://sourceware.org/ml/gdb-patches/2017-08/msg00277.html
> 
> Most of the patches were already approved, though of course it doesn't
> hurt to check again.  At least patch #1 needed some reworking in order
> to be rebased on top of the changes that have been made since August.
> 
> This version addresses most of the review comments from version 2.
> Also, in response to Pedro's feedback about the choice of "elide" as
> the option name, I've changed the new command to be "bt hide".  I
> think this should be unambiguous.
> 
> I have not addressed Phil's comments:
> 
> https://sourceware.org/ml/gdb-patches/2017-08/msg00360.html
> https://sourceware.org/ml/gdb-patches/2017-08/msg00359.html
> 
> One reason is that I agree with Pedro's comment here:
> 
> https://sourceware.org/ml/gdb-patches/2017-08/msg00297.html
> 
> Another reason is that I think that the two ideas can live together,
> if necessary: I'd still appreciate a way to hide frames from the
> command line, overriding whatever individual frame filters think is
> appropriate.  Similarly, adding this feature to frame filters would
> not impact this series -- it can always still be done after this.
> 
> Regression tested by the buildbot.

Thanks.  This is all fine with me, except the testsuite bit
which I sent a separate comment for.

Thanks,
Pedro Alves

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

* Re: [RFA v3 07/13] Throw a "quit" on a KeyboardException in py-framefilter.c
  2018-03-24 11:41   ` Pedro Alves
@ 2018-03-25 16:37     ` Tom Tromey
  2018-03-25 17:13       ` Pedro Alves
  0 siblings, 1 reply; 26+ messages in thread
From: Tom Tromey @ 2018-03-25 16:37 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, gdb-patches

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

Pedro> On 03/23/2018 08:55 PM, Tom Tromey wrote:
>> +set test "bt 1 with KeyboardInterrupt"
>> +gdb_test_multiple "bt 1" $test {
>> +    -re "Quit" {
>> +	pass $test
>> +    }
>> +}

Pedro> What's the gdb output in gdb.log in this case?  Is there a GDB prompt
Pedro> involved?  

Here's what gdb.log says:

bt 1
#0  Quit
(gdb) PASS: gdb.python/py-framefilter.exp: bt 1 with KeyboardInterrupt

The test is making a frame filter that throws a KeyboardInterrupt when
called, to simulate the user interrupting the backtrace.

Pedro> I'm wondering whether this is racy as is.  Does e.g., the test pass
Pedro> with "make check-read1"?

This works fine.

Pedro> Or, are we leaving a gdb prompt in the expect buffer unprocessed?

I don't think so but I guess I am not really sure.

Tom

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

* Re: [RFA v3 01/13] Rationalize "backtrace" command line parsing
  2018-03-24  6:31   ` Eli Zaretskii
@ 2018-03-25 16:50     ` Tom Tromey
  2018-03-25 17:11       ` Eli Zaretskii
  0 siblings, 1 reply; 26+ messages in thread
From: Tom Tromey @ 2018-03-25 16:50 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Tom Tromey, gdb-patches

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

Eli> Is it wise to delete the @table?  We always describe commands in that
Eli> format, AFAIR.

The patch actually just moves the table down a bit and removes some text
from each entry.  It isn't really deleted.  For example:

-@item backtrace @var{n}
-@itemx bt @var{n}
-Similar, but print only the innermost @var{n} frames.

becomes

+@table @code
+@item @var{n}
+@itemx @var{n}
+Print only the innermost @var{n} frames, where @var{n} is a positive
+number.

What here would you like changed?

>> +Print a backtrace of the entire stack, use the @code{backtrace}
>> +command, or its alias @code{bt}.  This command will print one line per

Eli> I gues you meant "To print a backtrace of the entire stack ...",
Eli> because otherwise the first sentence reads weirdly.

I'll make this change.

Tom

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

* Re: [RFA v3 01/13] Rationalize "backtrace" command line parsing
  2018-03-25 16:50     ` Tom Tromey
@ 2018-03-25 17:11       ` Eli Zaretskii
  2018-03-26 20:45         ` Tom Tromey
  0 siblings, 1 reply; 26+ messages in thread
From: Eli Zaretskii @ 2018-03-25 17:11 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

> From: Tom Tromey <tom@tromey.com>
> Cc: Tom Tromey <tom@tromey.com>,  gdb-patches@sourceware.org
> Date: Sun, 25 Mar 2018 10:50:52 -0600
> 
> >>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
> 
> Eli> Is it wise to delete the @table?  We always describe commands in that
> Eli> format, AFAIR.
> 
> The patch actually just moves the table down a bit and removes some text
> from each entry.  It isn't really deleted.  For example:
> 
> -@item backtrace @var{n}
> -@itemx bt @var{n}
> -Similar, but print only the innermost @var{n} frames.
> 
> becomes
> 
> +@table @code
> +@item @var{n}
> +@itemx @var{n}
> +Print only the innermost @var{n} frames, where @var{n} is a positive
> +number.

Yes, but the original table included the command, as we do with all
commands, whereas your new table includes only the various forms of
arguments to the command, and the command itself is never mentioned,
except in the surrounding text.

> What here would you like changed?

I expected to see something like this, before the description of
arguments:

 @table @code
 @item backtrace [@var{args}@dots{}]
 @itemx bt [@var{args}@dots{}]
 Print the backtrace of the entire stack.  The optional @var{args} can
 be one of the following:

followed by your table of arguments.

Thanks.

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

* Re: [RFA v3 07/13] Throw a "quit" on a KeyboardException in py-framefilter.c
  2018-03-25 16:37     ` Tom Tromey
@ 2018-03-25 17:13       ` Pedro Alves
  2018-03-26 21:14         ` Tom Tromey
  0 siblings, 1 reply; 26+ messages in thread
From: Pedro Alves @ 2018-03-25 17:13 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 03/25/2018 05:37 PM, Tom Tromey wrote:

> Pedro> Or, are we leaving a gdb prompt in the expect buffer unprocessed?
> 
> I don't think so but I guess I am not really sure.

We are then, since there's a prompt after the Quit that your
gdb_test_multiple is not consuming.  That is a recipe for making the
following gdb_test/gdb_test_multiple confused and hit the internal default
prompt/FAIL match in gdb_test_multiple:

...
	-re "\r\n$gdb_prompt $" {
	    if ![string match "" $message] then {
		fail "$message"
	    }
	    set result 1
	}
...

Given the "$", it's going to be racy, though make check-read1
is more likely to catch it.

Sounds like that doesn't happen currently because your test is
the last one before restarting gdb.  So if you add more tests after
yours, it's likely you'll see a problem.  Or even without more tests,
it can still happen with board files that issue gdb commands when
tearing down the connection or gdb.

So you need to consume the prompt, like:

 gdb_test_multiple "bt 1" $test {
-     -re "Quit" {
+     -re "Quit\r\n$gdb_prompt $" {
 	pass $test
     }
 }

Or simply instead:

 gdb_test "bt 1" "Quit" "bt 1 with KeyboardInterrupt"

since gdb_test adds the prompt to the expected string for you.

Thanks,
Pedro Alves

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

* Re: [RFA v3 01/13] Rationalize "backtrace" command line parsing
  2018-03-25 17:11       ` Eli Zaretskii
@ 2018-03-26 20:45         ` Tom Tromey
  2018-03-27  2:35           ` Eli Zaretskii
  0 siblings, 1 reply; 26+ messages in thread
From: Tom Tromey @ 2018-03-26 20:45 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Tom Tromey, gdb-patches

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

Eli> I expected to see something like this, before the description of
Eli> arguments:

Eli>  @table @code
Eli>  @item backtrace [@var{args}@dots{}]
Eli>  @itemx bt [@var{args}@dots{}]
Eli>  Print the backtrace of the entire stack.  The optional @var{args} can
Eli>  be one of the following:

Eli> followed by your table of arguments.

Like this?

Tom

commit f8f0cf13d9a8c66a85b287b533c231a44d18c516
Author: Tom Tromey <tom@tromey.com>
Date:   Sun Apr 23 10:54:33 2017 -0600

    Rationalize "backtrace" command line parsing
    
    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.
    
    gdb/ChangeLog
    2018-03-23  Tom Tromey  <tom@tromey.com>
    
            * stack.c (backtrace_command): Rewrite command line parsing.
    
    gdb/doc/ChangeLog
    2018-03-23  Tom Tromey  <tom@tromey.com>
    
            * gdb.texinfo (Backtrace): Describe options individually.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 47e6cff43d..90c66e3955 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2018-03-23  Tom Tromey  <tom@tromey.com>
+
+	* stack.c (backtrace_command): Rewrite command line parsing.
+
 2018-03-26  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* dwarf2read.c (DEF_VEC_I(offset_type)): Remove.
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 2441f15431..339f1d51ad 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,7 @@
+2018-03-23  Tom Tromey  <tom@tromey.com>
+
+	* gdb.texinfo (Backtrace): Describe options individually.
+
 2018-03-19  Tom Tromey  <tom@tromey.com>
 
 	* observer.texi: Remove.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 74e0fdb4a4..28254c9e68 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -7307,45 +7307,43 @@ 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}
+To 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 frames in the stack.  By default, all stack frames are
+printed.  You can stop the backtrace at any time by typing the system
+interrupt character, normally @kbd{Ctrl-c}.
+
+@table @code
+@item backtrace [@var{args}@dots{}]
+@itemx bt [@var{args}@dots{}]
+Print the backtrace of the entire stack.  The optional @var{args} can
+be one of the following:
+
+@table @code
+@item @var{n}
+@itemx @var{n}
+Print only the innermost @var{n} frames, where @var{n} is a positive
+number.
+
+@item -@var{n}
+@itemx -@var{n}
+Print only the outermost @var{n} frames, where @var{n} is a positive
+number.
+
+@item full
+Print the values of the local variables also.  This can be combined
+with a number to limit the number of frames shown.
+
+@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
 relevant when @value{GDBN} has been configured with @code{Python}
 support.
 @end table
+@end table
 
 @kindex where
 @kindex info stack
diff --git a/gdb/stack.c b/gdb/stack.c
index aad8fcd987..13af6594a9 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1850,61 +1850,39 @@ backtrace_command_1 (const char *count_exp, int show_locals, int no_filters,
 static void
 backtrace_command (const char *arg, int from_tty)
 {
-  int fulltrace_arg = -1, arglen = 0, argc = 0, no_filters  = -1;
-  int user_arg = 0;
+  bool fulltrace = false;
+  bool filters = true;
 
-  std::string reconstructed_arg;
   if (arg)
     {
-      char **argv;
-      int i;
+      bool done = false;
 
-      gdb_argv built_argv (arg);
-      argv = built_argv.get ();
-      argc = 0;
-      for (i = 0; argv[i]; i++)
+      while (!done)
 	{
-	  unsigned int j;
+	  const char *save_arg = arg;
+	  std::string this_arg = extract_arg (&arg);
 
-	  for (j = 0; j < strlen (argv[i]); j++)
-	    argv[i][j] = TOLOWER (argv[i][j]);
+	  if (this_arg.empty ())
+	    break;
 
-	  if (no_filters < 0 && subset_compare (argv[i], "no-filters"))
-	    no_filters = argc;
+	  if (subset_compare (this_arg.c_str (), "no-filters"))
+	    filters = false;
+	  else if (subset_compare (this_arg.c_str (), "full"))
+	    fulltrace = true;
 	  else
 	    {
-	      if (fulltrace_arg < 0 && subset_compare (argv[i], "full"))
-		fulltrace_arg = argc;
-	      else
-		{
-		  user_arg++;
-		  arglen += strlen (argv[i]);
-		}
-	    }
-	  argc++;
-	}
-      arglen += user_arg;
-      if (fulltrace_arg >= 0 || no_filters >= 0)
-	{
-	  if (arglen > 0)
-	    {
-	      for (i = 0; i < argc; i++)
-		{
-		  if (i != fulltrace_arg && i != no_filters)
-		    {
-		      reconstructed_arg += argv[i];
-		      reconstructed_arg += " ";
-		    }
-		}
-	      arg = reconstructed_arg.c_str ();
+	      /* Not a recognized argument, so stop.  */
+	      arg = save_arg;
+	      done = true;
 	    }
-	  else
-	    arg = NULL;
 	}
+
+      if (*arg == '\0')
+	arg = NULL;
     }
 
-  backtrace_command_1 (arg, fulltrace_arg >= 0 /* show_locals */,
-		       no_filters >= 0 /* no frame-filters */, from_tty);
+  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

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

* Re: [RFA v3 07/13] Throw a "quit" on a KeyboardException in py-framefilter.c
  2018-03-25 17:13       ` Pedro Alves
@ 2018-03-26 21:14         ` Tom Tromey
  0 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2018-03-26 21:14 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, gdb-patches

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

Pedro> Or simply instead:
Pedro>  gdb_test "bt 1" "Quit" "bt 1 with KeyboardInterrupt"
Pedro> since gdb_test adds the prompt to the expected string for you.

Thanks for the analysis.  I've made this change.

Tom

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

* Re: [RFA v3 01/13] Rationalize "backtrace" command line parsing
  2018-03-26 20:45         ` Tom Tromey
@ 2018-03-27  2:35           ` Eli Zaretskii
  0 siblings, 0 replies; 26+ messages in thread
From: Eli Zaretskii @ 2018-03-27  2:35 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

> From: Tom Tromey <tom@tromey.com>
> Cc: Tom Tromey <tom@tromey.com>,  gdb-patches@sourceware.org
> Date: Mon, 26 Mar 2018 14:44:54 -0600
> 
> >>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
> 
> Eli> I expected to see something like this, before the description of
> Eli> arguments:
> 
> Eli>  @table @code
> Eli>  @item backtrace [@var{args}@dots{}]
> Eli>  @itemx bt [@var{args}@dots{}]
> Eli>  Print the backtrace of the entire stack.  The optional @var{args} can
> Eli>  be one of the following:
> 
> Eli> followed by your table of arguments.
> 
> Like this?

Yes, thanks.

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

* Re: [RFA v3 00/13] various frame filter fixes and cleanups
  2018-03-24 11:42 ` [RFA v3 00/13] various frame filter fixes and cleanups Pedro Alves
@ 2018-03-27  4:01   ` Tom Tromey
  0 siblings, 0 replies; 26+ messages in thread
From: Tom Tromey @ 2018-03-27  4:01 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, gdb-patches

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

Pedro> Thanks.  This is all fine with me, except the testsuite bit
Pedro> which I sent a separate comment for.

Thanks.  I'm checking this in now.

Tom

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

end of thread, other threads:[~2018-03-27  4:01 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-23 20:55 [RFA v3 00/13] various frame filter fixes and cleanups Tom Tromey
2018-03-23 20:55 ` [RFA v3 07/13] Throw a "quit" on a KeyboardException in py-framefilter.c Tom Tromey
2018-03-24 11:41   ` Pedro Alves
2018-03-25 16:37     ` Tom Tromey
2018-03-25 17:13       ` Pedro Alves
2018-03-26 21:14         ` Tom Tromey
2018-03-23 20:55 ` [RFA v3 09/13] Return EXT_LANG_BT_ERROR in one more spot " Tom Tromey
2018-03-23 20:55 ` [RFA v3 05/13] Avoid manual resource management " Tom Tromey
2018-03-23 20:55 ` [RFA v3 06/13] Allow C-c to work in backtrace in more cases Tom Tromey
2018-03-23 20:55 ` [RFA v3 03/13] Allow hiding of some filtered frames Tom Tromey
2018-03-24  6:32   ` Eli Zaretskii
2018-03-23 20:55 ` [RFA v3 12/13] Simplify exception handling in py-framefilter.c Tom Tromey
2018-03-23 20:55 ` [RFA v3 02/13] Change backtrace_command_1 calling to use flags Tom Tromey
2018-03-23 20:55 ` [RFA v3 13/13] Remove verbose code from backtrace command Tom Tromey
2018-03-23 20:55 ` [RFA v3 04/13] Remove EXT_LANG_BT_COMPLETED Tom Tromey
2018-03-23 20:55 ` [RFA v3 08/13] Move some code later in backtrace_command_1 Tom Tromey
2018-03-23 20:55 ` [RFA v3 11/13] Improve "backtrace" help text Tom Tromey
2018-03-23 20:55 ` [RFA v3 01/13] Rationalize "backtrace" command line parsing Tom Tromey
2018-03-24  6:31   ` Eli Zaretskii
2018-03-25 16:50     ` Tom Tromey
2018-03-25 17:11       ` Eli Zaretskii
2018-03-26 20:45         ` Tom Tromey
2018-03-27  2:35           ` Eli Zaretskii
2018-03-23 20:55 ` [RFA v3 10/13] Call wrap_hint in one more spot in py-framefilter.c Tom Tromey
2018-03-24 11:42 ` [RFA v3 00/13] various frame filter fixes and cleanups Pedro Alves
2018-03-27  4:01   ` Tom Tromey

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