Re: [PATCH v2] RISC-V: Fix IFUNC resolver cannot access gp pointer

[Kito Cheng <kito.cheng@sifive.com>]

Hi Palmer:

> >>> In some cases, an IFUNC resolver may need to access the gp pointer to
> >>> resolve the function address. Such an object may have l_relocated == 0.
> >>> In this case, the GP register will not be set up. Thus, the IFUNC
> >>> resolver cannot access the gp pointer. This patch fixes this issue by
> >>> relaxing the check of l_relocated in elf_machine_runtime_setup.
> >>>
> >>> As for the original Bug 31317, since the static-linked executable has
> >>> already set up the gp pointer, we don't need to execute the code to set
> >>> up the gp pointer again. I have also reproduced and checked Bug 31317,
> >>> this patch can fix the issue.
> >
> > We had a pretty similar issue with the hwprobe IFUNC stuff, and IIRC the result there was that it's just not safe to look up global symbols from an IFUNC.

Do you remember if there are any other issues that need to be resolved
other than the GP issue?

Or do you think it's not the right solution for the GP issue or it may
have potential risk?

> >
>
> However, using global variables in IFUNC is unavoidable when using
> the data structure __riscv_feature_bits from RISC-V C-API [1], which
> stores a global variable and is initialized by a function in libgcc
> (GCC) [2] or compiler-rt (LLVM) [3]. I encountered this bug while
> implementing target_clones and target_version for RISC-V GCC [4].
>
> I have no better idea to solve this problem. Perhaps we should add
> some new restrictions to the ABI?
>
> [1] https://github.com/riscv-non-isa/riscv-c-api-doc/blob/main/src/c-api.adoc#extension-bitmask
> [2] https://github.com/gcc-mirror/gcc/blob/master/libgcc/config/riscv/feature_bits.c
> [3] https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/builtins/cpu_model/riscv.c
> [4] https://patchwork.sourceware.org/project/gcc/list/?series=39863&state=*
>
> >>> Closes: BZ #32269
> >>> Fixes: 96d1b9ac23 ("RISC-V: Fix the static-PIE non-relocated object check")
> >>> Signed-off-by: Yangyu Chen <cyy@cyyself.name>
> >>> ---
> >>>  sysdeps/riscv/dl-machine.h | 4 +++-
> >>>  1 file changed, 3 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h
> >>> index b2f28697f7..10a36d6701 100644
> >>> --- a/sysdeps/riscv/dl-machine.h
> >>> +++ b/sysdeps/riscv/dl-machine.h
> >>> @@ -348,7 +348,8 @@ elf_machine_runtime_setup (struct link_map *l, struct r_scope_elem *scope[],
> >>>        gotplt[1] = (ElfW(Addr)) l;
> >>>      }
> >>>
> >>> -  if (l->l_type == lt_executable && l->l_relocated)
> >>> +#ifdef SHARED
> >>> +  if (l->l_type == lt_executable)
> >>>      {
> >>>        /* The __global_pointer$ may not be defined by the linker if the
> >>>    $gp register does not be used to access the global variable
> >>> @@ -368,6 +369,7 @@ elf_machine_runtime_setup (struct link_map *l, struct r_scope_elem *scope[],
> >>>            : "r" (ref->st_value)
> >>>          );
> >>>      }
> >>> +#endif
> >>>  #endif
> >>>    return lazy;
> >>>  }
>
>