From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS31976 209.132.180.0/23 X-Spam-Status: No, score=-3.3 required=3.0 tests=AWL,BAYES_00,BODY_8BITS, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id 6230820248 for ; Fri, 8 Mar 2019 14:52:07 +0000 (UTC) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:subject:to:references:from:message-id:date :mime-version:in-reply-to:content-type :content-transfer-encoding; q=dns; s=default; b=wM0CLjSLs5gpUXze +z9gvfmfVBTjSB3fEt8JIrGZGF73LcpqBaVkWb13GybjRb55DcxEek+Ouq6nQs2S ox/I/OB2LHqJhBwc/reiOUO/RQMB9xyuUKIMfTxXd/Bb+A5d+pKlSjRizleNmmIy qEKvI4wj10Y3FqYCvQSKnKp4G8w= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:subject:to:references:from:message-id:date :mime-version:in-reply-to:content-type :content-transfer-encoding; s=default; bh=C1pOKoMOIZSVVEC7v171L0 auIs4=; b=Fef2kQtR2WLzic99eL0A7WhZF0bFpG2aAOQN7vmcLRcIzm97CtQuLJ HE92TxKZD6HKZDn9YXs2N31uIiLfvWZb3B35jM62ZvTHdHDAmJH2jR+Uq4s+ip3P x+Q1I3012iDBajg4ETBB85cvVN+9JgmdYUnZEHYNTCTA8j9NxaazE= Received: (qmail 53656 invoked by alias); 8 Mar 2019 14:52:04 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 53569 invoked by uid 89); 8 Mar 2019 14:52:03 -0000 Authentication-Results: sourceware.org; auth=none X-HELO: mail-qk1-f194.google.com Subject: Re: [PATCH] Fix output of LD_SHOW_AUXV=1. To: Stefan Liebler , GNU C Library References: From: Carlos O'Donell Openpgp: preference=signencrypt Message-ID: Date: Fri, 8 Mar 2019 09:51:56 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit On 3/8/19 7:52 AM, Stefan Liebler wrote: > Hi, > > starting with commit 1616d034b61622836d3a36af53dcfca7624c844e > the output was corrupted on some platforms as _dl_procinfo > was called for every auxv entry and on some architectures like s390 > all entries were represented as "AT_HWCAP". > > This patch fixes the condition which determines if _dl_procinfo > is called and adjusts _dl_procinfo implementations which assumed > that they are only called for AT_HWCAP or AT_HWCAP2. > > A further question is if we should always call _dl_procinfo without a > condition and let it decide if it is able to print an entry or if it > should be printed with help of the auxvars list? This is a design question: (a) Call machine-specific routine *after* filtering out generic options. vs. (b) Call machine-specific routine for *all* options and let the the machine-specific routine call a generic routine after it handles all the options it wants. We should *always* call _dl_procinfo, and let the architectures decide if they will handle the entry in a special way. We need to provide a "base" function helper to allow the arches to print the information in a generic manner if their own code decides it doesn't want to handle it. This would involve a refactoring of the existing code though, so I'm not going to require it as part of fixing this for s390x. However, if we implement (b), then the generic code doesn't need to know what will be handled by the machine-specific routines, so I see that as a good cleanup. $0.02. > Okay to commit? > Once it is committed I would also backport it to glibc 2.29 release branch. OK for master. Reviewed-by: Carlos O'Donell > Bye, > Stefan > > ChangeLog: > >     * elf/dl-sysdep.c (_dl_show_auxv): Fix condition if _dl_procinfo >     is called. >     * sysdeps/unix/sysv/linux/s390/dl-procinfo.h (_dl_procinfo): >     Ignore types other than AT_HWCAP. >     * sysdeps/sparc/dl-procinfo.h (_dl_procinfo): Likewise. >     * sysdeps/unix/sysv/linux/i386/dl-procinfo.h (_dl_procinfo): >     Likewise. > > 20190308_ldshowauxv.patch > > commit 5bbde0719d7c5fa58b0140d6fb1c6fa31a372772 > Author: Stefan Liebler > Date: Fri Mar 8 11:18:53 2019 +0100 > > Fix output of LD_SHOW_AUXV=1. > > Starting with commit 1616d034b61622836d3a36af53dcfca7624c844e > the output was corrupted on some platforms as _dl_procinfo > was called for every auxv entry and on some architectures like s390 > all entries were represented as "AT_HWCAP". > > This patch fixes the condition which determines if _dl_procinfo > is called and adjusts _dl_procinfo implementations which assumed > that they are only called for AT_HWCAP or AT_HWCAP2. OK. > ChangeLog: > > * elf/dl-sysdep.c (_dl_show_auxv): Fix condition if _dl_procinfo > is called. > * sysdeps/unix/sysv/linux/s390/dl-procinfo.h (_dl_procinfo): > Ignore types other than AT_HWCAP. > * sysdeps/sparc/dl-procinfo.h (_dl_procinfo): Likewise. > * sysdeps/unix/sysv/linux/i386/dl-procinfo.h (_dl_procinfo): Likewise. > > diff --git a/elf/dl-sysdep.c b/elf/dl-sysdep.c > index 5f6c679a3f..1588555651 100644 > --- a/elf/dl-sysdep.c > +++ b/elf/dl-sysdep.c > @@ -329,8 +329,10 @@ _dl_show_auxv (void) > assert (AT_IGNORE == 1); > > if (av->a_type == AT_HWCAP || av->a_type == AT_HWCAP2 > - || AT_L1I_CACHEGEOMETRY || AT_L1D_CACHEGEOMETRY > - || AT_L2_CACHEGEOMETRY || AT_L3_CACHEGEOMETRY) > + || av->a_type == AT_L1I_CACHEGEOMETRY > + || av->a_type == AT_L1D_CACHEGEOMETRY > + || av->a_type == AT_L2_CACHEGEOMETRY > + || av->a_type == AT_L3_CACHEGEOMETRY) OK. Fixes it so that we call into _dl_procinfo properly for the special cases. > { > /* These are handled in a special way per platform. */ > if (_dl_procinfo (av->a_type, av->a_un.a_val) == 0) > diff --git a/sysdeps/sparc/dl-procinfo.h b/sysdeps/sparc/dl-procinfo.h > index 282b8c5117..cc2687e99b 100644 > --- a/sysdeps/sparc/dl-procinfo.h > +++ b/sysdeps/sparc/dl-procinfo.h > @@ -32,7 +32,7 @@ _dl_procinfo (unsigned int type, unsigned long int word) > int i; > > /* Fallback to unknown output mechanism. */ > - if (type == AT_HWCAP2) > + if (type != AT_HWCAP) OK. > return -1; > > _dl_printf ("AT_HWCAP: "); > diff --git a/sysdeps/unix/sysv/linux/i386/dl-procinfo.h b/sysdeps/unix/sysv/linux/i386/dl-procinfo.h > index 22b43431bc..3aef14c6c1 100644 > --- a/sysdeps/unix/sysv/linux/i386/dl-procinfo.h > +++ b/sysdeps/unix/sysv/linux/i386/dl-procinfo.h > @@ -31,7 +31,7 @@ _dl_procinfo (unsigned int type, unsigned long int word) > int i; > > /* Fallback to unknown output mechanism. */ > - if (type == AT_HWCAP2) > + if (type != AT_HWCAP) OK. > return -1; > > _dl_printf ("AT_HWCAP: "); > diff --git a/sysdeps/unix/sysv/linux/s390/dl-procinfo.h b/sysdeps/unix/sysv/linux/s390/dl-procinfo.h > index 19329a335b..16739fd6cd 100644 > --- a/sysdeps/unix/sysv/linux/s390/dl-procinfo.h > +++ b/sysdeps/unix/sysv/linux/s390/dl-procinfo.h > @@ -33,7 +33,7 @@ _dl_procinfo (unsigned int type, unsigned long int word) > int i; > > /* Fallback to unknown output mechanism. */ > - if (type == AT_HWCAP2) > + if (type != AT_HWCAP) OK. > return -1; > > _dl_printf ("AT_HWCAP: "); OK, verified sysdeps/unix/sysv/linux/arm/dl-procinfo.h uses a switch statement that handles this correctly. OK, verified sysdeps/powerpc/dl-procinfo.h uses a switch statement that handles this correctly. -- Cheers, Carlos.