* [PATCH] Make GDBserver abort on internal error in development mode
@ 2022-06-24 13:26 Pedro Alves
2022-06-25 13:27 ` Simon Marchi
2022-06-27 12:06 ` Andrew Burgess
0 siblings, 2 replies; 3+ messages in thread
From: Pedro Alves @ 2022-06-24 13:26 UTC (permalink / raw)
To: gdb-patches
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
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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] Make GDBserver abort on internal error in development mode
2022-06-24 13:26 [PATCH] Make GDBserver abort on internal error in development mode Pedro Alves
@ 2022-06-25 13:27 ` Simon Marchi
2022-06-27 12:06 ` Andrew Burgess
1 sibling, 0 replies; 3+ messages in thread
From: Simon Marchi @ 2022-06-25 13:27 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
On 2022-06-24 09:26, Pedro Alves wrote:
> 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.
Thanks for mentioning this, I was going to ask "why don't we just abort
all the time". On non-embedded targets it could be useful to abort,
since it triggers the various distros error reporting tool (like ABRT on
Red Hat), and make reporting crashes easier. But the embedded target
case also makes sense. Your patch LGTM, it's a good change on its own.
If someone wants to add a --make-gdbserver-abort-on-internal-error
configure switch later (and convince distros to use it), I'd be ok with
that.
Simon
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Make GDBserver abort on internal error in development mode
2022-06-24 13:26 [PATCH] Make GDBserver abort on internal error in development mode Pedro Alves
2022-06-25 13:27 ` Simon Marchi
@ 2022-06-27 12:06 ` Andrew Burgess
1 sibling, 0 replies; 3+ messages in thread
From: Andrew Burgess @ 2022-06-27 12:06 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-06-27 12:06 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-24 13:26 [PATCH] Make GDBserver abort on internal error in development mode Pedro Alves
2022-06-25 13:27 ` Simon Marchi
2022-06-27 12:06 ` Andrew Burgess
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).