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

[Yangyu Chen]

On 1/13/25 22:46, Florian Weimer wrote:
> * Yangyu Chen:
> 
>> On 1/13/25 20:50, Florian Weimer wrote:
>>> * Yangyu Chen:
>>>
>>>> 2. There is no ABI grey area now.
>>>>
>>>>   From [3] we have: `__global_pointer$` must be exported in the dynamic
>>>> symbol table of dynamically-linked executables if there are any
>>>> GP-relative accesses present in the executable.
>>> If you can link editors to follow this even with version scripts
>>> that
>>> use ”local: *”, it will work.  But that's a big If.
>>>
>>
>> Thanks for that pointing out. For this issue, I think we should modify
>> the psABI to force the linker to export `__global_pointer$` even with
>> the existing version script. At least, the linker should output a
>> warning message. Whatever, it is still simpler than adding something
>> like DT_RISCV_GP_VALUE.
> 
> I would strongly suggest to unconditionally create the symbol.
> 
>>> This lookup should not use the link map's scope anyway because that
>>> could included preloaded objects with a conflicting definition of
>>> __global_pointer$.  You can use _dl_lookup_direct, similar to what
>>> _dl_call_libc_early_init does.  It wants the symbol in exactly this
>>> particular object, too.
>>
>> If we have checked l->l_type == lt_executable before looking for the
>> `__global_pointer$` symbol, I think some preloaded objects will not
>> cause any conflicting definitions. Is it right? Or am I missing
>> something?
> 
> No, the scope will typically contain more than just the object itself.
> That's why I suggested to use _dl_lookup_direct.  But this can be a
> separate change, and hopefully it does not matter in practice.
> 

Yeah. I think it does not matter in practice. Because GP relaxation can 
only applied to position-dependent binaries, and ld version scripts are 
used for shared binaries that are PIC, as we checked, l_type is an 
lt_executable, so it should not be an issue.

The only thing we missed here is if a user uses version script to link 
an executable (rather than shared libraries), but for the GLIBC side, we 
can regard it as if the user or linker violates the psABI. What we need 
to do is update the psABI documentation to clarify this and change the 
linker's behavior, to not relax to GP or always export this symbol to 
get this edge, but useless case works with IFUNC.

Is there any other issue that needs to be addressed?

I think this patch looks good now for GLIBC. If we could confirm this 
patch does not cause regression in GLIBC, I think we can merge this now 
so we have a working GP initialization in the next release (like 2.41), 
and make software to use some features rely on IFUNC like target_clones 
/ target_version safely without the risk of being corrupted because of 
the GP problem we have now.

I think my points 3 and 4 in that email [1] already provide sufficient 
information to ensure this does not cause any regression in GLIBC, and 
this patch offers more opportunities to make the GP initialized 
correctly for IFUNC to work.

[1] 
https://patchwork.sourceware.org/project/glibc/patch/tencent_EA6F621A42D41AFDF99A0561B51F1CB57109@qq.com/#191991

Thanks,
Yangyu Chen