Re: [PATCH] riscv: Resolve symbols directly for symbols with STO_RISCV_VARIANT_CC.

[DJ Delorie via Libc-alpha <libc-alpha@sourceware.org>]

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 <kai.wang@sifive.com> 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
> +   <https://www.gnu.org/licenses/>.  */
> +
> +/* 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)