cmd/link: choose one with larger size for duplicated BSS symbols

When two packages declare a variable with the same name (with
linkname at least on one side), the linker will choose one as the
actual definition of the symbol if one has content (i.e. a DATA
symbol) and the other does not (i.e. a BSS symbol). When both have
content, it is redefinition error. When neither has content,
currently the choice is sort of arbitrary (depending on symbol
loading order, etc. which are subject to change).

One use case for that is that one wants to reference a symbol
defined in another package, and the reference side just wants to
see some of the fields, so it may be declared with a smaller type.
In this case, we want to choose the one with the larger size as
the true definition. Otherwise the code accessing the larger
sized one may read/write out of bounds, corrupting the next
variable. This CL makes the linker do so.

Fixes #72032.

Change-Id: I160aa9e0234702066cb8f141c186eaa89d0fcfed
Reviewed-on: https://go-review.googlesource.com/c/go/+/660696
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Than McIntosh <thanm@golang.org>

1 files changed, 21 insertions, 8 deletions

diff --git a/src/cmd/link/internal/loader/loader.go b/src/cmd/link/internal/loader/loader.go
index 0c234e8975..182379f0df 100644
--- a/src/cmd/link/internal/loader/loader.go
+++ b/src/cmd/link/internal/loader/loader.go
@@ -432,16 +432,16 @@ func (st *loadState) addSym(name string, ver int, r *oReader, li uint32, kind in
 		return i
 	}
 	// symbol already exists
+	// Fix for issue #47185 -- given two dupok or BSS symbols with
+	// different sizes, favor symbol with larger size. See also
+	// issue #46653 and #72032.
+	oldsz := l.SymSize(oldi)
+	sz := int64(r.Sym(li).Siz())
 	if osym.Dupok() {
 		if l.flags&FlagStrictDups != 0 {
 			l.checkdup(name, r, li, oldi)
 		}
-		// Fix for issue #47185 -- given two dupok symbols with
-		// different sizes, favor symbol with larger size. See
-		// also issue #46653.
-		szdup := l.SymSize(oldi)
-		sz := int64(r.Sym(li).Siz())
-		if szdup < sz {
+		if oldsz < sz {
 			// new symbol overwrites old symbol.
 			l.objSyms[oldi] = objSym{r.objidx, li}
 		}
@@ -452,11 +452,24 @@ func (st *loadState) addSym(name string, ver int, r *oReader, li uint32, kind in
 	if oldsym.Dupok() {
 		return oldi
 	}
-	overwrite := r.DataSize(li) != 0
+	// If one is a DATA symbol (i.e. has content, DataSize != 0)
+	// and the other is BSS, the one with content wins.
+	// If both are BSS, the one with larger size wins.
+	// Specifically, the "overwrite" variable and the final result are
+	//
+	// new sym       old sym       overwrite
+	// ---------------------------------------------
+	// DATA          DATA          true  => ERROR
+	// DATA lg/eq    BSS  sm/eq    true  => new wins
+	// DATA small    BSS  large    true  => ERROR
+	// BSS  large    DATA small    true  => ERROR
+	// BSS  large    BSS  small    true  => new wins
+	// BSS  sm/eq    D/B  lg/eq    false => old wins
+	overwrite := r.DataSize(li) != 0 || oldsz < sz
 	if overwrite {
 		// new symbol overwrites old symbol.
 		oldtyp := sym.AbiSymKindToSymKind[objabi.SymKind(oldsym.Type())]
-		if !(oldtyp.IsData() && oldr.DataSize(oldli) == 0) {
+		if !(oldtyp.IsData() && oldr.DataSize(oldli) == 0) || oldsz > sz {
 			log.Fatalf("duplicated definition of symbol %s, from %s and %s", name, r.unit.Lib.Pkg, oldr.unit.Lib.Pkg)
 		}
 		l.objSyms[oldi] = objSym{r.objidx, li}