]> Cypherpunks repositories - gostls13.git/commitdiff
[release-branch.go1.23] cmd/link: permit a larger size BSS reference to a smaller...
authorCherry Mui <cherryyz@google.com>
Thu, 26 Jun 2025 19:46:31 +0000 (15:46 -0400)
committerGopher Robot <gobot@golang.org>
Fri, 27 Jun 2025 17:20:55 +0000 (10:20 -0700)
Currently, if there is a BSS reference and a DATA symbol
definition with the same name, we pick the DATA symbol, as it
contains the right content. In this case, if the BSS reference
has a larger size, we error out, because it is not safe to access
a smaller symbol as if it has a larger size.

Sometimes code declares a global variable in Go and defines it
in assembly with content. They are expected to be of the same
size. However, in ASAN mode, we insert a red zone for the variable
on the Go side, making it have a larger size, whereas the assembly
symbol is unchanged. This causes the Go reference (BSS) has a
larger size than the assembly definition (DATA). It results in an
error currently. This code is valid and safe, so we should permit
that.

We support this case by increasing the symbol size to match the
larger size (of the BSS side). The symbol content (from the DATA
side) is kept. In some sense, we merge the two symbols. When
loading symbols, it is not easy to change its size (as the object
file may be mapped read-only), so we add it to a fixup list, and
fix it up later after all Go symbols are loaded. This is a very
rare case, so the list should not be long.

We could limit this to just ASAN mode. But it seems okay to allow
this in general. As long as the symbol has the larger size, it is
safe to access it with the larger size.

Updates #74314.
Fixes #74402.

Change-Id: I3ee6e46161d8f59500e2b81befea11e563355a57
Reviewed-on: https://go-review.googlesource.com/c/go/+/684236
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
(cherry picked from commit 0f8ab2db177baee7b04182f5641693df3b212aa9)
Reviewed-on: https://go-review.googlesource.com/c/go/+/684456
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>

src/cmd/cgo/internal/testsanitizers/asan_test.go
src/cmd/cgo/internal/testsanitizers/cc_test.go
src/cmd/cgo/internal/testsanitizers/testdata/asan_global_asm/asm.s [new file with mode: 0644]
src/cmd/cgo/internal/testsanitizers/testdata/asan_global_asm/main.go [new file with mode: 0644]
src/cmd/cgo/internal/testsanitizers/testdata/asan_global_asm2_fail/asm.s [new file with mode: 0644]
src/cmd/cgo/internal/testsanitizers/testdata/asan_global_asm2_fail/main.go [new file with mode: 0644]
src/cmd/link/internal/loader/loader.go

