[release-branch.go1.20] cmd/link: better fix for arm32 trampgen problem with duff routines

This patch provides a fix for a problem linking large arm32 binaries
with external linking, specifically R_CALLARM relocations against
runtime.duff* routines being flagged by the external linker as not
reaching.

What appears to be happening in the bug in question is that the Go
linker and the external linker are using slightly different recipes to
decide whether a given R_CALLARM relocation will "fit" (e.g. will not
require a trampoline). The Go linker is taking into account the addend
on the call reloc (which for calls to runtime.duffcopy or
runtime.duffzero is nonzero), whereas the external linker appears to
be ignoring the addend.

Example to illustrate:

   Addr      Size   Func
   -----     -----  -----
   ...
   XYZ       1024   runtime.duffcopy
   ...
   ABC       ...    mypackge.MyFunc
     + R0: R_CALLARM  o=8 a=848 tgt=runtime.duffcopy<0>

Let's say that the distance between ABC (start address of
runtime.duffcopy) and XYZ (start of MyFunc) is just over the
architected 24-bit maximum displacement for an R_CALLARM (let's say
that ABC-XYZ is just over the architected limit by some small value,
say 36). Because we're calling into runtime.duffcopy at offset 848,
however, the relocation does in fact fit, but if the external linker
isn't taking into account the addend (assuming that all calls target
the first instruction of the called routine), then we'll get a
"doesn't fit" error from the linker.

To work around this problem, revise the ARM trampoline generation code
in the Go linker that computes the trampoline threshold to ignore the
addend on R_CALLARM relocations, so as to harmonize the two linkers.

Fixes #58503.
Updates #58428.
Updates #58425.

Change-Id: I56e580c05b7b47bbe8edf5532a1770bbd700fbe5
Reviewed-on: https://go-review.googlesource.com/c/go/+/469275
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
(cherry picked from commit 0b5affb193ed559f2cb646e0324827f261e2e767)
Reviewed-on: https://go-review.googlesource.com/c/go/+/471597

1 files changed, 20 insertions, 3 deletions

diff --git a/src/cmd/link/internal/arm/asm.go b/src/cmd/link/internal/arm/asm.go
index 9eaa7b9eea..1e4b36df48 100644
--- a/src/cmd/link/internal/arm/asm.go
+++ b/src/cmd/link/internal/arm/asm.go
@@ -396,9 +396,26 @@ func trampoline(ctxt *ld.Link, ldr *loader.Loader, ri int, rs, s loader.Sym) {
 		// laid out. Conservatively use a trampoline. This should be rare, as we lay out packages
 		// in dependency order.
 		if ldr.SymValue(rs) != 0 {
-			// r.Add is the instruction
-			// low 24-bit encodes the target address
-			t = (ldr.SymValue(rs) + int64(signext24(r.Add()&0xffffff)*4) - (ldr.SymValue(s) + int64(r.Off()))) / 4
+			// Workaround for issue #58425: it appears that the
+			// external linker doesn't always take into account the
+			// relocation addend when doing reachability checks. This
+			// means that if you have a call from function XYZ at
+			// offset 8 to runtime.duffzero with addend 800 (for
+			// example), where the distance between the start of XYZ
+			// and the start of runtime.duffzero is just over the
+			// limit (by 100 bytes, say), you can get "relocation
+			// doesn't fit" errors from the external linker. To deal
+			// with this, ignore the addend when performing the
+			// distance calculation (this assumes that we're only
+			// handling backward jumps; ideally we might want to check
+			// both with and without the addend).
+			if ctxt.IsExternal() {
+				t = (ldr.SymValue(rs) - (ldr.SymValue(s) + int64(r.Off()))) / 4
+			} else {
+				// r.Add is the instruction
+				// low 24-bit encodes the target address
+				t = (ldr.SymValue(rs) + int64(signext24(r.Add()&0xffffff)*4) - (ldr.SymValue(s) + int64(r.Off()))) / 4
+			}
 		}
 		if t > 0x7fffff || t < -0x800000 || ldr.SymValue(rs) == 0 || (*ld.FlagDebugTramp > 1 && ldr.SymPkg(s) != ldr.SymPkg(rs)) {
 			// direct call too far, need to insert trampoline.