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.0 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 1262C1F462 for ; Fri, 24 May 2019 14:42:42 +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:cc:references:from:message-id:date :mime-version:in-reply-to:content-type :content-transfer-encoding; q=dns; s=default; b=hKVcYt0oDyv0BHTX nV0UHcOHHUOX3V+lD7PlZZrLTYbDtusz99PS2axNPqZG+8PJNeGm69lXLeuqqAYQ nS0ugFF2Uh9W3g9xqicIF7uijQ5RYstYSmYzNjKY0xLHamJ9YD+neiA8TVKXAu3x 7ffq/v+eHgxwNV3MebW+8mBBV7g= 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:cc:references:from:message-id:date :mime-version:in-reply-to:content-type :content-transfer-encoding; s=default; bh=z5zMt5No9C7CixZbDhtEz+ P7puM=; b=hZGzsqaX8mAoSSIP+PYBgdZybkI1q4LauU51hfF1whMs1G/Y1mxakY D0Md3CyI2kszjDy91sQUwMEBDnYpzzEmu9pky7kONvJsrKZyrn4GQJltSV583jAW EvxPyDf+TruqfIlZJOi4iO6FBM200cNQdJJr80FE198UGw/85FFAI= Received: (qmail 68045 invoked by alias); 24 May 2019 14:42:39 -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 68032 invoked by uid 89); 24 May 2019 14:42:39 -0000 Authentication-Results: sourceware.org; auth=none X-HELO: mail-vs1-f66.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=subject:to:cc:references:from:openpgp:autocrypt:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=sAtNMvDeT6acM1VNci/O5EW6AFYb4Mkov0L3vbZUYJg=; b=PtezPJTMWrGtex2kicKdNJYttw613+lcrOXzoqk14+qJNp1deyVvD1elcQ52d9AK4s bwkiyCgK1/lkayxl+oQ39jIBfzVadAUcH6KLnfmwZHNZQB/upn1/QbOgT9LjQTmDo+IC ACJf2y0Ovyep9pJSkyl2PFSGeAALPbqF87LGuURWbRJDJg6OEmrV2F3uoaWxbP9lgiY9 HeZ1qS8Jzks4NUVyh4a3M4mgxmZvdVA7KFUEkvU25Gt2yesJZg5sraui/IZfORELqg5Q VZLdL0MXPNJLj+ZbhR7rLerebajwA+ONTnM8wtGus2TLMR0Kprq0hOFRCg9l/QHdRc4R Y3HA== Subject: Re: [PATCH] Add missing VDSO_{NAME,HASH}_* macros and use them for PREPARE_VERSION_KNOWN To: Tobias Klauser Cc: libc-alpha@sourceware.org References: <20190516115924.29027-1-tklauser@distanz.ch> <95538879-7ff2-275b-ad26-fd1e68ebbd1a@linaro.org> <20190524072356.it7dqmlkmt4vf3p2@distanz.ch> From: Adhemerval Zanella Openpgp: preference=signencrypt Message-ID: Date: Fri, 24 May 2019 11:42:32 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <20190524072356.it7dqmlkmt4vf3p2@distanz.ch> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 24/05/2019 04:23, Tobias Klauser wrote: > 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? I don't have a preference here. Refactor the vDSO code is something I have in mind for some time. > >>> --- >>> 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. >