[dev.link] cmd/link: create symbol updated lazily in amd64 adddynrel

Tweak the code in the amd64 version of adddynrel to avoid creating a
symbol updated for the symbol being processed until it's clear we need
to alter its relocations. This should help performance for the
PIE+internal linking scenario.

Reviewed-on: https://go-review.googlesource.com/c/go/+/229866
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Change-Id: Id25adfd81a5bbd2dde0f80a83b976397ba6abfb5
Reviewed-on: https://go-review.googlesource.com/c/go/+/230026
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>

diff --git a/src/cmd/link/internal/amd64/asm.go b/src/cmd/link/internal/amd64/asm.go
index fa0f6ab9b5cf9ec0f5715057763682cd904627b8..e2a66daf4f782e4352780032a4907cbe3ef824b5 100644 (file)
--- a/src/cmd/link/internal/amd64/asm.go
+++ b/src/cmd/link/internal/amd64/asm.go
@@ -85,7 +85,6 @@ func adddynrel2(target *ld.Target, ldr *loader.Loader, syms *ld.ArchSyms, s load
                targType = ldr.SymType(targ)
        }
 
-       su := ldr.MakeSymbolUpdater(s)
        switch r.Type() {
        default:
                if r.Type() >= objabi.ElfRelocOffset {
@@ -103,6 +102,7 @@ func adddynrel2(target *ld.Target, ldr *loader.Loader, syms *ld.ArchSyms, s load
                if (targType == 0 || targType == sym.SXREF) && !ldr.AttrVisibilityHidden(targ) {
                        ldr.Errorf(s, "unknown symbol %s in pcrel", ldr.SymName(targ))
                }
+               su := ldr.MakeSymbolUpdater(s)
                su.SetRelocType(rIdx, objabi.R_PCREL)
                su.SetRelocAdd(rIdx, r.Add()+4)
                return true
@@ -114,11 +114,13 @@ func adddynrel2(target *ld.Target, ldr *loader.Loader, syms *ld.ArchSyms, s load
                if targType == 0 || targType == sym.SXREF {
                        ldr.Errorf(s, "unknown symbol %s in pcrel", ldr.SymName(targ))
                }
+               su := ldr.MakeSymbolUpdater(s)
                su.SetRelocType(rIdx, objabi.R_PCREL)
                su.SetRelocAdd(rIdx, r.Add()+8)
                return true
 
        case objabi.ElfRelocOffset + objabi.RelocType(elf.R_X86_64_PLT32):
+               su := ldr.MakeSymbolUpdater(s)
                su.SetRelocType(rIdx, objabi.R_PCREL)
                su.SetRelocAdd(rIdx, r.Add()+4)
                if targType == sym.SDYNIMPORT {
@@ -132,11 +134,11 @@ func adddynrel2(target *ld.Target, ldr *loader.Loader, syms *ld.ArchSyms, s load
        case objabi.ElfRelocOffset + objabi.RelocType(elf.R_X86_64_GOTPCREL),
                objabi.ElfRelocOffset + objabi.RelocType(elf.R_X86_64_GOTPCRELX),
                objabi.ElfRelocOffset + objabi.RelocType(elf.R_X86_64_REX_GOTPCRELX):
+               su := ldr.MakeSymbolUpdater(s)
                if targType != sym.SDYNIMPORT {
                        // have symbol
                        sData := ldr.Data(s)
                        if r.Off() >= 2 && sData[r.Off()-2] == 0x8b {
-                               su := ldr.MakeSymbolUpdater(s)
                                su.MakeWritable()
                                // turn MOVQ of GOT entry into LEAQ of symbol itself
                                writeableData := su.Data()
@@ -160,6 +162,7 @@ func adddynrel2(target *ld.Target, ldr *loader.Loader, syms *ld.ArchSyms, s load
                if targType == sym.SDYNIMPORT {
                        ldr.Errorf(s, "unexpected R_X86_64_64 relocation for dynamic symbol %s", ldr.SymName(targ))
                }
+               su := ldr.MakeSymbolUpdater(s)
                su.SetRelocType(rIdx, objabi.R_ADDR)
                if target.IsPIE() && target.IsInternal() {
                        // For internal linking PIE, this R_ADDR relocation cannot
@@ -174,6 +177,7 @@ func adddynrel2(target *ld.Target, ldr *loader.Loader, syms *ld.ArchSyms, s load
                objabi.MachoRelocOffset + ld.MACHO_X86_64_RELOC_SIGNED*2 + 0,
                objabi.MachoRelocOffset + ld.MACHO_X86_64_RELOC_BRANCH*2 + 0:
                // TODO: What is the difference between all these?
+               su := ldr.MakeSymbolUpdater(s)
                su.SetRelocType(rIdx, objabi.R_ADDR)
 
                if targType == sym.SDYNIMPORT {
@@ -184,6 +188,7 @@ func adddynrel2(target *ld.Target, ldr *loader.Loader, syms *ld.ArchSyms, s load
        case objabi.MachoRelocOffset + ld.MACHO_X86_64_RELOC_BRANCH*2 + 1:
                if targType == sym.SDYNIMPORT {
                        addpltsym2(target, ldr, syms, targ)
+                       su := ldr.MakeSymbolUpdater(s)
                        su.SetRelocSym(rIdx, syms.PLT2)
                        su.SetRelocType(rIdx, objabi.R_PCREL)
                        su.SetRelocAdd(rIdx, int64(ldr.SymPlt(targ)))
@@ -196,6 +201,7 @@ func adddynrel2(target *ld.Target, ldr *loader.Loader, syms *ld.ArchSyms, s load
                objabi.MachoRelocOffset + ld.MACHO_X86_64_RELOC_SIGNED_1*2 + 1,
                objabi.MachoRelocOffset + ld.MACHO_X86_64_RELOC_SIGNED_2*2 + 1,
                objabi.MachoRelocOffset + ld.MACHO_X86_64_RELOC_SIGNED_4*2 + 1:
+               su := ldr.MakeSymbolUpdater(s)
                su.SetRelocType(rIdx, objabi.R_PCREL)
 
                if targType == sym.SDYNIMPORT {
@@ -227,6 +233,7 @@ func adddynrel2(target *ld.Target, ldr *loader.Loader, syms *ld.ArchSyms, s load
                        ldr.Errorf(s, "unexpected GOT reloc for non-dynamic symbol %s", ldr.SymName(targ))
                }
                addgotsym2(target, ldr, syms, targ)
+               su := ldr.MakeSymbolUpdater(s)
                su.SetRelocType(rIdx, objabi.R_PCREL)
                su.SetRelocSym(rIdx, syms.GOT2)
                su.SetRelocAdd(rIdx, r.Add()+int64(ldr.SymGot(targ)))
@@ -251,12 +258,14 @@ func adddynrel2(target *ld.Target, ldr *loader.Loader, syms *ld.ArchSyms, s load
                // Internal linking, for both ELF and Mach-O.
                // Build a PLT entry and change the relocation target to that entry.
                addpltsym2(target, ldr, syms, targ)
+               su := ldr.MakeSymbolUpdater(s)
                su.SetRelocSym(rIdx, syms.PLT2)
                su.SetRelocAdd(rIdx, int64(ldr.SymPlt(targ)))
                return true
 
        case objabi.R_ADDR:
                if ldr.SymType(s) == sym.STEXT && target.IsElf() {
+                       su := ldr.MakeSymbolUpdater(s)
                        if target.IsSolaris() {
                                addpltsym2(target, ldr, syms, targ)
                                su.SetRelocSym(rIdx, syms.PLT2)

diff --git a/src/cmd/link/internal/ld/data.go b/src/cmd/link/internal/ld/data.go
index 63e05a26453141b5844a67be669ddc62b6da4deb..d5286b428943e47ed435085702489c734dfff19b 100644 (file)
--- a/src/cmd/link/internal/ld/data.go
+++ b/src/cmd/link/internal/ld/data.go
@@ -684,10 +684,6 @@ func dynrelocsym2(ctxt *Link, s loader.Sym) {
        relocs := ldr.Relocs(s)
        for ri := 0; ri < relocs.Count(); ri++ {
                r := relocs.At2(ri)
-               // FIXME: the call to Adddynrel2 below is going to wind up
-               // eagerly promoting the symbol to external, which is not great--
-               // it would improve things for internal/PIE if we could
-               // create the symbol updater lazily.
                if ctxt.BuildMode == BuildModePIE && ctxt.LinkMode == LinkInternal {
                        // It's expected that some relocations will be done
                        // later by relocsym (R_TLS_LE, R_ADDROFF), so