]> Cypherpunks repositories - gostls13.git/commitdiff
Revert "cmd/link/internal/ld: assign temporary addresses to per-package text"
authorThan McIntosh <thanm@google.com>
Tue, 10 Oct 2023 19:56:41 +0000 (19:56 +0000)
committerGopher Robot <gobot@golang.org>
Tue, 10 Oct 2023 20:47:17 +0000 (20:47 +0000)
This reverts commit http://go.dev/cl/349650

Reason for revert: CL breaks multiple builders with failure in TestLargeTextSectionSplitting

Change-Id: I4894ffd101b2757a6e6359212d5b8a64da1fcdf0
Reviewed-on: https://go-review.googlesource.com/c/go/+/534299
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
src/cmd/link/internal/ld/data.go
src/cmd/link/internal/ld/ld_test.go

index fbbd53465fb4021f85bf54b97901c43cc7e60ddd..dbaf68621283f21c7cfa654a1ca9c9897c2ddddd 100644 (file)
@@ -84,15 +84,14 @@ func maxSizeTrampolines(ctxt *Link, ldr *loader.Loader, s loader.Sym, isTramp bo
                }
        }
 
-       switch {
-       case ctxt.IsARM():
+       if ctxt.IsARM() {
                return n * 20 // Trampolines in ARM range from 3 to 5 instructions.
-       case ctxt.IsARM64():
-               return n * 12 // Trampolines in ARM64 are 3 instructions.
-       case ctxt.IsPPC64():
+       }
+       if ctxt.IsPPC64() {
                return n * 16 // Trampolines in PPC64 are 4 instructions.
-       case ctxt.IsRISCV64():
-               return n * 8 // Trampolines in RISCV64 are 2 instructions.
+       }
+       if ctxt.IsARM64() {
+               return n * 12 // Trampolines in ARM64 are 3 instructions.
        }
        panic("unreachable")
 }
@@ -119,21 +118,18 @@ func trampoline(ctxt *Link, s loader.Sym) {
                        continue // something is wrong. skip it here and we'll emit a better error later
                }
 
