From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 6CF99386F0EB for ; Mon, 27 Jun 2022 12:06:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6CF99386F0EB Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-271-6cZbvcwSNj2DPknNEKgCcg-1; Mon, 27 Jun 2022 08:06:44 -0400 X-MC-Unique: 6cZbvcwSNj2DPknNEKgCcg-1 Received: by mail-wr1-f71.google.com with SMTP id v8-20020adfa1c8000000b0021b81a553fbso1140330wrv.18 for ; Mon, 27 Jun 2022 05:06:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:subject:in-reply-to:references:date :message-id:mime-version; bh=Q3I98UIjMViob6piy/T/SEXGrAPACqmZhkUwEN6RVrM=; b=BL7OEawLjZYHhE2FshBnIuuUR6MYm7zTyNjoF7bFSqHy8RHjZgTPoS/yomOK+R06g4 17gte2++p4SJVPU/PUF2FT7+1m4wqVhvP02Dh6z+jog2w6kuHfhbyWTzxdsTChrj1ATa GDsN0vdH1rpZwd7X2XCTFQuRqjcCkqZ5zv4WGi7iPb8YSgckDVB6XLOej/x+wO91H2DC gnipE8GXKpx8eXb0h8C4A3vhNqFp8TrCmEhHCJyWAbIy4HO8TwUs0IqWzR/KRA9j3A7G I0tu/LYfCAqmfO+CChMVIUFIdAWGKkLPdoLZn7mIqqbOb7fB2p5ritGSTsVPpAlOCTvn SG5Q== X-Gm-Message-State: AJIora8DipQrsbUQloJm3ZtqxiTkFKBJb/uWfo6cjbX3Pg3/KAdMebb+ JeIcvsVp9g9TJEGhO+J9uDo7Z4uVswMbmY4uxQvQ/55D6urD+DH/JLzo4By0D5OnaTDNzg/XYP1 USbKlDPDCUy3+QXtuKRMPQw== X-Received: by 2002:a5d:6d86:0:b0:21b:ce00:4163 with SMTP id l6-20020a5d6d86000000b0021bce004163mr5237295wrs.511.1656331603119; Mon, 27 Jun 2022 05:06:43 -0700 (PDT) X-Google-Smtp-Source: AGRyM1t3Ik3fNEe2urBmg6F9DGxcmXexdZFBvfGWoMD5Cfe/nWxvS16Zyr6LrFNn54eeXTIElBEoXQ== X-Received: by 2002:a5d:6d86:0:b0:21b:ce00:4163 with SMTP id l6-20020a5d6d86000000b0021bce004163mr5237274wrs.511.1656331602821; Mon, 27 Jun 2022 05:06:42 -0700 (PDT) Received: from localhost (15.72.115.87.dyn.plus.net. [87.115.72.15]) by smtp.gmail.com with ESMTPSA id w9-20020a5d6089000000b0020e5b4ebaecsm10325936wrt.4.2022.06.27.05.06.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 27 Jun 2022 05:06:42 -0700 (PDT) From: Andrew Burgess To: Pedro Alves , gdb-patches@sourceware.org Subject: Re: [PATCH] Make GDBserver abort on internal error in development mode In-Reply-To: <20220624132645.1117504-1-pedro@palves.net> References: <20220624132645.1117504-1-pedro@palves.net> Date: Mon, 27 Jun 2022 13:06:40 +0100 Message-ID: <87sfnqe2en.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-10.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 27 Jun 2022 12:06:50 -0000 Pedro Alves 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