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 <[email protected]>
Reviewed-by: David Chase <[email protected]>
Reviewed-by: Than McIntosh <[email protected]>

Author: cherrymui

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}

src/cmd/link/link_test.go

@@ -20,6 +20,7 @@ import (
 	"testing"
 
 	imacho "cmd/internal/macho"
+	"cmd/internal/objfile"
 	"cmd/internal/sys"
 )
 
@@ -1541,3 +1542,52 @@ func TestCheckLinkname(t *testing.T) {
 		})
 	}
 }
+
+func TestLinknameBSS(t *testing.T) {
+	// Test that the linker chooses the right one as the definition
+	// for linknamed variables. See issue #72032.
+	testenv.MustHaveGoBuild(t)
+	t.Parallel()
+
+	tmpdir := t.TempDir()
+
+	src := filepath.Join("testdata", "linkname", "sched.go")
+	exe := filepath.Join(tmpdir, "sched.exe")
+	cmd := testenv.Command(t, testenv.GoToolPath(t), "build", "-o", exe, src)
+	out, err := cmd.CombinedOutput()
+	if err != nil {
+		t.Fatalf("build failed unexpectedly: %v:\n%s", err, out)
+	}
+
+	// Check the symbol size.
+	f, err := objfile.Open(exe)
+	if err != nil {
+		t.Fatalf("fail to open executable: %v", err)
+	}
+	syms, err := f.Symbols()
+	if err != nil {
+		t.Fatalf("fail to get symbols: %v", err)
+	}
+	found := false
+	for _, s := range syms {
+		if s.Name == "runtime.sched" || s.Name == "_runtime.sched" {
+			found = true
+			if s.Size < 100 {
+				// As of Go 1.25 (Mar 2025), runtime.sched has 6848 bytes on
+				// darwin/arm64. It should always be larger than 100 bytes on
+				// all platforms.
+				t.Errorf("runtime.sched symbol size too small: want > 100, got %d", s.Size)
+			}
+		}
+	}
+	if !found {
+		t.Errorf("runtime.sched symbol not found")
+	}
+
+	// Executable should run.
+	cmd = testenv.Command(t, exe)
+	out, err = cmd.CombinedOutput()
+	if err != nil {
+		t.Errorf("executable failed to run: %v\n%s", err, out)
+	}
+}

src/cmd/link/testdata/linkname/sched.go

@@ -0,0 +1,19 @@
+// Copyright 2025 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+import _ "unsafe"
+
+type schedt struct{}
+
+//go:linkname sched runtime.sched
+var sched schedt
+
+func main() {
+	select {
+	default:
+		println("hello")
+	}
+}