Skip to content

Commit 0f8ab2d

Browse files
committed
cmd/link: permit a larger size BSS reference to a smaller DATA symbol
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. Fixes #74314. 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>
1 parent 988a20c commit 0f8ab2d

File tree

7 files changed

+84
-8
lines changed

7 files changed

+84
-8
lines changed

‎src/cmd/cgo/internal/testsanitizers/asan_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ func TestASAN(t *testing.T) {
4242
{src: "asan_global3_fail.go", memoryAccessError: "global-buffer-overflow", errorLocation: "asan_global3_fail.go:13"},
4343
{src: "asan_global4_fail.go", memoryAccessError: "global-buffer-overflow", errorLocation: "asan_global4_fail.go:21"},
4444
{src: "asan_global5.go"},
45+
{src: "asan_global_asm"},
46+
{src: "asan_global_asm2_fail", memoryAccessError: "global-buffer-overflow", errorLocation: "main.go:17"},
4547
{src: "arena_fail.go", memoryAccessError: "use-after-poison", errorLocation: "arena_fail.go:26", experiments: []string{"arenas"}},
4648
}
4749
for _, tc := range cases {

‎src/cmd/cgo/internal/testsanitizers/cc_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -554,7 +554,7 @@ func (c *config) checkRuntime() (skip bool, err error) {
554554

555555
// srcPath returns the path to the given file relative to this test's source tree.
556556
func srcPath(path string) string {
557-
return filepath.Join("testdata", path)
557+
return "./testdata/" + path
558558
}
559559

560560
// A tempDir manages a temporary directory within a test.
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// Copyright 2025 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
#include "textflag.h"
6+
7+
DATA ·x(SB)/8, $123
8+
GLOBL ·x(SB), NOPTR, $8
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// Copyright 2025 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package main
6+
7+
var x uint64
8+
9+
func main() {
10+
println(x)
11+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// Copyright 2025 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
#include "textflag.h"
6+
7+
DATA ·x(SB)/8, $123
8+
GLOBL ·x(SB), NOPTR, $8
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Copyright 2025 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package main
6+
7+
import "unsafe"
8+
9+
var x uint64
10+
11+
func main() {
12+
bar(&x)
13+
}
14+
15+
func bar(a *uint64) {
16+
p := (*uint64)(unsafe.Add(unsafe.Pointer(a), 1*unsafe.Sizeof(uint64(1))))
17+
if *p == 10 { // BOOM
18+
println("its value is 10")
19+
}
20+
}

‎src/cmd/link/internal/loader/loader.go

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,12 @@ type Loader struct {
253253

254254
WasmExports []Sym
255255

256+
// sizeFixups records symbols that we need to fix up the size
257+
// after loading. It is very rarely needed, only for a DATA symbol
258+
// and a BSS symbol with the same name, and the BSS symbol has
259+
// larger size.
260+
sizeFixups []symAndSize
261+
256262
flags uint32
257263

258264
strictDupMsgs int // number of strict-dup warning/errors, when FlagStrictDups is enabled
@@ -469,31 +475,46 @@ func (st *loadState) addSym(name string, ver int, r *oReader, li uint32, kind in
469475
// In summary, the "overwrite" variable and the final result are
470476
//
471477
// new sym old sym result
472-
// ---------------------------------------------
478+
// -------------------------------------------------------
473479
// TEXT BSS new wins
474480
// DATA DATA ERROR
475481
// DATA lg/eq BSS sm/eq new wins
476-
// DATA small BSS large ERROR
477-
// BSS large DATA small ERROR
482+
// DATA small BSS large merge: new with larger size
483+
// BSS large DATA small merge: old with larger size
478484
// BSS large BSS small new wins
479485
// BSS sm/eq D/B lg/eq old wins
480486
// BSS TEXT old wins
481487
oldtyp := sym.AbiSymKindToSymKind[objabi.SymKind(oldsym.Type())]
482488
newtyp := sym.AbiSymKindToSymKind[objabi.SymKind(osym.Type())]
483-
oldIsText := oldtyp.IsText()
484489
newIsText := newtyp.IsText()
485490
oldHasContent := oldr.DataSize(oldli) != 0
486491
newHasContent := r.DataSize(li) != 0
487492
oldIsBSS := oldtyp.IsData() && !oldHasContent
488493
newIsBSS := newtyp.IsData() && !newHasContent
489494
switch {
490495
case newIsText && oldIsBSS,
491-
newHasContent && oldIsBSS && sz >= oldsz,
496+
newHasContent && oldIsBSS,
492497
newIsBSS && oldIsBSS && sz > oldsz:
493498
// new symbol overwrites old symbol.
494499
l.objSyms[oldi] = objSym{r.objidx, li}
495-
case newIsBSS && (oldsz >= sz || oldIsText):
500+
if oldsz > sz {
501+
// If the BSS symbol has a larger size, expand the data
502+
// symbol's size so access from the BSS side cannot overrun.
503+
// It is hard to modify the symbol size until all Go objects
504+
// (potentially read-only) are loaded, so we record it in
505+
// a fixup table and apply them later. This is very rare.
506+
// One case is a global variable with a Go declaration and an
507+
// assembly definition, which typically have the same size,
508+
// but in ASAN mode the Go declaration has a larger size due
509+
// to the inserted red zone.
510+
l.sizeFixups = append(l.sizeFixups, symAndSize{oldi, uint32(oldsz)})
511+
}
512+
case newIsBSS:
496513
// old win, just ignore the new symbol.
514+
if sz > oldsz {
515+
// See the comment above for sizeFixups.
516+
l.sizeFixups = append(l.sizeFixups, symAndSize{oldi, uint32(sz)})
517+
}
497518
default:
498519
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)
499520
}
@@ -2277,6 +2298,10 @@ func (l *Loader) LoadSyms(arch *sys.Arch) {
22772298
st.preloadSyms(r, hashedDef)
22782299
st.preloadSyms(r, nonPkgDef)
22792300
}
2301+
for _, sf := range l.sizeFixups {
2302+
pp := l.cloneToExternal(sf.sym)
2303+
pp.size = int64(sf.size)
2304+
}
22802305
for _, vr := range st.linknameVarRefs {
22812306
l.checkLinkname(vr.pkg, vr.name, vr.sym)
22822307
}
@@ -2498,7 +2523,7 @@ func topLevelSym(sname string, skind sym.SymKind) bool {
24982523
// a symbol originally discovered as part of an object file, it's
24992524
// easier to do this if we make the updates to an external symbol
25002525
// payload.
2501-
func (l *Loader) cloneToExternal(symIdx Sym) {
2526+
func (l *Loader) cloneToExternal(symIdx Sym) *extSymPayload {
25022527
if l.IsExternal(symIdx) {
25032528
panic("sym is already external, no need for clone")
25042529
}
@@ -2550,6 +2575,8 @@ func (l *Loader) cloneToExternal(symIdx Sym) {
25502575
// Some attributes were encoded in the object file. Copy them over.
25512576
l.SetAttrDuplicateOK(symIdx, r.Sym(li).Dupok())
25522577
l.SetAttrShared(symIdx, r.Shared())
2578+
2579+
return pp
25532580
}
25542581

25552582
// Copy the payload of symbol src to dst. Both src and dst must be external

0 commit comments

Comments
 (0)