From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id 39FD539484A9 for ; Fri, 12 Feb 2021 19:12:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 39FD539484A9 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tdevries@suse.de X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 46D89AFF3; Fri, 12 Feb 2021 19:12:00 +0000 (UTC) Subject: Re: [PATCH][gdb/threads] Fix lin_thread_get_thread_signals for glibc 2.28 To: Luis Machado , gdb-patches@sourceware.org References: <20210205111502.GA1510@delia> From: Tom de Vries Message-ID: <96aa0f3f-98f0-d907-b795-39ac5f3b3196@suse.de> Date: Fri, 12 Feb 2021 20:11:59 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.6 required=5.0 tests=BAYES_00, BODY_8BITS, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_NUMSUBJECT, NICE_REPLY_A, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Fri, 12 Feb 2021 19:12:03 -0000 On 2/5/21 12:54 PM, Luis Machado wrote: > On 2/5/21 8:15 AM, Tom de Vries wrote: >> Hi, >> >> When running test-case gdb.threads/create-fail.exp on openSUSE Factory >> (with glibc version 2.32) I run into: > > I'd mention Ubuntu 20.04 as well. > >> ... >> (gdb) continue >> Continuing. >> [New Thread 0x7ffff7c83700 (LWP 626354)] >> [New Thread 0x7ffff7482700 (LWP 626355)] >> [Thread 0x7ffff7c83700 (LWP 626354) exited] >> [New Thread 0x7ffff6c81700 (LWP 626356)] >> [Thread 0x7ffff7482700 (LWP 626355) exited] >> [New Thread 0x7ffff6480700 (LWP 626357)] >> [Thread 0x7ffff6c81700 (LWP 626356) exited] >> [New Thread 0x7ffff5c7f700 (LWP 626358)] >> [Thread 0x7ffff6480700 (LWP 626357) exited] >> pthread_create: 22: Invalid argument >> >> Thread 6 "create-fail" received signal SIG32, Real-time event 32. >> [Switching to Thread 0x7ffff5c7f700 (LWP 626358)] >> 0x00007ffff7d87695 in clone () from /lib64/libc.so.6 >> (gdb) FAIL: gdb.threads/create-fail.exp: iteration 1: run till end >> ... >> The problem is that glibc-internal signal SIGCANCEL is not recognized >> by gdb. >> >> There's code in check_thread_signals that is supposed to take care of >> that, >> but it's not working because this code in >> lin_thread_get_thread_signals has >> stopped working: >> ... >>    /* NPTL reserves the first two RT signals, but does not provide any >>       way for the debugger to query the signal numbers - fortunately >>       they don't change.  */ >>    sigaddset (set, __SIGRTMIN); >>    sigaddset (set, __SIGRTMIN + 1); >> ... >> >> Since glibc commit d2dc5467c6 "Filter out NPTL internal signals (BZ >> #22391)" >> (first released as part of glibc 2.28), a sigaddset with a glibc-internal >> signal has no other effect than setting errno to EINVALID. >> >> Fix this by eliminating the usage of sigset_t in check_thread_signals and >> lin_thread_get_thread_signals. >> >> Tested on x86_64-linux. > > Also validated on aarch64-linux/Ubuntu 20.04 and Ubuntu 18.04. > Thanks. Added mention of that, and committed. - Tom >> >> Any comments? >> >> Thanks, >> - Tom >> >> [gdb/threads] Fix lin_thread_get_thread_signals for glibc 2.28 >> >> gdb/ChangeLog: >> >> 2021-02-05  Tom de Vries  >> >>     PR threads/26228 >>     * linux-nat.c (lin_thread_get_thread_signals): Remove. >>     (lin_thread_signals): New static var. >>     (lin_thread_get_thread_signal_num, lin_thread_get_thread_signal): >>     New function. >>     * linux-nat.h (lin_thread_get_thread_signals): Remove. >>     (lin_thread_get_thread_signal_num, lin_thread_get_thread_signal): >>     Declare. >>     * linux-thread-db.c (check_thread_signals): Use >>     lin_thread_get_thread_signal_num and lin_thread_get_thread_signal. >> >> --- >>   gdb/linux-nat.c       | 26 +++++++++++++++++--------- >>   gdb/linux-nat.h       |  7 +++++-- >>   gdb/linux-thread-db.c | 21 +++++---------------- >>   3 files changed, 27 insertions(+), 27 deletions(-) >> >> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c >> index 10419dc7bb5..42dcd77ac45 100644 >> --- a/gdb/linux-nat.c >> +++ b/gdb/linux-nat.c >> @@ -4411,16 +4411,24 @@ Enables printf debugging output."), >>      the GNU/Linux Threads library and therefore doesn't really belong >>      here.  */ >>   -/* Return the set of signals used by the threads library in *SET.  */ >> +/* NPTL reserves the first two RT signals, but does not provide any >> +   way for the debugger to query the signal numbers - fortunately >> +   they don't change.  */ >> +static int lin_thread_signals[] = { __SIGRTMIN, __SIGRTMIN + 1 }; >>   -void >> -lin_thread_get_thread_signals (sigset_t *set) >> +/* See linux-nat.h.  */ >> + >> +unsigned int >> +lin_thread_get_thread_signal_num (void) >>   { >> -  sigemptyset (set); >> +  return sizeof (lin_thread_signals) / sizeof (lin_thread_signals[0]); >> +} >>   -  /* NPTL reserves the first two RT signals, but does not provide any >> -     way for the debugger to query the signal numbers - fortunately >> -     they don't change.  */ >> -  sigaddset (set, __SIGRTMIN); >> -  sigaddset (set, __SIGRTMIN + 1); >> +/* See linux-nat.h.  */ >> + >> +int >> +lin_thread_get_thread_signal (unsigned int i) >> +{ >> +  gdb_assert (i < lin_thread_get_thread_signal_num ()); >> +  return lin_thread_signals[i]; >>   } >> diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h >> index a5547f282ad..ff4d753422d 100644 >> --- a/gdb/linux-nat.h >> +++ b/gdb/linux-nat.h >> @@ -304,8 +304,11 @@ void check_for_thread_db (void); >>      true on success, false if the process isn't using libpthread.  */ >>   extern int thread_db_notice_clone (ptid_t parent, ptid_t child); >>   -/* Return the set of signals used by the threads library.  */ >> -extern void lin_thread_get_thread_signals (sigset_t *mask); >> +/* Return the number of signals used by the threads library.  */ >> +extern unsigned int lin_thread_get_thread_signal_num (void); >> + >> +/* Return the i-th signal used by the threads library.  */ >> +extern int lin_thread_get_thread_signal (unsigned int i); >>     /* Find process PID's pending signal set from /proc/pid/status.  */ >>   void linux_proc_pending_signals (int pid, sigset_t *pending, >> diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c >> index dce4bd23c1b..4dab64ac344 100644 >> --- a/gdb/linux-thread-db.c >> +++ b/gdb/linux-thread-db.c >> @@ -161,8 +161,6 @@ static thread_db_target the_thread_db_target; >>   /* Non-zero if we have determined the signals used by the threads >>      library.  */ >>   static int thread_signals; >> -static sigset_t thread_stop_set; >> -static sigset_t thread_print_set; >>     struct thread_db_info >>   { >> @@ -1225,23 +1223,14 @@ check_thread_signals (void) >>   { >>     if (!thread_signals) >>       { >> -      sigset_t mask; >>         int i; >>   -      lin_thread_get_thread_signals (&mask); >> -      sigemptyset (&thread_stop_set); >> -      sigemptyset (&thread_print_set); >> - >> -      for (i = 1; i < NSIG; i++) >> +      for (i = 0; i < lin_thread_get_thread_signal_num (); i++) >>       { >> -      if (sigismember (&mask, i)) >> -        { >> -          if (signal_stop_update (gdb_signal_from_host (i), 0)) >> -        sigaddset (&thread_stop_set, i); >> -          if (signal_print_update (gdb_signal_from_host (i), 0)) >> -        sigaddset (&thread_print_set, i); >> -          thread_signals = 1; >> -        } >> +      int sig = lin_thread_get_thread_signal (i); >> +      signal_stop_update (gdb_signal_from_host (sig), 0); >> +      signal_print_update (gdb_signal_from_host (sig), 0); >> +      thread_signals = 1; >>       } >>       } >>   } >>