Re: [PATCH] Make GDBserver abort on internal error in development mode

[Andrew Burgess <aburgess@redhat.com>]

Pedro Alves <pedro@palves.net> writes:

> Currently, if GDBserver hits some internal assertion, it exits with
> error status, instead of aborting.  This makes it harder to debug
> GDBserver, as you can't just debug a core file if GDBserver fails an
> assertion.  I've had to hack the code to make GDBserver abort to debug
> something several times before.
>
> I believe the reason it exits instead of aborting, is to prevent
> potentially littering the filesystem of smaller embedded targets with
> core files.  I think I recall Daniel Jacobowitz once saying that many
> years ago, but I can't be sure.  Anyhow, that seems reasonable to me.
>
> Since we nowadays have a distinction between development and release
> modes, I propose to make GDBserver abort on internal error is in
> development mode, while keeping the status quo when in release mode.
>
> Thus, after this patch, in development mode, you get:
>
>  $ ../gdbserver/gdbserver
>  ../../src/gdbserver/server.cc:3711: A problem internal to GDBserver has been detected.
>  captured_main: Assertion `0' failed.
>  Aborted (core dumped)
>  $
>
> while in release mode, you'll continue to get:
>
>  $ ../gdbserver/gdbserver
>  ../../src/gdbserver/server.cc:3711: A problem internal to GDBserver has been detected.
>  captured_main: Assertion `0' failed.
>  $ echo $?
>  1

LGTM.

Thanks,
Andrew


>
> I do not think that this requires a separate configure switch.
>
> A "--target_board=native-extended-gdbserver" run on Ubuntu 20.04 ends
> up with:
>
> 		 === gdb Summary ===
>
>  # of unexpected core files      29
>  ...
>
> for me, of which 8 are GDBserver core dumps, 7 more than without this
> patch.
>
> Change-Id: I6861e08ad71f65a0332c91ec95ca001d130b0e9d
> ---
>  gdbserver/utils.cc      | 20 +++++++++++++++++---
>  gdbsupport/config.in    |  3 +++
>  gdbsupport/configure    | 13 +++++++++++++
>  gdbsupport/configure.ac | 10 ++++++++++
>  4 files changed, 43 insertions(+), 3 deletions(-)
>
> diff --git a/gdbserver/utils.cc b/gdbserver/utils.cc
> index 4f6516c9cf2..d24057c6012 100644
> --- a/gdbserver/utils.cc
> +++ b/gdbserver/utils.cc
> @@ -28,13 +28,27 @@
>  
>  /* Generally useful subroutines used throughout the program.  */
>  
> +/* If in release mode, just exit.  This avoids potentially littering
> +   the filesystem of small embedded targets with core files.  If in
> +   development mode however, abort, producing core files to help with
> +   debugging GDBserver.  */
> +static void ATTRIBUTE_NORETURN
> +abort_or_exit ()
> +{
> +#ifdef DEVELOPMENT
> +  abort ();
> +#else
> +  exit (1);
> +#endif
> +}
> +
>  void
>  malloc_failure (long size)
>  {
>    fprintf (stderr,
>  	   PREFIX "ran out of memory while trying to allocate %lu bytes\n",
>  	   (unsigned long) size);
> -  exit (1);
> +  abort_or_exit ();
>  }
>  
>  /* Print the system error message for errno, and also mention STRING
> @@ -82,7 +96,7 @@ vwarning (const char *string, va_list args)
>    fprintf (stderr, "\n");
>  }
>  
> -/* Report a problem internal to GDBserver, and exit.  */
> +/* Report a problem internal to GDBserver, and abort/exit.  */
>  
>  void
>  internal_verror (const char *file, int line, const char *fmt, va_list args)
> @@ -91,7 +105,7 @@ internal_verror (const char *file, int line, const char *fmt, va_list args)
>  %s:%d: A problem internal to " TOOLNAME " has been detected.\n", file, line);
>    vfprintf (stderr, fmt, args);
>    fprintf (stderr, "\n");
> -  exit (1);
> +  abort_or_exit ();
>  }
>  
>  /* Report a problem internal to GDBserver.  */
> diff --git a/gdbsupport/config.in b/gdbsupport/config.in
> index a7ae23b4984..577866c97b3 100644
> --- a/gdbsupport/config.in
> +++ b/gdbsupport/config.in
> @@ -11,6 +11,9 @@
>  /* Define to 1 if using `alloca.c'. */
>  #undef C_ALLOCA
>  
> +/* Define if development-mode features are enabled. */
> +#undef DEVELOPMENT
> +
>  /* Define to 1 if translation of program messages to the user's native
>     language is requested. */
>  #undef ENABLE_NLS
> diff --git a/gdbsupport/configure b/gdbsupport/configure
> index 618f487749f..0b48521deb6 100755
> --- a/gdbsupport/configure
> +++ b/gdbsupport/configure
> @@ -624,6 +624,7 @@ ac_subst_vars='am__EXEEXT_FALSE
>  am__EXEEXT_TRUE
>  LTLIBOBJS
>  LIBOBJS
> +CONFIG_STATUS_DEPENDENCIES
>  WERROR_CFLAGS
>  WARN_CFLAGS
>  HAVE_PIPE_OR_PIPE2_FALSE
> @@ -10452,6 +10453,15 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu
>  
>  
>  
> +# Set the 'development' global.
> +. $srcdir/../bfd/development.sh
> +
> +if test "$development" = true ; then
> +
> +$as_echo "#define DEVELOPMENT 1" >>confdefs.h
> +
> +fi
> +
>  case ${host} in
>    *mingw32*)
>  
> @@ -10460,6 +10470,9 @@ $as_echo "#define USE_WIN32API 1" >>confdefs.h
>      ;;
>  esac
>  
> +CONFIG_STATUS_DEPENDENCIES='$srcdir/../bfd/development.sh'
> +
> +
>  ac_config_files="$ac_config_files Makefile"
>  
>  cat >confcache <<\_ACEOF
> diff --git a/gdbsupport/configure.ac b/gdbsupport/configure.ac
> index 1f794605f3c..ac2ade6a220 100644
> --- a/gdbsupport/configure.ac
> +++ b/gdbsupport/configure.ac
> @@ -63,6 +63,14 @@ GDB_AC_PTRACE
>  AM_GDB_COMPILER_TYPE
>  AM_GDB_WARNINGS
>  
> +# Set the 'development' global.
> +. $srcdir/../bfd/development.sh
> +
> +if test "$development" = true ; then
> +   AC_DEFINE(DEVELOPMENT, 1,
> +	     [Define if development-mode features are enabled.])
> +fi
> +
>  case ${host} in
>    *mingw32*)
>      AC_DEFINE(USE_WIN32API, 1,
> @@ -73,5 +81,7 @@ case ${host} in
>      ;;
>  esac
>  
> +AC_SUBST([CONFIG_STATUS_DEPENDENCIES], ['$srcdir/../bfd/development.sh'])
> +
>  AC_CONFIG_FILES([Makefile])
>  AC_OUTPUT
>
> base-commit: e83907ff5ffbac3d0224d31ee99e6dc056205f39
> -- 
> 2.36.0