Skip to content

Commit c58f58b

Browse files
mknyszekgopherbot
authored andcommitted
runtime: mark and identify tiny blocks in checkfinalizers mode
This change adds support for identifying cleanups and finalizers attached to tiny blocks to checkfinalizers mode. It also notes a subtle pitfall, which is that the cleanup arg, if tiny-allocated, could end up co-located with the object with the cleanup attached! Oops... For #72949. Change-Id: Icbe0112f7dcfc63f35c66cf713216796a70121ce Reviewed-on: https://go-review.googlesource.com/c/go/+/662037 LUCI-TryBot-Result: Go LUCI <[email protected]> Auto-Submit: Michael Knyszek <[email protected]> Reviewed-by: Michael Pratt <[email protected]> Reviewed-by: Carlos Amedee <[email protected]>
1 parent 913c069 commit c58f58b

File tree

6 files changed

+151
-38
lines changed

6 files changed

+151
-38
lines changed

src/runtime/gc_test.go

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1076,14 +1076,22 @@ func TestMSpanQueue(t *testing.T) {
10761076

10771077
func TestDetectFinalizerAndCleanupLeaks(t *testing.T) {
10781078
got := runTestProg(t, "testprog", "DetectFinalizerAndCleanupLeaks", "GODEBUG=checkfinalizers=1")
1079-
sp := strings.SplitN(got, "runtime: detected", 2)
1079+
sp := strings.SplitN(got, "detected possible issues with cleanups and/or finalizers", 2)
10801080
if len(sp) != 2 {
10811081
t.Fatalf("expected the runtime to throw, got:\n%s", got)
10821082
}
1083-
if strings.Count(sp[0], "is reachable from cleanup or finalizer") != 2 {
1083+
if strings.Count(sp[0], "is reachable from") != 2 {
10841084
t.Fatalf("expected exactly two leaked cleanups and/or finalizers, got:\n%s", got)
10851085
}
1086-
if strings.Count(sp[0], "created at: main.DetectFinalizerAndCleanupLeaks") != 2 {
1087-
t.Fatalf("expected two symbolized locations, got:\n%s", got)
1086+
// N.B. Disable in race mode and in asan mode. Both disable the tiny allocator.
1087+
wantSymbolizedLocations := 2
1088+
if !race.Enabled && !asan.Enabled {
1089+
if strings.Count(sp[0], "is in a tiny block") != 1 {
1090+
t.Fatalf("expected exactly one report for allocation in a tiny block, got:\n%s", got)
1091+
}
1092+
wantSymbolizedLocations++
1093+
}
1094+
if strings.Count(sp[0], "main.DetectFinalizerAndCleanupLeaks()") != wantSymbolizedLocations {
1095+
t.Fatalf("expected %d symbolized locations, got:\n%s", wantSymbolizedLocations, got)
10881096
}
10891097
}

src/runtime/malloc.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1670,6 +1670,12 @@ func postMallocgcDebug(x unsafe.Pointer, elemsize uintptr, typ *_type) {
16701670
traceRelease(trace)
16711671
}
16721672
}
1673+
1674+
// N.B. elemsize == 0 indicates a tiny allocation, since no new slot was
1675+
// allocated to fulfill this call to mallocgc.
1676+
if debug.checkfinalizers != 0 && elemsize == 0 {
1677+
setTinyBlockContext(unsafe.Pointer(alignDown(uintptr(x), maxTinySize)))
1678+
}
16731679
}
16741680

16751681
// deductAssistCredit reduces the current G's assist credit

src/runtime/mcheckmark.go

Lines changed: 57 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -148,15 +148,29 @@ func runCheckmark(prepareRootSet func(*gcWork)) {
148148
func checkFinalizersAndCleanups() {
149149
assertWorldStopped()
150150

151+
const (
152+
reportCycle = 1 << iota
153+
reportTiny
154+
)
155+
156+
// Find the arena and page index into that arena for this shard.
151157
type report struct {
152-
ptr uintptr
153-
sp *special
158+
issues int
159+
ptr uintptr
160+
sp *special
154161
}
155-
var reports [25]report
162+
var reports [50]report
156163
var nreports int
157164
var more bool
165+
var lastTinyBlock uintptr
158166

159167
forEachSpecial(func(p uintptr, s *mspan, sp *special) bool {
168+
// N.B. The tiny block specials are sorted first in the specials list.
169+
if sp.kind == _KindSpecialTinyBlock {
170+
lastTinyBlock = s.base() + sp.offset
171+
return true
172+
}
173+
160174
// We only care about finalizers and cleanups.
161175
if sp.kind != _KindSpecialFinalizer && sp.kind != _KindSpecialCleanup {
162176
return true
@@ -180,19 +194,28 @@ func checkFinalizersAndCleanups() {
180194
if bytep == nil {
181195
return true
182196
}
197+
var issues int
183198
if atomic.Load8(bytep)&mask != 0 {
199+
issues |= reportCycle
200+
}
201+
if p >= lastTinyBlock && p < lastTinyBlock+maxTinySize {
202+
issues |= reportTiny
203+
}
204+
if issues != 0 {
184205
if nreports >= len(reports) {
185206
more = true
186207
return false
187208
}
188-
reports[nreports] = report{p, sp}
209+
reports[nreports] = report{issues, p, sp}
189210
nreports++
190211
}
191212
return true
192213
})
193214

194215
if nreports > 0 {
195216
lastPtr := uintptr(0)
217+
println("WARNING: LIKELY CLEANUP/FINALIZER ISSUES")
218+
println()
196219
for _, r := range reports[:nreports] {
197220
var ctx *specialCheckFinalizer
198221
var kind string
@@ -210,36 +233,54 @@ func checkFinalizersAndCleanups() {
210233
if lastPtr != 0 {
211234
println()
212235
}
213-
print("runtime: value of type ", toRType(ctx.ptrType).string(), " @ ", hex(r.ptr), " is reachable from cleanup or finalizer\n")
214-
println("value reachable from function or argument at one of:")
236+
print("Value of type ", toRType(ctx.ptrType).string(), " at ", hex(r.ptr), "\n")
237+
if r.issues&reportCycle != 0 {
238+
if r.sp.kind == _KindSpecialFinalizer {
239+
println(" is reachable from finalizer")
240+
} else {
241+
println(" is reachable from cleanup or cleanup argument")
242+
}
243+
}
244+
if r.issues&reportTiny != 0 {
245+
println(" is in a tiny block with other (possibly long-lived) values")
246+
}
247+
if r.issues&reportTiny != 0 && r.issues&reportCycle != 0 {
248+
if r.sp.kind == _KindSpecialFinalizer {
249+
println(" may be in the same tiny block as finalizer")
250+
} else {
251+
println(" may be in the same tiny block as cleanup or cleanup argument")
252+
}
253+
}
215254
}
255+
println()
216256

257+
println("Has", kind, "at", hex(uintptr(unsafe.Pointer(r.sp))))
217258
funcInfo := findfunc(ctx.funcPC)
218259
if funcInfo.valid() {
219-
file, line := funcline(funcInfo, ctx.createPC)
220-
print(funcname(funcInfo), " (", kind, ")\n")
221-
print("\t", file, ":", line, "\n")
260+
file, line := funcline(funcInfo, ctx.funcPC)
261+
print(" ", funcname(funcInfo), "()\n")
262+
print(" ", file, ":", line, " +", hex(ctx.funcPC-funcInfo.entry()), "\n")
222263
} else {
223-
print("<bad pc ", hex(ctx.funcPC), ">\n")
264+
print(" <bad pc ", hex(ctx.funcPC), ">\n")
224265
}
225266

226-
print("created at: ")
267+
println("created at: ")
227268
createInfo := findfunc(ctx.createPC)
228269
if createInfo.valid() {
229270
file, line := funcline(createInfo, ctx.createPC)
230-
print(funcname(createInfo), "\n")
231-
print("\t", file, ":", line, "\n")
271+
print(" ", funcname(createInfo), "()\n")
272+
print(" ", file, ":", line, " +", hex(ctx.createPC-createInfo.entry()), "\n")
232273
} else {
233-
print("<bad pc ", hex(ctx.createPC), ">\n")
274+
print(" <bad pc ", hex(ctx.createPC), ">\n")
234275
}
235276

236277
lastPtr = r.ptr
237278
}
238279
println()
239280
if more {
240-
println("runtime: too many errors")
281+
println("... too many potential issues ...")
241282
}
242-
throw("runtime: detected possible cleanup and/or finalizer leaks")
283+
throw("detected possible issues with cleanups and/or finalizers")
243284
}
244285
}
245286

src/runtime/mheap.go

Lines changed: 58 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,7 @@ type mheap struct {
218218
specialfinalizeralloc fixalloc // allocator for specialfinalizer*
219219
specialCleanupAlloc fixalloc // allocator for specialCleanup*
220220
specialCheckFinalizerAlloc fixalloc // allocator for specialCheckFinalizer*
221+
specialTinyBlockAlloc fixalloc // allocator for specialTinyBlock*
221222
specialprofilealloc fixalloc // allocator for specialprofile*
222223
specialReachableAlloc fixalloc // allocator for specialReachable
223224
specialPinCounterAlloc fixalloc // allocator for specialPinCounter
@@ -793,6 +794,7 @@ func (h *mheap) init() {
793794
h.specialfinalizeralloc.init(unsafe.Sizeof(specialfinalizer{}), nil, nil, &memstats.other_sys)
794795
h.specialCleanupAlloc.init(unsafe.Sizeof(specialCleanup{}), nil, nil, &memstats.other_sys)
795796
h.specialCheckFinalizerAlloc.init(unsafe.Sizeof(specialCheckFinalizer{}), nil, nil, &memstats.other_sys)
797+
h.specialTinyBlockAlloc.init(unsafe.Sizeof(specialTinyBlock{}), nil, nil, &memstats.other_sys)
796798
h.specialprofilealloc.init(unsafe.Sizeof(specialprofile{}), nil, nil, &memstats.other_sys)
797799
h.specialReachableAlloc.init(unsafe.Sizeof(specialReachable{}), nil, nil, &memstats.other_sys)
798800
h.specialPinCounterAlloc.init(unsafe.Sizeof(specialPinCounter{}), nil, nil, &memstats.other_sys)
@@ -1967,23 +1969,28 @@ func (q *mSpanQueue) popN(n int) mSpanQueue {
19671969
}
19681970

19691971
const (
1972+
// _KindSpecialTinyBlock indicates that a given allocation is a tiny block.
1973+
// Ordered before KindSpecialFinalizer and KindSpecialCleanup so that it
1974+
// always appears first in the specials list.
1975+
// Used only if debug.checkfinalizers != 0.
1976+
_KindSpecialTinyBlock = 1
19701977
// _KindSpecialFinalizer is for tracking finalizers.
1971-
_KindSpecialFinalizer = 1
1978+
_KindSpecialFinalizer = 2
19721979
// _KindSpecialWeakHandle is used for creating weak pointers.
1973-
_KindSpecialWeakHandle = 2
1980+
_KindSpecialWeakHandle = 3
19741981
// _KindSpecialProfile is for memory profiling.
1975-
_KindSpecialProfile = 3
1982+
_KindSpecialProfile = 4
19761983
// _KindSpecialReachable is a special used for tracking
19771984
// reachability during testing.
1978-
_KindSpecialReachable = 4
1985+
_KindSpecialReachable = 5
19791986
// _KindSpecialPinCounter is a special used for objects that are pinned
19801987
// multiple times
1981-
_KindSpecialPinCounter = 5
1988+
_KindSpecialPinCounter = 6
19821989
// _KindSpecialCleanup is for tracking cleanups.
1983-
_KindSpecialCleanup = 6
1990+
_KindSpecialCleanup = 7
19841991
// _KindSpecialCheckFinalizer adds additional context to a finalizer or cleanup.
19851992
// Used only if debug.checkfinalizers != 0.
1986-
_KindSpecialCheckFinalizer = 7
1993+
_KindSpecialCheckFinalizer = 8
19871994
)
19881995

19891996
type special struct {
@@ -2347,6 +2354,45 @@ func clearCleanupContext(ptr uintptr, cleanupID uint64) {
23472354
unlock(&mheap_.speciallock)
23482355
}
23492356

2357+
// Indicates that an allocation is a tiny block.
2358+
// Used only if debug.checkfinalizers != 0.
2359+
type specialTinyBlock struct {
2360+
_ sys.NotInHeap
2361+
special special
2362+
}
2363+
2364+
// setTinyBlockContext marks an allocation as a tiny block to diagnostics like
2365+
// checkfinalizer.
2366+
//
2367+
// A tiny block is only marked if it actually contains more than one distinct
2368+
// value, since we're using this for debugging.
2369+
func setTinyBlockContext(ptr unsafe.Pointer) {
2370+
lock(&mheap_.speciallock)
2371+
s := (*specialTinyBlock)(mheap_.specialTinyBlockAlloc.alloc())
2372+
unlock(&mheap_.speciallock)
2373+
s.special.kind = _KindSpecialTinyBlock
2374+
2375+
mp := acquirem()
2376+
addspecial(ptr, &s.special, false)
2377+
releasem(mp)
2378+
KeepAlive(ptr)
2379+
}
2380+
2381+
// inTinyBlock returns whether ptr is in a tiny alloc block, at one point grouped
2382+
// with other distinct values.
2383+
func inTinyBlock(ptr uintptr) bool {
2384+
assertWorldStopped()
2385+
2386+
ptr = alignDown(ptr, maxTinySize)
2387+
span := spanOfHeap(ptr)
2388+
if span == nil {
2389+
return false
2390+
}
2391+
offset := ptr - span.base()
2392+
_, exists := span.specialFindSplicePoint(offset, _KindSpecialTinyBlock)
2393+
return exists
2394+
}
2395+
23502396
// The described object has a weak pointer.
23512397
//
23522398
// Weak pointers in the GC have the following invariants:
@@ -2766,6 +2812,11 @@ func freeSpecial(s *special, p unsafe.Pointer, size uintptr) {
27662812
lock(&mheap_.speciallock)
27672813
mheap_.specialCheckFinalizerAlloc.free(unsafe.Pointer(sc))
27682814
unlock(&mheap_.speciallock)
2815+
case _KindSpecialTinyBlock:
2816+
st := (*specialTinyBlock)(unsafe.Pointer(s))
2817+
lock(&mheap_.speciallock)
2818+
mheap_.specialTinyBlockAlloc.free(unsafe.Pointer(st))
2819+
unlock(&mheap_.speciallock)
27692820
default:
27702821
throw("bad special kind")
27712822
panic("not reached")

src/runtime/runtime1.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -333,14 +333,14 @@ var debug struct {
333333
traceCheckStackOwnership int32
334334
profstackdepth int32
335335
dataindependenttiming int32
336-
checkfinalizers int32
337336

338337
// debug.malloc is used as a combined debug check
339338
// in the malloc function and should be set
340339
// if any of the below debug options is != 0.
341-
malloc bool
342-
inittrace int32
343-
sbrk int32
340+
malloc bool
341+
inittrace int32
342+
sbrk int32
343+
checkfinalizers int32
344344
// traceallocfree controls whether execution traces contain
345345
// detailed trace data about memory allocation. This value
346346
// affects debug.malloc only if it is != 0 and the execution
@@ -440,7 +440,7 @@ func parsedebugvars() {
440440
// apply environment settings
441441
parsegodebug(godebug, nil)
442442

443-
debug.malloc = (debug.inittrace | debug.sbrk) != 0
443+
debug.malloc = (debug.inittrace | debug.sbrk | debug.checkfinalizers) != 0
444444
debug.profstackdepth = min(debug.profstackdepth, maxProfStackDepth)
445445

446446
// Disable async preemption in checkmark mode. The following situation is

src/runtime/testdata/testprog/checkfinalizers.go

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ func init() {
1313
register("DetectFinalizerAndCleanupLeaks", DetectFinalizerAndCleanupLeaks)
1414
}
1515

16+
type tiny uint8
17+
18+
var tinySink *tiny
19+
1620
// Intended to be run only with `GODEBUG=checkfinalizers=1`.
1721
func DetectFinalizerAndCleanupLeaks() {
1822
type T *int
@@ -34,6 +38,15 @@ func DetectFinalizerAndCleanupLeaks() {
3438
**cNoLeak = x
3539
}, int(0)).Stop()
3640

41+
// Ensure we create an allocation into a tiny block that shares space among several values.
42+
var ctLeak *tiny
43+
for i := 0; i < 18; i++ {
44+
tinySink = ctLeak
45+
ctLeak = new(tiny)
46+
*ctLeak = tiny(i)
47+
}
48+
runtime.AddCleanup(ctLeak, func(_ struct{}) {}, struct{}{})
49+
3750
// Leak a finalizer.
3851
fLeak := new(T)
3952
runtime.SetFinalizer(fLeak, func(_ *T) {
@@ -49,10 +62,4 @@ func DetectFinalizerAndCleanupLeaks() {
4962
// runtime.GC here should crash.
5063
runtime.GC()
5164
println("OK")
52-
53-
// Keep everything alive.
54-
runtime.KeepAlive(cLeak)
55-
runtime.KeepAlive(cNoLeak)
56-
runtime.KeepAlive(fLeak)
57-
runtime.KeepAlive(fNoLeak)
5865
}

0 commit comments

Comments
 (0)