From a9d9b5570964c2c2ef07992150174bd3d1b2d891 Mon Sep 17 00:00:00 2001
From: Cherry Mui <cherryyz@google.com>
Date: Tue, 25 Mar 2025 16:55:54 -0400
Subject: [PATCH] [release-branch.go1.24] 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.
Also include the followup fix CL 661915.
Fixes #73092.
Updates #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>
(cherry picked from commit 8f6c083d7bf68a766073c50ceb8ea405a3fe7bed)
Reviewed-on: https://go-review.googlesource.com/c/go/+/662335
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
---
src/cmd/link/internal/loader/loader.go | 29 ++++++++++----
src/cmd/link/link_test.go | 51 +++++++++++++++++++++++++
src/cmd/link/testdata/linkname/sched.go | 19 +++++++++
3 files changed, 91 insertions(+), 8 deletions(-)
create mode 100644 src/cmd/link/testdata/linkname/sched.go
diff --git a/src/cmd/link/internal/loader/loader.go b/src/cmd/link/internal/loader/loader.go
index 0c234e89758..182379f0df0 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}
diff --git a/src/cmd/link/link_test.go b/src/cmd/link/link_test.go
index ab56b49e15d..cd2f9e3953c 100644
--- a/src/cmd/link/link_test.go
+++ b/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,53 @@ 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)
+ }
+ defer f.Close()
+ 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)
+ }
+}
diff --git a/src/cmd/link/testdata/linkname/sched.go b/src/cmd/link/testdata/linkname/sched.go
new file mode 100644
index 00000000000..7a9d66f4950
--- /dev/null
+++ b/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")
+ }
+}
--
GitLab