]> Cypherpunks repositories - gostls13.git/commitdiff
[release-branch.go1.24] cmd/link: choose one with larger size for duplicated BSS...
authorCherry Mui <cherryyz@google.com>
Tue, 25 Mar 2025 20:55:54 +0000 (16:55 -0400)
committerGopher Robot <gobot@golang.org>
Thu, 10 Apr 2025 15:34:54 +0000 (08:34 -0700)
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
src/cmd/link/link_test.go
src/cmd/link/testdata/linkname/sched.go [new file with mode: 0644]

index 0c234e89758df32960660194784fcad0367fe4dd..182379f0df0f68b8f49cb5e15006aebacb81e143 100644 (file)
@@ -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}
index ab56b49e15d3b17a09554baa1647dc771a3119a9..cd2f9e3953ca5aa509a6cbd56f9cc1c157428354 100644 (file)
@@ -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 (file)
index 0000000..7a9d66f
--- /dev/null
@@ -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")
+       }
+}