Skip to content

Commit 112a7cb

Browse files
cherrymuicherrymui
committed
[dev.link] cmd/link: remove the second result of MakeSymbolUpdater
With unique global indices, MakeSymbolUpdater will not change the symbol's index. So no need to return a new index. Change-Id: I5b4fd6a0167cc74476880bbf4382c524ecde7721 Reviewed-on: https://go-review.googlesource.com/c/go/+/219227 Run-TryBot: Cherry Zhang <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Jeremy Faller <[email protected]> Reviewed-by: Than McIntosh <[email protected]>
1 parent 5a8b150 commit 112a7cb

8 files changed

Lines changed: 70 additions & 72 deletions

File tree

‎src/cmd/link/internal/ld/go.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ func setCgoAttr(ctxt *Link, lookup func(string, int) loader.Sym, file string, pk
202202
l.SetSymExtname(s, remote)
203203
l.SetSymDynimpvers(s, q)
204204
if st != sym.SHOSTOBJ {
205-
su, _ := l.MakeSymbolUpdater(s)
205+
su := l.MakeSymbolUpdater(s)
206206
su.SetType(sym.SDYNIMPORT)
207207
} else {
208208
hostObjSyms[s] = struct{}{}
@@ -218,7 +218,8 @@ func setCgoAttr(ctxt *Link, lookup func(string, int) loader.Sym, file string, pk
218218
}
219219
local := f[1]
220220

221-
su, s := l.MakeSymbolUpdater(lookup(local, 0))
221+
s := lookup(local, 0)
222+
su := l.MakeSymbolUpdater(s)
222223
su.SetType(sym.SHOSTOBJ)
223224
su.SetSize(0)
224225
hostObjSyms[s] = struct{}{}
@@ -260,7 +261,7 @@ func setCgoAttr(ctxt *Link, lookup func(string, int) loader.Sym, file string, pk
260261
l.SetSymDynimpvers(s, "")
261262
l.SetSymExtname(s, "")
262263
var su *loader.SymbolBuilder
263-
su, s = l.MakeSymbolUpdater(s)
264+
su = l.MakeSymbolUpdater(s)
264265
su.SetType(0)
265266
}
266267

‎src/cmd/link/internal/ld/lib.go‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,7 @@ func (ctxt *Link) loadcgodirectives() {
567567
// cgo_import_static and cgo_import_dynamic,
568568
// then we want to make it cgo_import_dynamic
569569
// now.
570-
su, _ := l.MakeSymbolUpdater(symIdx)
570+
su := l.MakeSymbolUpdater(symIdx)
571571
if l.SymExtname(symIdx) != "" && l.SymDynimplib(symIdx) != "" && !(l.AttrCgoExportStatic(symIdx) || l.AttrCgoExportDynamic(symIdx)) {
572572
su.SetType(sym.SDYNIMPORT)
573573
} else {
@@ -584,12 +584,12 @@ func (ctxt *Link) linksetup() {
584584
switch ctxt.BuildMode {
585585
case BuildModeCShared, BuildModePlugin:
586586
symIdx := ctxt.loader.LookupOrCreateSym("runtime.islibrary", 0)
587-
sb, _ := ctxt.loader.MakeSymbolUpdater(symIdx)
587+
sb := ctxt.loader.MakeSymbolUpdater(symIdx)
588588
sb.SetType(sym.SNOPTRDATA)
589589
sb.AddUint8(1)
590590
case BuildModeCArchive:
591591
symIdx := ctxt.loader.LookupOrCreateSym("runtime.isarchive", 0)
592-
sb, _ := ctxt.loader.MakeSymbolUpdater(symIdx)
592+
sb := ctxt.loader.MakeSymbolUpdater(symIdx)
593593
sb.SetType(sym.SNOPTRDATA)
594594
sb.AddUint8(1)
595595
}
@@ -621,16 +621,16 @@ func (ctxt *Link) linksetup() {
621621

622622
if ctxt.LinkMode == LinkExternal && ctxt.Arch.Family == sys.PPC64 && objabi.GOOS != "aix" {
623623
toc := ctxt.loader.LookupOrCreateSym(".TOC.", 0)
624-
sb, _ := ctxt.loader.MakeSymbolUpdater(toc)
624+
sb := ctxt.loader.MakeSymbolUpdater(toc)
625625
sb.SetType(sym.SDYNIMPORT)
626626
}
627627

628628
// The Android Q linker started to complain about underalignment of the our TLS
629629
// section. We don't actually use the section on android, so don't
630630
// generate it.
631631
if objabi.GOOS != "android" {
632-
symIdx := ctxt.loader.LookupOrCreateSym("runtime.tlsg", 0)
633-
sb, tlsg := ctxt.loader.MakeSymbolUpdater(symIdx)
632+
tlsg := ctxt.loader.LookupOrCreateSym("runtime.tlsg", 0)
633+
sb := ctxt.loader.MakeSymbolUpdater(tlsg)
634634

635635
// runtime.tlsg is used for external linking on platforms that do not define
636636
// a variable to hold g in assembly (currently only intel).
@@ -647,12 +647,12 @@ func (ctxt *Link) linksetup() {
647647
var moduledata loader.Sym
648648
var mdsb *loader.SymbolBuilder
649649
if ctxt.BuildMode == BuildModePlugin {
650-
pmd := ctxt.loader.LookupOrCreateSym("local.pluginmoduledata", 0)
651-
mdsb, moduledata = ctxt.loader.MakeSymbolUpdater(pmd)
650+
moduledata = ctxt.loader.LookupOrCreateSym("local.pluginmoduledata", 0)
651+
mdsb = ctxt.loader.MakeSymbolUpdater(moduledata)
652652
ctxt.loader.SetAttrLocal(moduledata, true)
653653
} else {
654-
fmd := ctxt.loader.LookupOrCreateSym("runtime.firstmoduledata", 0)
655-
mdsb, moduledata = ctxt.loader.MakeSymbolUpdater(fmd)
654+
moduledata = ctxt.loader.LookupOrCreateSym("runtime.firstmoduledata", 0)
655+
mdsb = ctxt.loader.MakeSymbolUpdater(moduledata)
656656
}
657657
if mdsb.Type() != 0 && mdsb.Type() != sym.SDYNIMPORT {
658658
// If the module (toolchain-speak for "executable or shared
@@ -666,24 +666,24 @@ func (ctxt *Link) linksetup() {
666666
// recording the value of GOARM.
667667
if ctxt.Arch.Family == sys.ARM {
668668
goarm := ctxt.loader.LookupOrCreateSym("runtime.goarm", 0)
669-
sb, _ := ctxt.loader.MakeSymbolUpdater(goarm)
669+
sb := ctxt.loader.MakeSymbolUpdater(goarm)
670670
sb.SetType(sym.SDATA)
671671
sb.SetSize(0)
672672
sb.AddUint8(uint8(objabi.GOARM))
673673
}
674674

675675
if objabi.Framepointer_enabled(objabi.GOOS, objabi.GOARCH) {
676676
fpe := ctxt.loader.LookupOrCreateSym("runtime.framepointer_enabled", 0)
677-
sb, _ := ctxt.loader.MakeSymbolUpdater(fpe)
677+
sb := ctxt.loader.MakeSymbolUpdater(fpe)
678678
sb.SetType(sym.SNOPTRDATA)
679679
sb.SetSize(0)
680680
sb.AddUint8(1)
681681
}
682682
} else {
683683
// If OTOH the module does not contain the runtime package,
684684
// create a local symbol for the moduledata.
685-
lmd := ctxt.loader.LookupOrCreateSym("local.moduledata", 0)
686-
mdsb, moduledata = ctxt.loader.MakeSymbolUpdater(lmd)
685+
moduledata = ctxt.loader.LookupOrCreateSym("local.moduledata", 0)
686+
mdsb = ctxt.loader.MakeSymbolUpdater(moduledata)
687687
ctxt.loader.SetAttrLocal(moduledata, true)
688688
}
689689
// In all cases way we mark the moduledata as noptrdata to hide it from
@@ -704,8 +704,8 @@ func (ctxt *Link) linksetup() {
704704

705705
if ctxt.Arch == sys.Arch386 && ctxt.HeadType != objabi.Hwindows {
706706
if (ctxt.BuildMode == BuildModeCArchive && ctxt.IsELF) || ctxt.BuildMode == BuildModeCShared || ctxt.BuildMode == BuildModePIE || ctxt.DynlinkingGo() {
707-
symIdx := ctxt.loader.LookupOrCreateSym("_GLOBAL_OFFSET_TABLE_", 0)
708-
sb, got := ctxt.loader.MakeSymbolUpdater(symIdx)
707+
got := ctxt.loader.LookupOrCreateSym("_GLOBAL_OFFSET_TABLE_", 0)
708+
sb := ctxt.loader.MakeSymbolUpdater(got)
709709
sb.SetType(sym.SDYNIMPORT)
710710
ctxt.loader.SetAttrReachable(got, true)
711711
}
@@ -1970,16 +1970,16 @@ func ldshlibsyms(ctxt *Link, shlib string) {
19701970
}
19711971

19721972
l := ctxt.loader
1973-
symIdx := l.LookupOrCreateSym(elfsym.Name, ver)
1973+
s := l.LookupOrCreateSym(elfsym.Name, ver)
19741974

19751975
// Because loadlib above loads all .a files before loading
19761976
// any shared libraries, any non-dynimport symbols we find
19771977
// that duplicate symbols already loaded should be ignored
19781978
// (the symbols from the .a files "win").
1979-
if l.SymType(symIdx) != 0 && l.SymType(symIdx) != sym.SDYNIMPORT {
1979+
if l.SymType(s) != 0 && l.SymType(s) != sym.SDYNIMPORT {
19801980
continue
19811981
}
1982-
su, s := l.MakeSymbolUpdater(symIdx)
1982+
su := l.MakeSymbolUpdater(s)
19831983
su.SetType(sym.SDYNIMPORT)
19841984
l.SetSymElfType(s, elf.ST_TYPE(elfsym.Info))
19851985
su.SetSize(int64(elfsym.Size))
@@ -2019,7 +2019,7 @@ func ldshlibsyms(ctxt *Link, shlib string) {
20192019
if l.SymType(alias) != 0 {
20202020
continue
20212021
}
2022-
su, _ := l.MakeSymbolUpdater(alias)
2022+
su := l.MakeSymbolUpdater(alias)
20232023
su.SetType(sym.SABIALIAS)
20242024
su.AddReloc(loader.Reloc{Sym: s})
20252025
}

‎src/cmd/link/internal/loadelf/ldelf.go‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -721,7 +721,7 @@ func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, f *bio.Reader,
721721
}
722722
sectsymNames[name] = true
723723

724-
sb, _ := l.MakeSymbolUpdater(lookup(name, localSymVersion))
724+
sb := l.MakeSymbolUpdater(lookup(name, localSymVersion))
725725

726726
switch int(sect.flags) & (ElfSectFlagAlloc | ElfSectFlagWrite | ElfSectFlagExec) {
727727
default:
@@ -768,15 +768,13 @@ func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, f *bio.Reader,
768768
continue
769769
}
770770
if elfsym.shndx == ElfSymShnCommon || elfsym.type_ == ElfSymTypeCommon {
771-
sb, ns := l.MakeSymbolUpdater(elfsym.sym)
771+
sb := l.MakeSymbolUpdater(elfsym.sym)
772772
if uint64(sb.Size()) < elfsym.size {
773773
sb.SetSize(int64(elfsym.size))
774774
}
775775
if sb.Type() == 0 || sb.Type() == sym.SXREF {
776776
sb.SetType(sym.SNOPTRBSS)
777777
}
778-
symbols[i] = ns
779-
elfsym.sym = ns
780778
continue
781779
}
782780

@@ -822,8 +820,8 @@ func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, f *bio.Reader,
822820
l.SymName(s), l.SymName(l.OuterSym(s)), l.SymName(sect.sym))
823821
}
824822

825-
sectsb, _ := l.MakeSymbolUpdater(sect.sym)
826-
sb, _ := l.MakeSymbolUpdater(s)
823+
sectsb := l.MakeSymbolUpdater(sect.sym)
824+
sb := l.MakeSymbolUpdater(s)
827825

828826
sb.SetType(sectsb.Type())
829827
sectsb.PrependSub(s)
@@ -856,8 +854,7 @@ func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, f *bio.Reader,
856854
if s == 0 {
857855
continue
858856
}
859-
sb, _ := l.MakeSymbolUpdater(s)
860-
s = sb.Sym()
857+
sb := l.MakeSymbolUpdater(s)
861858
if l.SubSym(s) != 0 {
862859
sb.SortSub()
863860
}
@@ -992,7 +989,7 @@ func Load(l *loader.Loader, arch *sys.Arch, localSymVersion int, f *bio.Reader,
992989
sort.Sort(loader.RelocByOff(r[:n]))
993990
// just in case
994991

995-
sb, _ := l.MakeSymbolUpdater(sect.sym)
992+
sb := l.MakeSymbolUpdater(sect.sym)
996993
r = r[:n]
997994
sb.SetRelocs(r)
998995
}
@@ -1090,7 +1087,7 @@ func readelfsym(newSym, lookup func(string, int) loader.Sym, l *loader.Loader, a
10901087
// comment #5 for details.
10911088
if s != 0 && elfsym.other == 2 {
10921089
if !l.IsExternal(s) {
1093-
_, s = l.MakeSymbolUpdater(s)
1090+
l.MakeSymbolUpdater(s)
10941091
}
10951092
l.SetAttrDuplicateOK(s, true)
10961093
l.SetAttrVisibilityHidden(s, true)
@@ -1147,7 +1144,7 @@ func readelfsym(newSym, lookup func(string, int) loader.Sym, l *loader.Loader, a
11471144
// TODO(mwhudson): the test of VisibilityHidden here probably doesn't make
11481145
// sense and should be removed when someone has thought about it properly.
11491146
if s != 0 && l.SymType(s) == 0 && !l.AttrVisibilityHidden(s) && elfsym.type_ != ElfSymTypeSection {
1150-
sb, _ := l.MakeSymbolUpdater(s)
1147+
sb := l.MakeSymbolUpdater(s)
11511148
sb.SetType(sym.SXREF)
11521149
}
11531150
elfsym.sym = s

‎src/cmd/link/internal/loader/loader_test.go‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,9 @@ func TestAddMaterializedSymbol(t *testing.T) {
5858
}
5959

6060
// Grab symbol builder pointers
61-
sb1, es1 := ldr.MakeSymbolUpdater(es1)
62-
sb2, es2 := ldr.MakeSymbolUpdater(es2)
63-
sb3, es3 := ldr.MakeSymbolUpdater(es3)
61+
sb1 := ldr.MakeSymbolUpdater(es1)
62+
sb2 := ldr.MakeSymbolUpdater(es2)
63+
sb3 := ldr.MakeSymbolUpdater(es3)
6464

6565
// Suppose we create some more symbols, which triggers a grow.
6666
// Make sure the symbol builder's payload pointer is valid,
@@ -116,8 +116,8 @@ func TestAddMaterializedSymbol(t *testing.T) {
116116
}
117117
}
118118

119-
sb1, es1 = ldr.MakeSymbolUpdater(es1)
120-
sb2, es2 = ldr.MakeSymbolUpdater(es2)
119+
sb1 = ldr.MakeSymbolUpdater(es1)
120+
sb2 = ldr.MakeSymbolUpdater(es2)
121121

122122
// Get/set a few other attributes
123123
if ldr.AttrVisibilityHidden(es3) {
@@ -248,51 +248,51 @@ func TestAddDataMethods(t *testing.T) {
248248
{
249249
which: "AddUint8",
250250
addDataFunc: func(l *Loader, s Sym, _ Sym) Sym {
251-
sb, ns := l.MakeSymbolUpdater(s)
251+
sb := l.MakeSymbolUpdater(s)
252252
sb.AddUint8('a')
253-
return ns
253+
return s
254254
},
255255
expData: []byte{'a'},
256256
expKind: sym.SDATA,
257257
},
258258
{
259259
which: "AddUintXX",
260260
addDataFunc: func(l *Loader, s Sym, _ Sym) Sym {
261-
sb, ns := l.MakeSymbolUpdater(s)
261+
sb := l.MakeSymbolUpdater(s)
262262
sb.AddUintXX(arch, 25185, 2)
263-
return ns
263+
return s
264264
},
265265
expData: []byte{'a', 'b'},
266266
expKind: sym.SDATA,
267267
},
268268
{
269269
which: "SetUint8",
270270
addDataFunc: func(l *Loader, s Sym, _ Sym) Sym {
271-
sb, ns := l.MakeSymbolUpdater(s)
271+
sb := l.MakeSymbolUpdater(s)
272272
sb.AddUint8('a')
273273
sb.AddUint8('b')
274274
sb.SetUint8(arch, 1, 'c')
275-
return ns
275+
return s
276276
},
277277
expData: []byte{'a', 'c'},
278278
expKind: sym.SDATA,
279279
},
280280
{
281281
which: "AddString",
282282
addDataFunc: func(l *Loader, s Sym, _ Sym) Sym {
283-
sb, ns := l.MakeSymbolUpdater(s)
283+
sb := l.MakeSymbolUpdater(s)
284284
sb.Addstring("hello")
285-
return ns
285+
return s
286286
},
287287
expData: []byte{'h', 'e', 'l', 'l', 'o', 0},
288288
expKind: sym.SNOPTRDATA,
289289
},
290290
{
291291
which: "AddAddrPlus",
292292
addDataFunc: func(l *Loader, s Sym, s2 Sym) Sym {
293-
sb, ns := l.MakeSymbolUpdater(s)
293+
sb := l.MakeSymbolUpdater(s)
294294
sb.AddAddrPlus(arch, s2, 3)
295-
return ns
295+
return s
296296
},
297297
expData: []byte{0, 0, 0, 0, 0, 0, 0, 0},
298298
expKind: sym.SDATA,
@@ -301,9 +301,9 @@ func TestAddDataMethods(t *testing.T) {
301301
{
302302
which: "AddAddrPlus4",
303303
addDataFunc: func(l *Loader, s Sym, s2 Sym) Sym {
304-
sb, ns := l.MakeSymbolUpdater(s)
304+
sb := l.MakeSymbolUpdater(s)
305305
sb.AddAddrPlus4(arch, s2, 3)
306-
return ns
306+
return s
307307
},
308308
expData: []byte{0, 0, 0, 0},
309309
expKind: sym.SDATA,
@@ -312,9 +312,9 @@ func TestAddDataMethods(t *testing.T) {
312312
{
313313
which: "AddCURelativeAddrPlus",
314314
addDataFunc: func(l *Loader, s Sym, s2 Sym) Sym {
315-
sb, ns := l.MakeSymbolUpdater(s)
315+
sb := l.MakeSymbolUpdater(s)
316316
sb.AddCURelativeAddrPlus(arch, s2, 7)
317-
return ns
317+
return s
318318
},
319319
expData: []byte{0, 0, 0, 0, 0, 0, 0, 0},
320320
expKind: sym.SDATA,

‎src/cmd/link/internal/loader/symbolbuilder.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,8 @@ func (l *Loader) MakeSymbolBuilder(name string) *SymbolBuilder {
3636
// symbol 'symIdx'. If 'symIdx' is not an external symbol, then create
3737
// a clone of it (copy name, properties, etc) fix things up so that
3838
// the lookup tables and caches point to the new version, not the old
39-
// version. Returns a SymbolBuilder and a Sym (which may be different
40-
// from the original if we had to clone).
41-
func (l *Loader) MakeSymbolUpdater(symIdx Sym) (*SymbolBuilder, Sym) {
39+
// version.
40+
func (l *Loader) MakeSymbolUpdater(symIdx Sym) *SymbolBuilder {
4241
if symIdx == 0 {
4342
panic("can't update the null symbol")
4443
}
@@ -53,7 +52,7 @@ func (l *Loader) MakeSymbolUpdater(symIdx Sym) (*SymbolBuilder, Sym) {
5352
// Construct updater and return.
5453
sb := &SymbolBuilder{l: l, symIdx: symIdx}
5554
sb.extSymPayload = l.getPayload(symIdx)
56-
return sb, symIdx
55+
return sb
5756
}
5857

5958
// Getters for properties of the symbol we're working on.

0 commit comments

Comments
 (0)