From patchwork Tue Sep 9 05:54:01 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Evans X-Patchwork-Id: 2700 Received: (qmail 2640 invoked by alias); 9 Sep 2014 05:54:47 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 2630 invoked by uid 89); 9 Sep 2014 05:54:46 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL, BAYES_00, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pd0-f171.google.com Received: from mail-pd0-f171.google.com (HELO mail-pd0-f171.google.com) (209.85.192.171) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Tue, 09 Sep 2014 05:54:44 +0000 Received: by mail-pd0-f171.google.com with SMTP id p10so5340811pdj.2 for ; Mon, 08 Sep 2014 22:54:43 -0700 (PDT) X-Received: by 10.66.236.38 with SMTP id ur6mr54168057pac.49.1410242082873; Mon, 08 Sep 2014 22:54:42 -0700 (PDT) Received: from seba.sebabeach.org.gmail.com (173-13-178-50-sfba.hfc.comcastbusiness.net. [173.13.178.50]) by mx.google.com with ESMTPSA id i9sm10444786pbq.17.2014.09.08.22.54.41 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 08 Sep 2014 22:54:42 -0700 (PDT) From: Doug Evans To: gdb-patches@sourceware.org Subject: Re: [PATCH][PR guile/17247] Block SIGCHLD while initializing Guile References: Date: Mon, 08 Sep 2014 22:54:01 -0700 In-Reply-To: (Doug Evans's message of "Sun, 31 Aug 2014 12:07:58 -0700") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 X-IsSubscribed: yes Doug Evans writes: > Hi. > > This patch fixes PR 17247. > > Basically, current Guile (git) starts an internal thread > (the "finalizer" thread), and libgc as of 7.4 now starts several > marker threads by default (before 7.4.0 one needed to configure > libgc with --enable-parallel-mark). > > When other threads are running, and they haven't blocked SIGCHLD, > then the kernel may send SIGCHLD to these threads, leaving gdb > hung in the sigsuspend calls in linux-nat.c. > > P.S. I have a tentative patch for PR 17314, which also fixes 17247, > but for a bit of robustness I'm applying both. > > 2014-08-31 Doug Evans > > PR 17247 > * guile.c: #include . > (_initialize_guile): Block SIGCHLD while initializing Guile. > > Replaces the following, which is reverted. > > 2014-07-26 Doug Evans > > PR 17185 > * configure.ac: Add check for header gc/gc.h. > Add check for function setenv. > * configure: Regenerate. > * config.in: Regenerate. > * guile/guile.c (_initialize_guile): Add workaround for libgc 7.4.0. Hi. I have checked in this slight variation to trunk, and will commit it to the 7.8 branch tomorrow. commit 92d8d229d9a310ebfcfc13bf4a75a286c1add1ac Author: Doug Evans Date: Mon Sep 8 22:45:34 2014 -0700 Fix for PR 17247: Block SIGCHLD while initializing Guile. The problem here is that if a thread other than gdb's main thread gets a SIGCHLD (it's an asynchronous signal so the kernel will essentially pick a random thread) then gdb will hang if it is in sigsuspend when the SIGCHLD is delivered. The other thread will see the signal and the sigsuspend won't "wake up". Guile and libgc should be blocking SIGCHLD in their threads, but we need to work with Guile 2.0 and libgc 7.4. The problem first shows up in libgc 7.4 because it is the first release that enables multiple marker threads by default. gdb/ChangeLog: PR 17247 * guile.c: #include . (_initialize_guile): Block SIGCHLD while initializing Guile. Replaces the following, which is reverted. 2014-07-26 Doug Evans PR 17185 * configure.ac: Add check for header gc/gc.h. Add check for function setenv. * configure: Regenerate. * config.in: Regenerate. * guile/guile.c (_initialize_guile): Add workaround for libgc 7.4.0. diff --git a/gdb/ChangeLog b/gdb/ChangeLog index ad5ef6f..9d3f392 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,22 @@ 2014-09-08 Doug Evans + PR 17247 + * guile.c: #include . + (_initialize_guile): Block SIGCHLD while initializing Guile. + + Replaces the following, which is reverted. + + 2014-07-26 Doug Evans + + PR 17185 + * configure.ac: Add check for header gc/gc.h. + Add check for function setenv. + * configure: Regenerate. + * config.in: Regenerate. + * guile/guile.c (_initialize_guile): Add workaround for libgc 7.4.0. + +2014-09-08 Doug Evans + * guile/scm-cmd.c (gdbscm_parse_command_name): Replace magic number with named constant. Fix style of pointer comparison. * python/py-cmd.c (gdbpy_parse_command_name): Ditto. diff --git a/gdb/config.in b/gdb/config.in index b853412..f1593bd 100644 --- a/gdb/config.in +++ b/gdb/config.in @@ -141,9 +141,6 @@ /* Define if has fpregset_t. */ #undef HAVE_FPREGSET_T -/* Define to 1 if you have the header file. */ -#undef HAVE_GC_GC_H - /* Define to 1 if you have the `getgid' function. */ #undef HAVE_GETGID @@ -348,9 +345,6 @@ /* Define to 1 if you have the `scm_new_smob' function. */ #undef HAVE_SCM_NEW_SMOB -/* Define to 1 if you have the `setenv' function. */ -#undef HAVE_SETENV - /* Define to 1 if you have the `setlocale' function. */ #undef HAVE_SETLOCALE diff --git a/gdb/configure b/gdb/configure index bf141f1..350ec0c 100755 --- a/gdb/configure +++ b/gdb/configure @@ -9276,32 +9276,6 @@ else fi -# PR 17185, see if we can get the libgc version to see if we need -# to apply the workaround. -for ac_header in gc/gc.h -do : - ac_fn_c_check_header_mongrel "$LINENO" "gc/gc.h" "ac_cv_header_gc_gc_h" "$ac_includes_default" -if test "x$ac_cv_header_gc_gc_h" = x""yes; then : - cat >>confdefs.h <<_ACEOF -#define HAVE_GC_GC_H 1 -_ACEOF - -fi - -done - -for ac_func in setenv -do : - ac_fn_c_check_func "$LINENO" "setenv" "ac_cv_func_setenv" -if test "x$ac_cv_func_setenv" = x""yes; then : - cat >>confdefs.h <<_ACEOF -#define HAVE_SETENV 1 -_ACEOF - -fi -done - - # --------------------- # # Check for libmcheck. # # --------------------- # diff --git a/gdb/configure.ac b/gdb/configure.ac index 26c8ecf..ab7b1c2 100644 --- a/gdb/configure.ac +++ b/gdb/configure.ac @@ -1233,11 +1233,6 @@ AC_SUBST(GUILE_CPPFLAGS) AC_SUBST(GUILE_LIBS) AM_CONDITIONAL(HAVE_GUILE, test "${have_libguile}" != no) -# PR 17185, see if we can get the libgc version to see if we need -# to apply the workaround. -AC_CHECK_HEADERS(gc/gc.h) -AC_CHECK_FUNCS([setenv]) - # --------------------- # # Check for libmcheck. # # --------------------- # diff --git a/gdb/guile/guile.c b/gdb/guile/guile.c index 575bb6c..97da042 100644 --- a/gdb/guile/guile.c +++ b/gdb/guile/guile.c @@ -34,10 +34,8 @@ #ifdef HAVE_GUILE #include "guile.h" #include "guile-internal.h" -#ifdef HAVE_GC_GC_H -#include /* PR 17185 */ -#endif #endif +#include /* The Guile version we're using. We *could* use the macros in libguile/version.h but that would preclude @@ -833,39 +831,46 @@ extern initialize_file_ftype _initialize_guile; void _initialize_guile (void) { - char *msg; - install_gdb_commands (); #if HAVE_GUILE - /* The Python support puts the C side in module "_gdb", leaving the Python - side to define module "gdb" which imports "_gdb". There is evidently no - similar convention in Guile so we skip this. */ - - /* PR 17185 There are problems with using libgc 7.4.0. - Copy over the workaround Guile uses (Guile is working around a different - problem, but the workaround is the same). */ -#if (GC_VERSION_MAJOR == 7 && GC_VERSION_MINOR == 4 && GC_VERSION_MICRO == 0) - /* The bug is only known to appear with pthreads. We assume any system - using pthreads also uses setenv (and not putenv). That is why we don't - have a similar call to putenv here. */ -#if defined (HAVE_SETENV) - setenv ("GC_MARKERS", "1", 1); + { +#ifdef HAVE_SIGPROCMASK + sigset_t sigchld_mask, prev_mask; #endif + + /* The Python support puts the C side in module "_gdb", leaving the Python + side to define module "gdb" which imports "_gdb". There is evidently no + similar convention in Guile so we skip this. */ + +#ifdef HAVE_SIGPROCMASK + /* Before we initialize Guile, block SIGCHLD. + This is done so that all threads created during Guile initialization + have SIGCHLD blocked. PR 17247. + Really libgc and Guile should do this, but we need to work with + libgc 7.4.x. */ + sigemptyset (&sigchld_mask); + sigaddset (&sigchld_mask, SIGCHLD); + sigprocmask (SIG_BLOCK, &sigchld_mask, &prev_mask); +#endif + + /* scm_with_guile is the most portable way to initialize Guile. + Plus we need to initialize the Guile support while in Guile mode + (e.g., called from within a call to scm_with_guile). */ + scm_with_guile (call_initialize_gdb_module, NULL); + +#ifdef HAVE_SIGPROCMASK + sigprocmask (SIG_SETMASK, &prev_mask, NULL); #endif - /* scm_with_guile is the most portable way to initialize Guile. - Plus we need to initialize the Guile support while in Guile mode - (e.g., called from within a call to scm_with_guile). */ - scm_with_guile (call_initialize_gdb_module, NULL); - - /* Set Guile's backtrace to match the "set guile print-stack" default. - [N.B. The two settings are still separate.] - But only do this after we've initialized Guile, it's nice to see a - backtrace if there's an error during initialization. - OTOH, if the error is that gdb/init.scm wasn't found because gdb is being - run from the build tree, the backtrace is more noise than signal. - Sigh. */ - gdbscm_set_backtrace (0); + /* Set Guile's backtrace to match the "set guile print-stack" default. + [N.B. The two settings are still separate.] + But only do this after we've initialized Guile, it's nice to see a + backtrace if there's an error during initialization. + OTOH, if the error is that gdb/init.scm wasn't found because gdb is + being run from the build tree, the backtrace is more noise than signal. + Sigh. */ + gdbscm_set_backtrace (0); + } #endif }