index 7db356244a85a8bd164468acca74dcd9f736e0ec..20f6ef2d242f1b7675a2b3b88385503cad27dfd3 100644 (file)
@@ -65,6 +65,8 @@ func TestASAN(t *testing.T) {
                {src: "asan_global3_fail.go", memoryAccessError: "global-buffer-overflow", errorLocation: "asan_global3_fail.go:13"},
                {src: "asan_global4_fail.go", memoryAccessError: "global-buffer-overflow", errorLocation: "asan_global4_fail.go:21"},
                {src: "asan_global5.go"},
+               {src: "asan_global_asm"},
+               {src: "asan_global_asm2_fail", memoryAccessError: "global-buffer-overflow", errorLocation: "main.go:17"},
                {src: "arena_fail.go", memoryAccessError: "use-after-poison", errorLocation: "arena_fail.go:26", experiments: []string{"arenas"}},
        }
        for _, tc := range cases {
index e650de835ab42ecfd348820b1c094dad885a7642..c493597ded898a12294b4a96c78cc7715aa1c1f4 100644 (file)
@@ -536,7 +536,7 @@ func (c *config) checkRuntime() (skip bool, err error) {
 
 // srcPath returns the path to the given file relative to this test's source tree.
 func srcPath(path string) string {
-       return filepath.Join("testdata", path)
+       return "./testdata/" + path
 }
 
 // A tempDir manages a temporary directory within a test.
diff --git a/src/cmd/cgo/internal/testsanitizers/testdata/asan_global_asm/asm.s b/src/cmd/cgo/internal/testsanitizers/testdata/asan_global_asm/asm.s
new file mode 100644 (file)
index 0000000..b4b9766
--- /dev/null
@@ -0,0 +1,8 @@
+// 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.
+
+#include "textflag.h"
+
+DATA   ·x(SB)/8, $123
+GLOBL  ·x(SB), NOPTR, $8
diff --git a/src/cmd/cgo/internal/testsanitizers/testdata/asan_global_asm/main.go b/src/cmd/cgo/internal/testsanitizers/testdata/asan_global_asm/main.go
new file mode 100644 (file)
index 0000000..2ae5448
--- /dev/null
@@ -0,0 +1,11 @@
+// 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
+
+var x uint64
+
+func main() {
+       println(x)
+}
diff --git a/src/cmd/cgo/internal/testsanitizers/testdata/asan_global_asm2_fail/asm.s b/src/cmd/cgo/internal/testsanitizers/testdata/asan_global_asm2_fail/asm.s
new file mode 100644 (file)
index 0000000..b4b9766
--- /dev/null
@@ -0,0 +1,8 @@
+// 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.
+
+#include "textflag.h"
+
+DATA   ·x(SB)/8, $123
+GLOBL  ·x(SB), NOPTR, $8
diff --git a/src/cmd/cgo/internal/testsanitizers/testdata/asan_global_asm2_fail/main.go b/src/cmd/cgo/internal/testsanitizers/testdata/asan_global_asm2_fail/main.go
new file mode 100644 (file)
index 0000000..2d02a1b
--- /dev/null
@@ -0,0 +1,20 @@
+// 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"
+
+var x uint64
+
+func main() {
+       bar(&x)
+}
+
+func bar(a *uint64) {
+       p := (*uint64)(unsafe.Add(unsafe.Pointer(a), 1*unsafe.Sizeof(uint64(1))))
+       if *p == 10 { // BOOM
+               println("its value is 10")
+       }
+}
index 6697095fbaab9f6c18f025404a34a92370e91d41..ca1736fd2e559e747d1cc358b11ded70e20d1658 100644 (file)
@@ -251,6 +251,12 @@ type Loader struct {
        // CgoExports records cgo-exported symbols by SymName.
        CgoExports map[string]Sym
 
+       // sizeFixups records symbols that we need to fix up the size
+       // after loading. It is very rarely needed, only for a DATA symbol
+       // and a BSS symbol with the same name, and the BSS symbol has
+       // larger size.
+       sizeFixups []symAndSize
+
        flags uint32
 
        strictDupMsgs int // number of strict-dup warning/errors, when FlagStrictDups is enabled
@@ -467,18 +473,17 @@ func (st *loadState) addSym(name string, ver int, r *oReader, li uint32, kind in
        // In summary, the "overwrite" variable and the final result are
        //
        // new sym       old sym       result
-       // ---------------------------------------------
+       // -------------------------------------------------------
        // TEXT          BSS           new wins
        // DATA          DATA          ERROR
        // DATA lg/eq    BSS  sm/eq    new wins
-       // DATA small    BSS  large    ERROR
-       // BSS  large    DATA small    ERROR
+       // DATA small    BSS  large    merge: new with larger size
+       // BSS  large    DATA small    merge: old with larger size
        // BSS  large    BSS  small    new wins
        // BSS  sm/eq    D/B  lg/eq    old wins
        // BSS           TEXT          old wins
        oldtyp := sym.AbiSymKindToSymKind[objabi.SymKind(oldsym.Type())]
        newtyp := sym.AbiSymKindToSymKind[objabi.SymKind(osym.Type())]
-       oldIsText := oldtyp == sym.STEXT
        newIsText := newtyp == sym.STEXT
        oldHasContent := oldr.DataSize(oldli) != 0
        newHasContent := r.DataSize(li) != 0
@@ -486,12 +491,28 @@ func (st *loadState) addSym(name string, ver int, r *oReader, li uint32, kind in
        newIsBSS := newtyp.IsData() && !newHasContent
        switch {
        case newIsText && oldIsBSS,
-               newHasContent && oldIsBSS && sz >= oldsz,
+               newHasContent && oldIsBSS,
                newIsBSS && oldIsBSS && sz > oldsz:
                // new symbol overwrites old symbol.
                l.objSyms[oldi] = objSym{r.objidx, li}
-       case newIsBSS && (oldsz >= sz || oldIsText):
+               if oldsz > sz {
+                       // If the BSS symbol has a larger size, expand the data
+                       // symbol's size so access from the BSS side cannot overrun.
+                       // It is hard to modify the symbol size until all Go objects
+                       // (potentially read-only) are loaded, so we record it in
+                       // a fixup table and apply them later. This is very rare.
+                       // One case is a global variable with a Go declaration and an
+                       // assembly definition, which typically have the same size,
+                       // but in ASAN mode the Go declaration has a larger size due
+                       // to the inserted red zone.
+                       l.sizeFixups = append(l.sizeFixups, symAndSize{oldi, uint32(oldsz)})
+               }
+       case newIsBSS:
                // old win, just ignore the new symbol.
+               if sz > oldsz {
+                       // See the comment above for sizeFixups.
+                       l.sizeFixups = append(l.sizeFixups, symAndSize{oldi, uint32(sz)})
+               }
        default:
                log.Fatalf("duplicated definition of symbol %s, from %s (type %s size %d) and %s (type %s size %d)", name, r.unit.Lib.Pkg, newtyp, sz, oldr.unit.Lib.Pkg, oldtyp, oldsz)
        }
@@ -2286,6 +2307,10 @@ func (l *Loader) LoadSyms(arch *sys.Arch) {
                st.preloadSyms(r, hashedDef)
                st.preloadSyms(r, nonPkgDef)
        }
+       for _, sf := range l.sizeFixups {
+               pp := l.cloneToExternal(sf.sym)
+               pp.size = int64(sf.size)
+       }
        for _, vr := range st.linknameVarRefs {
                l.checkLinkname(vr.pkg, vr.name, vr.sym)
        }
@@ -2443,7 +2468,7 @@ func topLevelSym(sname string, skind sym.SymKind) bool {
 // a symbol originally discovered as part of an object file, it's
 // easier to do this if we make the updates to an external symbol
 // payload.
-func (l *Loader) cloneToExternal(symIdx Sym) {
+func (l *Loader) cloneToExternal(symIdx Sym) *extSymPayload {
        if l.IsExternal(symIdx) {
                panic("sym is already external, no need for clone")
        }
@@ -2495,6 +2520,8 @@ func (l *Loader) cloneToExternal(symIdx Sym) {
        // Some attributes were encoded in the object file. Copy them over.
        l.SetAttrDuplicateOK(symIdx, r.Sym(li).Dupok())
        l.SetAttrShared(symIdx, r.Shared())
+
+       return pp
 }
 
 // Copy the payload of symbol src to dst. Both src and dst must be external