Re: fatal() -> internal_error() jumbo patch

[Andrew Cagney <ac131313@cygnus.com>]

"J.T. Conklin" wrote:
> 
> >>>>> "Andrew" == Andrew Cagney <ac131313@cygnus.com> writes:
> Andrew> Hello, I'm looking for comments.  Mainly on the new function
> Andrew> internal_error().
> 
> It would have been a little easier had you split out that part of the
> patch.  It took a bit of time to find amidst the boilerplate changes.

Good point. I've attached the interesting bit.

> I think the idea is a good one.  Although, I must admit I've never
> seen fatal() getting tripped in quite some years.

Two examples:

The multi-arch code, when it detects a configuration botch, currently
triggers a fatal error and dumps the user out.  Not very friendly :-(

GDB likes to dump core when corrupt symbol table information is
detected.


> When internal_error() recurses three times, you write() to filedesc 3.
> Did you mean 2?  Also that case calls then abort(), which has already
> failed twice by that point.  Perhaps it should call _exit() or some
> such, otherwise it may never exit.

Good point, I'll fix that.

> Andrew> The new function asks the user before dumping core (quiting
> Andrew> GDB) this gives the user a fighting chance of still being able
> Andrew> to salvage something from a debug session.
> 
> IMO the question about dumping core and continuing should be separate.
> I can envision answering yes to dump core and no to quit.  That way I
> can dump the frotzed state of GDB (for later debugging), yet still try
> to recover useful info from my debug session.

Again, good point. Would you have a patch?

	Andrew
Index: utils.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/utils.c,v
retrieving revision 1.195
diff -p -r1.195 utils.c
*** utils.c	1999/08/08 01:25:48	1.195
--- utils.c	1999/08/08 01:55:43
*************** static void malloc_botch PARAMS ((void))
*** 66,74 ****
  #endif
  
  static void
- fatal_dump_core PARAMS ((char *,...));
- 
- static void
  prompt_for_continue PARAMS ((void));
  
  static void
--- 66,71 ----
*************** error (const char *string,...)
*** 492,535 ****
  }
  
  
! /* Print an error message and exit reporting failure.
!    This is for a error that we cannot continue from.
!    The arguments are printed a la printf.
  
-    This function cannot be declared volatile (NORETURN) in an
-    ANSI environment because exit() is not declared volatile. */
- 
  NORETURN void
! fatal (char *string,...)
  {
    va_list args;
-   va_start (args, string);
-   fprintf_unfiltered (gdb_stderr, "\ngdb: ");
-   vfprintf_unfiltered (gdb_stderr, string, args);
-   fprintf_unfiltered (gdb_stderr, "\n");
-   va_end (args);
-   exit (1);
- }
  
! /* Print an error message and exit, dumping core.
!    The arguments are printed a la printf ().  */
  
! static void
! fatal_dump_core (char *string,...)
! {
!   va_list args;
    va_start (args, string);
-   /* "internal error" is always correct, since GDB should never dump
-      core, no matter what the input.  */
-   fprintf_unfiltered (gdb_stderr, "\ngdb internal error: ");
    vfprintf_unfiltered (gdb_stderr, string, args);
-   fprintf_unfiltered (gdb_stderr, "\n");
    va_end (args);
  
!   signal (SIGQUIT, SIG_DFL);
!   kill (getpid (), SIGQUIT);
!   /* We should never get here, but just in case...  */
!   exit (1);
  }
  
  /* The strerror() function can return NULL for errno values that are
--- 489,534 ----
  }
  
  
! /* Print a message reporting an internal error. Ask the user if they
!    want to continue, dump core, or just exit. */
  
  NORETURN void
! internal_error (char *string, ...)
  {
+   static char msg[] = "Internal GDB error: recursive internal error.\n";
+   static int dejavu = 0;
    va_list args;
  
!   /* don't allow infinite error recursion. */
!   switch (dejavu)
!     {
!     case 0:
!       dejavu = 1;
!       break;
!     case 1:
!       dejavu = 2;
!       fputs_unfiltered (msg, gdb_stderr);
!       abort ();
!     default:
!       dejavu = 3;
!       write (3, msg, sizeof (msg));
!       exit (1);
!     }
  
!   /* Try to get the message out */
!   fputs_unfiltered ("\nGDB-INTERNAL-ERROR: ", gdb_stderr);
    va_start (args, string);
    vfprintf_unfiltered (gdb_stderr, string, args);
    va_end (args);
+   fputs_unfiltered ("\n", gdb_stderr);
+ 
+   if (query ("\
+ An internal GDB error has been detected.\n\
+ Do you want to quit GDB (dumping core)? "))
+     abort ();
  
!   dejavu = 0;
!   return_to_top_level (RETURN_ERROR);
  }
  
  /* The strerror() function can return NULL for errno values that are
*************** init_malloc (md)
*** 790,796 ****
  static void
  malloc_botch ()
  {
!   fatal_dump_core ("Memory corruption");
  }
  
  /* Attempt to install hooks in mmalloc/mrealloc/mfree for the heap specified
--- 789,796 ----
  static void
  malloc_botch ()
  {
!   fprintf_unfiltered (gdb_stderr, "Memory corruption\n");
!   abort ();
  }
  
  /* Attempt to install hooks in mmalloc/mrealloc/mfree for the heap specified
*************** nomem (size)
*** 841,851 ****
  {
    if (size > 0)
      {
!       fatal ("virtual memory exhausted: can't allocate %ld bytes.", size);
      }
    else
      {
!       fatal ("virtual memory exhausted.");
      }
  }
  
--- 841,851 ----
  {
    if (size > 0)
      {
!       internal_error ("virtual memory exhausted: can't allocate %ld bytes.", size);
      }
    else
      {
!       internal_error ("virtual memory exhausted.");
      }
  }