Skip to content

Commit 40b19b5

Browse files
runtime: add valgrind instrumentation
Add build tag gated Valgrind annotations to the runtime which let it understand how the runtime manages memory. This allows for Go binaries to be run under Valgrind without emitting spurious errors. Instead of adding the Valgrind headers to the tree, and using cgo to call the various Valgrind client request macros, we just add an assembly function which emits the necessary instructions to trigger client requests. In particular we add instrumentation of the memory allocator, using a two-level mempool structure (as described in the Valgrind manual [0]). We also add annotations which allow Valgrind to track which memory we use for stacks, which seems necessary to let it properly function. We describe the memory model to Valgrind as follows: we treat heap arenas as a "pool" created with VALGRIND_CREATE_MEMPOOL_EXT (so that we can use VALGRIND_MEMPOOL_METAPOOL and VALGRIND_MEMPOOL_AUTO_FREE). Within the pool we treat spans as "superblocks", annotated with VALGRIND_MEMPOOL_ALLOC. We then allocate individual objects within spans with VALGRIND_MALLOCLIKE_BLOCK. It should be noted that running binaries under Valgrind can be _quite slow_, and certain operations, such as running the GC, can be _very slow_. It is recommended to run programs with GOGC=off. Additionally, async preemption should be turned off, since it'll cause strange behavior (GODEBUG=asyncpreemptoff=1). Running Valgrind with --leak-check=yes will result in some errors resulting from some things not being marked fully free'd. These likely need more annotations to rectify, but for now it is recommended to run with --leak-check=off. Updates #73602 [0] https://valgrind.org/docs/manual/mc-manual.html#mc-manual.mempools Change-Id: I71b26c47d7084de71ef1e03947ef6b1cc6d38301 Reviewed-on: https://go-review.googlesource.com/c/go/+/674077 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
1 parent 2a5ac1a commit 40b19b5

15 files changed

+364
-3
lines changed

