From: Than McIntosh Date: Wed, 18 Dec 2019 20:14:46 +0000 (-0500) Subject: [dev.link] cmd/link: handle multiple levels of overwrite X-Git-Tag: go1.15beta1~679^2~155 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=db48d458388e62aa1ae8947a5f5bbd60b467bcf6;p=gostls13.git [dev.link] cmd/link: handle multiple levels of overwrite Revamp the way that symbol overwrites are handled to deal with symbols that are overwritten more than once (such as "_cgo_mmap"). The scenario here is that a symbol can be overwritten twice, once during preload, and then again when host objects are read during internal linking. This can result in a situation where we have two entries in the overwrite map, from X -> Y and then from Y -> Z. Rather than search the overwrite map when adding new entries, add a helper routine for querying the map that catches this situation and fixes it up. Also with this patch is a couple of tweaks to the loader.Dump method to insure that it can dump the entire global index space without crashing due to odd overwrites (as in the scenario above). Change-Id: Ib6c8a0e03e92fc2b57318001711b501eeaf12249 Reviewed-on: https://go-review.googlesource.com/c/go/+/212098 Run-TryBot: Than McIntosh TryBot-Result: Gobot Gobot Reviewed-by: Jeremy Faller --- diff --git a/src/cmd/link/internal/loader/loader.go b/src/cmd/link/internal/loader/loader.go index 2cf4dd02ce..369381ec27 100644 --- a/src/cmd/link/internal/loader/loader.go +++ b/src/cmd/link/internal/loader/loader.go @@ -428,12 +428,72 @@ func (l *Loader) growSyms(i int) { l.growAttrBitmaps(int(i) + 1) } +// getOverwrite returns the overwrite symbol for 'symIdx', while +// collapsing any chains of overwrites along the way. This is +// apparently needed in cases where we add an overwrite entry X -> Y +// during preload (where both X and Y are non-external symbols), and +// then we add an additional entry to the overwrite map Y -> W in +// cloneToExternal when we encounter the real definition of the symbol +// in a host object file, and we need to build up W's content. +// +// Note: it would be nice to avoid this sort of complexity. One of the +// main reasons we wind up with overwrites has to do with the way the +// compiler handles link-named symbols that are 'defined elsewhere': +// at the moment they wind up as no-package defs. For example, consider +// the variable "runtime.no_pointers_stackmap". This variable is defined +// in an assembly file as RODATA, then in one of the Go files it is +// declared this way: +// +// var no_pointers_stackmap uint64 // defined in assembly +// +// This generates what amounts to a weak definition (in the object +// containing the line of code above), which is then overriden by the +// stronger def from the assembly file. Rather than have things work +// this way, it would be better if in the Go file we emitted a +// no-package ref instead of a no-package def, which would eliminate +// the need for overwrites. Doing this would also require changing the +// semantics of //go:linkname, however; we'd have to insure that in +// the cross-package case there is a go:linkname directive on both +// ends. +func (l *Loader) getOverwrite(symIdx Sym) Sym { + var seen map[Sym]bool + result := symIdx + cur := symIdx + for { + if ov, ok := l.overwrite[cur]; ok { + if seen == nil { + seen = make(map[Sym]bool) + seen[symIdx] = true + } + if _, ok := seen[ov]; ok { + panic("cycle in overwrite map") + } else { + seen[cur] = true + } + cur = ov + } else { + break + } + } + if cur != symIdx { + result = cur + cur = symIdx + for { + if ov, ok := l.overwrite[cur]; ok { + l.overwrite[cur] = result + cur = ov + } else { + break + } + } + } + return result +} + // Convert a local index to a global index. func (l *Loader) toGlobal(r *oReader, i int) Sym { g := l.startIndex(r) + Sym(i) - if ov, ok := l.overwrite[g]; ok { - return ov - } + g = l.getOverwrite(g) return g } @@ -491,7 +551,11 @@ func (l *Loader) resolve(r *oReader, s goobj2.SymRef) Sym { case goobj2.PkgIdxNone: // Check for cached version first if cached := r.rcacheGet(s.SymIdx); cached != 0 { - return cached + ov := l.getOverwrite(cached) + if cached != ov { + r.rcacheSet(s.SymIdx, ov) + return ov + } } // Resolve by name i := int(s.SymIdx) + r.NSym() @@ -499,7 +563,7 @@ func (l *Loader) resolve(r *oReader, s goobj2.SymRef) Sym { osym.Read(r.Reader, r.SymOff(i)) name := strings.Replace(osym.Name, "\"\".", r.pkgprefix, -1) v := abiToVer(osym.ABI, r.version) - gsym := l.Lookup(name, v) + gsym := l.getOverwrite(l.Lookup(name, v)) // Add to cache, then return. r.rcacheSet(s.SymIdx, gsym) return gsym @@ -1498,6 +1562,9 @@ func (l *Loader) LoadFull(arch *sys.Arch, syms *sym.Symbols) { s.Attr.Set(sym.AttrReachable, l.attrReachable.has(i)) continue } + if i != l.getOverwrite(i) { + continue + } sname := l.RawSymName(i) if !l.attrReachable.has(i) && !strings.HasPrefix(sname, "gofile..") { // XXX file symbols are used but not marked continue @@ -1864,14 +1931,6 @@ func (l *Loader) cloneToExternal(symIdx Sym) Sym { // the old symbol). l.overwrite[symIdx] = ns - // There may be relocations against this symbol from other symbols - // in the object -- we want those relocations to target the new - // external sym version of this symbol, not the old overwritten - // one. Update the rcache accordingly. - if li > r.NSym() { - r.rcacheSet(uint32(li-r.NSym()), ns) - } - // FIXME: copy other attributes? reachable is the main one, and we // don't expect it to be set at this point. @@ -2235,6 +2294,7 @@ func (l *Loader) Dump() { } } fmt.Println("extStart:", l.extStart) + fmt.Println("max:", l.max) fmt.Println("syms") for i, s := range l.Syms { if i == 0 { @@ -2243,7 +2303,13 @@ func (l *Loader) Dump() { if s != nil { fmt.Println(i, s, s.Type) } else { - fmt.Println(i, l.SymName(Sym(i)), "") + otag := "" + si := Sym(i) + if _, ok := l.overwrite[si]; ok { + si = l.getOverwrite(si) + otag = fmt.Sprintf(" ", si) + } + fmt.Println(i, l.SymName(si), "", otag) } } fmt.Println("overwrite:", l.overwrite) diff --git a/src/cmd/link/internal/loader/symbolbuilder.go b/src/cmd/link/internal/loader/symbolbuilder.go index 6d3d0186e7..a815a69617 100644 --- a/src/cmd/link/internal/loader/symbolbuilder.go +++ b/src/cmd/link/internal/loader/symbolbuilder.go @@ -41,9 +41,7 @@ func (l *Loader) MakeSymbolUpdater(symIdx Sym) (*SymbolBuilder, Sym) { if symIdx == 0 { panic("can't update the null symbol") } - if ov, ok := l.overwrite[symIdx]; ok { - symIdx = ov - } + symIdx = l.getOverwrite(symIdx) if !l.IsExternal(symIdx) { // Create a clone with the same name/version/kind etc. symIdx = l.cloneToExternal(symIdx)