Message ID | 20230326111334.9920-1-xry111@xry111.site |
---|---|
State | New |
Headers | show |
Series | LoongArch: ldconfig: Ignore EF_LARCH_OBJABI_V1 in shared objects | expand |
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? Why isn't this handled below via SUPPORTED_ELF_FLAGS? > > /* LoongArch linkers encode the floating point ABI as part of the ELF headers. */ > switch (flags & SUPPORTED_ELF_FLAGS)
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. > 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.
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. >
On Mon, 2023-03-27 at 13:54 -0300, Adhemerval Zanella Netto wrote: > 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. */ Looks like Yinyu has already landed the patch. Yinyu: could you add this comment before the changed line? I don't have a write access.
On 3/27/23 12:54, Adhemerval Zanella Netto wrote: > > > 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: OK, so the final form of the shared objects is compatible, this is only about static link compatibility and non-shared relocations. I would suggest reviewing ".gnu_attribute" for an extension to Ehdr->ef_flags that other architectures use, like ppc64le, to do such compatibility checking and error detection. >> 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. */ Agreed. commit 07dd75589ecbedec5162a5645d57f8bd093a45db Author: caiyinyu <caiyinyu@loongson.cn> Date: Tue Mar 28 09:19:53 2023 +0800 LoongArch: ldconfig: Add comments for using EF_LARCH_OBJABI_V1 We added Adhemerval Zanella's comment to explain the reason for using EF_LARCH_OBJABI_V1. And the comment is now in the sources. This is better, but I have a post-commit review comment here about the overall usage of ELF flags. While it is possible to use Ehdr->e_flags for such purposes as marking static relocation compatibility, you also have the broader use of ".gnu_attribute" via: GNU Object Attributes: https://sourceware.org/binutils/docs/as/GNU-Object-Attributes.html Which allows a richer set of non-flag-based attributes to be used for object files and for static link diagnostics. -- Cheers, Carlos.
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; /* LoongArch linkers encode the floating point ABI as part of the ELF headers. */ switch (flags & SUPPORTED_ELF_FLAGS)