From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dragonfly.birch.relay.mailchannels.net (dragonfly.birch.relay.mailchannels.net [23.83.209.51]) by sourceware.org (Postfix) with ESMTPS id C17AE3836428 for ; Thu, 19 May 2022 14:57:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C17AE3836428 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=gotplt.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gotplt.org X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org Received: from relay.mailchannels.net (localhost [127.0.0.1]) by relay.mailchannels.net (Postfix) with ESMTP id 21CA35A1083; Thu, 19 May 2022 14:57:02 +0000 (UTC) Received: from pdx1-sub0-mail-a307.dreamhost.com (unknown [127.0.0.6]) (Authenticated sender: dreamhost) by relay.mailchannels.net (Postfix) with ESMTPA id AF7845A169C; Thu, 19 May 2022 14:57:01 +0000 (UTC) ARC-Seal: i=1; s=arc-2022; d=mailchannels.net; t=1652972221; a=rsa-sha256; cv=none; b=DLJRvvNpjhM5y3mv/IDUMw1Qf/gXUcb0tP9hmJTdkC5zr1pt14kYVRpulRUxUlBdmusUEZ Ux78Fx16wAxv/a4dMfgPbBCywIBGAVBbT0R0a1brkqg5IYagsqjQ8kQoy8NUZgyNG5nl0o Dfm1EUiVD03C8nW+3Ax4IWG3e2K7xG4YiymGwJo9h2s6XdcWHDgvcmi7hz4imshAiSTWxW jegOD1nBns09vNp9TQT4PEXF8nhUcH51XEaP4Z9LPVc1W69IEbhFjgvz5cZaZVEGTGZUt4 uBgv91qQ9V7rqAt9YB9+Mc9qV3uK8HxVvJGjGKV8g/Srpa9eksdY+w78wSD9XQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=mailchannels.net; s=arc-2022; t=1652972221; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=TSfDRqRMvJIFvJXkt7Lm0HAK66TBR8UaXA/eYcZB2Js=; b=bkEuar5Upck29uWLw8tJcn4ZQftOKU9ONnUenj8dtaG+/0lOVepS2hUNNwB1h2c+PZXMeK yIG2xArwRIjC5ydW/VPC46FqU5sTCpvKslkzFwNG0I9EWlNokZv77Td2DWbF7UQpXbVC3Z BjhlIvofoviVWMzSFy5YivmjB8NkBeGEdW8pMQjWaMCOOJV40jlFcUnEjXC1aMxTQt37wv DRn0TClpr7BIR0pIevdiw3qKkqawEH1WuoZO0FyxNN9ownfThHiQkZ5eq4TJI3iVceOCJI r8Cmb2NkeqAZqCBCcYKAQpV/Q1afd4qWEcroNE8YQBUHk2ptDk/sgDBJ3TLM7A== ARC-Authentication-Results: i=1; rspamd-554c8f6c56-tc62w; auth=pass smtp.auth=dreamhost smtp.mailfrom=siddhesh@gotplt.org X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org X-MC-Relay: Neutral X-MailChannels-SenderId: dreamhost|x-authsender|siddhesh@gotplt.org X-MailChannels-Auth-Id: dreamhost X-Suffer-Reaction: 7a80fd0744422a43_1652972221964_3098172984 X-MC-Loop-Signature: 1652972221964:1411881260 X-MC-Ingress-Time: 1652972221964 Received: from pdx1-sub0-mail-a307.dreamhost.com (pop.dreamhost.com [64.90.62.162]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384) by 100.116.106.119 (trex/6.7.1); Thu, 19 May 2022 14:57:01 +0000 Received: from [192.168.1.174] (unknown [1.186.223.88]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) (Authenticated sender: siddhesh@gotplt.org) by pdx1-sub0-mail-a307.dreamhost.com (Postfix) with ESMTPSA id 4L3tHJ1pSxz3B; Thu, 19 May 2022 07:56:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gotplt.org; s=dreamhost; t=1652972221; bh=TSfDRqRMvJIFvJXkt7Lm0HAK66TBR8UaXA/eYcZB2Js=; h=Date:Subject:To:Cc:From:Content-Type:Content-Transfer-Encoding; b=M64YBnJKhI5v2oqz/IPWcH3gSK5sn1fhdcyyIYCZSXWh94lhNI+EgEJxJEyto+7Gx JZ3Hly6sKBExdEXyToFwDmYeXKLwBFUB9t1S7bUQ2LGHlbHPX/WBjOG5GglFoixIh4 ZWF56rariKF6u8Zkea+2Ni7vcO60BrDBumptthDYadNsVNVhyT9AQMgoIufs5sU9dl /FKnz83SrW31UHTcdJq+7tOIWuN4bp9sMdFFYauMdRl+NhjXRkjiqxHsiDp1vuOuRR N+jvn4aEMqMQNVdc+NaClYrLNem3WdeHns9glr+Fk+xLmgtZaJhoqFqT8IIodjUL39 HOLcL5L6uZxWg== Message-ID: Date: Thu, 19 May 2022 20:26:55 +0530 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.8.0 Subject: Re: [PATCH v10 1/6] elf: Refactor dl_new_hash so it can be tested / benchmarked Content-Language: en-US To: Noah Goldstein Cc: GNU C Library References: <20220414041231.926415-1-goldstein.w.n@gmail.com> <20220518172635.291119-1-goldstein.w.n@gmail.com> From: Siddhesh Poyarekar In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3037.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, NICE_REPLY_A, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 19 May 2022 14:57:07 -0000 On 19/05/2022 20:20, Noah Goldstein wrote: > On Thu, May 19, 2022 at 9:47 AM Siddhesh Poyarekar wrote: >> >> On 18/05/2022 22:56, Noah Goldstein via Libc-alpha wrote: >>> No change to the code other than moving the function to >>> dl-new-hash.h. Changed name so its now in the reserved namespace. >>> --- >>> elf/dl-lookup.c | 13 ++----------- >>> elf/dl-new-hash.h | 40 ++++++++++++++++++++++++++++++++++++++++ >>> 2 files changed, 42 insertions(+), 11 deletions(-) >>> create mode 100644 elf/dl-new-hash.h >>> >>> diff --git a/elf/dl-lookup.c b/elf/dl-lookup.c >>> index 989b073e4f..a42f6d5390 100644 >>> --- a/elf/dl-lookup.c >>> +++ b/elf/dl-lookup.c >>> @@ -24,6 +24,7 @@ >>> #include >>> #include >>> #include >>> +#include >>> #include >>> #include >>> #include >>> @@ -558,16 +559,6 @@ skip: >>> } >>> >>> >>> -static uint32_t >>> -dl_new_hash (const char *s) >>> -{ >>> - uint32_t h = 5381; >>> - for (unsigned char c = *s; c != '\0'; c = *++s) >>> - h = h * 33 + c; >>> - return h; >>> -} >>> - >>> - >>> /* Add extra dependency on MAP to UNDEF_MAP. */ >>> static int >>> add_dependency (struct link_map *undef_map, struct link_map *map, int flags) >>> @@ -816,7 +807,7 @@ _dl_lookup_symbol_x (const char *undef_name, struct link_map *undef_map, >>> const struct r_found_version *version, >>> int type_class, int flags, struct link_map *skip_map) >>> { >>> - const unsigned int new_hash = dl_new_hash (undef_name); >>> + const unsigned int new_hash = _dl_new_hash (undef_name); >>> unsigned long int old_hash = 0xffffffff; >>> struct sym_val current_value = { NULL, NULL }; >>> struct r_scope_elem **scope = symbol_scope; >>> diff --git a/elf/dl-new-hash.h b/elf/dl-new-hash.h >>> new file mode 100644 >>> index 0000000000..8641bb4196 >>> --- /dev/null >>> +++ b/elf/dl-new-hash.h >>> @@ -0,0 +1,40 @@ >>> +/* _dl_new_hash for elf symbol lookup >>> + Copyright (C) 2022 Free Software Foundation, Inc. >>> + This file is part of the GNU C Library. >>> + >>> + The GNU C Library is free software; you can redistribute it and/or >>> + modify it under the terms of the GNU Lesser General Public >>> + License as published by the Free Software Foundation; either >>> + version 2.1 of the License, or (at your option) any later version. >>> + >>> + The GNU C Library is distributed in the hope that it will be useful, >>> + but WITHOUT ANY WARRANTY; without even the implied warranty of >>> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >>> + Lesser General Public License for more details. >>> + >>> + You should have received a copy of the GNU Lesser General Public >>> + License along with the GNU C Library; if not, see >>> + . */ >>> + >>> +#ifndef _DL_NEW_HASH_H >>> +#define _DL_NEW_HASH_H 1 >>> + >>> +#include >>> +/* For __always_inline. */ >>> +#include >>> + >>> +static __always_inline uint32_t >>> +__attribute__ ((unused)) >>> +_dl_new_hash (const char *s) >>> +{ >>> + uint32_t h = 5381; >>> + for (unsigned char c = *s; c != '\0'; c = *++s) >>> + h = h * 33 + c; >>> + return h; >>> +} >>> + >>> +/* For testing/benchmarking purposes. */ >>> +#define __simple_dl_new_hash _dl_new_hash >>> + >>> + >>> +#endif /* dl-new-hash.h */ >> >> Uhmm, you're going to call it __simple_dl_new_hash. A bit roundabout, >> but OK. > > It's in a header. Doesn't it need to be in reserved namespace? Ah not that. What I'd have done was to keep it as a real implementation and just have the sysdeps one override it, like some of the posix implementations that return ENOSYS and then implementations in sysdeps override them. Then you include in the test case as a reference implementations, like the string tests do for some of the C string implementations. But what you've done is also fine. Siddhesh