src/os/pidfd_linux.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,10 @@ func checkPidfd() error {
170170

171171
// Check waitid(P_PIDFD) works.
172172
err = ignoringEINTR(func() error {
173-
return unix.Waitid(unix.P_PIDFD, int(fd), nil, syscall.WEXITED, nil)
173+
var info unix.SiginfoChild
174+
// We don't actually care about the info, but passing a nil pointer
175+
// makes valgrind complain because 0x0 is unaddressable.
176+
return unix.Waitid(unix.P_PIDFD, int(fd), &info, syscall.WEXITED, nil)
174177
})
175178
// Expect ECHILD from waitid since we're not our own parent.
176179
if err != syscall.ECHILD {

src/runtime/arena.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -950,6 +950,9 @@ func freeUserArenaChunk(s *mspan, x unsafe.Pointer) {
950950
if asanenabled {
951951
asanpoison(unsafe.Pointer(s.base()), s.elemsize)
952952
}
953+
if valgrindenabled {
954+
valgrindFree(unsafe.Pointer(s.base()))
955+
}
953956

954957
// Make ourselves non-preemptible as we manipulate state and statistics.
955958
//

src/runtime/malloc.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -754,6 +754,11 @@ func (h *mheap) sysAlloc(n uintptr, hintList **arenaHint, arenaList *[]arenaIdx)
754754
}
755755

756756
mapped:
757+
if valgrindenabled {
758+
valgrindCreateMempool(v)
759+
valgrindMakeMemNoAccess(v, size)
760+
}
761+
757762
// Create arena metadata.
758763
for ri := arenaIndex(uintptr(v)); ri <= arenaIndex(uintptr(v)+size-1); ri++ {
759764
l2 := h.arenas[ri.l1()]
@@ -1084,6 +1089,9 @@ func mallocgc(size uintptr, typ *_type, needzero bool) unsafe.Pointer {
10841089
asanpoison(unsafe.Add(x, size-asanRZ), asanRZ)
10851090
asanunpoison(x, size-asanRZ)
10861091
}
1092+
if valgrindenabled {
1093+
valgrindMalloc(x, size-asanRZ)
1094+
}
10871095

10881096
// Adjust our GC assist debt to account for internal fragmentation.
10891097
if gcBlackenEnabled != 0 && elemsize != 0 {

src/runtime/mgcmark.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,10 @@ func markrootFreeGStacks() {
315315
stackfree(gp.stack)
316316
gp.stack.lo = 0
317317
gp.stack.hi = 0
318+
if valgrindenabled {
319+
valgrindDeregisterStack(gp.valgrindStackID)
320+
gp.valgrindStackID = 0
321+
}
318322
}
319323

320324
q := gQueue{list.head, tail.guintptr(), list.size}

src/runtime/mgcsweep.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -641,6 +641,9 @@ func (sl *sweepLocked) sweep(preserve bool) bool {
641641
if asanenabled && !s.isUserArenaChunk {
642642
asanpoison(unsafe.Pointer(x), size)
643643
}
644+
if valgrindenabled && !s.isUserArenaChunk {
645+
valgrindFree(unsafe.Pointer(x))
646+
}
644647
}
645648
mbits.advance()
646649
abits.advance()

src/runtime/mheap.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1388,6 +1388,10 @@ HaveSpan:
13881388
// Initialize the span.
13891389
h.initSpan(s, typ, spanclass, base, npages)
13901390

1391+
if valgrindenabled {
1392+
valgrindMempoolMalloc(unsafe.Pointer(arenaBase(arenaIndex(base))), unsafe.Pointer(base), npages*pageSize)
1393+
}
1394+
13911395
// Commit and account for any scavenged memory that the span now owns.
13921396
nbytes := npages * pageSize
13931397
if scav != 0 {
@@ -1643,6 +1647,10 @@ func (h *mheap) freeSpan(s *mspan) {
16431647
bytes := s.npages << gc.PageShift
16441648
asanpoison(base, bytes)
16451649
}
1650+
if valgrindenabled {
1651+
base := s.base()
1652+
valgrindMempoolFree(unsafe.Pointer(arenaBase(arenaIndex(base))), unsafe.Pointer(base))
1653+
}
16461654
h.freeSpanLocked(s, spanAllocHeap)
16471655
unlock(&h.lock)
16481656
})
@@ -1671,6 +1679,10 @@ func (h *mheap) freeManual(s *mspan, typ spanAllocType) {
16711679

16721680
s.needzero = 1
16731681
lock(&h.lock)
1682+
if valgrindenabled {
1683+
base := s.base()
1684+
valgrindMempoolFree(unsafe.Pointer(arenaBase(arenaIndex(base))), unsafe.Pointer(base))
1685+
}
16741686
h.freeSpanLocked(s, typ)
16751687
unlock(&h.lock)
16761688
}

src/runtime/proc.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1955,6 +1955,10 @@ func mexit(osStack bool) {
19551955
// Free the gsignal stack.
19561956
if mp.gsignal != nil {
19571957
stackfree(mp.gsignal.stack)
1958+
if valgrindenabled {
1959+
valgrindDeregisterStack(mp.gsignal.valgrindStackID)
1960+
mp.gsignal.valgrindStackID = 0
1961+
}
19581962
// On some platforms, when calling into VDSO (e.g. nanotime)
19591963
// we store our g on the gsignal stack, if there is one.
19601964
// Now the stack is freed, unlink it from the m, so we
@@ -2252,6 +2256,10 @@ func allocm(pp *p, fn func(), id int64) *m {
22522256
// startm.
22532257
systemstack(func() {
22542258
stackfree(freem.g0.stack)
2259+
if valgrindenabled {
2260+
valgrindDeregisterStack(freem.g0.valgrindStackID)
2261+
freem.g0.valgrindStackID = 0
2262+
}
22552263
})
22562264
}
22572265
freem = freem.freelink
@@ -5046,6 +5054,9 @@ func malg(stacksize int32) *g {
50465054
stacksize = round2(stackSystem + stacksize)
50475055
systemstack(func() {
50485056
newg.stack = stackalloc(uint32(stacksize))
5057+
if valgrindenabled {
5058+
newg.valgrindStackID = valgrindRegisterStack(unsafe.Pointer(newg.stack.lo), unsafe.Pointer(newg.stack.hi))
5059+
}
50495060
})
50505061
newg.stackguard0 = newg.stack.lo + stackGuard
50515062
newg.stackguard1 = ^uintptr(0)
@@ -5234,6 +5245,10 @@ func gfput(pp *p, gp *g) {
52345245
gp.stack.lo = 0
52355246
gp.stack.hi = 0
52365247
gp.stackguard0 = 0
5248+
if valgrindenabled {
5249+
valgrindDeregisterStack(gp.valgrindStackID)
5250+
gp.valgrindStackID = 0
5251+
}
52375252
}
52385253

52395254
pp.gFree.push(gp)
@@ -5291,12 +5306,19 @@ retry:
52915306
gp.stack.lo = 0
52925307
gp.stack.hi = 0
52935308
gp.stackguard0 = 0
5309+
if valgrindenabled {
5310+
valgrindDeregisterStack(gp.valgrindStackID)
5311+
gp.valgrindStackID = 0
5312+
}
52945313
})
52955314
}
52965315
if gp.stack.lo == 0 {
52975316
// Stack was deallocated in gfput or just above. Allocate a new one.
52985317
systemstack(func() {
52995318
gp.stack = stackalloc(startingStackSize)
5319+
if valgrindenabled {
5320+
gp.valgrindStackID = valgrindRegisterStack(unsafe.Pointer(gp.stack.lo), unsafe.Pointer(gp.stack.hi))
5321+
}
53005322
})
53015323
gp.stackguard0 = gp.stack.lo + stackGuard
53025324
} else {

src/runtime/runtime2.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -504,6 +504,10 @@ type g struct {
504504
// and check for debt in the malloc hot path. The assist ratio
505505
// determines how this corresponds to scan work debt.
506506
gcAssistBytes int64
507+
508+
// valgrindStackID is used to track what memory is used for stacks when a program is
509+
// built with the "valgrind" build tag, otherwise it is unused.
510+
valgrindStackID uintptr
507511
}
508512

509513
// gTrackingPeriod is the number of transitions out of _Grunning between

src/runtime/sizeof_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ func TestSizeof(t *testing.T) {
2020
_32bit uintptr // size on 32bit platforms
2121
_64bit uintptr // size on 64bit platforms
2222
}{
23-
{runtime.G{}, 276, 432}, // g, but exported for testing
23+
{runtime.G{}, 280, 440}, // g, but exported for testing
2424
{runtime.Sudog{}, 56, 88}, // sudog, but exported for testing
2525
}
2626

src/runtime/stack.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,13 @@ func stackpoolalloc(order uint8) gclinkptr {
211211
s.elemsize = fixedStack << order
212212
for i := uintptr(0); i < _StackCacheSize; i += s.elemsize {
213213
x := gclinkptr(s.base() + i)
214+
if valgrindenabled {
215+
// The address of x.ptr() becomes the base of stacks. We need to
216+
// mark it allocated here and in stackfree and stackpoolfree, and free'd in
217+
// stackalloc in order to avoid overlapping allocations and
218+
// uninitialized memory errors in valgrind.
219+
valgrindMalloc(unsafe.Pointer(x.ptr()), unsafe.Sizeof(x.ptr()))
220+
}
214221
x.ptr().next = s.manualFreeList
215222
s.manualFreeList = x
216223
}
@@ -388,6 +395,12 @@ func stackalloc(n uint32) stack {
388395
c.stackcache[order].list = x.ptr().next
389396
c.stackcache[order].size -= uintptr(n)
390397
}
398+
if valgrindenabled {
399+
// We're about to allocate the stack region starting at x.ptr().
400+
// To prevent valgrind from complaining about overlapping allocations,
401+
// we need to mark the (previously allocated) memory as free'd.
402+
valgrindFree(unsafe.Pointer(x.ptr()))
403+
}
391404
v = unsafe.Pointer(x)
392405
} else {
393406
var s *mspan
@@ -432,6 +445,9 @@ func stackalloc(n uint32) stack {
432445
if asanenabled {
433446
asanunpoison(v, uintptr(n))
434447
}
448+
if valgrindenabled {
449+
valgrindMalloc(v, uintptr(n))
450+
}
435451
if stackDebug >= 1 {
436452
print(" allocated ", v, "\n")
437453
}
@@ -479,6 +495,9 @@ func stackfree(stk stack) {
479495
if asanenabled {
480496
asanpoison(v, n)
481497
}
498+
if valgrindenabled {
499+
valgrindFree(v)
500+
}
482501
if n < fixedStack<<_NumStackOrders && n < _StackCacheSize {
483502
order := uint8(0)
484503
n2 := n
@@ -489,13 +508,24 @@ func stackfree(stk stack) {
489508
x := gclinkptr(v)
490509
if stackNoCache != 0 || gp.m.p == 0 || gp.m.preemptoff != "" {
491510
lock(&stackpool[order].item.mu)
511+
if valgrindenabled {
512+
// x.ptr() is the head of the list of free stacks, and will be used
513+
// when allocating a new stack, so it has to be marked allocated.
514+
valgrindMalloc(unsafe.Pointer(x.ptr()), unsafe.Sizeof(x.ptr()))
515+
}
492516
stackpoolfree(x, order)
493517
unlock(&stackpool[order].item.mu)
494518
} else {
495519
c := gp.m.p.ptr().mcache
496520
if c.stackcache[order].size >= _StackCacheSize {
497521
stackcacherelease(c, order)
498522
}
523+
if valgrindenabled {
524+
// x.ptr() is the head of the list of free stacks, and will
525+
// be used when allocating a new stack, so it has to be
526+
// marked allocated.
527+
valgrindMalloc(unsafe.Pointer(x.ptr()), unsafe.Sizeof(x.ptr()))
528+
}
499529
x.ptr().next = c.stackcache[order].list
500530
c.stackcache[order].list = x
501531
c.stackcache[order].size += n
@@ -583,6 +613,16 @@ func adjustpointer(adjinfo *adjustinfo, vpp unsafe.Pointer) {
583613
if stackDebug >= 4 {
584614
print(" ", pp, ":", hex(p), "\n")
585615
}
616+
if valgrindenabled {
617+
// p is a pointer on a stack, it is inherently initialized, as
618+
// everything on the stack is, but valgrind for _some unknown reason_
619+
// sometimes thinks it's uninitialized, and flags operations on p below
620+
// as uninitialized. We just initialize it if valgrind thinks its
621+
// uninitialized.
622+
//
623+
// See go.dev/issues/73801.
624+
valgrindMakeMemDefined(unsafe.Pointer(&p), unsafe.Sizeof(&p))
625+
}
586626
if adjinfo.old.lo <= p && p < adjinfo.old.hi {
587627
*pp = p + adjinfo.delta
588628
if stackDebug >= 3 {
@@ -936,6 +976,14 @@ func copystack(gp *g, newsize uintptr) {
936976
adjustframe(&u.frame, &adjinfo)
937977
}
938978

979+
if valgrindenabled {
980+
if gp.valgrindStackID == 0 {
981+
gp.valgrindStackID = valgrindRegisterStack(unsafe.Pointer(new.lo), unsafe.Pointer(new.hi))
982+
} else {
983+
valgrindChangeStack(gp.valgrindStackID, unsafe.Pointer(new.lo), unsafe.Pointer(new.hi))
984+
}
985+
}
986+
939987
// free old stack
940988
if stackPoisonCopy != 0 {
941989
fillstack(old, 0xfc)

0 commit comments

Comments
 (0)