From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTPS id 7852F3858405 for ; Fri, 29 Oct 2021 00:20:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7852F3858405 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-371-WdjZ7LxbM6mMKVydE6ncmg-1; Thu, 28 Oct 2021 20:20:46 -0400 X-MC-Unique: WdjZ7LxbM6mMKVydE6ncmg-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id E96676A29A; Fri, 29 Oct 2021 00:20:45 +0000 (UTC) Received: from greed.delorie.com (ovpn-112-186.phx2.redhat.com [10.3.112.186]) by smtp.corp.redhat.com (Postfix) with ESMTPS id BA71D1A26A; Fri, 29 Oct 2021 00:20:45 +0000 (UTC) Received: from greed.delorie.com.redhat.com (localhost [127.0.0.1]) by greed.delorie.com (8.15.2/8.15.2) with ESMTP id 19T0KiVP1764931; Thu, 28 Oct 2021 20:20:44 -0400 From: DJ Delorie To: Hsiangkai Wang Cc: libc-alpha@sourceware.org Subject: Re: [PATCH] riscv: Resolve symbols directly for symbols with STO_RISCV_VARIANT_CC. In-Reply-To: <1628480827-366232-1-git-send-email-kai.wang@sifive.com> Date: Thu, 28 Oct 2021 20:20:44 -0400 Message-ID: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-12.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Fri, 29 Oct 2021 00:20:53 -0000 This patch looks good to me and follows the spec as committed. I think it's good to have a "we can't trust the ABI" bit that bypasses all the problems with lazy relocation and ABI registers. However, it no longer compiles due to 490e6c62aa31a8aa5c4a059f6e646ede121edf0a https://sourceware.org/pipermail/libc-alpha/2021-October/131680.html https://patchwork.sourceware.org/project/glibc/patch/20211006050935.1311740-1-maskray@google.com/ Speficially, this function changed parameters: -elf_machine_rela (struct link_map *map, const ElfW(Rela) *reloc, - const ElfW(Sym) *sym, const struct r_found_version *version, +elf_machine_rela (struct link_map *map, struct r_scope_elem *scope[], + const ElfW(Rela) *reloc, const ElfW(Sym) *sym, + const struct r_found_version *version, void *const reloc_addr, int skip_ifunc) Hsiangkai Wang writes: > +/* RISC-V specific values for the Dyn d_tag field. */ > +#define DT_RISCV_VARIANT_CC (DT_LOPROC + 1) > +#define DT_RISCV_NUM 2 This matches the spec, ok. I note that (DT_LOPROC) is left unused, so while there is only one DT_* tag, the "number" of used tags is 2, with the first tag left undefined. > +/* RISC-V specific values for the st_other field. */ > +#define STO_RISCV_VARIANT_CC 0x80 This matches the spec, ok. > diff --git a/sysdeps/riscv/dl-dtprocnum.h b/sysdeps/riscv/dl-dtprocnum.h > new file mode 100644 > index 0000000..f189fd7 > --- /dev/null > +++ b/sysdeps/riscv/dl-dtprocnum.h > @@ -0,0 +1,21 @@ > +/* Configuration of lookup functions. RISC-V version. > + Copyright (C) 2019-2021 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 > + . */ > + > +/* Number of extra dynamic section entries for this architecture. By > + default there are none. */ > +#define DT_THISPROCNUM DT_RISCV_NUM Ok. > diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h > +/* Translate a processor specific dynamic tag to the index in l_info array. */ > +#define DT_RISCV(x) (DT_RISCV_##x - DT_LOPROC + DT_NUM) Ok. > @@ -299,6 +302,29 @@ elf_machine_lazy_rel (struct link_map *map, ElfW(Addr) l_addr, > /* Check for unexpected PLT reloc type. */ > if (__glibc_likely (r_type == R_RISCV_JUMP_SLOT)) > { > + if (__glibc_unlikely (map->l_info[DT_RISCV (VARIANT_CC)] != NULL)) > + { If the object MAY have CC's... > + /* Check the symbol table for variant CC symbols. */ > + const Elf_Symndx symndx = ELFW(R_SYM) (reloc->r_info); > + const ElfW(Sym) *symtab = > + (const void *)D_PTR (map, l_info[DT_SYMTAB]); > + const ElfW(Sym) *sym = &symtab[symndx]; Find the symbol we're relocating against; we already know reloc is not NULL. > + if (__glibc_unlikely (sym->st_other & STO_RISCV_VARIANT_CC)) > + { and if it IS a CC type.. > + /* Avoid lazy resolution of variant CC symbols. */ > + const struct r_found_version *version = NULL; > + if (map->l_info[VERSYMIDX (DT_VERSYM)] != NULL) > + { > + const ElfW(Half) *vernum = > + (const void *)D_PTR (map, l_info[VERSYMIDX (DT_VERSYM)]); > + version = &map->l_versions[vernum[symndx] & 0x7fff]; > + } > + elf_machine_rela (map, reloc, sym, version, reloc_addr, > + skip_ifunc); THEN we relocate it now > + return; > + } > + } > + Else we continue to keep it lazy. (conveniently, this code is the same as aarch64's) > if (__glibc_unlikely (map->l_mach.plt == 0)) > { > if (l_addr)