From: Cherry Zhang Date: Tue, 11 Feb 2020 15:27:15 +0000 (-0500) Subject: [dev.link] cmd/link: add defined package symbols first, before any non-package symbols X-Git-Tag: go1.15beta1~679^2~128 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=b0275bfbf4992e3e63f279b3fc7ba1f93700abff;p=gostls13.git [dev.link] cmd/link: add defined package symbols first, before any non-package symbols Currently, the loader adds defined package symbols and non-package symbols to the global index space object by object. This CL changes it to add all the defined package symbols first, then all the non-package symbols. The advantage of doing this is that when adding package symbols, by definition they cannot be dup to each other, so we don't need to do a name lookup when adding them. We still add them to the lookup table (for now), since they may still be referenced by name (e.g. through linkname). This CL is also a prerequisite if we want to move to not adding package symbols to the lookup table entirely (e.g. by using pre-generated in-file lookup table). Also update some comments to reflect the current state. Change-Id: Ib757e070b48a9ef6215e47dc3421fc5c055b746c Reviewed-on: https://go-review.googlesource.com/c/go/+/219078 Run-TryBot: Cherry Zhang TryBot-Result: Gobot Gobot Reviewed-by: Than McIntosh --- diff --git a/src/cmd/link/internal/ld/lib.go b/src/cmd/link/internal/ld/lib.go index 7ced27ec79..fd43dd4885 100644 --- a/src/cmd/link/internal/ld/lib.go +++ b/src/cmd/link/internal/ld/lib.go @@ -422,30 +422,33 @@ func (ctxt *Link) loadlib() { loadobjfile(ctxt, lib) } } + // At this point, the Go objects are "preloaded". Not all the symbols are + // added to the symbol table (only defined package symbols are). Looking + // up symbol by name may not get expected result. - iscgo = ctxt.loader.Lookup("x_cgo_init", 0) != 0 - ctxt.canUsePlugins = ctxt.loader.Lookup("plugin.Open", sym.SymVerABIInternal) != 0 + iscgo = ctxt.LibraryByPkg["runtime/cgo"] != nil + ctxt.canUsePlugins = ctxt.LibraryByPkg["plugin"] != nil // We now have enough information to determine the link mode. determineLinkMode(ctxt) - if ctxt.LinkMode == LinkExternal && !iscgo && ctxt.LibraryByPkg["runtime/cgo"] == nil && !(objabi.GOOS == "darwin" && ctxt.BuildMode != BuildModePlugin && (ctxt.Arch.Family == sys.AMD64 || ctxt.Arch.Family == sys.I386)) { + if ctxt.LinkMode == LinkExternal && !iscgo && !(objabi.GOOS == "darwin" && ctxt.BuildMode != BuildModePlugin && (ctxt.Arch.Family == sys.AMD64 || ctxt.Arch.Family == sys.I386)) { // This indicates a user requested -linkmode=external. // The startup code uses an import of runtime/cgo to decide // whether to initialize the TLS. So give it one. This could // be handled differently but it's an unusual case. - if lib := loadinternal(ctxt, "runtime/cgo"); lib != nil { - if lib.Shlib != "" { - ldshlibsyms(ctxt, lib.Shlib) - } else { - if ctxt.BuildMode == BuildModeShared || ctxt.linkShared { - Exitf("cannot implicitly include runtime/cgo in a shared library") - } - loadobjfile(ctxt, lib) + if lib := loadinternal(ctxt, "runtime/cgo"); lib != nil && lib.Shlib == "" { + if ctxt.BuildMode == BuildModeShared || ctxt.linkShared { + Exitf("cannot implicitly include runtime/cgo in a shared library") } + loadobjfile(ctxt, lib) } } + // Add non-package symbols and references of externally defined symbols. + ctxt.loader.LoadNonpkgSyms(ctxt.Arch, ctxt.Syms) + + // Load symbols from shared libraries, after all Go object symbols are loaded. for _, lib := range ctxt.Library { if lib.Shlib != "" { if ctxt.Debugvlog > 1 { @@ -455,9 +458,6 @@ func (ctxt *Link) loadlib() { } } - // Add references of externally defined symbols. - ctxt.loader.LoadRefs(ctxt.Arch, ctxt.Syms) - // Process cgo directives (has to be done before host object loading). ctxt.loadcgodirectives() diff --git a/src/cmd/link/internal/loader/loader.go b/src/cmd/link/internal/loader/loader.go index 45ddef9574..b80f2c568b 100644 --- a/src/cmd/link/internal/loader/loader.go +++ b/src/cmd/link/internal/loader/loader.go @@ -125,31 +125,20 @@ func growBitmap(reqLen int, b bitmap) bitmap { // // Notes on the layout of global symbol index space: // -// TODO: rework index space reservation. -// // - Go object files are read before host object files; each Go object -// read adds its defined (package + non-package) symbols to the global -// index space. +// read adds its defined package symbols to the global index space. +// Nonpackage symbols are not yet added. // -// - In loader.LoadRefs(), the loader makes a sweep through all of the -// non-package references in each object file and allocates sym indices -// for any symbols that have not yet been defined (start of this space -// is marked by loader.extStart). +// - In loader.LoadNonpkgSyms, add non-package defined symbols and +// references in all object files to the global index space. // // - Host object file loading happens; the host object loader does a // name/version lookup for each symbol it finds; this can wind up // extending the external symbol index space range. The host object -// loader currently stores symbol payloads in sym.Symbol objects, -// which get handed off to the loader. +// loader stores symbol payloads in loader.payloads using SymbolBuilder. // -// - A given external symbol (Sym) either has a sym.Symbol acting as -// its backing store (this will continue to be the case until we -// finish rewriting the host object loader to work entirely with -// loader.Sym) or it has a "payload" backing store (represented by -// extSymPayload). Newly created external symbols (created by -// a call to AddExtSym or equivalent) start out in the "has payload" -// state, and continue until installSym is called for the sym -// index in question. +// - For now, in loader.LoadFull we convert all symbols (Go + external) +// to sym.Symbols. // // - At some point (when the wayfront is pushed through all of the // linker), all external symbols will be payload-based, and we can @@ -237,6 +226,12 @@ type Loader struct { elfsetstring elfsetstringFunc } +const ( + pkgDef = iota + nonPkgDef + nonPkgRef +) + type elfsetstringFunc func(s *sym.Symbol, str string, off int) // extSymPayload holds the payload (data + relocations) for linker-synthesized @@ -299,17 +294,15 @@ func (l *Loader) addObj(pkg string, r *oReader) Sym { if _, ok := l.objByPkg[pkg]; !ok { l.objByPkg[pkg] = r } - n := r.NSym() + r.NNonpkgdef() i := Sym(len(l.objSyms)) l.start[r] = i l.objs = append(l.objs, objIdx{r, i}) - l.growValues(int(i) + n - 1) return i } // Add a symbol from an object file, return the global index and whether it is added. // If the symbol already exist, it returns the index of that symbol. -func (l *Loader) AddSym(name string, ver int, r *oReader, li int, dupok bool, typ sym.SymKind) (Sym, bool) { +func (l *Loader) AddSym(name string, ver int, r *oReader, li int, kind int, dupok bool, typ sym.SymKind) (Sym, bool) { if l.extStart != 0 { panic("AddSym called after AddExtSym is called") } @@ -328,6 +321,18 @@ func (l *Loader) AddSym(name string, ver int, r *oReader, li int, dupok bool, ty addToGlobal() return i, true } + if kind == pkgDef { + // Defined package symbols cannot be dup to each other. + // We load all the package symbols first, so we don't need + // to check dup here. + // We still add it to the lookup table, as it may still be + // referenced by name (e.g. through linkname). + l.symsByName[ver][name] = i + addToGlobal() + return i, true + } + + // Non-package (named) symbol. Check if it already exists. oldi, existed := l.symsByName[ver][name] if !existed { l.symsByName[ver][name] = i @@ -1468,8 +1473,9 @@ func (x RelocByOff) Len() int { return len(x) } func (x RelocByOff) Swap(i, j int) { x[i], x[j] = x[j], x[i] } func (x RelocByOff) Less(i, j int) bool { return x[i].Off < x[j].Off } -// Preload a package: add autolibs, add symbols to the symbol table. -// Does not read symbol data yet. +// Preload a package: add autolibs, add defined package symbols to the symbol table. +// Does not add non-package symbols yet, which will be done in LoadNonpkgSyms. +// Does not read symbol data. func (l *Loader) Preload(arch *sys.Arch, syms *sym.Symbols, f *bio.Reader, lib *sym.Library, unit *sym.CompilationUnit, length int64, pn string, flags int) { roObject, readonly, err := f.Slice(uint64(length)) if err != nil { @@ -1495,16 +1501,38 @@ func (l *Loader) Preload(arch *sys.Arch, syms *sym.Symbols, f *bio.Reader, lib * unit.DWARFFileTable[i] = r.DwarfFile(i) } - istart := l.addObj(lib.Pkg, or) - l.growAttrBitmaps(int(istart) + ndef + nnonpkgdef) - for i, n := 0, ndef+nnonpkgdef; i < n; i++ { + l.addObj(lib.Pkg, or) + l.preloadSyms(or, pkgDef) + + // The caller expects us consuming all the data + f.MustSeek(length, os.SEEK_CUR) +} + +// Preload symbols of given kind from an object. +func (l *Loader) preloadSyms(r *oReader, kind int) { + ndef := r.NSym() + nnonpkgdef := r.NNonpkgdef() + var start, end int + switch kind { + case pkgDef: + start = 0 + end = ndef + case nonPkgDef: + start = ndef + end = ndef + nnonpkgdef + default: + panic("preloadSyms: bad kind") + } + l.growSyms(len(l.objSyms) + end - start) + l.growAttrBitmaps(len(l.objSyms) + end - start) + for i := start; i < end; i++ { osym := goobj2.Sym{} - osym.Read(r, r.SymOff(i)) - name := strings.Replace(osym.Name, "\"\".", pkgprefix, -1) - v := abiToVer(osym.ABI, localSymVersion) + osym.Read(r.Reader, r.SymOff(i)) + name := strings.Replace(osym.Name, "\"\".", r.pkgprefix, -1) + v := abiToVer(osym.ABI, r.version) dupok := osym.Dupok() - gi, added := l.AddSym(name, v, or, i, dupok, sym.AbiSymKindToSymKind[objabi.SymKind(osym.Type)]) - or.syms[i] = gi + gi, added := l.AddSym(name, v, r, i, kind, dupok, sym.AbiSymKindToSymKind[objabi.SymKind(osym.Type)]) + r.syms[i] = gi if !added { continue } @@ -1522,14 +1550,14 @@ func (l *Loader) Preload(arch *sys.Arch, syms *sym.Symbols, f *bio.Reader, lib * l.SetAttrNotInSymbolTable(gi, true) } } - - // The caller expects us consuming all the data - f.MustSeek(length, os.SEEK_CUR) } -// Make sure referenced symbols are added. Most of them should already be added. -// This should only be needed for referenced external symbols. -func (l *Loader) LoadRefs(arch *sys.Arch, syms *sym.Symbols) { +// Add non-package symbols and references to external symbols (which are always +// named). +func (l *Loader) LoadNonpkgSyms(arch *sys.Arch, syms *sym.Symbols) { + for _, o := range l.objs[1:] { + l.preloadSyms(o.r, nonPkgDef) + } for _, o := range l.objs[1:] { loadObjRefs(l, o.r, arch, syms) } diff --git a/src/cmd/link/internal/loader/loader_test.go b/src/cmd/link/internal/loader/loader_test.go index fc1665cc7d..47a5355994 100644 --- a/src/cmd/link/internal/loader/loader_test.go +++ b/src/cmd/link/internal/loader/loader_test.go @@ -20,7 +20,7 @@ import ( // data or relocations). func addDummyObjSym(t *testing.T, ldr *Loader, or *oReader, name string) Sym { idx := len(ldr.objSyms) - s, ok := ldr.AddSym(name, 0, or, idx, false, sym.SRODATA) + s, ok := ldr.AddSym(name, 0, or, idx, nonPkgDef, false, sym.SRODATA) if !ok { t.Errorf("AddrSym failed for '" + name + "'") }