[v2,1/5] LoongArch: Reject R_LARCH_32 from becoming a runtime reloc in ELFCLASS64
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_binutils_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_binutils_check--master-arm |
success
|
Test passed
|
Commit Message
We were converting R_LARCH_32 to R_LARCH_RELATIVE for ELFCLASS64:
$ cat t.s
.data
x:
.4byte x
.4byte 0xdeadbeef
$ as/as-new t.s -o t.o
$ ld/ld-new -shared t.o
$ objdump -R
a.out: file format elf64-loongarch
DYNAMIC RELOCATION RECORDS
OFFSET TYPE VALUE
00000000000001a8 R_LARCH_RELATIVE *ABS*+0x00000000000001a8
But this is just wrong: at runtime the dynamic linker will run
*(uintptr *)&x += load_address, clobbering the next 4 bytes of data
("0xdeadbeef" in the example).
If we keep the R_LARCH_32 reloc as-is in ELFCLASS64, it'll be rejected
by the Glibc dynamic linker anyway. And it does not make too much sense
to modify Glibc to support it. So we can just reject it like x86_64:
relocation R_X86_64_32 against `.data' can not be used when making a
shared object; recompile with -fPIC
or RISC-V:
relocation R_RISCV_32 against non-absolute symbol `a local symbol'
can not be used in RV64 when making a shared object"
Signed-off-by: Xi Ruoyao <xry111@xry111.site>
---
bfd/elfnn-loongarch.c | 23 ++++++++++++++++++-
.../ld-loongarch-elf/ld-loongarch-elf.exp | 1 +
.../ld-loongarch-elf/r_larch_32_elf64.d | 4 ++++
.../ld-loongarch-elf/r_larch_32_elf64.s | 3 +++
4 files changed, 30 insertions(+), 1 deletion(-)
create mode 100644 ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.d
create mode 100644 ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.s
Comments
On 2024-06-26 18:04, Xi Ruoyao wrote:
> We were converting R_LARCH_32 to R_LARCH_RELATIVE for ELFCLASS64:
>
> $ cat t.s
> .data
> x:
> .4byte x
> .4byte 0xdeadbeef
> $ as/as-new t.s -o t.o
> $ ld/ld-new -shared t.o
> $ objdump -R
> a.out: file format elf64-loongarch
>
> DYNAMIC RELOCATION RECORDS
> OFFSET TYPE VALUE
> 00000000000001a8 R_LARCH_RELATIVE *ABS*+0x00000000000001a8
>
> But this is just wrong: at runtime the dynamic linker will run
> *(uintptr *)&x += load_address, clobbering the next 4 bytes of data
> ("0xdeadbeef" in the example).
>
> If we keep the R_LARCH_32 reloc as-is in ELFCLASS64, it'll be rejected
> by the Glibc dynamic linker anyway. And it does not make too much sense
> to modify Glibc to support it. So we can just reject it like x86_64:
>
> relocation R_X86_64_32 against `.data' can not be used when making a
> shared object; recompile with -fPIC
>
> or RISC-V:
>
> relocation R_RISCV_32 against non-absolute symbol `a local symbol'
> can not be used in RV64 when making a shared object"
>
> Signed-off-by: Xi Ruoyao <xry111@xry111.site>
> ---
> bfd/elfnn-loongarch.c | 23 ++++++++++++++++++-
> .../ld-loongarch-elf/ld-loongarch-elf.exp | 1 +
> .../ld-loongarch-elf/r_larch_32_elf64.d | 4 ++++
> .../ld-loongarch-elf/r_larch_32_elf64.s | 3 +++
> 4 files changed, 30 insertions(+), 1 deletion(-)
> create mode 100644 ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.d
> create mode 100644 ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.s
>
> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> index b1720760475..3a55ac93e20 100644
> --- a/bfd/elfnn-loongarch.c
> +++ b/bfd/elfnn-loongarch.c
> @@ -2861,7 +2861,28 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info,
> /* No alloc space of func allocate_dynrelocs. */
> if (unresolved_reloc
> && !(h && (h->is_weakalias || !h->dyn_relocs)))
> - loongarch_elf_append_rela (output_bfd, sreloc, &outrel);
> + {
> + if (is_pic && r_type != R_LARCH_NN)
> + {
> + /* Not to use ELFCLASSNN in string literal or it'll
> + puzzle gettext. */
> +
> + /* xgettext:c-format */
> + char *msg = bfd_asprintf (
> + _("reloc is unresolved and cannot be turned to "
> + "a runtime reloc in ELFCLASS%d"),
> + NN);
> +
> + /* loongarch_reloc_is_fatal will output
> + "R_LARCH_32" or "R_LARCH_64" for us. */
I'd like to do this rejection earlier in `check_relocs` than
`relocate_section`. Generally loongarch32 do not produce R_LARCH_64,
so this rejection should be efficient only for R_LARCH_32 on loongarch64.
Thanks.
> + fatal = loongarch_reloc_is_fatal (
> + info, input_bfd, input_section, rel, howto,
> + bfd_reloc_notsupported, is_undefweak, name, msg);
> + }
> + else
> + loongarch_elf_append_rela (output_bfd, sreloc,
> + &outrel);
> + }
> }
>
> relocation += rel->r_addend;
> diff --git a/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp b/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp
> index 2d67c4f2668..4d9f4aebaff 100644
> --- a/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp
> +++ b/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp
> @@ -132,6 +132,7 @@ if [istarget "loongarch64-*-*"] {
> run_dump_test "reloc_le_with_shared"
> run_dump_test "reloc_ler_with_shared"
> run_dump_test "reloc_abs_with_shared"
> + run_dump_test "r_larch_32_elf64"
> }
>
> if [check_pie_support] {
> diff --git a/ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.d b/ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.d
> new file mode 100644
> index 00000000000..df61f3a36c5
> --- /dev/null
> +++ b/ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.d
> @@ -0,0 +1,4 @@
> +#name: R_LARCH_32 in ELFCLASS64
> +#source: r_larch_32_elf64.s
> +#ld: -shared -melf64loongarch
> +#error: R_LARCH_32 against `x':\nreloc is unresolved and cannot be turned to a runtime reloc in ELFCLASS64
> diff --git a/ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.s b/ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.s
> new file mode 100644
> index 00000000000..6649f2bce01
> --- /dev/null
> +++ b/ld/testsuite/ld-loongarch-elf/r_larch_32_elf64.s
> @@ -0,0 +1,3 @@
> +.data
> +x:
> + .4byte x
On Thu, 2024-06-27 at 18:58 +0800, Jinyang He wrote:
/* snip */
> > @@ -2861,7 +2861,28 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info,
> > /* No alloc space of func allocate_dynrelocs. */
> > if (unresolved_reloc
> > && !(h && (h->is_weakalias || !h->dyn_relocs)))
> > - loongarch_elf_append_rela (output_bfd, sreloc, &outrel);
> > + {
> > + if (is_pic && r_type != R_LARCH_NN)
> > + {
> > + /* Not to use ELFCLASSNN in string literal or it'll
> > + puzzle gettext. */
> > +
> > + /* xgettext:c-format */
> > + char *msg = bfd_asprintf (
> > + _("reloc is unresolved and cannot be turned to "
> > + "a runtime reloc in ELFCLASS%d"),
> > + NN);
> > +
> > + /* loongarch_reloc_is_fatal will output
> > + "R_LARCH_32" or "R_LARCH_64" for us. */
> I'd like to do this rejection earlier in `check_relocs` than
> `relocate_section`. Generally loongarch32 do not produce R_LARCH_64,
> so this rejection should be efficient only for R_LARCH_32 on loongarch64.
Ok, in V3 I'll use the same approach as RISC-V then.
On Thu, 2024-06-27 at 20:39 +0800, Xi Ruoyao wrote:
> > I'd like to do this rejection earlier in `check_relocs` than
> > `relocate_section`. Generally loongarch32 do not produce R_LARCH_64,
> > so this rejection should be efficient only for R_LARCH_32 on
> > loongarch64.
>
> Ok, in V3 I'll use the same approach as RISC-V then.
And after some thinking: R_LARCH_64 on loongarch32 should be fine. On
little-endian hardware *(uint64 *)pc += load_addr should be same as
*(uint32 *)pc += load_addr unless the latter wraps, but if it wraps
Glibc dynamic linker should complain anyway.
RISC-V also converts R_RISCV_64 to R_RISCV_RELATIVE for rv32.
On 2024-06-28 01:19, Xi Ruoyao wrote:
> On Thu, 2024-06-27 at 20:39 +0800, Xi Ruoyao wrote:
>>> I'd like to do this rejection earlier in `check_relocs` than
>>> `relocate_section`. Generally loongarch32 do not produce R_LARCH_64,
>>> so this rejection should be efficient only for R_LARCH_32 on
>>> loongarch64.
>> Ok, in V3 I'll use the same approach as RISC-V then.
> And after some thinking: R_LARCH_64 on loongarch32 should be fine. On
> little-endian hardware *(uint64 *)pc += load_addr should be same as
> *(uint32 *)pc += load_addr unless the latter wraps, but if it wraps
> Glibc dynamic linker should complain anyway.
>
> RISC-V also converts R_RISCV_64 to R_RISCV_RELATIVE for rv32.
Yes, you're right. I think we cannot avoid asm like `.8byte .L1` or
`.8byte .L1 - .L2`, so emiting 64bits static reloc type on loongarch32
makes sense.
On Thu, Jun 27, 2024 at 6:53 PM Jinyang He <hejinyang@loongson.cn> wrote:
>
> On 2024-06-28 01:19, Xi Ruoyao wrote:
>
> > On Thu, 2024-06-27 at 20:39 +0800, Xi Ruoyao wrote:
> >>> I'd like to do this rejection earlier in `check_relocs` than
> >>> `relocate_section`. Generally loongarch32 do not produce R_LARCH_64,
> >>> so this rejection should be efficient only for R_LARCH_32 on
> >>> loongarch64.
> >> Ok, in V3 I'll use the same approach as RISC-V then.
> > And after some thinking: R_LARCH_64 on loongarch32 should be fine. On
> > little-endian hardware *(uint64 *)pc += load_addr should be same as
> > *(uint32 *)pc += load_addr unless the latter wraps, but if it wraps
> > Glibc dynamic linker should complain anyway.
> >
> > RISC-V also converts R_RISCV_64 to R_RISCV_RELATIVE for rv32.
> Yes, you're right. I think we cannot avoid asm like `.8byte .L1` or
> `.8byte .L1 - .L2`, so emiting 64bits static reloc type on loongarch32
> makes sense.
>
x86-32, aarch32, ppc32, and s390 ports do not support `.quad .L1`.
mips32 and riscv32 accept `.quad .L1`, which might be a mistake.
Why cannot '.8byte .L1` be avoided?
在 2024/6/29 1:04, Fangrui Song 写道:
> On Thu, Jun 27, 2024 at 6:53 PM Jinyang He <hejinyang@loongson.cn> wrote:
>> On 2024-06-28 01:19, Xi Ruoyao wrote:
>>
>>> On Thu, 2024-06-27 at 20:39 +0800, Xi Ruoyao wrote:
>>>>> I'd like to do this rejection earlier in `check_relocs` than
>>>>> `relocate_section`. Generally loongarch32 do not produce R_LARCH_64,
>>>>> so this rejection should be efficient only for R_LARCH_32 on
>>>>> loongarch64.
>>>> Ok, in V3 I'll use the same approach as RISC-V then.
>>> And after some thinking: R_LARCH_64 on loongarch32 should be fine. On
>>> little-endian hardware *(uint64 *)pc += load_addr should be same as
>>> *(uint32 *)pc += load_addr unless the latter wraps, but if it wraps
>>> Glibc dynamic linker should complain anyway.
>>>
>>> RISC-V also converts R_RISCV_64 to R_RISCV_RELATIVE for rv32.
>> Yes, you're right. I think we cannot avoid asm like `.8byte .L1` or
>> `.8byte .L1 - .L2`, so emiting 64bits static reloc type on loongarch32
>> makes sense.
>>
> x86-32, aarch32, ppc32, and s390 ports do not support `.quad .L1`.
> mips32 and riscv32 accept `.quad .L1`, which might be a mistake.
>
> Why cannot '.8byte .L1` be avoided?
Thank you for providing the detailed list. To be frankly, I sometimes see
how RISCV does and think how LoongArch should do. I'm sorry for lack
of in-depth thinking.
I think users may handwrite asm codes for 32-bits and 64 bits machines.
If we accept it, we can reduce the error. Users need to understand that
the value is 8 bytes as they excepted rather than the arch bits size.
Compilers such as gcc should not generate these asm codes.
If it is a clear mistake, I think we should fix it.
Thanks.
On Sat, 2024-06-29 at 21:56 +0800, Jinyang He wrote:
> 在 2024/6/29 1:04, Fangrui Song 写道:
>
> > On Thu, Jun 27, 2024 at 6:53 PM Jinyang He <hejinyang@loongson.cn> wrote:
> > > On 2024-06-28 01:19, Xi Ruoyao wrote:
> > >
> > > > On Thu, 2024-06-27 at 20:39 +0800, Xi Ruoyao wrote:
> > > > > > I'd like to do this rejection earlier in `check_relocs` than
> > > > > > `relocate_section`. Generally loongarch32 do not produce R_LARCH_64,
> > > > > > so this rejection should be efficient only for R_LARCH_32 on
> > > > > > loongarch64.
> > > > > Ok, in V3 I'll use the same approach as RISC-V then.
> > > > And after some thinking: R_LARCH_64 on loongarch32 should be fine. On
> > > > little-endian hardware *(uint64 *)pc += load_addr should be same as
> > > > *(uint32 *)pc += load_addr unless the latter wraps, but if it wraps
> > > > Glibc dynamic linker should complain anyway.
> > > >
> > > > RISC-V also converts R_RISCV_64 to R_RISCV_RELATIVE for rv32.
> > > Yes, you're right. I think we cannot avoid asm like `.8byte .L1` or
> > > `.8byte .L1 - .L2`, so emiting 64bits static reloc type on loongarch32
> > > makes sense.
> > >
> > x86-32, aarch32, ppc32, and s390 ports do not support `.quad .L1`.
> > mips32 and riscv32 accept `.quad .L1`, which might be a mistake.
> >
> > Why cannot '.8byte .L1` be avoided?
> Thank you for providing the detailed list. To be frankly, I sometimes see
> how RISCV does and think how LoongArch should do. I'm sorry for lack
> of in-depth thinking.
>
> I think users may handwrite asm codes for 32-bits and 64 bits machines.
For supporting this IMO we should add more functionalities like
ld.p/st.p/ldptr.p/stptr.p/addi.p/add.p/sub.p
which is assembled to ld.d etc. for lp64, and ld.w etc. for ilp32. Now
every project is defining a bunch of macros for these, and IMO it's
better to just support them in GAS so new projects won't need to define
their own macros anymore.
And similarly, for hard coding a pointer to x we can also add a
directive like ".ptr x", which behaves like ".4byte x" for ilp32 and
".8byte x" for lp64.
I'm modifying my code to "do things RISC-V is doing" for now. When we
finalize ilp32 support we can revise.
@@ -2861,7 +2861,28 @@ loongarch_elf_relocate_section (bfd *output_bfd, struct bfd_link_info *info,
/* No alloc space of func allocate_dynrelocs. */
if (unresolved_reloc
&& !(h && (h->is_weakalias || !h->dyn_relocs)))
- loongarch_elf_append_rela (output_bfd, sreloc, &outrel);
+ {
+ if (is_pic && r_type != R_LARCH_NN)
+ {
+ /* Not to use ELFCLASSNN in string literal or it'll
+ puzzle gettext. */
+
+ /* xgettext:c-format */
+ char *msg = bfd_asprintf (
+ _("reloc is unresolved and cannot be turned to "
+ "a runtime reloc in ELFCLASS%d"),
+ NN);
+
+ /* loongarch_reloc_is_fatal will output
+ "R_LARCH_32" or "R_LARCH_64" for us. */
+ fatal = loongarch_reloc_is_fatal (
+ info, input_bfd, input_section, rel, howto,
+ bfd_reloc_notsupported, is_undefweak, name, msg);
+ }
+ else
+ loongarch_elf_append_rela (output_bfd, sreloc,
+ &outrel);
+ }
}
relocation += rel->r_addend;
@@ -132,6 +132,7 @@ if [istarget "loongarch64-*-*"] {
run_dump_test "reloc_le_with_shared"
run_dump_test "reloc_ler_with_shared"
run_dump_test "reloc_abs_with_shared"
+ run_dump_test "r_larch_32_elf64"
}
if [check_pie_support] {
new file mode 100644
@@ -0,0 +1,4 @@
+#name: R_LARCH_32 in ELFCLASS64
+#source: r_larch_32_elf64.s
+#ld: -shared -melf64loongarch
+#error: R_LARCH_32 against `x':\nreloc is unresolved and cannot be turned to a runtime reloc in ELFCLASS64
new file mode 100644
@@ -0,0 +1,3 @@
+.data
+x:
+ .4byte x