Re: [PATCH] LoongArch: ldconfig: Ignore EF_LARCH_OBJABI_V1 in shared objects

[Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>]

On 27/03/23 11:57, Xi Ruoyao wrote:
> On Mon, 2023-03-27 at 09:44 -0400, Carlos O'Donell wrote:
>> On 3/26/23 07:13, Xi Ruoyao via Libc-alpha wrote:
>>> Binutils 2.40 sets EF_LARCH_OBJABI_V1 for shared objects:
>>>
>>>     $ ld --version | head -n1
>>>     GNU ld (GNU Binutils) 2.40
>>>     $ echo 'int dummy;' > dummy.c
>>>     $ cc dummy.c -shared
>>>     $ readelf -h a.out | grep Flags
>>>     Flags:                             0x43, DOUBLE-FLOAT, OBJ-v1
>>>
>>> We need to ignore it in ldconfig or ldconfig will consider all shared
>>> objects linked by Binutils 2.40 "unsupported".  Maybe we should stop
>>> setting EF_LARCH_OBJABI_V1 for shared objects, but Binutils 2.40 is
>>> already released and we cannot change it.
>>> ---
>>>  sysdeps/unix/sysv/linux/loongarch/readelflib.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/sysdeps/unix/sysv/linux/loongarch/readelflib.c b/sysdeps/unix/sysv/linux/loongarch/readelflib.c
>>> index bcaff86b36..ceba355959 100644
>>> --- a/sysdeps/unix/sysv/linux/loongarch/readelflib.c
>>> +++ b/sysdeps/unix/sysv/linux/loongarch/readelflib.c
>>> @@ -40,7 +40,7 @@ process_elf_file (const char *file_name, const char *lib, int *flag,
>>>  
>>>    ret = process_elf64_file (file_name, lib, flag, isa_level, soname,
>>>                                 file_contents, file_length);
>>> -  flags = elf64_header->e_flags;
>>> +  flags = elf64_header->e_flags & ~EF_LARCH_OBJABI_V1;
>>
>> Are such objects ABI compatible?
> 
> This flag was designed for indicating the relocation type usage in a .o
> file:
> 
> If EF_LARCH_OBJABI_V1 is set, it's allowed to use relocation types with
> ID in [64, 100], but it's prohibited to use relocation types with ID in
> [22, 46].
> 
> If EF_LARCH_OBJABI_V1 is not set, it's allowed to use relocation types
> with ID in [22, 46], but it's prohibited to use relocation types with ID
> in [64, 100].
> 
> A linker may only support the .o files with EF_LARCH_OBJABI_V1 unset
> (for example, GNU ld 2.38), or only support the .o files with
> EF_LARCH_OBJABI_V1 set (for example, LLD under review at
> https://reviews.llvm.org/D138135), or support both (for example, GNU ld
> 2.40).
> 
> If a linker supports both, it's OK to link a EF_LARCH_OBJABI_V1 .o file
> together with a non-EF_LARCH_OBJABI_V1 .o file into an executable or
> shared object.
> 
> But none of relocation type ID in [22, 46] or [64, 100] is runtime
> relocation.  I. e. those relocation types should not show up in a shared
> object at all (if one shows up, the linker is buggy).  So
> EF_LARCH_OBJABI_V1 makes no difference for shared objects. 

Since the boat is already sailed with 2.40, I think the proposed patch
is fine (it would be better if you indeed fix it on 2.41).  I would 
just like to add a comment on why this is required:

  /* The EF_LARCH_OBJABI_V1 flag indicate which set of static relocations
     the object might use and it only considered during static linking, 
     it does not reflect in runtime relocations.  However some binutils 
     version might set it on dynamic shared object, so clear it to avoid 
     see the SO as unsupported.  */ 

> 
>> Why isn't this handled below via SUPPORTED_ELF_FLAGS?
> 
> If we add this into SUPPORTED_ELF_FLAGS, we'll need
> 
>   switch (flags & SUPPORTED_ELF_FLAGS)
>     {
>       case EF_LARCH_ABI_SOFT_FLOAT:
> +     case EF_LARCH_ABI_SOFT_FLOAT | EF_LARCH_OBJABI_V1:
>         *flag |= FLAG_LARCH_FLOAT_ABI_SOFT;
>        break;
>       case EF_LARCH_ABI_DOUBLE_FLOAT:
> +     case EF_LARCH_ABI_DOUBLE_FLOAT | EF_LARCH_OBJABI_V1:
>         *flag |= FLAG_LARCH_FLOAT_ABI_DOUBLE;
>        break;
>       default:
>         return 1;
>     }
> 
> I don't think it's better...  But if really you prefer this way I can
> send a v2.
>