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

[Yangyu Chen <cyy@cyyself.name>]

> On Oct 30, 2024, at 11:10, Palmer Dabbelt <palmer@rivosinc.com> wrote:
> 
> On Tue, 29 Oct 2024 19:58:14 PDT (-0700), cyy@cyyself.name wrote:
>> Friendly ping.
>> 
>> FYI, this bug can be reproduced through the steps outlined in the BZ #32269.
>> 
>> Link: https://sourceware.org/bugzilla/show_bug.cgi?id=32269
>> 
>> On 10/16/24 01:01, Yangyu Chen wrote:
>>> 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.
> 

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;
>>>  }