From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x1032.google.com (mail-pj1-x1032.google.com [IPv6:2607:f8b0:4864:20::1032]) by sourceware.org (Postfix) with ESMTPS id 0775F385841F for ; Thu, 24 Feb 2022 20:56:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0775F385841F Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=dabbelt.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=dabbelt.com Received: by mail-pj1-x1032.google.com with SMTP id j10-20020a17090a94ca00b001bc2a9596f6so2995201pjw.5 for ; Thu, 24 Feb 2022 12:56:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=dabbelt-com.20210112.gappssmtp.com; s=20210112; h=date:subject:in-reply-to:cc:from:to:message-id:mime-version :content-transfer-encoding; bh=+4/+JGkV5zwiLA27Ieb0SvN2D+lj8Wrbn+OzzKysTvk=; b=sGJXzykbm0VvS6FuMGRM4D/3BweJkF890Zlxt2S4tZHhKrkTDDxW4eBrXWOrbk077Z 4gPdVdZ3WM2tiNf9jXSmkodSlsNoskgWn+bIYnHBC0vitoIUN495QAplwbdQsYIixnyW 3PrcMK3K3VH0lhfzEUyMxMJ40VaA7X5tgGQJ1K9udaJMrlSYtlAKx8CZ08mgTwnYIsyA mVWg3mczWFk1lQl9+y/v4USM79hDb6hpcQTMI5ggv1+nxmiYkUtCIhhRq3e8VEwe9UlI u7V78rkxSDrz0PGY/zmW2F0k1XKyWiQKWbeig3B2L8Ua7gqLri+PeP9Zp4AfMfFkYGAq oUPw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:subject:in-reply-to:cc:from:to:message-id :mime-version:content-transfer-encoding; bh=+4/+JGkV5zwiLA27Ieb0SvN2D+lj8Wrbn+OzzKysTvk=; b=l27PdOpaZyspELvjIIPbdjZ4pgv9pMhMn2Pe7uUGVj6wqMHLaZsVzGD068GUr6+sDZ 940gyrmNtrLEhEPVkif9avF6tzHeFWvHlIqCAl/vbytwIeY0TsMA3WBb1uTkqdnHmPDE sQAbhuVilrCSOKK5tO3hMkFnQ9DMorCkiLNZ1D543ytDyeuFNUp/43CPovkS9Q6YY/X0 END8Mfu6mPMp3QNJ47TvzmFuFjzq5wW38RNveHerwA79PbaESZI4oKsQ+RrBjI89LZZe BXAtFFa68PgNY3+QoHR7LAiUvy8PLMb4l/u0xADSQxiwcWSso5GakQHezsMNcI1S4XFI IrBw== X-Gm-Message-State: AOAM530qoH6DIfs1dKrnncU5ziqULkMPG/I/+hilGcEcx6YkOh6eU9Gb VZIi1b7C6it0YD5eHgQBkrObgYhsdxgimg== X-Google-Smtp-Source: ABdhPJwbq0/2UEraUmk9/IvOgu6tIeGQIwYzRrcF9xz4hyap6+sKXW3hbKk84WNXTDhl9K32U0xvSw== X-Received: by 2002:a17:90a:168f:b0:1b9:453a:fe79 with SMTP id o15-20020a17090a168f00b001b9453afe79mr16269357pja.107.1645736214723; Thu, 24 Feb 2022 12:56:54 -0800 (PST) Received: from localhost ([12.3.194.138]) by smtp.gmail.com with ESMTPSA id mq5-20020a17090b380500b001bc770d1df4sm215982pjb.55.2022.02.24.12.56.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 24 Feb 2022 12:56:54 -0800 (PST) Date: Thu, 24 Feb 2022 12:56:54 -0800 (PST) X-Google-Original-Date: Thu, 24 Feb 2022 12:56:01 PST (-0800) Subject: Re: [PATCH v2 2/2] riscv: Resolve symbols directly for symbols with STO_RISCV_VARIANT_CC. In-Reply-To: CC: libc-alpha@sourceware.org, Darius Rad , Andrew Waterman , dj@redhat.com, kito.cheng@sifive.com, greentime.hu@sifive.com, kai.wang@sifive.com From: Palmer Dabbelt To: vincent.chen@sifive.com Message-ID: Mime-Version: 1.0 (MHng) Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE 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: Thu, 24 Feb 2022 20:56:59 -0000 On Thu, 20 Jan 2022 17:43:14 PST (-0800), vincent.chen@sifive.com wrote: > On Thu, Jan 20, 2022 at 10:21 AM Palmer Dabbelt wrote: >> >> Sorry, I missed the fixed-up patch set (which is why I just sent out a >> similar bit of documentation). >> >> On Mon, 17 Jan 2022 20:31:59 PST (-0800), vincent.chen@sifive.com wrote: >> > From: Hsiangkai Wang >> > >> > In some cases, we do not want to go through the resolver for function >> > calls. For example, functions with vector arguments will use vector >> > registers to pass arguments. In the resolver, we do not save/restore the >> > vector argument registers for lazy binding efficiency. To avoid ruining >> > the vector arguments, functions with vector arguments will not go >> > through the resolver. >> > >> > To achieve the goal, we will annotate the function symbols with >> > STO_RISCV_VARIANT_CC flag and add DT_RISCV_VARIANT_CC tag in the dynamic >> > section. In the first pass on PLT relocations, we do not set up to call >> > _dl_runtime_resolve. Instead, we resolve the functions directly. >> > >> > Signed-off-by: Hsiangkai Wang >> > Signed-off-by: Vincent Chen >> > --- >> > elf/elf.h | 7 +++++++ >> > manual/platform.texi | 6 ++++++ >> > sysdeps/riscv/dl-dtprocnum.h | 21 +++++++++++++++++++++ >> > sysdeps/riscv/dl-machine.h | 26 ++++++++++++++++++++++++++ >> > 4 files changed, 60 insertions(+) >> > create mode 100644 sysdeps/riscv/dl-dtprocnum.h >> > >> > diff --git a/elf/elf.h b/elf/elf.h >> > index 0735f6b579..9c95544050 100644 >> > --- a/elf/elf.h >> > +++ b/elf/elf.h >> > @@ -3911,6 +3911,13 @@ enum >> > >> > #define R_TILEGX_NUM 130 >> > >> > +/* RISC-V specific values for the Dyn d_tag field. */ >> > +#define DT_RISCV_VARIANT_CC (DT_LOPROC + 1) >> > +#define DT_RISCV_NUM 2 >> > + >> > +/* RISC-V specific values for the st_other field. */ >> > +#define STO_RISCV_VARIANT_CC 0x80 >> > + >> > /* RISC-V ELF Flags */ >> > #define EF_RISCV_RVC 0x0001 >> > #define EF_RISCV_FLOAT_ABI 0x0006 >> > diff --git a/manual/platform.texi b/manual/platform.texi >> > index d5fdc5bd05..a1a740f381 100644 >> > --- a/manual/platform.texi >> > +++ b/manual/platform.texi >> > @@ -121,6 +121,12 @@ when it is not allowed, the priority is set to medium. >> > @node RISC-V >> > @appendixsec RISC-V-specific Facilities >> > >> > +Functions that are lazily bound must be compatible with the standard calling >> > +convention. When a function is annotated with STO_RISCV_VARIANT_CC, it means >> > +this function is not compatible with the standard calling convention. The >> > +dynamic linker will directly resolve it instead of using the lazy binding >> > +mechanism. >> >> IMO this is the wrong way to go: we're essentially re-defining a bit >> used be the standard ABI to mean something else. I guess we've already >> defacto forked from the psABI with that "standard calling convention" >> language, but IMO it'd be prudent to use a different bit to represent >> this new behavior. In the long term one could imagine trying to get >> back in line with the psABI, but if we're repurposing two bit patterns >> it'll be a bit harder than if we're just repurposing one. >> > OK, I understand. I reviewed the psABI spec again and did some > modifications. Did you think is it better? > > Functions that are lazily bound must be compatible with the standard > calling convention. Any functions that use additional argument > registers must be annotated with STO_RISCV_VARIANT_CC. To prevent > these additional argument registers from being corrupted during the > lazy binding process, this patch makes such functions be always > resolved at load time, not lazily. I was trying to suggest using a different bit (with a different name) for the "does not follow the standard calling convention" behavior, rather than re-defining the bit allocated for STO_RISCV_VARIANT_CC in the psABI for that behavior. Maybe it just doesn't matter, given that we're forking, but re-using the same bit will just make things more confusing for everyone in the future. Aside from that the original text looked OK. >> > + >> > Cache management facilities specific to RISC-V systems that implement the Linux >> > ABI are declared in @file{sys/cachectl.h}. >> > >> > diff --git a/sysdeps/riscv/dl-dtprocnum.h b/sysdeps/riscv/dl-dtprocnum.h >> > new file mode 100644 >> > index 0000000000..f189fd700a >> > --- /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 >> > diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h >> > index 1d3e2e588c..cdbaca6533 100644 >> > --- a/sysdeps/riscv/dl-machine.h >> > +++ b/sysdeps/riscv/dl-machine.h >> > @@ -53,6 +53,9 @@ >> > || (__WORDSIZE == 64 && (type) == R_RISCV_TLS_TPREL64))) \ >> > | (ELF_RTYPE_CLASS_COPY * ((type) == R_RISCV_COPY))) >> > >> > +//* Translate a processor specific dynamic tag to the index in l_info array. */ >> > +#define DT_RISCV(x) (DT_RISCV_##x - DT_LOPROC + DT_NUM) >> > + >> > /* Return nonzero iff ELF header is compatible with the running host. */ >> > static inline int __attribute_used__ >> > elf_machine_matches_host (const ElfW(Ehdr) *ehdr) >> > @@ -305,6 +308,29 @@ elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[], >> > /* 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)) >> > + { >> > + /* 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]; >> > + if (__glibc_unlikely (sym->st_other & STO_RISCV_VARIANT_CC)) >> > + { >> > + /* 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, scope, reloc, sym, version, reloc_addr, >> > + skip_ifunc); >> > + return; >> > + } >> > + } >> > + >> > if (__glibc_unlikely (map->l_mach.plt == 0)) >> > { >> > if (l_addr) >> >> Aside from that this one looks fine to me. >> >> Given the complexity around this psABI spec deviation and how close we >> are to release I'd prefer to wait and see if we can come up with a >> better solution, though -- for example, I'd been kicking around some >> ideas related to ELF object attributes saying "this follows the >> psABI-1.0" vs "this follows the legacy GNU psABI extensions". That way >> we could at least tag binaries that explicitly rely on this new behavior >> as such, which would give us a shot at eventually getting rid of them. > > I agree that we don't need to rush to come up with a solution in this > release. But, I have a little confused. Even if the ELF object > attribute is able to say "this follows the psABI-1.0" vs "this follows > the legacy GNU psABI extensions", we still need to use > STO_RISCV_VARIANT_CC to tell ld.so whether needs to directly resolve > this symbol. Is it correct? We need to directly resolve all symbols compatible with psABI-1.0, lazy binding will only be legal for symbols that follow the legacy GNU extensions.