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.133.124]) by sourceware.org (Postfix) with ESMTP id 0BD1239A141D for ; Fri, 11 Jun 2021 16:35:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0BD1239A141D Received: from mail-qk1-f200.google.com (mail-qk1-f200.google.com [209.85.222.200]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-516-kyeIZy7NMte9jG3OkztcEQ-1; Fri, 11 Jun 2021 12:35:39 -0400 X-MC-Unique: kyeIZy7NMte9jG3OkztcEQ-1 Received: by mail-qk1-f200.google.com with SMTP id k15-20020a05620a138fb02903aadd467ff1so9346149qki.7 for ; Fri, 11 Jun 2021 09:35:37 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:to:subject:from:organization:message-id:date :user-agent:mime-version:content-language:content-transfer-encoding; bh=LXDwunJC+aU+ojZe8VOdwnMrb25Dl0kRdQDgl95U/Zc=; b=qhd8nnZJpdCjHREYf1Lfqp8CRHG476qWVM1yAeFPooWA6LpK9E+Dx7489HBwZEqaHn GIR3rbw9lJuPChA0FZEvNs6+Eqn3MszpDz6rkPChtD+8g/yYod6YtCq4XiCfsVbgjMIw dJ26NuVIiccUNdUYYOCQIbYq6ka2RlhQHb0hON5/bdWRDce40n/F0w6bDMkf+lPwEbpo LM6mGqrydVW9upQDNnIH0B1n/xUibBuYCR+BzUt1lgXBN9d2vp36mp9j5AW2S02KK/N5 Z+VDOn46TFhpZ1s2XFBD6AlBkduFuIwABfItLGoGSB5IDv8IBVz3lgkNBXqh6Pds4Oqo bfYQ== X-Gm-Message-State: AOAM532vVy32aOf6KuRNS5haOGYEDT+K/atbaps2ZEsBAyyjrt8JxejN 8kEFUFcPUAT3brAdYaHUqQsZ1+AYzyXrCPWajKRSdNor+Yazjni8gZ1kEHljooKEtnJ+jvfANrj E79sWoeD6NLGZMudLrEMONXQTHNOnhXV1z/DZY/9Ha6mBWkbZEr9PEHmYM6to/rQCdoDOJ9U= X-Received: by 2002:ac8:5315:: with SMTP id t21mr4609019qtn.180.1623429337221; Fri, 11 Jun 2021 09:35:37 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwAb3Or9pBQeuHQ/pPBgAD9hK62eNJ0mHpwDsPEEZ0pwCV1dxNZgUtCXh0ojq0LWsU8ZNncdQ== X-Received: by 2002:ac8:5315:: with SMTP id t21mr4608990qtn.180.1623429336868; Fri, 11 Jun 2021 09:35:36 -0700 (PDT) Received: from [192.168.1.16] (198-84-214-74.cpe.teksavvy.com. [198.84.214.74]) by smtp.gmail.com with ESMTPSA id 75sm2814322qkm.57.2021.06.11.09.35.36 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 11 Jun 2021 09:35:36 -0700 (PDT) To: kevinb@redhat.com, "gdb-patches@sourceware.org" Subject: Re: [PATCH 1/4] libthread_db initialization changes related to upcoming glibc-2.34 From: Carlos O'Donell Organization: Red Hat Message-ID: <44c561eb-2676-4b8a-0bc0-d924e7d7ae74@redhat.com> Date: Fri, 11 Jun 2021 12:35:35 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 MIME-Version: 1.0 X-Mimecast-Spam-Score: 1 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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, FAKE_REPLY_A1, GIT_PATCH_0, KAM_NUMSUBJECT, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, 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, 11 Jun 2021 16:35:45 -0000 This looks good to me. I've tested on x86_64 Fedora Rawhide. Reviewed-by: Carlos O'Donell Tested-by: Carlos O'Donell > This commit makes some adjustments to accomodate the upcoming > glibc-2.34 release. Beginning with glibc-2.34, functionality formerly > contained in libpthread has been moved to libc. For the time being, > libpthread.so still exists in the file system, but it won't show up in > ldd output and therefore won't be able to trigger initialization of > libthread_db related code. E.g... > > Fedora 34 / glibc-2.33.9000: > > [kev@f34-2 gdb]$ ldd testsuite/outputs/gdb.threads/tls/tls > linux-vdso.so.1 (0x00007ffcf94fa000) > libstdc++.so.6 => /lib64/libstdc++.so.6 (0x00007ff0ba9af000) > libm.so.6 => /lib64/libm.so.6 (0x00007ff0ba8d4000) > libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007ff0ba8b9000) > libc.so.6 => /lib64/libc.so.6 (0x00007ff0ba6c6000) > /lib64/ld-linux-x86-64.so.2 (0x00007ff0babf0000) > > Fedora 34 / glibc-2.33: > > [kev@f34-1 gdb]$ ldd testsuite/outputs/gdb.threads/tls/tls > linux-vdso.so.1 (0x00007fff32dc0000) > libpthread.so.0 => /lib64/libpthread.so.0 (0x00007f815f6de000) > libstdc++.so.6 => /lib64/libstdc++.so.6 (0x00007f815f4bf000) > libm.so.6 => /lib64/libm.so.6 (0x00007f815f37b000) > libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007f815f360000) > libc.so.6 => /lib64/libc.so.6 (0x00007f815f191000) > /lib64/ld-linux-x86-64.so.2 (0x00007f815f721000) > > Note that libpthread is missing from the ldd output for the > glibc-2.33.9000 machine. > > This means that (unless we happen to think of some entirely different > mechanism), we'll now need to potentially match "libc" in addition to > "libpthread" as libraries which might be thread libraries. This > accounts for the change made in solib.c. Note that the new code > attempts to match "/libc." via strstr(). That trailing dot (".") > avoids inadvertently matching libraries such as libcrypt (and > all the other many libraries which begin with "libc"). This is correct and the solution looks good. You do need to trigger libthread_db loading for libc.so.6 now. > To avoid attempts to load libthread_db when encountering older > versions of libc, we now attempt to find "pthread_create" (which is a > symbol that we'd expect to be in any pthread library) in the > associated objfile. This accounts for the changes in > linux-thread-db.c. That is a good heuristic. > I think that other small adjustments will need to be made elsewhere > too. I've been working through regressions on my glibc-2.33.9000 > machine; I've fixed some fairly "obvious" changes in the testsuite > (which are in other commits). For the rest, it's not yet clear to me > whether the handful of remaining failures represent a problem in glibc > or gdb. I'm still investigating, however, I'll note that these are > problems that I only see on my glibc-2.33.9000 machine. > > gdb/ChangeLog: > > * solib.c (libpthread_name_p): Match "libc" in addition > to "libpthread". > * linux-thread-db.c (libpthread_objfile_p): New function. > (libpthread_name_p): Adjust preexisting callers to use > libpthread_objfile_p(). > --- > gdb/linux-thread-db.c | 24 +++++++++++++++++++++--- > gdb/solib.c | 10 ++++++++-- > 2 files changed, 29 insertions(+), 5 deletions(-) > > diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c > index d1e8c22ac96..5b4e5a8654f 100644 > --- a/gdb/linux-thread-db.c > +++ b/gdb/linux-thread-db.c > @@ -799,6 +799,24 @@ check_thread_db (struct thread_db_info *info, bool log_progress) > return test_passed; > } > > +/* Predicate which tests whether objfile OBJ refers to the library > + containing pthread related symbols. Historically, this library has > + been named in such a way that looking for "libpthread" in the name > + was sufficient to identify it. As of glibc-2.34, the C library > + (libc) contains the thread library symbols. Therefore we check > + that the name matches a possible thread library, but we also check > + that it contains at least one of the symbols (pthread_create) that > + we'd expect to find in the thread library. */ > + > +static bool > +libpthread_objfile_p (objfile *obj) > +{ > + return (libpthread_name_p (objfile_name (obj)) > + && lookup_minimal_symbol ("pthread_create", > + NULL, > + obj).minsym != NULL); > +} OK. If the loaded file matches the name list then do a global symbol search to see if pthread_create is present and if it is then we will indirectly trigger an attempt to load libthread_db. > + > /* Attempt to initialize dlopen()ed libthread_db, described by INFO. > Return true on success. > Failure could happen if libthread_db does not have symbols we expect, > @@ -1072,7 +1090,7 @@ try_thread_db_load_from_pdir (const char *subdir) > return false; > > for (objfile *obj : current_program_space->objfiles ()) > - if (libpthread_name_p (objfile_name (obj))) > + if (libpthread_objfile_p (obj)) OK. > { > if (try_thread_db_load_from_pdir_1 (obj, subdir)) > return true; > @@ -1181,7 +1199,7 @@ static bool > has_libpthread (void) > { > for (objfile *obj : current_program_space->objfiles ()) > - if (libpthread_name_p (objfile_name (obj))) > + if (libpthread_objfile_p (obj)) OK. > return true; > > return false; > @@ -1286,7 +1304,7 @@ thread_db_new_objfile (struct objfile *objfile) > of the list of shared libraries to load, and in an app of several > thousand shared libraries, this can otherwise be painful. */ > && ((objfile->flags & OBJF_MAINLINE) != 0 > - || libpthread_name_p (objfile_name (objfile)))) > + || libpthread_objfile_p (objfile))) OK. > check_for_thread_db (); > } > > diff --git a/gdb/solib.c b/gdb/solib.c > index 2df52118143..88b201ff6a0 100644 > --- a/gdb/solib.c > +++ b/gdb/solib.c > @@ -900,12 +900,18 @@ Do you need \"set solib-search-path\" or \"set sysroot\"?"), > > Uses a fairly simplistic heuristic approach where we check > the file name against "/libpthread". This can lead to false > - positives, but this should be good enough in practice. */ > + positives, but this should be good enough in practice. > + > + In glibc-2.34 (and later), functions formerly residing > + in libpthread have been moved to libc, so "/libc." needs > + to be checked too. (Matching the "." will avoid matching > + libraries such as libcrypt.) */ > > bool > libpthread_name_p (const char *name) > { > - return (strstr (name, "/libpthread") != NULL); > + return (strstr (name, "/libpthread") != NULL > + || strstr (name, "/libc.") != NULL ); OK. So you look for both libpthread.* and libc.*. > } > > /* Return non-zero if SO is the libpthread shared library. */ > -- > 2.31.1 -- Cheers, Carlos.