[RFA]: issue warnings for frame offenses

[RFA]: issue warnings for frame offenses

[Jeff Johnston]

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

The attached patch changes a few backtrace termination scenarios in frame.c to 
issue a warning and terminate the backtrace rather than use the error() 
function.  The change is being made to help out end-users that use macros with 
backtraces in them.  At present, there a number of platforms where backtraces 
through assembler code (e.g. glibc thread creation) cause backtraces to get 
somewhat lost.  When the frame code issues the error, any macro issuing the 
backtrace is terminated.  If an end-user is applying such a macro to all 
threads, it ends prematurely for no good reason.  With the change, the message 
is still issued and the backtrace is stopped.

Ok to commit?

2004-10-26  Jeff Johnston  <jjohnstn@redhat.com>

         * frame.c (get_prev_frame_1): Change inner frame and equivalent
         frame tests to issue a warning rather than an error.
         (get_prev_frame): When backtrace limit is exceeded, terminate
         with a warning rather than an error.


[-- Attachment #2: frame.patch --]
[-- Type: text/plain, Size: 1642 bytes --]

Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.191
diff -u -p -r1.191 frame.c
--- frame.c	1 Sep 2004 14:13:33 -0000	1.191
+++ frame.c	26 Oct 2004 21:13:18 -0000
@@ -1036,14 +1036,20 @@ get_prev_frame_1 (struct frame_info *thi
   if (this_frame->next->level >= 0
       && this_frame->next->unwind->type != SIGTRAMP_FRAME
       && frame_id_inner (this_id, get_frame_id (this_frame->next)))
-    error ("Previous frame inner to this frame (corrupt stack?)");
+    {
+      warning ("Previous frame inner to this frame (corrupt stack?)");
+      return NULL;
+    }
 
   /* Check that this and the next frame are not identical.  If they
      are, there is most likely a stack cycle.  As with the inner-than
      test above, avoid comparing the inner-most and sentinel frames.  */
   if (this_frame->level > 0
       && frame_id_eq (this_id, get_frame_id (this_frame->next)))
-    error ("Previous frame identical to this frame (corrupt stack?)");
+    {
+      warning ("Previous frame identical to this frame (corrupt stack?)");
+      return NULL;
+    }
 
   /* Allocate the new frame but do not wire it in to the frame chain.
      Some (bad) code in INIT_FRAME_EXTRA_INFO tries to look along
@@ -1199,7 +1205,8 @@ get_prev_frame (struct frame_info *this_
 
   if (this_frame->level > backtrace_limit)
     {
-      error ("Backtrace limit of %d exceeded", backtrace_limit);
+      warning ("Backtrace limit of %d exceeded", backtrace_limit);
+      return NULL;
     }
 
   /* If we're already inside the entry function for the main objfile,

Re: [RFA]: issue warnings for frame offenses

[Joel Brobecker]

> The attached patch changes a few backtrace termination scenarios in frame.c 
> to issue a warning and terminate the backtrace rather than use the error() 
> function.  The change is being made to help out end-users that use macros 
> with backtraces in them.  At present, there a number of platforms where 
> backtraces through assembler code (e.g. glibc thread creation) cause 
> backtraces to get somewhat lost.  When the frame code issues the error, any 
> macro issuing the backtrace is terminated.  If an end-user is applying such 
> a macro to all threads, it ends prematurely for no good reason.  With the 
> change, the message is still issued and the backtrace is stopped.

Interestingly, a customer of ours reported about a year or two ago
a situation similar to this. They were using the backtrace command
inside breakpoint commands looking like this:

        break somewhere
        commands
           backtrace
           continue
        end

The idea was that they wanted to see how the program would get to
a certain piece of code (in our case raise an exception), and how
often. But they didn't want to stop the execution and start debugging.
But the unwinder would sometimes be confused, and an error would be
raised, and the commands execution would be aborted.

We modified the debugger in a rather radical way: We wrapped the
backtrace command under control of the catch_error() routine, and
transformed any error into a warning. We didn't feel that this would
be interesting for the FSF tree, but maybe our assesment was wrong.
Let us know if this approach would also be useful, in which case
we'll be happy to contribute it.

> Ok to commit?
> 
> 2004-10-26  Jeff Johnston  <jjohnstn@redhat.com>
> 
>         * frame.c (get_prev_frame_1): Change inner frame and equivalent
>         frame tests to issue a warning rather than an error.
>         (get_prev_frame): When backtrace limit is exceeded, terminate
>         with a warning rather than an error.
> 
-- 
Joel

Re: [RFA]: issue warnings for frame offenses

[Andrew Cagney]

Joel Brobecker wrote:
>>The attached patch changes a few backtrace termination scenarios in frame.c 
>>to issue a warning and terminate the backtrace rather than use the error() 
>>function.  The change is being made to help out end-users that use macros 
>>with backtraces in them.  At present, there a number of platforms where 
>>backtraces through assembler code (e.g. glibc thread creation) cause 
>>backtraces to get somewhat lost.  When the frame code issues the error, any 
>>macro issuing the backtrace is terminated.  If an end-user is applying such 
>>a macro to all threads, it ends prematurely for no good reason.  With the 
>>change, the message is still issued and the backtrace is stopped.
> 
> 
> Interestingly, a customer of ours reported about a year or two ago
> a situation similar to this. They were using the backtrace command
> inside breakpoint commands looking like this:
> 
>         break somewhere
>         commands
>            backtrace
>            continue
>         end
> 
> The idea was that they wanted to see how the program would get to
> a certain piece of code (in our case raise an exception), and how
> often. But they didn't want to stop the execution and start debugging.
> But the unwinder would sometimes be confused, and an error would be
> raised, and the commands execution would be aborted.
> 
> We modified the debugger in a rather radical way: We wrapped the
> backtrace command under control of the catch_error() routine, and
> transformed any error into a warning. We didn't feel that this would
> be interesting for the FSF tree, but maybe our assesment was wrong.
> Let us know if this approach would also be useful, in which case
> we'll be happy to contribute it.

(Somewhere there is a try / finally command patch, just waiting to be 
integrated.)

How does the following sound:

- add a new fatal_error():

There should be a new error mechanism that throws a QUIT instead of 
ERROR.  Code should not normally be catching QUIT (much unfortunately 
does).  A fatal error is things like: syntax error; no target; lost 
target.  Things like a memory access violation though are not fatal.

- think about stopping code catching and then discarding QUIT
Grep for RETURN_MASK_ALL in the sources - it should be RETURN_MASK_ERROR :-/

- modify the backtrace code to catch ERROR, but not QUIT

Along the lines of Joel's suggestion, the backtrace command should catch 
ERROR (but should not catch QUIT).  That way simple problems don't abort 
the script but fatal ones do.


Why?

Lets try to categorize commands as being of two types:

- display
These commands print information from the inferior.  In general they 
don't cause an error - for instance:
   (gdb) print *(char *)0
   $1 = <memory error>
(yes I know, this isn't currently the case :-)

- modify
These commands try to change the state of GDB or the inferior.  In 
general problems do lead to an error as not performing the operation 
puts things into a relatively undefined state.

(The pedant will realize that ``(gdb) print (*(char *)0)++'' can be 
thought of as a  `modify', lets ignore that in this simple model :-).

The breakpoint command falls into the ``display'' category.
The up and run commands fall into the ``modify'' category.


Why not s/error/warning/:

I'm trying to avoid a bigger mess.  Given a corrupt stack, I'm pretty 
sure that this:

   (gdb) up
   Previous frame identical to this frame (corrupt stack?)
   (gdb) up
   Initial frame selected; you cannot go up.

(the same error should be printed both times) and this:

   (gdb) frame 0x12345678
   Previous frame identical to this frame (corrupt stack?)
   (gdb) frame 0x12345678
   #1  0x1005e5f8 in fprintf_filtered ....

(the error should be completely suppressed) occur!

Fixing this would mean more radical (but still overdue) changes to both 
the stack and frame code.

My guess is publish a method:

	int safe_get_prev_frame (this_frame, &prev_frame, &error_if_nonnull)

and use that.

How, if you're up for a challenge.

Andrew

Re: [RFA]: issue warnings for frame offenses

[Jeff Johnston]

Andrew Cagney wrote:
> Joel Brobecker wrote:
> 
>>> The attached patch changes a few backtrace termination scenarios in 
>>> frame.c to issue a warning and terminate the backtrace rather than 
>>> use the error() function.  The change is being made to help out 
>>> end-users that use macros with backtraces in them.  At present, there 
>>> a number of platforms where backtraces through assembler code (e.g. 
>>> glibc thread creation) cause backtraces to get somewhat lost.  When 
>>> the frame code issues the error, any macro issuing the backtrace is 
>>> terminated.  If an end-user is applying such a macro to all threads, 
>>> it ends prematurely for no good reason.  With the change, the message 
>>> is still issued and the backtrace is stopped.
>>
>>
>>
>> Interestingly, a customer of ours reported about a year or two ago
>> a situation similar to this. They were using the backtrace command
>> inside breakpoint commands looking like this:
>>
>>         break somewhere
>>         commands
>>            backtrace
>>            continue
>>         end
>>
>> The idea was that they wanted to see how the program would get to
>> a certain piece of code (in our case raise an exception), and how
>> often. But they didn't want to stop the execution and start debugging.
>> But the unwinder would sometimes be confused, and an error would be
>> raised, and the commands execution would be aborted.
>>
>> We modified the debugger in a rather radical way: We wrapped the
>> backtrace command under control of the catch_error() routine, and
>> transformed any error into a warning. We didn't feel that this would
>> be interesting for the FSF tree, but maybe our assesment was wrong.
>> Let us know if this approach would also be useful, in which case
>> we'll be happy to contribute it.
> 
> 
> (Somewhere there is a try / finally command patch, just waiting to be 
> integrated.)
> 
> How does the following sound:
> 

Sounds fine.  I'll start working on it.  I assume you meant non_fatal_error 
below for the new function that issues the quit.

> - add a new fatal_error():
> 
> There should be a new error mechanism that throws a QUIT instead of 
> ERROR.  Code should not normally be catching QUIT (much unfortunately 
> does).  A fatal error is things like: syntax error; no target; lost 
> target.  Things like a memory access violation though are not fatal.
> 
> - think about stopping code catching and then discarding QUIT
> Grep for RETURN_MASK_ALL in the sources - it should be RETURN_MASK_ERROR 
> :-/
> 
> - modify the backtrace code to catch ERROR, but not QUIT
> 
> Along the lines of Joel's suggestion, the backtrace command should catch 
> ERROR (but should not catch QUIT).  That way simple problems don't abort 
> the script but fatal ones do.
> 
> 
> Why?
> 
> Lets try to categorize commands as being of two types:
> 
> - display
> These commands print information from the inferior.  In general they 
> don't cause an error - for instance:
>   (gdb) print *(char *)0
>   $1 = <memory error>
> (yes I know, this isn't currently the case :-)
> 
> - modify
> These commands try to change the state of GDB or the inferior.  In 
> general problems do lead to an error as not performing the operation 
> puts things into a relatively undefined state.
> 
> (The pedant will realize that ``(gdb) print (*(char *)0)++'' can be 
> thought of as a  `modify', lets ignore that in this simple model :-).
> 
> The breakpoint command falls into the ``display'' category.
> The up and run commands fall into the ``modify'' category.
> 
> 
> Why not s/error/warning/:
> 
> I'm trying to avoid a bigger mess.  Given a corrupt stack, I'm pretty 
> sure that this:
> 
>   (gdb) up
>   Previous frame identical to this frame (corrupt stack?)
>   (gdb) up
>   Initial frame selected; you cannot go up.
> 
> (the same error should be printed both times) and this:
> 
>   (gdb) frame 0x12345678
>   Previous frame identical to this frame (corrupt stack?)
>   (gdb) frame 0x12345678
>   #1  0x1005e5f8 in fprintf_filtered ....
> 
> (the error should be completely suppressed) occur!
> 
> Fixing this would mean more radical (but still overdue) changes to both 
> the stack and frame code.
> 
> My guess is publish a method:
> 
>     int safe_get_prev_frame (this_frame, &prev_frame, &error_if_nonnull)
> 
> and use that.
> 
> How, if you're up for a challenge.
> 
> Andrew
> 

Re: [RFA]: issue warnings for frame offenses

[Jeff Johnston]

Jeff Johnston wrote:
> 
> Sounds fine.  I'll start working on it.  I assume you meant 
> non_fatal_error below for the new function that issues the quit.
>

Ignore my question above.  I understand what is asked for now.  I was reading 
QUIT as being QUIT the current operation, but you mean it to be more severe so 
what you say below makes sense.

-- Jeff J.

>> - add a new fatal_error():
>>
>> There should be a new error mechanism that throws a QUIT instead of 
>> ERROR.  Code should not normally be catching QUIT (much unfortunately 
>> does).  A fatal error is things like: syntax error; no target; lost 
>> target.  Things like a memory access violation though are not fatal.
>>
>> - think about stopping code catching and then discarding QUIT
>> Grep for RETURN_MASK_ALL in the sources - it should be 
>> RETURN_MASK_ERROR :-/
>>
>> - modify the backtrace code to catch ERROR, but not QUIT
>>
>> Along the lines of Joel's suggestion, the backtrace command should 
>> catch ERROR (but should not catch QUIT).  That way simple problems 
>> don't abort the script but fatal ones do.
>>
>>

Re: [RFA]: issue warnings for frame offenses

[Jeff Johnston]

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

Andrew,

   The following updated patch implements your suggestion of having a new error 
routine that throw RETURN_QUIT.  The backtrace command is now protected with a 
catch_errors call.  I did not change other existing calls to catch_errors to use 
RETURN_MASK_ERROR instead of RETURN_MASK_ALL.

   Ok or is there something I have missed?

-- Jeff J.

2004-11-04  Jeff Johnston  <jjohnstn@redhat.com>

         * defs.h (fatal_error, vfatal_error): New function prototypes.
         * stack.c (backtrace_command_stub): Stub to call backtrace_command_1
         via catch_errors.
         (backtrace_command): Change to call backtrace_command_stub via
         catch_errors instead of calling backtrace_command_1 directly.
         (backtrace_full_command): Ditto.
         * utils.c (error_stream_1): New static function.
         (verror): Change to call error_stream_1 instead of error_stream.
         (error_stream): Call error_stream_1 with RETURN_ERROR argument.
         (vfatal_error, fatal_error): New functions.


[-- Attachment #2: backtrace.patch --]
[-- Type: text/plain, Size: 5003 bytes --]

Index: defs.h
===================================================================
RCS file: /cvs/src/src/gdb/defs.h,v
retrieving revision 1.170
diff -u -p -r1.170 defs.h
--- defs.h	12 Oct 2004 10:06:14 -0000	1.170
+++ defs.h	4 Nov 2004 22:55:02 -0000
@@ -911,6 +911,10 @@ extern char *error_last_message (void);
 /* Output arbitrary error message.  */
 extern void error_output_message (char *pre_print, char *msg);
 
+extern NORETURN void vfatal_error (const char *fmt, va_list ap) ATTR_NORETURN;
+
+extern NORETURN void fatal_error (const char *fmt, ...) ATTR_NORETURN ATTR_FORMAT (printf, 1, 2);
+
 extern NORETURN void internal_verror (const char *file, int line,
 				      const char *, va_list ap) ATTR_NORETURN;
 
Index: stack.c
===================================================================
RCS file: /cvs/src/src/gdb/stack.c,v
retrieving revision 1.115
diff -u -p -r1.115 stack.c
--- stack.c	30 Oct 2004 21:16:10 -0000	1.115
+++ stack.c	4 Nov 2004 22:55:03 -0000
@@ -1204,6 +1204,22 @@ backtrace_command_1 (char *count_exp, in
     printf_filtered ("(More stack frames follow...)\n");
 }
 
+struct backtrace_command_args
+  {
+    char *count_exp;
+    int show_locals;
+    int from_tty;
+  };
+
+/* Stub to call backtrace_command_1 by way of an error catcher.  */
+static int
+backtrace_command_stub (void *data)
+{
+  struct backtrace_command_args *args = (struct backtrace_command_args *)data;
+  backtrace_command_1 (args->count_exp, args->show_locals, args->from_tty);
+  return 0;
+}
+
 static void
 backtrace_command (char *arg, int from_tty)
 {
@@ -1211,6 +1227,7 @@ backtrace_command (char *arg, int from_t
   char **argv = (char **) NULL;
   int argIndicatingFullTrace = (-1), totArgLen = 0, argc = 0;
   char *argPtr = arg;
+  struct backtrace_command_args btargs;
 
   if (arg != (char *) NULL)
     {
@@ -1260,7 +1277,10 @@ backtrace_command (char *arg, int from_t
 	}
     }
 
-  backtrace_command_1 (argPtr, (argIndicatingFullTrace >= 0), from_tty);
+  btargs.count_exp = argPtr;
+  btargs.show_locals = (argIndicatingFullTrace >= 0);
+  btargs.from_tty = from_tty;
+  catch_errors (backtrace_command_stub, (char *)&btargs, "", RETURN_MASK_ERROR);
 
   if (argIndicatingFullTrace >= 0 && totArgLen > 0)
     xfree (argPtr);
@@ -1273,7 +1293,11 @@ static void backtrace_full_command (char
 static void
 backtrace_full_command (char *arg, int from_tty)
 {
-  backtrace_command_1 (arg, 1, from_tty);
+  struct backtrace_command_args btargs;
+  btargs.count_exp = arg;
+  btargs.show_locals = 1;
+  btargs.from_tty = from_tty;
+  catch_errors (backtrace_command_stub, (char *)&btargs, "", RETURN_MASK_ERROR);
 }
 \f
 
Index: utils.c
===================================================================
RCS file: /cvs/src/src/gdb/utils.c,v
retrieving revision 1.137
diff -u -p -r1.137 utils.c
--- utils.c	30 Sep 2004 19:57:54 -0000	1.137
+++ utils.c	4 Nov 2004 22:55:03 -0000
@@ -104,6 +104,9 @@ static void prompt_for_continue (void);
 static void set_screen_size (void);
 static void set_width (void);
 
+static NORETURN void error_stream_1 (struct ui_file *stream, 
+				     enum return_reason reason) ATTR_NORETURN;
+
 /* Chain of cleanup actions established with make_cleanup,
    to be executed if an error happens.  */
 
@@ -620,7 +623,7 @@ verror (const char *string, va_list args
   struct ui_file *tmp_stream = mem_fileopen ();
   make_cleanup_ui_file_delete (tmp_stream);
   vfprintf_unfiltered (tmp_stream, string, args);
-  error_stream (tmp_stream);
+  error_stream_1 (tmp_stream, RETURN_ERROR);
 }
 
 NORETURN void
@@ -632,6 +635,28 @@ error (const char *string, ...)
   va_end (args);
 }
 
+/* Print an error message and quit.
+   The first argument STRING is the error message, used as a fprintf string,
+   and the remaining args are passed as arguments to it.  */
+
+NORETURN void
+vfatal_error (const char *string, va_list args)
+{
+  struct ui_file *tmp_stream = mem_fileopen ();
+  make_cleanup_ui_file_delete (tmp_stream);
+  vfprintf_unfiltered (tmp_stream, string, args);
+  error_stream_1 (tmp_stream, RETURN_QUIT);
+}
+
+NORETURN void
+fatal_error (const char *string, ...)
+{
+  va_list args;
+  va_start (args, string);
+  vfatal_error (string, args);
+  va_end (args);
+}
+
 static void
 do_write (void *data, const char *buffer, long length_buffer)
 {
@@ -670,8 +695,8 @@ error_output_message (char *pre_print, c
   fprintf_filtered (gdb_stderr, "\n");
 }
 
-NORETURN void
-error_stream (struct ui_file *stream)
+static NORETURN void
+error_stream_1 (struct ui_file *stream, enum return_reason reason)
 {
   if (deprecated_error_begin_hook)
     deprecated_error_begin_hook ();
@@ -690,7 +715,13 @@ error_stream (struct ui_file *stream)
   ui_file_put (stream, do_write, gdb_stderr);
   fprintf_filtered (gdb_stderr, "\n");
 
-  throw_exception (RETURN_ERROR);
+  throw_exception (reason);
+}
+
+NORETURN void
+error_stream (struct ui_file *stream)
+{
+  error_stream_1 (stream, RETURN_ERROR);
 }
 
 /* Get the last error message issued by gdb */

Re: [RFA]: issue warnings for frame offenses

[Andrew Cagney]

Jeff Johnston wrote:
> Andrew,
> 
>   The following updated patch implements your suggestion of having a new 
> error routine that throw RETURN_QUIT.  The backtrace command is now 
> protected with a catch_errors call.  I did not change other existing 
> calls to catch_errors to use RETURN_MASK_ERROR instead of RETURN_MASK_ALL.
> 
>   Ok or is there something I have missed?

No.

With regard to s/RETURN_MASK_ALL/RETURN_MASK_ERROR/ that while more 
correct gives me the willies, so yes we can worry about that later.

I need to think about s/RETURN_QUIT/RETURN_FATAL/.

I'm just trying to decide which of:
	fatal_error OR fatal
and
	vfatal_error, fatal_verror, and vfatal
are better.  Having seen the code, I kind of prefer the shorter "fatal" 
and "vfatal".  Ok with that.

This should be mentioned in NEWS, I guess with "thread apply all bt" now 
"works".

thanks,
Andrew

> -- Jeff J.
> 
> 2004-11-04  Jeff Johnston  <jjohnstn@redhat.com>
> 
>         * defs.h (fatal_error, vfatal_error): New function prototypes.
>         * stack.c (backtrace_command_stub): Stub to call 
> backtrace_command_1
>         via catch_errors.
>         (backtrace_command): Change to call backtrace_command_stub via
>         catch_errors instead of calling backtrace_command_1 directly.
>         (backtrace_full_command): Ditto.
>         * utils.c (error_stream_1): New static function.
>         (verror): Change to call error_stream_1 instead of error_stream.
>         (error_stream): Call error_stream_1 with RETURN_ERROR argument.
>         (vfatal_error, fatal_error): New functions.
> 
> 
> ------------------------------------------------------------------------
> 
> Index: defs.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/defs.h,v
> retrieving revision 1.170
> diff -u -p -r1.170 defs.h
> --- defs.h	12 Oct 2004 10:06:14 -0000	1.170
> +++ defs.h	4 Nov 2004 22:55:02 -0000
> @@ -911,6 +911,10 @@ extern char *error_last_message (void);
>  /* Output arbitrary error message.  */
>  extern void error_output_message (char *pre_print, char *msg);
>  
> +extern NORETURN void vfatal_error (const char *fmt, va_list ap) ATTR_NORETURN;
> +
> +extern NORETURN void fatal_error (const char *fmt, ...) ATTR_NORETURN ATTR_FORMAT (printf, 1, 2);
> +
>  extern NORETURN void internal_verror (const char *file, int line,
>  				      const char *, va_list ap) ATTR_NORETURN;
>  
> Index: stack.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/stack.c,v
> retrieving revision 1.115
> diff -u -p -r1.115 stack.c
> --- stack.c	30 Oct 2004 21:16:10 -0000	1.115
> +++ stack.c	4 Nov 2004 22:55:03 -0000
> @@ -1204,6 +1204,22 @@ backtrace_command_1 (char *count_exp, in
>      printf_filtered ("(More stack frames follow...)\n");
>  }
>  
> +struct backtrace_command_args
> +  {
> +    char *count_exp;
> +    int show_locals;
> +    int from_tty;
> +  };
> +
> +/* Stub to call backtrace_command_1 by way of an error catcher.  */
> +static int
> +backtrace_command_stub (void *data)
> +{
> +  struct backtrace_command_args *args = (struct backtrace_command_args *)data;
> +  backtrace_command_1 (args->count_exp, args->show_locals, args->from_tty);
> +  return 0;
> +}
> +
>  static void
>  backtrace_command (char *arg, int from_tty)
>  {
> @@ -1211,6 +1227,7 @@ backtrace_command (char *arg, int from_t
>    char **argv = (char **) NULL;
>    int argIndicatingFullTrace = (-1), totArgLen = 0, argc = 0;
>    char *argPtr = arg;
> +  struct backtrace_command_args btargs;
>  
>    if (arg != (char *) NULL)
>      {
> @@ -1260,7 +1277,10 @@ backtrace_command (char *arg, int from_t
>  	}
>      }
>  
> -  backtrace_command_1 (argPtr, (argIndicatingFullTrace >= 0), from_tty);
> +  btargs.count_exp = argPtr;
> +  btargs.show_locals = (argIndicatingFullTrace >= 0);
> +  btargs.from_tty = from_tty;
> +  catch_errors (backtrace_command_stub, (char *)&btargs, "", RETURN_MASK_ERROR);
>  
>    if (argIndicatingFullTrace >= 0 && totArgLen > 0)
>      xfree (argPtr);
> @@ -1273,7 +1293,11 @@ static void backtrace_full_command (char
>  static void
>  backtrace_full_command (char *arg, int from_tty)
>  {
> -  backtrace_command_1 (arg, 1, from_tty);
> +  struct backtrace_command_args btargs;
> +  btargs.count_exp = arg;
> +  btargs.show_locals = 1;
> +  btargs.from_tty = from_tty;
> +  catch_errors (backtrace_command_stub, (char *)&btargs, "", RETURN_MASK_ERROR);
>  }
>  \f
>  
> Index: utils.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/utils.c,v
> retrieving revision 1.137
> diff -u -p -r1.137 utils.c
> --- utils.c	30 Sep 2004 19:57:54 -0000	1.137
> +++ utils.c	4 Nov 2004 22:55:03 -0000
> @@ -104,6 +104,9 @@ static void prompt_for_continue (void);
>  static void set_screen_size (void);
>  static void set_width (void);
>  
> +static NORETURN void error_stream_1 (struct ui_file *stream, 
> +				     enum return_reason reason) ATTR_NORETURN;
> +
>  /* Chain of cleanup actions established with make_cleanup,
>     to be executed if an error happens.  */
>  
> @@ -620,7 +623,7 @@ verror (const char *string, va_list args
>    struct ui_file *tmp_stream = mem_fileopen ();
>    make_cleanup_ui_file_delete (tmp_stream);
>    vfprintf_unfiltered (tmp_stream, string, args);
> -  error_stream (tmp_stream);
> +  error_stream_1 (tmp_stream, RETURN_ERROR);
>  }
>  
>  NORETURN void
> @@ -632,6 +635,28 @@ error (const char *string, ...)
>    va_end (args);
>  }
>  
> +/* Print an error message and quit.
> +   The first argument STRING is the error message, used as a fprintf string,
> +   and the remaining args are passed as arguments to it.  */
> +
> +NORETURN void
> +vfatal_error (const char *string, va_list args)
> +{
> +  struct ui_file *tmp_stream = mem_fileopen ();
> +  make_cleanup_ui_file_delete (tmp_stream);
> +  vfprintf_unfiltered (tmp_stream, string, args);
> +  error_stream_1 (tmp_stream, RETURN_QUIT);
> +}
> +
> +NORETURN void
> +fatal_error (const char *string, ...)
> +{
> +  va_list args;
> +  va_start (args, string);
> +  vfatal_error (string, args);
> +  va_end (args);
> +}
> +
>  static void
>  do_write (void *data, const char *buffer, long length_buffer)
>  {
> @@ -670,8 +695,8 @@ error_output_message (char *pre_print, c
>    fprintf_filtered (gdb_stderr, "\n");
>  }
>  
> -NORETURN void
> -error_stream (struct ui_file *stream)
> +static NORETURN void
> +error_stream_1 (struct ui_file *stream, enum return_reason reason)
>  {
>    if (deprecated_error_begin_hook)
>      deprecated_error_begin_hook ();
> @@ -690,7 +715,13 @@ error_stream (struct ui_file *stream)
>    ui_file_put (stream, do_write, gdb_stderr);
>    fprintf_filtered (gdb_stderr, "\n");
>  
> -  throw_exception (RETURN_ERROR);
> +  throw_exception (reason);
> +}
> +
> +NORETURN void
> +error_stream (struct ui_file *stream)
> +{
> +  error_stream_1 (stream, RETURN_ERROR);
>  }
>  
>  /* Get the last error message issued by gdb */

Re: [RFA]: issue warnings for frame offenses

[Jeff Johnston]

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

Andrew Cagney wrote:
> Jeff Johnston wrote:
> 
>> Andrew,
>>
>>   The following updated patch implements your suggestion of having a 
>> new error routine that throw RETURN_QUIT.  The backtrace command is 
>> now protected with a catch_errors call.  I did not change other 
>> existing calls to catch_errors to use RETURN_MASK_ERROR instead of 
>> RETURN_MASK_ALL.
>>
>>   Ok or is there something I have missed?
> 
> 
> No.
> 
> With regard to s/RETURN_MASK_ALL/RETURN_MASK_ERROR/ that while more 
> correct gives me the willies, so yes we can worry about that later.
> 
> I need to think about s/RETURN_QUIT/RETURN_FATAL/.
> 
> I'm just trying to decide which of:
>     fatal_error OR fatal
> and
>     vfatal_error, fatal_verror, and vfatal
> are better.  Having seen the code, I kind of prefer the shorter "fatal" 
> and "vfatal".  Ok with that.
>

Ok, change made and attached patch has been committed.

2004-11-05  Jeff Johnston  <jjohnstn@redhat.com>

         * defs.h (fatal, vfatal): New function prototypes.
         * stack.c (backtrace_command_stub): Stub to call backtrace_command_1
         via catch_errors.
         (backtrace_command): Change to call backtrace_command_stub via
         catch_errors instead of calling backtrace_command_1 directly.
         (backtrace_full_command): Ditto.
         * utils.c (error_stream_1): New static function.
         (verror): Change to call error_stream_1 instead of error_stream.
         (error_stream): Call error_stream_1 with RETURN_ERROR argument.
         (vfatal, fatal): New functions.


> This should be mentioned in NEWS, I guess with "thread apply all bt" now 
> "works".
>

I'll post a change for this as well as a test case I have.

-- Jeff J.


[-- Attachment #2: backtrace.patch2 --]
[-- Type: text/plain, Size: 4972 bytes --]

Index: defs.h
===================================================================
RCS file: /cvs/src/src/gdb/defs.h,v
retrieving revision 1.170
diff -u -p -r1.170 defs.h
--- defs.h	12 Oct 2004 10:06:14 -0000	1.170
+++ defs.h	5 Nov 2004 20:29:57 -0000
@@ -911,6 +911,10 @@ extern char *error_last_message (void);
 /* Output arbitrary error message.  */
 extern void error_output_message (char *pre_print, char *msg);
 
+extern NORETURN void vfatal (const char *fmt, va_list ap) ATTR_NORETURN;
+
+extern NORETURN void fatal (const char *fmt, ...) ATTR_NORETURN ATTR_FORMAT (printf, 1, 2);
+
 extern NORETURN void internal_verror (const char *file, int line,
 				      const char *, va_list ap) ATTR_NORETURN;
 
Index: stack.c
===================================================================
RCS file: /cvs/src/src/gdb/stack.c,v
retrieving revision 1.117
diff -u -p -r1.117 stack.c
--- stack.c	5 Nov 2004 18:58:29 -0000	1.117
+++ stack.c	5 Nov 2004 20:29:57 -0000
@@ -1218,6 +1218,22 @@ backtrace_command_1 (char *count_exp, in
     printf_filtered ("(More stack frames follow...)\n");
 }
 
+struct backtrace_command_args
+  {
+    char *count_exp;
+    int show_locals;
+    int from_tty;
+  };
+
+/* Stub to call backtrace_command_1 by way of an error catcher.  */
+static int
+backtrace_command_stub (void *data)
+{
+  struct backtrace_command_args *args = (struct backtrace_command_args *)data;
+  backtrace_command_1 (args->count_exp, args->show_locals, args->from_tty);
+  return 0;
+}
+
 static void
 backtrace_command (char *arg, int from_tty)
 {
@@ -1225,6 +1241,7 @@ backtrace_command (char *arg, int from_t
   char **argv = (char **) NULL;
   int argIndicatingFullTrace = (-1), totArgLen = 0, argc = 0;
   char *argPtr = arg;
+  struct backtrace_command_args btargs;
 
   if (arg != (char *) NULL)
     {
@@ -1274,7 +1291,10 @@ backtrace_command (char *arg, int from_t
 	}
     }
 
-  backtrace_command_1 (argPtr, (argIndicatingFullTrace >= 0), from_tty);
+  btargs.count_exp = argPtr;
+  btargs.show_locals = (argIndicatingFullTrace >= 0);
+  btargs.from_tty = from_tty;
+  catch_errors (backtrace_command_stub, (char *)&btargs, "", RETURN_MASK_ERROR);
 
   if (argIndicatingFullTrace >= 0 && totArgLen > 0)
     xfree (argPtr);
@@ -1287,7 +1307,11 @@ static void backtrace_full_command (char
 static void
 backtrace_full_command (char *arg, int from_tty)
 {
-  backtrace_command_1 (arg, 1, from_tty);
+  struct backtrace_command_args btargs;
+  btargs.count_exp = arg;
+  btargs.show_locals = 1;
+  btargs.from_tty = from_tty;
+  catch_errors (backtrace_command_stub, (char *)&btargs, "", RETURN_MASK_ERROR);
 }
 \f
 
Index: utils.c
===================================================================
RCS file: /cvs/src/src/gdb/utils.c,v
retrieving revision 1.137
diff -u -p -r1.137 utils.c
--- utils.c	30 Sep 2004 19:57:54 -0000	1.137
+++ utils.c	5 Nov 2004 20:29:58 -0000
@@ -104,6 +104,9 @@ static void prompt_for_continue (void);
 static void set_screen_size (void);
 static void set_width (void);
 
+static NORETURN void error_stream_1 (struct ui_file *stream, 
+				     enum return_reason reason) ATTR_NORETURN;
+
 /* Chain of cleanup actions established with make_cleanup,
    to be executed if an error happens.  */
 
@@ -620,7 +623,7 @@ verror (const char *string, va_list args
   struct ui_file *tmp_stream = mem_fileopen ();
   make_cleanup_ui_file_delete (tmp_stream);
   vfprintf_unfiltered (tmp_stream, string, args);
-  error_stream (tmp_stream);
+  error_stream_1 (tmp_stream, RETURN_ERROR);
 }
 
 NORETURN void
@@ -632,6 +635,28 @@ error (const char *string, ...)
   va_end (args);
 }
 
+/* Print an error message and quit.
+   The first argument STRING is the error message, used as a fprintf string,
+   and the remaining args are passed as arguments to it.  */
+
+NORETURN void
+vfatal (const char *string, va_list args)
+{
+  struct ui_file *tmp_stream = mem_fileopen ();
+  make_cleanup_ui_file_delete (tmp_stream);
+  vfprintf_unfiltered (tmp_stream, string, args);
+  error_stream_1 (tmp_stream, RETURN_QUIT);
+}
+
+NORETURN void
+fatal (const char *string, ...)
+{
+  va_list args;
+  va_start (args, string);
+  vfatal (string, args);
+  va_end (args);
+}
+
 static void
 do_write (void *data, const char *buffer, long length_buffer)
 {
@@ -670,8 +695,8 @@ error_output_message (char *pre_print, c
   fprintf_filtered (gdb_stderr, "\n");
 }
 
-NORETURN void
-error_stream (struct ui_file *stream)
+static NORETURN void
+error_stream_1 (struct ui_file *stream, enum return_reason reason)
 {
   if (deprecated_error_begin_hook)
     deprecated_error_begin_hook ();
@@ -690,7 +715,13 @@ error_stream (struct ui_file *stream)
   ui_file_put (stream, do_write, gdb_stderr);
   fprintf_filtered (gdb_stderr, "\n");
 
-  throw_exception (RETURN_ERROR);
+  throw_exception (reason);
+}
+
+NORETURN void
+error_stream (struct ui_file *stream)
+{
+  error_stream_1 (stream, RETURN_ERROR);
 }
 
 /* Get the last error message issued by gdb */