[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 <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>

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 {
+	g = l.getOverwrite(g)
-		return ov
-	}
 	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)), "<not loaded>")
+			otag := ""
+			si := Sym(i)
+			if _, ok := l.overwrite[si]; ok {
+				si = l.getOverwrite(si)
+				otag = fmt.Sprintf(" <overwritten to %d>", si)
+			}
+			fmt.Println(i, l.SymName(si), "<not loaded>", otag)
 		}
 	}
 	fmt.Println("overwrite:", l.overwrite)

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 = l.getOverwrite(symIdx)
-		symIdx = ov
-	}
 	if !l.IsExternal(symIdx) {
 		// Create a clone with the same name/version/kind etc.
 		symIdx = l.cloneToExternal(symIdx)