* [PATCH v6] MIPS: Reject branch absolute relocs for PIC for linking
@ 2024-02-21 14:52 YunQiang Su
2024-02-22 3:13 ` Hans-Peter Nilsson
0 siblings, 1 reply; 6+ messages in thread
From: YunQiang Su @ 2024-02-21 14:52 UTC (permalink / raw)
To: nickc; +Cc: binutils, macro, xry111
The asm code like:
b 8
will emit absolute relocs like:
R_MIPS_PC16 *ABS*
If they are included into PIC shared objects or PIE executables,
the branch target will be like 0x12340000, which will make the
programs crash.
---
bfd/elfxx-mips.c | 9 +++++++++
ld/testsuite/ld-mips-elf/mips-elf.exp | 1 +
ld/testsuite/ld-mips-elf/pic-reject-abs-reloc.d | 5 +++++
ld/testsuite/ld-mips-elf/pic-reject-abs-reloc.s | 2 ++
4 files changed, 17 insertions(+)
create mode 100644 ld/testsuite/ld-mips-elf/pic-reject-abs-reloc.d
create mode 100644 ld/testsuite/ld-mips-elf/pic-reject-abs-reloc.s
diff --git a/bfd/elfxx-mips.c b/bfd/elfxx-mips.c
index 69dd71419ff..9542250dec4 100644
--- a/bfd/elfxx-mips.c
+++ b/bfd/elfxx-mips.c
@@ -9258,6 +9258,15 @@ _bfd_mips_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
(h) ? h->root.root.string : "a local symbol");
break;
default:
+ if (branch_reloc_p (r_type) && r_symndx == STN_UNDEF)
+ {
+ howto = MIPS_ELF_RTYPE_TO_HOWTO (abfd, r_type, NEWABI_P (abfd));
+ info->callbacks->einfo
+ /* xgettext:c-format */
+ (_("%X%H: relocation %s against `*ABS*' cannot be used"
+ " when making a PIC/PIE object\n"),
+ abfd, sec, rel->r_offset, howto->name);
+ }
break;
}
}
diff --git a/ld/testsuite/ld-mips-elf/mips-elf.exp b/ld/testsuite/ld-mips-elf/mips-elf.exp
index 50af78d1430..a8e1b91b3a1 100644
--- a/ld/testsuite/ld-mips-elf/mips-elf.exp
+++ b/ld/testsuite/ld-mips-elf/mips-elf.exp
@@ -1678,6 +1678,7 @@ run_dump_test_o32 "pic-reloc-6"
run_dump_test_n64 "pic-reloc-7"
run_dump_test_n64 "pic-reloc-7" [list [list name (microMIPS)] \
[list as "-mmicromips"]]
+run_dump_test "pic-reject-abs-reloc"
run_dump_test_o32 "reloc-pcrel-r6"
diff --git a/ld/testsuite/ld-mips-elf/pic-reject-abs-reloc.d b/ld/testsuite/ld-mips-elf/pic-reject-abs-reloc.d
new file mode 100644
index 00000000000..16d9f4e8553
--- /dev/null
+++ b/ld/testsuite/ld-mips-elf/pic-reject-abs-reloc.d
@@ -0,0 +1,5 @@
+#name: MIPS PIC rejects branch absolute
+#ld: -shared -T pic-reloc-absolute-lo.ld
+#target: [check_shared_lib_support]
+#error: \A[^\n]*: in function `foo':\n
+#error: \(\.text\+0x0\): relocation R_MIPS_PC16 against `\*ABS\*' cannot be used when making a PIC/PIE object
diff --git a/ld/testsuite/ld-mips-elf/pic-reject-abs-reloc.s b/ld/testsuite/ld-mips-elf/pic-reject-abs-reloc.s
new file mode 100644
index 00000000000..a845fd50a77
--- /dev/null
+++ b/ld/testsuite/ld-mips-elf/pic-reject-abs-reloc.s
@@ -0,0 +1,2 @@
+foo:
+ b 8
--
2.39.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v6] MIPS: Reject branch absolute relocs for PIC for linking
2024-02-21 14:52 [PATCH v6] MIPS: Reject branch absolute relocs for PIC for linking YunQiang Su
@ 2024-02-22 3:13 ` Hans-Peter Nilsson
2024-02-22 6:29 ` YunQiang Su
0 siblings, 1 reply; 6+ messages in thread
From: Hans-Peter Nilsson @ 2024-02-22 3:13 UTC (permalink / raw)
To: YunQiang Su; +Cc: Nick Clifton, binutils, macro, xry111
A random comment:
On Wed, 21 Feb 2024, YunQiang Su wrote:
> The asm code like:
> b 8
> will emit absolute relocs like:
> R_MIPS_PC16 *ABS*
>
> If they are included into PIC shared objects or PIE executables,
> the branch target will be like 0x12340000, which will make the
> programs crash.
> ---
> bfd/elfxx-mips.c | 9 +++++++++
> ld/testsuite/ld-mips-elf/mips-elf.exp | 1 +
> ld/testsuite/ld-mips-elf/pic-reject-abs-reloc.d | 5 +++++
> ld/testsuite/ld-mips-elf/pic-reject-abs-reloc.s | 2 ++
> 4 files changed, 17 insertions(+)
> create mode 100644 ld/testsuite/ld-mips-elf/pic-reject-abs-reloc.d
> create mode 100644 ld/testsuite/ld-mips-elf/pic-reject-abs-reloc.s
>
> diff --git a/bfd/elfxx-mips.c b/bfd/elfxx-mips.c
> index 69dd71419ff..9542250dec4 100644
> --- a/bfd/elfxx-mips.c
> +++ b/bfd/elfxx-mips.c
> @@ -9258,6 +9258,15 @@ _bfd_mips_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
> (h) ? h->root.root.string : "a local symbol");
> break;
> default:
> + if (branch_reloc_p (r_type) && r_symndx == STN_UNDEF)
> + {
> + howto = MIPS_ELF_RTYPE_TO_HOWTO (abfd, r_type, NEWABI_P (abfd));
> + info->callbacks->einfo
> + /* xgettext:c-format */
> + (_("%X%H: relocation %s against `*ABS*' cannot be used"
There's no "*ABS*" in the source and IMHO that'd look confusing
to innocent users. How about "...against an absolute value"?
Or "...against an absolute value or absolute symbol"? Perhaps
the latter is a bit too wordy, but also more complete.
> + " when making a PIC/PIE object\n"),
> + abfd, sec, rel->r_offset, howto->name);
> + }
> break;
brgds, H-P
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v6] MIPS: Reject branch absolute relocs for PIC for linking
2024-02-22 3:13 ` Hans-Peter Nilsson
@ 2024-02-22 6:29 ` YunQiang Su
2024-02-22 13:13 ` Maciej W. Rozycki
0 siblings, 1 reply; 6+ messages in thread
From: YunQiang Su @ 2024-02-22 6:29 UTC (permalink / raw)
To: Hans-Peter Nilsson; +Cc: Nick Clifton, binutils, macro, xry111
> > + howto = MIPS_ELF_RTYPE_TO_HOWTO (abfd, r_type, NEWABI_P (abfd));
> > + info->callbacks->einfo
> > + /* xgettext:c-format */
> > + (_("%X%H: relocation %s against `*ABS*' cannot be used"
>
> There's no "*ABS*" in the source and IMHO that'd look confusing
> to innocent users. How about "...against an absolute value"?
> Or "...against an absolute value or absolute symbol"? Perhaps
> the latter is a bit too wordy, but also more complete.
>
Good idea. I think that we should use "...against an absolute value".
"absolute symbol" is not included in this code, as it's r_symndx is
not STN_UNDEF.
An example is gas/testsuite/gas/mips/branch-absolute.s.
Thank you,
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v6] MIPS: Reject branch absolute relocs for PIC for linking
2024-02-22 6:29 ` YunQiang Su
@ 2024-02-22 13:13 ` Maciej W. Rozycki
2024-02-25 14:08 ` YunQiang Su
0 siblings, 1 reply; 6+ messages in thread
From: Maciej W. Rozycki @ 2024-02-22 13:13 UTC (permalink / raw)
To: YunQiang Su; +Cc: Hans-Peter Nilsson, Nick Clifton, binutils, xry111
On Thu, 22 Feb 2024, YunQiang Su wrote:
> > > + howto = MIPS_ELF_RTYPE_TO_HOWTO (abfd, r_type, NEWABI_P (abfd));
> > > + info->callbacks->einfo
> > > + /* xgettext:c-format */
> > > + (_("%X%H: relocation %s against `*ABS*' cannot be used"
> >
> > There's no "*ABS*" in the source and IMHO that'd look confusing
> > to innocent users. How about "...against an absolute value"?
> > Or "...against an absolute value or absolute symbol"? Perhaps
> > the latter is a bit too wordy, but also more complete.
> >
>
> Good idea. I think that we should use "...against an absolute value".
> "absolute symbol" is not included in this code, as it's r_symndx is
> not STN_UNDEF.
Well code says otherwise:
+ if (branch_reloc_p (r_type) && r_symndx == STN_UNDEF)
did you mean "the relocation's r_symndx is STN_UNDEF"?
But it can be a relocation against an absolute symbol, for example if the
symbol referred to by a relocation is supplied by another module in the
link or a linker script and said symbol is absolute. This case has to be
covered in testing as well, as I infer from your code it won't be handled
correctly.
In fact this situation is not unique to branch relocations, because no
PC-relative relocation against an absolute symbol or value can be resolved
at link time for PIC/PIE links. So you need to make your code generic for
any relocations that have the `pc_relative' flag set in their howto unless
we have a way to make a dynamic relocation out of such a relocation (which
is none right now, as we still only have R_MIPS_REL32 as the sole dynamic
relocation, except for TLS stuff).
Extra test cases will be appreciated if you can come up with ones.
Maciej
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v6] MIPS: Reject branch absolute relocs for PIC for linking
2024-02-22 13:13 ` Maciej W. Rozycki
@ 2024-02-25 14:08 ` YunQiang Su
2024-03-01 17:23 ` Maciej W. Rozycki
0 siblings, 1 reply; 6+ messages in thread
From: YunQiang Su @ 2024-02-25 14:08 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Hans-Peter Nilsson, Nick Clifton, binutils, xry111
Maciej W. Rozycki <macro@orcam.me.uk> 于2024年2月22日周四 21:13写道:
>
> On Thu, 22 Feb 2024, YunQiang Su wrote:
>
> > > > + howto = MIPS_ELF_RTYPE_TO_HOWTO (abfd, r_type, NEWABI_P (abfd));
> > > > + info->callbacks->einfo
> > > > + /* xgettext:c-format */
> > > > + (_("%X%H: relocation %s against `*ABS*' cannot be used"
> > >
> > > There's no "*ABS*" in the source and IMHO that'd look confusing
> > > to innocent users. How about "...against an absolute value"?
> > > Or "...against an absolute value or absolute symbol"? Perhaps
> > > the latter is a bit too wordy, but also more complete.
> > >
> >
> > Good idea. I think that we should use "...against an absolute value".
> > "absolute symbol" is not included in this code, as it's r_symndx is
> > not STN_UNDEF.
>
> Well code says otherwise:
>
> + if (branch_reloc_p (r_type) && r_symndx == STN_UNDEF)
>
> did you mean "the relocation's r_symndx is STN_UNDEF"?
>
> But it can be a relocation against an absolute symbol, for example if the
> symbol referred to by a relocation is supplied by another module in the
> link or a linker script and said symbol is absolute. This case has to be
I have no idea whether we should emit this error for this case:
If the symbol is set by a linker script, and is near enough with
our instruction,
the object should be ok for PIC?
If not near enough, a "relocation truncated to fit" error will emit.
> covered in testing as well, as I infer from your code it won't be handled
> correctly.
>
> In fact this situation is not unique to branch relocations, because no
> PC-relative relocation against an absolute symbol or value can be resolved
> at link time for PIC/PIE links. So you need to make your code generic for
> any relocations that have the `pc_relative' flag set in their howto unless
> we have a way to make a dynamic relocation out of such a relocation (which
> is none right now, as we still only have R_MIPS_REL32 as the sole dynamic
> relocation, except for TLS stuff).
>
> Extra test cases will be appreciated if you can come up with ones.
>
> Maciej
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v6] MIPS: Reject branch absolute relocs for PIC for linking
2024-02-25 14:08 ` YunQiang Su
@ 2024-03-01 17:23 ` Maciej W. Rozycki
0 siblings, 0 replies; 6+ messages in thread
From: Maciej W. Rozycki @ 2024-03-01 17:23 UTC (permalink / raw)
To: YunQiang Su; +Cc: Hans-Peter Nilsson, Nick Clifton, binutils, xry111
On Sun, 25 Feb 2024, YunQiang Su wrote:
> > > Good idea. I think that we should use "...against an absolute value".
> > > "absolute symbol" is not included in this code, as it's r_symndx is
> > > not STN_UNDEF.
> >
> > Well code says otherwise:
> >
> > + if (branch_reloc_p (r_type) && r_symndx == STN_UNDEF)
> >
> > did you mean "the relocation's r_symndx is STN_UNDEF"?
> >
> > But it can be a relocation against an absolute symbol, for example if the
> > symbol referred to by a relocation is supplied by another module in the
> > link or a linker script and said symbol is absolute. This case has to be
>
> I have no idea whether we should emit this error for this case:
> If the symbol is set by a linker script, and is near enough with
> our instruction,
> the object should be ok for PIC?
> If not near enough, a "relocation truncated to fit" error will emit.
If the symbol referred is absolute, then the case is no different from an
absolute relocation. You just can't calculate a PC-relative reference to
an absolute value at static link time, because the distance from PC to
that value will depend on the base address at load time. It is different
from a reference to a regular (non-absolute) symbol that just turns out
too distant for a PC-relative relocation to reach (where the "relocation
truncated to fit" case applies).
Maciej
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-03-01 17:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-21 14:52 [PATCH v6] MIPS: Reject branch absolute relocs for PIC for linking YunQiang Su
2024-02-22 3:13 ` Hans-Peter Nilsson
2024-02-22 6:29 ` YunQiang Su
2024-02-22 13:13 ` Maciej W. Rozycki
2024-02-25 14:08 ` YunQiang Su
2024-03-01 17:23 ` Maciej W. Rozycki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).