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=-4.1 required=3.0 tests=AWL,BAYES_00,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 59DE31F462 for ; Fri, 24 May 2019 07:24:06 +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:date:from:to:cc:subject:message-id:references :mime-version:content-type:in-reply-to; q=dns; s=default; b=Fm68 Ff1XOHxASSGuyrp/6mJn4BatftHoXMDLmy2tVHZNh0pL8TOnrIyrgz1RUBrP/9DS wEAsMgQocLz3NjRRhF4x82UTHi52qJLIQdWpnz1M2LoFQD4KObiCvI/Jw9Z+iL1A dn8+K5Dj5SBuAavQehZCThwPv8Mux5W1hJaYuSk= 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:date:from:to:cc:subject:message-id:references :mime-version:content-type:in-reply-to; s=default; bh=RaWBfI3aI6 LH/pWvkjBmRdnld+g=; b=TzoOZ7qiTKe/lgOc4OFSt7gKxlnLrfDqDQg2HmuiwB N1/Dcds8HVrkEf78tu8hJQZH7cig1gt8DSfblqWuq1pUut6hUwQEgDI+Beic8vsc mwh+r+znabbzRpfcjJduTFjqrPBVjbWFCbEye7XZflYsQvg9kXlMHL2/Q46awUlw M= Received: (qmail 88283 invoked by alias); 24 May 2019 07:24:03 -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 88259 invoked by uid 89); 24 May 2019 07:24:02 -0000 Authentication-Results: sourceware.org; auth=none X-HELO: sym2.noone.org Date: Fri, 24 May 2019 09:23:56 +0200 From: Tobias Klauser To: Adhemerval Zanella Cc: libc-alpha@sourceware.org Subject: Re: [PATCH] Add missing VDSO_{NAME,HASH}_* macros and use them for PREPARE_VERSION_KNOWN Message-ID: <20190524072356.it7dqmlkmt4vf3p2@distanz.ch> References: <20190516115924.29027-1-tklauser@distanz.ch> <95538879-7ff2-275b-ad26-fd1e68ebbd1a@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <95538879-7ff2-275b-ad26-fd1e68ebbd1a@linaro.org> User-Agent: NeoMutt/20170113 (1.7.2) Thank you for the review. On 2019-05-23 at 21:36:01 +0200, Adhemerval Zanella wrote: > On 16/05/2019 08:59, Tobias Klauser wrote: > > Define all currently used Linux versions used for > > PREPARE_VERSION{,_KNOWN} in sysdeps/unix/sysv/linux/dl-vdso.h and use > > them instead of duplicating the versions and precomputed hashes across > > architecture specific files. > > > > 2019-05-16 Tobias Klauser > > > > * sysdeps/unix/sysv/linux/aarch64/gettimeofday.c (INIT_ARCH): Use > > PREPARE_VERSION_KNOWN. > > * sysdeps/unix/sysv/linux/aarch64/init-first.c: Likewise. > > * sysdeps/unix/sysv/linux/dl-vdso.h (VDSO_NAME_LINUX_2_6_39): New > > define. > > (VDSO_HASH_LINUX_2_6_39): Likewise. > > (VDSO_NAME_LINUX_4_9): Likewise. > > (VDSO_HASH_LINUX_4_9): Likewise. > > * sysdeps/unix/sysv/linux/m68k/init-first.c (_libc_vdso_platform_setup): > > Use PREPARE_VERSION_KNOWN. > > * sysdeps/unix/sysv/linux/powerpc/gettimeofday.c (INIT_ARCH): Likewise. > > * sysdeps/unix/sysv/linux/powerpc/init-first.c > > (_libc_vdso_platform_setup): Likewise. > > * sysdeps/unix/sysv/linux/powerpc/time.c (INIT_ARCH): Likewise. > > * sysdeps/unix/sysv/linux/s390/init-first.c (_libc_vdso_platform_setup): > > Likewise. > > * sysdeps/unix/sysv/linux/x86_64/init-first.c (__vdso_platform_setup): > > Likewise. > > > LGTM, I assume you checked a build against affected architectures. As a side > note I think also should just remove PREPARE_VERSION macro and replace it with > a more sane interface: I (cross-)compile tested on all architectures and ran the test suite on x86_64. > -- > static inline struct r_found_version > prepare_version_base (const char *name, ElfW(Word) hash) > { > assert (hash == _dl_elf_hash (name)); > return (struct r_found_version) { name, hash, 1, NULL }; > } > #define prepare_version(vname) \ > prepare_version_base (VDSO_NAME_##vname, VDSO_HASH_##vname) > -- Should I add this to a v2 of this patch? Or should I send a follow-up patch with the change? > > --- > > sysdeps/unix/sysv/linux/aarch64/gettimeofday.c | 4 ++-- > > sysdeps/unix/sysv/linux/aarch64/init-first.c | 4 ++-- > > sysdeps/unix/sysv/linux/dl-vdso.h | 4 ++++ > > sysdeps/unix/sysv/linux/m68k/init-first.c | 2 +- > > sysdeps/unix/sysv/linux/powerpc/gettimeofday.c | 4 ++-- > > sysdeps/unix/sysv/linux/powerpc/init-first.c | 2 +- > > sysdeps/unix/sysv/linux/powerpc/time.c | 4 ++-- > > sysdeps/unix/sysv/linux/s390/init-first.c | 2 +- > > sysdeps/unix/sysv/linux/x86_64/init-first.c | 2 +- > > 9 files changed, 16 insertions(+), 12 deletions(-) > > > > diff --git a/sysdeps/unix/sysv/linux/aarch64/gettimeofday.c b/sysdeps/unix/sysv/linux/aarch64/gettimeofday.c > > index 6c008ed9357f..9180b50bf7c3 100644 > > --- a/sysdeps/unix/sysv/linux/aarch64/gettimeofday.c > > +++ b/sysdeps/unix/sysv/linux/aarch64/gettimeofday.c > > @@ -38,13 +38,13 @@ __gettimeofday_vsyscall (struct timeval *tv, struct timezone *tz) > > return INLINE_VSYSCALL (gettimeofday, 2, tv, tz); > > } > > > > -/* PREPARE_VERSION will need an __LP64__ ifdef when ILP32 support > > +/* PREPARE_VERSION_KNOWN will need an __LP64__ ifdef when ILP32 support > > goes in. See _libc_vdso_platform_setup in > > sysdeps/unix/sysv/linux/aarch64/init-first.c. */ > > > > # undef INIT_ARCH > > # define INIT_ARCH() \ > > - PREPARE_VERSION (linux_version, "LINUX_2.6.39", 123718537); \ > > + PREPARE_VERSION_KNOWN (linux_version, LINUX_2_6_39); \ > > void *vdso_gettimeofday = \ > > _dl_vdso_vsym ("__kernel_gettimeofday", &linux_version); > > Also a side note, this vsdo setup is not required. The init-first.c > already setups the VDSO_SYMBOL(gettimeofday). So a possible followup > cleanup would be: > > -- > # undef INIT_ARCH > # define INIT_ARCH() > > static inline void * > gettimeofday_vdso (void) > { > __typeof (VDSO_SYMBOL(gettimeofday)) vdsop = VDSO_SYMBOL(gettimeofday); > PTR_DEMANGLE (vdsop); > return vdsop != NULL ? vdsop : (void *) __gettimeofday_vsyscall; > } > > libc_ifunc_hidden (__redirect___gettimeofday, __gettimeofday, > gettimeofday_vdso ()) > -- Thanks, will send a follow-up patch.