From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-1.mimecast.com (us-smtp-2.mimecast.com [207.211.31.81]) by sourceware.org (Postfix) with ESMTP id 51CDC3948490 for ; Wed, 22 Apr 2020 16:27:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 51CDC3948490 Received: from mail-qt1-f199.google.com (mail-qt1-f199.google.com [209.85.160.199]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-393-aZNEJYiRO5SxPWfIb5FAyQ-1; Wed, 22 Apr 2020 12:27:51 -0400 X-MC-Unique: aZNEJYiRO5SxPWfIb5FAyQ-1 Received: by mail-qt1-f199.google.com with SMTP id g8so3106167qtq.2 for ; Wed, 22 Apr 2020 09:27:51 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:organization :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=UFybEsOCH0QpRmbogJFLvfLUd4Dh2ys8hwtvIU0vk60=; b=XdqPSIsPWwqUqs3/uhUsx5VF6eig0WYfLu65YnJfxHl4nusj8qeKEwD7Di/7XVPG5u EbqeUL32fHRv8FOI/HFnWGQblaY2vclv2j7nP5bMLa5z/6Lg87czejjEpmrOzAydxudi JH9TEZl4D5HICsK8NcBik+DlmmTEAZasvWWy5luuaW3aZybZjtaO0oz3NwtmK44gex++ ERJHJAumjlwhLjkRbk10NWAKWLU4Nc21ehcsUVi2/4WGQ/1SYz1bL291xIVNemh5wzkU YvBCDy5KNXc53vp9IeoDjYmSNnxetSFqtcp3dnLtC1srlI1oeE9uIR781R7CBUu/XjY9 qyVg== X-Gm-Message-State: AGi0PuZi9wmLOw6Rnx2m0zlYs8sy9mbkmV9a/YHLBpfK5dxL3cmTANym dnkyeRCijbH9qMKS4WHK/+JbyNP2jK05o7T1duo9AEV8Zz9pEowj+BA8gNWMyHF535LTr23imKx c9sbHqln1bxPOBaIgH2WW X-Received: by 2002:ac8:470e:: with SMTP id f14mr26927242qtp.87.1587572869967; Wed, 22 Apr 2020 09:27:49 -0700 (PDT) X-Google-Smtp-Source: APiQypKlFDRmrO/NHMvcH89BGcErEHCSsHqjofsOHZrxeC9vQwjVq+8Rrtpfn+6hE0p6e6pZOi0mbg== X-Received: by 2002:ac8:470e:: with SMTP id f14mr26927210qtp.87.1587572869587; Wed, 22 Apr 2020 09:27:49 -0700 (PDT) Received: from [192.168.1.4] (198-84-170-103.cpe.teksavvy.com. [198.84.170.103]) by smtp.gmail.com with ESMTPSA id f130sm4188662qke.22.2020.04.22.09.27.48 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 22 Apr 2020 09:27:49 -0700 (PDT) Subject: Re: [PATCH glibc 1/9] Introduce To: Mathieu Desnoyers Cc: Florian Weimer , Joseph Myers , Szabolcs Nagy , libc-alpha@sourceware.org References: <20200326155633.18236-1-mathieu.desnoyers@efficios.com> <20200326155633.18236-2-mathieu.desnoyers@efficios.com> From: Carlos O'Donell Organization: Red Hat Message-ID: <14122d2c-4455-c3a7-eaab-ee07d03703be@redhat.com> Date: Wed, 22 Apr 2020 12:27:48 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 MIME-Version: 1.0 In-Reply-To: <20200326155633.18236-2-mathieu.desnoyers@efficios.com> Content-Language: en-US X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-21.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, KAM_STOCKGEN, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, 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: 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: Wed, 22 Apr 2020 16:27:57 -0000 On 3/26/20 11:56 AM, Mathieu Desnoyers wrote: > From: Florian Weimer > > MIPS needs to ignore certain existing symbols during symbol lookup. > The old scheme uses the ELF_MACHINE_SYM_NO_MATCH macro, with an > inline function, within its own header, with a sysdeps override for > MIPS. This allows re-use of the function from another file (without > having to include or providing the default definition > for ELF_MACHINE_SYM_NO_MATCH). > > Built with build-many-glibcs.py, with manual verification that > sysdeps/mips/elf_machine_sym_no_match.h is picked up on MIPS. Tested > on aarch64-linux-gnu, i686-linux-gnu, powerpc64-linux-gnu, > s390x-linux-gnu, x86_64-linux-gnu. The refactoring from a macro to a static inline function looks good to me. There isn't anything that changes. This patch needs squashing with the second patch in the series. Technically speaking it's ready to be committed, but I want to see the squashed patch before ACK'ing it. > --- > elf/dl-lookup.c | 10 ++---- > elf/elf_machine_sym_no_match.h | 34 +++++++++++++++++++ > sysdeps/mips/dl-machine.h | 15 --------- > sysdeps/mips/elf_machine_sym_no_match.h | 43 +++++++++++++++++++++++++ > 4 files changed, 79 insertions(+), 23 deletions(-) > create mode 100644 elf/elf_machine_sym_no_match.h > create mode 100644 sysdeps/mips/elf_machine_sym_no_match.h > > diff --git a/elf/dl-lookup.c b/elf/dl-lookup.c > index 12a229f06c..807f3ea9b6 100644 > --- a/elf/dl-lookup.c > +++ b/elf/dl-lookup.c > @@ -28,18 +28,12 @@ > #include > #include > #include > +#include OK. > > #include > > -/* Return nonzero if check_match should consider SYM to fail to match a > - symbol reference for some machine-specific reason. */ > -#ifndef ELF_MACHINE_SYM_NO_MATCH > -# define ELF_MACHINE_SYM_NO_MATCH(sym) 0 > -#endif > - > #define VERSTAG(tag) (DT_NUM + DT_THISPROCNUM + DT_VERSIONTAGIDX (tag)) > > - > struct sym_val > { > const ElfW(Sym) *s; > @@ -78,7 +72,7 @@ check_match (const char *const undef_name, > if (__glibc_unlikely ((sym->st_value == 0 /* No value. */ > && sym->st_shndx != SHN_ABS > && stt != STT_TLS) > - || ELF_MACHINE_SYM_NO_MATCH (sym) > + || elf_machine_sym_no_match (sym) OK. You are my hero every time you kill a macro and make an static inline function. > || (type_class & (sym->st_shndx == SHN_UNDEF)))) > return NULL; > > diff --git a/elf/elf_machine_sym_no_match.h b/elf/elf_machine_sym_no_match.h > new file mode 100644 > index 0000000000..6e299e5ee8 > --- /dev/null > +++ b/elf/elf_machine_sym_no_match.h > @@ -0,0 +1,34 @@ > +/* Function to ignore certain symbol matches for machine-specific reasons. > + Copyright (C) 2019 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 _ELF_MACHINE_SYM_NO_MATCH_H > +#define _ELF_MACHINE_SYM_NO_MATCH_H > + > +#include > +#include > + > +/* This can be customized to ignore certain symbols during lookup in > + case there are machine-specific rules to disregard some > + symbols. */ > +static inline bool > +elf_machine_sym_no_match (const ElfW(Sym) *sym) > +{ > + return false; > +} OK. > + > +#endif /* _ELF_MACHINE_SYM_NO_MATCH_H */ > diff --git a/sysdeps/mips/dl-machine.h b/sysdeps/mips/dl-machine.h > index 06021ea9ab..e3b11a4f21 100644 > --- a/sysdeps/mips/dl-machine.h > +++ b/sysdeps/mips/dl-machine.h > @@ -467,21 +467,6 @@ elf_machine_plt_value (struct link_map *map, const ElfW(Rel) *reloc, > return value; > } > > -/* The semantics of zero/non-zero values of undefined symbols differs > - depending on whether the non-PIC ABI is in use. Under the non-PIC > - ABI, a non-zero value indicates that there is an address reference > - to the symbol and thus it must always be resolved (except when > - resolving a jump slot relocation) to the PLT entry whose address is > - provided as the symbol's value; a zero value indicates that this > - canonical-address behaviour is not required. Yet under the classic > - MIPS psABI, a zero value indicates that there is an address > - reference to the function and the dynamic linker must resolve the > - symbol immediately upon loading. To avoid conflict, symbols for > - which the dynamic linker must assume the non-PIC ABI semantics are > - marked with the STO_MIPS_PLT flag. */ > -#define ELF_MACHINE_SYM_NO_MATCH(sym) \ > - ((sym)->st_shndx == SHN_UNDEF && !((sym)->st_other & STO_MIPS_PLT)) > - OK. > #endif /* !dl_machine_h */ > > #ifdef RESOLVE_MAP > diff --git a/sysdeps/mips/elf_machine_sym_no_match.h b/sysdeps/mips/elf_machine_sym_no_match.h > new file mode 100644 > index 0000000000..f2be74caaf > --- /dev/null > +++ b/sysdeps/mips/elf_machine_sym_no_match.h > @@ -0,0 +1,43 @@ > +/* MIPS-specific handling of undefined symbols. > + Copyright (C) 2008-2019 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 _ELF_MACHINE_SYM_NO_MATCH_H > +#define _ELF_MACHINE_SYM_NO_MATCH_H > + > +#include > +#include > + > +/* The semantics of zero/non-zero values of undefined symbols differs > + depending on whether the non-PIC ABI is in use. Under the non-PIC > + ABI, a non-zero value indicates that there is an address reference > + to the symbol and thus it must always be resolved (except when > + resolving a jump slot relocation) to the PLT entry whose address is > + provided as the symbol's value; a zero value indicates that this > + canonical-address behaviour is not required. Yet under the classic > + MIPS psABI, a zero value indicates that there is an address > + reference to the function and the dynamic linker must resolve the > + symbol immediately upon loading. To avoid conflict, symbols for > + which the dynamic linker must assume the non-PIC ABI semantics are > + marked with the STO_MIPS_PLT flag. */ > +static inline bool > +elf_machine_sym_no_match (const ElfW(Sym) *sym) > +{ > + return sym->st_shndx == SHN_UNDEF && !(sym->st_other & STO_MIPS_PLT); > +} > + > +#endif /* _ELF_MACHINE_SYM_NO_MATCH_H */ > OK. Conditional matches original code. -- Cheers, Carlos.