diff mbox

[GAS] intercept signal crashes

Message ID eaa8de8e-fe49-1057-db0a-c1df12cd2b1c@acm.org
State New
Headers show

Commit Message

Nathan Sidwell Jan. 12, 2017, 4:41 p.m. UTC 
gas's fatal signal handling is not as nice as gcc's has become.  This 
patch is a move towards fixing that.

1) rather than have as_assert and as_abort fatal error functions with 
almost the same behaviour -- they differ only in their initial message. 
This patch kills 'as_assert' and rewords 'as_abort' to meet assert fail 
behaviour.  So now on a failed assert, instead of:
   Internal error!
   Assertion failure in FOO at BAZ:17.
   Please report this bug.
we get
   Internal error in FOO at BAZ:17.
   Please report this bug.

That's now the same message for any explicit call to 'abort'.

2) Added a signal handler to catch SEGV, ILL, BUS, ABRT, IOT and FPE 
(when they exist), and forward it to as_abort using a NULL FILE, so we 
get a message such as:
   Internal error (Segmentation fault).
   Please report this bug.
or in the worst case:
   Internal error (unknown).
   Please report this bug.

I tested that on an x86_64-linux system by using gdb to inject a signal 
into gas.

Most of that handling code is stolen from gcc (toplev.c, diagnostic.c 
and system.h)

ok?

nathan
-- 
Nathan Sidwell

Comments

Alan Modra Jan. 13, 2017, 11:31 a.m. UTC | #1 
On Thu, Jan 12, 2017 at 11:41:58AM -0500, Nathan Sidwell wrote:
> 	* as.h (gas_assert): Use abort.

> 	(as_assert): Remove.

> 	(signal_init): Declare.

> 	* as.c (main): Call signal_init.

> 	* messages.c: #include <signal.h>

> 	(as_assert): Delete.

> 	(as_abort): Allow NULL FILE.

> 	(signal_crash): New.

> 	(signal_init): Register fatal signal handlers.

> 	* configure.ac: Check for strsignal.

> 	* config.in: Rebuilt.

> 	* configure: Rebuilt.


I like it!

-- 
Alan Modra
Australia Development Lab, IBM
diff mbox

Patch

2017-01-12  Nathan Sidwell  <nathan@acm.org>

	* as.h (gas_assert): Use abort.
	(as_assert): Remove.
	(signal_init): Declare.
	* as.c (main): Call signal_init.
	* messages.c: #include <signal.h>
	(as_assert): Delete.
	(as_abort): Allow NULL FILE.
	(signal_crash): New.
	(signal_init): Register fatal signal handlers.
	* configure.ac: Check for strsignal.
	* config.in: Rebuilt.
	* configure: Rebuilt.

diff --git a/gas/as.c b/gas/as.c
index 6c55e76..83a572b 100644
--- a/gas/as.c
+++ b/gas/as.c
@@ -1186,6 +1186,7 @@  main (int argc, char ** argv)
   int macro_strip_at;
 
   start_time = get_run_time ();
+  signal_init ();
 #ifdef HAVE_SBRK
   start_sbrk = (char *) sbrk (0);
 #endif
diff --git a/gas/as.h b/gas/as.h
index 76aa9ac..fee7c75 100644
--- a/gas/as.h
+++ b/gas/as.h
@@ -85,8 +85,7 @@ 
 #if __GNUC__ < 2 || (__GNUC__ == 2 && __GNUC_MINOR__ < 6)
 #define __PRETTY_FUNCTION__  ((char *) NULL)
 #endif
-#define gas_assert(P) \
-  ((void) ((P) ? 0 : (as_assert (__FILE__, __LINE__, __PRETTY_FUNCTION__), 0)))
+#define gas_assert(P)	((void) ((P) ? 0 : (abort (), 0)))
 #undef abort
 #define abort()		as_abort (__FILE__, __LINE__, __PRETTY_FUNCTION__)
 
@@ -459,8 +458,8 @@  PRINTF_LIKE (as_warn);
 PRINTF_WHERE_LIKE (as_bad_where);
 PRINTF_WHERE_LIKE (as_warn_where);
 
-void   as_assert (const char *, int, const char *) ATTRIBUTE_NORETURN;
 void   as_abort (const char *, int, const char *) ATTRIBUTE_NORETURN;
+void   signal_init (void);
 void   sprint_value (char *, addressT);
 int    had_errors (void);
 int    had_warnings (void);
diff --git a/gas/config.in b/gas/config.in
index 5129c28..0855179 100644
--- a/gas/config.in
+++ b/gas/config.in
@@ -144,6 +144,9 @@ 
 /* Define to 1 if you have the <string.h> header file. */
 #undef HAVE_STRING_H
 
+/* Define to 1 if you have the `strsignal' function. */
+#undef HAVE_STRSIGNAL
+
 /* Define if <sys/stat.h> has struct stat.st_mtim.tv_nsec */
 #undef HAVE_ST_MTIM_TV_NSEC
 
diff --git a/gas/configure b/gas/configure
index 2b54054..d3ae96e 100755
--- a/gas/configure
+++ b/gas/configure
@@ -13899,6 +13899,17 @@  _ACEOF
 fi
 done
 