-               if ldr.SymValue(rs) == 0 && ldr.SymType(rs) != sym.SDYNIMPORT && ldr.SymType(rs) != sym.SUNDEFEXT {
-                       // Symbols in the same package are laid out together.
-                       // Except that if SymPkg(s) == "", it is a host object symbol
-                       // which may call an external symbol via PLT.
+               // RISC-V is only able to reach +/-1MiB via a JAL instruction,
+               // which we can readily exceed in the same package. As such, we
+               // need to generate trampolines when the address is unknown.
+               if ldr.SymValue(rs) == 0 && !ctxt.Target.IsRISCV64() && ldr.SymType(rs) != sym.SDYNIMPORT && ldr.SymType(rs) != sym.SUNDEFEXT {
                        if ldr.SymPkg(s) != "" && ldr.SymPkg(rs) == ldr.SymPkg(s) {
-                               // RISC-V is only able to reach +/-1MiB via a JAL instruction.
-                               // We need to generate a trampoline when an address is
-                               // currently unknown.
-                               if !ctxt.Target.IsRISCV64() {
-                                       continue
-                               }
+                               // Symbols in the same package are laid out together.
+                               // Except that if SymPkg(s) == "", it is a host object symbol
+                               // which may call an external symbol via PLT.
+                               continue
                        }
-                       // Runtime packages are laid out together.
                        if isRuntimeDepPkg(ldr.SymPkg(s)) && isRuntimeDepPkg(ldr.SymPkg(rs)) {
-                               continue
+                               continue // runtime packages are laid out together
                        }
                }
                thearch.Trampoline(ctxt, ldr, ri, rs, s)
@@ -2439,8 +2435,8 @@ func (ctxt *Link) textaddress() {
                limit = 1
        }
 
-       // First pass: assign addresses assuming the program is small and will
-       // not require trampoline generation.
+       // First pass: assign addresses assuming the program is small and
+       // don't generate trampolines.
        big := false
        for _, s := range ctxt.Textp {
                sect, n, va = assignAddress(ctxt, sect, n, s, va, false, big)
@@ -2455,43 +2451,21 @@ func (ctxt *Link) textaddress() {
        if big {
                // reset addresses
                for _, s := range ctxt.Textp {
-                       if s != text {
-                               resetAddress(ctxt, s)
+                       if ldr.OuterSym(s) != 0 || s == text {
+                               continue
+                       }
+                       oldv := ldr.SymValue(s)
+                       for sub := s; sub != 0; sub = ldr.SubSym(sub) {
+                               ldr.SetSymValue(sub, ldr.SymValue(sub)-oldv)
                        }
                }
                va = start
 
                ntramps := 0
-               var curPkg string
-               for i, s := range ctxt.Textp {
-                       // When we find the first symbol in a package, perform a
-                       // single iteration that assigns temporary addresses to all
-                       // of the text in the same package, using the maximum possible
-                       // number of trampolines. This allows for better decisions to
-                       // be made regarding reachability and the need for trampolines.
-                       if symPkg := ldr.SymPkg(s); symPkg != "" && curPkg != symPkg {
-                               curPkg = symPkg
-                               vaTmp := va
-                               for j := i; j < len(ctxt.Textp); j++ {
-                                       curSym := ctxt.Textp[j]
-                                       if symPkg := ldr.SymPkg(curSym); symPkg == "" || curPkg != symPkg {
-                                               break
-                                       }
-                                       sect, n, vaTmp = assignAddress(ctxt, sect, n, curSym, vaTmp, false, big)
-                                       vaTmp += maxSizeTrampolines(ctxt, ldr, curSym, false)
-                               }
-                       }
-
-                       // Reset address for current symbol.
-                       if s != text {
-                               resetAddress(ctxt, s)
-                       }
-
-                       // Assign actual address for current symbol.
+               for _, s := range ctxt.Textp {
                        sect, n, va = assignAddress(ctxt, sect, n, s, va, false, big)
 
-                       // Resolve jumps, adding trampolines if they are needed.
-                       trampoline(ctxt, s)
+                       trampoline(ctxt, s) // resolve jumps, may add trampolines if jump too far
 
                        // lay down trampolines after each function
                        for ; ntramps < len(ctxt.tramps); ntramps++ {
@@ -2639,17 +2613,6 @@ func assignAddress(ctxt *Link, sect *sym.Section, n int, s loader.Sym, va uint64
        return sect, n, va
 }
 
-func resetAddress(ctxt *Link, s loader.Sym) {
-       ldr := ctxt.loader
-       if ldr.OuterSym(s) != 0 {
-               return
-       }
-       oldv := ldr.SymValue(s)
-       for sub := s; sub != 0; sub = ldr.SubSym(sub) {
-               ldr.SetSymValue(sub, ldr.SymValue(sub)-oldv)
-       }
-}
-
 // Return whether we may need to split text sections.
 //
 // On PPC64x, when external linking, a text section should not be
index 1767667759ec948734543449608c8d7870716c66..a7a6082f54b922730eb385ee83c5b9f0b9d991eb 100644 (file)
@@ -344,71 +344,3 @@ func main() {
                })
        }
 }
-
-func TestRISCVTrampolines(t *testing.T) {
-       testenv.MustHaveGoBuild(t)
-       t.Parallel()
-
-       tmpDir := t.TempDir()
-       tmpFile := filepath.Join(tmpDir, "x.s")
-
-       // Calling b from a or c should not use trampolines, however
-       // calling from d to a will require one.
-       buf := new(bytes.Buffer)
-       fmt.Fprintf(buf, "TEXT a(SB),$0-0\n")
-       for i := 0; i < 1<<17; i++ {
-               fmt.Fprintf(buf, "\tADD $0, X0, X0\n")
-       }
-       fmt.Fprintf(buf, "\tCALL b(SB)\n")
-       fmt.Fprintf(buf, "\tRET\n")
-       fmt.Fprintf(buf, "TEXT b(SB),$0-0\n")
-       fmt.Fprintf(buf, "\tRET\n")
-       fmt.Fprintf(buf, "TEXT c(SB),$0-0\n")
-       fmt.Fprintf(buf, "\tCALL b(SB)\n")
-       fmt.Fprintf(buf, "\tRET\n")
-       fmt.Fprintf(buf, "TEXT ·d(SB),0,$0-0\n")
-       for i := 0; i < 1<<17; i++ {
-               fmt.Fprintf(buf, "\tADD $0, X0, X0\n")
-       }
-       fmt.Fprintf(buf, "\tCALL a(SB)\n")
-       fmt.Fprintf(buf, "\tCALL c(SB)\n")
-       fmt.Fprintf(buf, "\tRET\n")
-       if err := os.WriteFile(tmpFile, buf.Bytes(), 0644); err != nil {
-               t.Fatalf("Failed to write assembly file: %v", err)
-       }
-
-       if err := os.WriteFile(filepath.Join(tmpDir, "go.mod"), []byte("module riscvtramp"), 0644); err != nil {
-               t.Fatalf("Failed to write file: %v\n", err)
-       }
-       main := `package main
-func main() {
-       d()
-}
-
-func d()
-`
-       if err := os.WriteFile(filepath.Join(tmpDir, "x.go"), []byte(main), 0644); err != nil {
-               t.Fatalf("failed to write main: %v\n", err)
-       }
-       cmd := testenv.Command(t, testenv.GoToolPath(t), "build", "-ldflags=-linkmode=internal")
-       cmd.Dir = tmpDir
-       cmd.Env = append(os.Environ(), "GOARCH=riscv64", "GOOS=linux")
-       out, err := cmd.CombinedOutput()
-       if err != nil {
-               t.Fatalf("Build failed: %v, output: %s", err, out)
-       }
-
-       // Check what trampolines exist.
-       cmd = testenv.Command(t, testenv.GoToolPath(t), "tool", "nm", filepath.Join(tmpDir, "riscvtramp"))
-       cmd.Env = append(os.Environ(), "GOARCH=riscv64", "GOOS=linux")
-       out, err = cmd.CombinedOutput()
-       if err != nil {
-               t.Fatalf("nm failure: %s\n%s\n", err, string(out))
-       }
-       if !bytes.Contains(out, []byte(" T a-tramp0")) {
-               t.Errorf("Trampoline a-tramp0 is missing")
-       }
-       if bytes.Contains(out, []byte(" T b-tramp0")) {
-               t.Errorf("Trampoline b-tramp0 exists unnecessarily")
-       }
-}