[PATCH] alpha: don't delete a caller's gp reload after a call
Matt Turner <[email protected]>
| Newsgroups | gmane.comp.gnu.binutils |
|---|---|
| Message-ID | <[email protected]> |
elf64_alpha_relax_with_lituse turns
ldq t12,lit(gp) bsr ra,callee+8
jsr ra,(t12) ==> unop
ldah gp,hi(ra) unop
lda gp,lo(gp)
whenever elf64_alpha_relax_opt_call returns a non-zero destination, i.e.
when the callee starts with our gp (or is marked NOPV). That justifies
entering the callee past its own ldgp, since the ldgp would recompute the
gp we already have, but it does not justify deleting the reload. $gp is
caller-saved; the callee is under no obligation to return with it intact.
A callee ending in a tail call
ldq ra,0(sp)
lda sp,N(sp)
ldq t12,lit(gp)
jmp zero,(t12)
hands control to a function that runs its own ldgp and returns straight to
our caller. When the tail target is in a different sub-GOT the caller
resumes with the wrong gp, and every later disp(gp) names a slot in the
wrong sub-GOT. The value loaded is a valid GOT entry belonging to some
other reference, so nothing traps until it is used, typically an indirect
call landing in .data.rel.ro.
This needs a multi-sub-GOT link to be observable, which is why it went
unnoticed. It was found in libLLVM.so, where SelectionDAG::getConstant
called a gp-sharing callee that tail-called into another sub-GOT and then
jumped through a slot holding a vtable pointer.
Recovering the optimization needs proof that the callee returns with gp
intact. A callee containing no tail call, direct or indirect, always
does, but establishing that requires scanning the target function here.
Until then, keep the reload. It costs no code size: the two instructions
are already emitted and were only being rewritten to unops.
The new test only needs a single sub-GOT: it checks that ld --relax still
converts the jsr to a bsr into foo+8 while leaving the ldah/lda pair
alone.
* elf64-alpha.c (elf64_alpha_relax_with_lituse): Don't turn the
GPDISP ldah/lda pair into unops when the callee shares our gp.
ld/
* testsuite/ld-alpha/relax-gpdisp.s: New test.
* testsuite/ld-alpha/relax-gpdisp.d: New test.
---
bfd/elf64-alpha.c | 46 +++++++++-------------------
ld/testsuite/ld-alpha/relax-gpdisp.d | 17 ++++++++++
ld/testsuite/ld-alpha/relax-gpdisp.s | 26 ++++++++++++++++
3 files changed, 58 insertions(+), 31 deletions(-)
create mode 100644 ld/testsuite/ld-alpha/relax-gpdisp.d
create mode 100644 ld/testsuite/ld-alpha/relax-gpdisp.s
diff --git a/bfd/elf64-alpha.c b/bfd/elf64-alpha.c
index 3e0b03608a6..7a0763be459 100644
--- a/bfd/elf64-alpha.c
+++ b/bfd/elf64-alpha.c
@@ -3414,37 +3414,21 @@ elf64_alpha_relax_with_lituse (struct alpha_relax_info *info,
else
all_optimized = false;
- /* Even if the target is not in range for a direct branch,
- if we share a GP, we can eliminate the gp reload. */
- if (optdest)
- {
- Elf_Internal_Rela *gpdisp
- = (elf64_alpha_find_reloc_at_ofs
- (info->relocs, irelend, urel_r_offset + 4,
- R_ALPHA_GPDISP));
- if (gpdisp)
- {
- bfd_byte *p_ldah = contents + gpdisp->r_offset;
- bfd_byte *p_lda = p_ldah + gpdisp->r_addend;
- unsigned int ldah = bfd_get_32 (abfd, p_ldah);
- unsigned int lda = bfd_get_32 (abfd, p_lda);
-
- /* Verify that the instruction is "ldah $29,0($26)".
- Consider a function that ends in a noreturn call,
- and that the next function begins with an ldgp,
- and that by accident there is no padding between.
- In that case the insn would use $27 as the base. */
- if (ldah == 0x27ba0000 && lda == 0x23bd0000)
- {
- bfd_put_32 (abfd, (bfd_vma) INSN_UNOP, p_ldah);
- bfd_put_32 (abfd, (bfd_vma) INSN_UNOP, p_lda);
-
- gpdisp->r_info = ELF64_R_INFO (0, R_ALPHA_NONE);
- changed_contents = true;
- changed_relocs = true;
- }
- }
- }
+ /* Keep the caller's GPDISP ldah/lda pair. A non-zero optdest
+ says the callee starts with our gp, which is enough to enter
+ it past its own ldgp, but says nothing about the gp we get
+ back: $gp is caller-saved, and a callee ending in a tail call
+ returns with the tail target's gp in $29. If that target
+ lives in another sub-GOT, later displacements off $gp in this
+ function silently name slots in the wrong sub-GOT. Eliding
+ the reload again would mean proving here that the callee
+ returns with gp intact, which needs a scan of its
+ instructions. Cheaper would be a new STO_ALPHA_* bit that
+ gas sets at .end, alongside STO_ALPHA_STD_GPLOAD, when it saw
+ nothing in the function that can leave $29 clobbered. Either
+ way the payoff is two unops: the pair is already emitted, so
+ the rewrite never shrinks .text, and dropping the got entry
+ depends on optdest rather than on this. */
}
break;
}
diff --git a/ld/testsuite/ld-alpha/relax-gpdisp.d b/ld/testsuite/ld-alpha/relax-gpdisp.d
new file mode 100644
index 00000000000..b4198171e84
--- /dev/null
+++ b/ld/testsuite/ld-alpha/relax-gpdisp.d
@@ -0,0 +1,17 @@
+#ld: -relax
+#objdump: -dw --no-show-raw-insn
+
+.*: +file format .*
+
+
+Disassembly of section .text:
+
+[a-f0-9]+ <_start>:
+ +[a-f0-9]+: ldah gp,[0-9]+\(t12\)
+ +[a-f0-9]+: lda gp,-?[0-9]+\(gp\)
+ +[a-f0-9]+: unop[ ]*
+ +[a-f0-9]+: bsr ra,[a-f0-9]+ <foo\+0x8>
+ +[a-f0-9]+: ldah gp,[0-9]+\(ra\)
+ +[a-f0-9]+: lda gp,-?[0-9]+\(gp\)
+ +[a-f0-9]+: ret
+#pass
diff --git a/ld/testsuite/ld-alpha/relax-gpdisp.s b/ld/testsuite/ld-alpha/relax-gpdisp.s
new file mode 100644
index 00000000000..f8faf542dde
--- /dev/null
+++ b/ld/testsuite/ld-alpha/relax-gpdisp.s
@@ -0,0 +1,26 @@
+/* --relax turns the LITERAL+LITUSE_JSR call into a bsr straight to
+ foo+8, since foo shares our gp and starts with an ldgp. The GPDISP
+ ldah/lda pair after the call must survive: foo is free to return with
+ some other gp in $29. */
+
+ .text
+
+ .globl _start
+ .ent _start
+_start:
+ ldgp $29,0($27)
+ .prologue 1
+ ldq $27,foo($29) !literal!1
+ jsr $26,($27),foo !lituse_jsr!1
+ ldah $29,0($26) !gpdisp!2
+ lda $29,0($29) !gpdisp!2
+ ret $31,($26),1
+ .end _start
+
+ .globl foo
+ .ent foo
+foo:
+ ldgp $29,0($27)
+ .prologue 1
+ ret $31,($26),1
+ .end foo
--
2.54.0