+for ac_func in strsignal
+do :
+  ac_fn_c_check_func "$LINENO" "strsignal" "ac_cv_func_strsignal"
+if test "x$ac_cv_func_strsignal" = x""yes; then :
+  cat >>confdefs.h <<_ACEOF
+#define HAVE_STRSIGNAL 1
+_ACEOF
+
+fi
+done
+
 
 
   { $as_echo "$as_me:${as_lineno-$LINENO}: checking for LC_MESSAGES" >&5
diff --git a/gas/configure.ac b/gas/configure.ac
index c4c3036..cc70aa7 100644
--- a/gas/configure.ac
+++ b/gas/configure.ac
@@ -826,6 +826,7 @@  AC_C_INLINE
 # VMS doesn't have unlink.
 AC_CHECK_FUNCS(unlink remove, break)
 AC_CHECK_FUNCS(sbrk setlocale)
+AC_CHECK_FUNCS(strsignal)
 
 AM_LC_MESSAGES
 
diff --git a/gas/messages.c b/gas/messages.c
index f452f22..57d4ed7 100644
--- a/gas/messages.c
+++ b/gas/messages.c
@@ -18,11 +18,19 @@ 
    02110-1301, USA.  */
 
 #include "as.h"
+#include <signal.h>
+
+/* If the system doesn't provide strsignal, we get it defined in
+   libiberty but no declaration is supplied.  Because, reasons. */
+#if !defined (HAVE_STRSIGNAL) && !defined (strsignal)
+extern const char *strsignal (int);
+#endif
 
 static void identify (const char *);
 static void as_show_where (void);
 static void as_warn_internal (const char *, unsigned int, char *);
 static void as_bad_internal (const char *, unsigned int, char *);
+static void signal_crash (int) ATTRIBUTE_NORETURN;
 
 /* Despite the rest of the comments in this file, (FIXME-SOON),
    here is the current scheme for error messages etc:
@@ -58,7 +66,10 @@  static void as_bad_internal (const char *, unsigned int, char *);
    as_tsktsk() is used when we see a minor error for which
    our error recovery action is almost certainly correct.
    In this case, we print a message and then assembly
-   continues as though no error occurred.  */
+   continues as though no error occurred.
+
+   as_abort () is used for logic failure (assert or abort, signal).
+*/
 
 static void
 identify (const char *file)
@@ -286,38 +297,61 @@  as_fatal (const char *format, ...)
   xexit (EXIT_FAILURE);
 }
 
-/* Indicate assertion failure.
-   Arguments: Filename, line number, optional function name.  */
+/* Indicate internal constency error.
+   Arguments: Filename, line number, optional function name.
+   FILENAME may be NULL, which we use for crash-via-signal.  */
 
 void
-as_assert (const char *file, int line, const char *fn)
+as_abort (const char *file, int line, const char *fn)
 {
   as_show_where ();
-  fprintf (stderr, _("Internal error!\n"));
-  if (fn)
-    fprintf (stderr, _("Assertion failure in %s at %s:%d.\n"),
-	     fn, file, line);
+
+  if (!file)
+    fprintf (stderr, _("Internal error (%s).\n"), fn ? fn : "unknown");
+  else if (fn)
+    fprintf (stderr, _("Internal error in %s at %s:%d.\n"), fn, file, line);
   else
-    fprintf (stderr, _("Assertion failure at %s:%d.\n"), file, line);
+    fprintf (stderr, _("Internal error at %s:%d.\n"), file, line);
+
   fprintf (stderr, _("Please report this bug.\n"));
+
   xexit (EXIT_FAILURE);
 }
 
-/* as_abort: Print a friendly message saying how totally hosed we are,
-   and exit without producing a core file.  */
+/* Handler for fatal signals, such as SIGSEGV. */
+
+static void
+signal_crash (int signo)
+{
+  /* Reset, to prevent unbounded recursion.  */
+  signal (signo, SIG_DFL);
+
+  as_abort (NULL, 0, strsignal (signo));
+}
+
+/* Register signal handlers, for less abrubt crashes.  */
 
 void
-as_abort (const char *file, int line, const char *fn)
+signal_init (void)
 {
-  as_show_where ();
-  if (fn)
-    fprintf (stderr, _("Internal error, aborting at %s:%d in %s\n"),
-	     file, line, fn);
-  else
-    fprintf (stderr, _("Internal error, aborting at %s:%d\n"),
-	     file, line);
-  fprintf (stderr, _("Please report this bug.\n"));
-  xexit (EXIT_FAILURE);
+#ifdef SIGSEGV
+  signal (SIGSEGV, signal_crash);
+#endif
+#ifdef SIGILL
+  signal (SIGILL, signal_crash);
+#endif
+#ifdef SIGBUS
+  signal (SIGBUS, signal_crash);
+#endif
+#ifdef SIGABRT
+  signal (SIGABRT, signal_crash);
+#endif
+#if defined SIGIOT && (!defined SIGABRT || SIGABRT != SIGIOT)
+  signal (SIGIOT, signal_crash);
+#endif
+#ifdef SIGFPE
+  signal (SIGFPE, signal_crash);
+#endif
 }
 
 /* Support routines.  */