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

[Yangyu Chen <cyy@cyyself.name>]

On 10/30/24 11:10, Palmer Dabbelt wrote:
>> 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.
> 

Hi Palmer,

I think there is no ABI grey area for dynamically linked executables 
since we have the following statements in the riscv-elf doc [1]:

`__global_pointer$` must be exported in the dynamic symbol table of 
dynamically linked executables if there are any GP-relative accesses in 
the executable.

And after some discussions [2] about adding norelax attribute in the 
riscv-c-api-doc, I think this patch is the right way to go.

How do you think about this explanation?

Thanks,
Yangyu Chen

[1] 
https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blame/3799de41be668bc42cad7fac9ccc28d20f3fcb5e/riscv-elf.adoc#L389
[2] https://github.com/riscv-non-isa/riscv-c-api-doc/pull/94

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