]> Cypherpunks repositories - gostls13.git/commitdiff
runtime: mark and identify tiny blocks in checkfinalizers mode
authorMichael Anthony Knyszek <mknyszek@google.com>
Fri, 9 May 2025 18:53:06 +0000 (18:53 +0000)
committerGopher Robot <gobot@golang.org>
Tue, 20 May 2025 18:15:12 +0000 (11:15 -0700)
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 <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
src/runtime/gc_test.go
src/runtime/malloc.go
src/runtime/mcheckmark.go
src/runtime/mheap.go
src/runtime/runtime1.go
src/runtime/testdata/testprog/checkfinalizers.go

index b5e69062355b69f15d70310996ae186ecbdb0ab5..0a1e01cbcf9d7c2acdee436d692df95a2c5bf196 100644 (file)
@@ -1076,14 +1076,22 @@ func TestMSpanQueue(t *testing.T) {
 
 func TestDetectFinalizerAndCleanupLeaks(t *testing.T) {
        got := runTestProg(t, "testprog", "DetectFinalizerAndCleanupLeaks", "GODEBUG=checkfinalizers=1")
-       sp := strings.SplitN(got, "runtime: detected", 2)
+       sp := strings.SplitN(got, "detected possible issues with cleanups and/or finalizers", 2)
        if len(sp) != 2 {
                t.Fatalf("expected the runtime to throw, got:\n%s", got)
        }
-       if strings.Count(sp[0], "is reachable from cleanup or finalizer") != 2 {
+       if strings.Count(sp[0], "is reachable from") != 2 {
                t.Fatalf("expected exactly two leaked cleanups and/or finalizers, got:\n%s", got)
        }
-       if strings.Count(sp[0], "created at: main.DetectFinalizerAndCleanupLeaks") != 2 {
-               t.Fatalf("expected two symbolized locations, got:\n%s", got)
+       // N.B. Disable in race mode and in asan mode. Both disable the tiny allocator.
+       wantSymbolizedLocations := 2
+       if !race.Enabled && !asan.Enabled {
+               if strings.Count(sp[0], "is in a tiny block") != 1 {
+                       t.Fatalf("expected exactly one report for allocation in a tiny block, got:\n%s", got)
+               }
+               wantSymbolizedLocations++
+       }
+       if strings.Count(sp[0], "main.DetectFinalizerAndCleanupLeaks()") != wantSymbolizedLocations {
+               t.Fatalf("expected %d symbolized locations, got:\n%s", wantSymbolizedLocations, got)
        }
 }
index 46200037e285fdf26dc7c5eb59077015fe146e11..25caf0625beed58226249cf68f011908ac25ce8c 100644 (file)
@@ -1670,6 +1670,12 @@ func postMallocgcDebug(x unsafe.Pointer, elemsize uintptr, typ *_type) {
                        traceRelease(trace)
                }
        }
+
+       // N.B. elemsize == 0 indicates a tiny allocation, since no new slot was
+       // allocated to fulfill this call to mallocgc.
+       if debug.checkfinalizers != 0 && elemsize == 0 {
+               setTinyBlockContext(unsafe.Pointer(alignDown(uintptr(x), maxTinySize)))
+       }
 }
 
 // deductAssistCredit reduces the current G's assist credit
index cdd9c48115ee26cf093f20fb5b2eb6f53003017e..ebb19a0ceb9d39cfbc1d8ec72693ab441d1f87cd 100644 (file)
@@ -148,15 +148,29 @@ func runCheckmark(prepareRootSet func(*gcWork)) {
 func checkFinalizersAndCleanups() {
        assertWorldStopped()
 
+       const (
+               reportCycle = 1 << iota
+               reportTiny
+       )
+
+       // Find the arena and page index into that arena for this shard.
        type report struct {
-               ptr uintptr
-               sp  *special
+               issues int
+               ptr    uintptr
+               sp     *special
        }
-       var reports [25]report
+       var reports [50]report
        var nreports int
        var more bool
+       var lastTinyBlock uintptr
 
        forEachSpecial(func(p uintptr, s *mspan, sp *special) bool {
+               // N.B. The tiny block specials are sorted first in the specials list.
+               if sp.kind == _KindSpecialTinyBlock {
+                       lastTinyBlock = s.base() + sp.offset
+                       return true
+               }
+
                // We only care about finalizers and cleanups.
                if sp.kind != _KindSpecialFinalizer && sp.kind != _KindSpecialCleanup {
                        return true
@@ -180,12 +194,19 @@ func checkFinalizersAndCleanups() {
                if bytep == nil {
                        return true
                }
+               var issues int
                if atomic.Load8(bytep)&mask != 0 {
+                       issues |= reportCycle
+               }
+               if p >= lastTinyBlock && p < lastTinyBlock+maxTinySize {
+                       issues |= reportTiny
+               }
+               if issues != 0 {
                        if nreports >= len(reports) {
                                more = true
                                return false
                        }
-                       reports[nreports] = report{p, sp}
+                       reports[nreports] = report{issues, p, sp}
                        nreports++
                }
                return true
@@ -193,6 +214,8 @@ func checkFinalizersAndCleanups() {
 
        if nreports > 0 {
                lastPtr := uintptr(0)
+               println("WARNING: LIKELY CLEANUP/FINALIZER ISSUES")
+               println()
                for _, r := range reports[:nreports] {
                        var ctx *specialCheckFinalizer
                        var kind string
@@ -210,36 +233,54 @@ func checkFinalizersAndCleanups() {
                                if lastPtr != 0 {
                                        println()
                                }
-                               print("runtime: value of type ", toRType(ctx.ptrType).string(), " @ ", hex(r.ptr), " is reachable from cleanup or finalizer\n")
-                               println("value reachable from function or argument at one of:")
+                               print("Value of type ", toRType(ctx.ptrType).string(), " at ", hex(r.ptr), "\n")
+                               if r.issues&reportCycle != 0 {
+                                       if r.sp.kind == _KindSpecialFinalizer {
+                                               println("  is reachable from finalizer")
+                                       } else {
+                                               println("  is reachable from cleanup or cleanup argument")
+                                       }
+                               }
+                               if r.issues&reportTiny != 0 {
+                                       println("  is in a tiny block with other (possibly long-lived) values")
+                               }
+                               if r.issues&reportTiny != 0 && r.issues&reportCycle != 0 {
+                                       if r.sp.kind == _KindSpecialFinalizer {
+                                               println("  may be in the same tiny block as finalizer")
+                                       } else {
+                                               println("  may be in the same tiny block as cleanup or cleanup argument")
+                                       }
+                               }
                        }
+                       println()
 
+                       println("Has", kind, "at", hex(uintptr(unsafe.Pointer(r.sp))))
                        funcInfo := findfunc(ctx.funcPC)
                        if funcInfo.valid() {
-                               file, line := funcline(funcInfo, ctx.createPC)
-                               print(funcname(funcInfo), " (", kind, ")\n")
-                               print("\t", file, ":", line, "\n")
+                               file, line := funcline(funcInfo, ctx.funcPC)
+                               print("  ", funcname(funcInfo), "()\n")
+                               print("      ", file, ":", line, " +", hex(ctx.funcPC-funcInfo.entry()), "\n")
                        } else {
-                               print("<bad pc ", hex(ctx.funcPC), ">\n")
+                               print("  <bad pc ", hex(ctx.funcPC), ">\n")
                        }
 
-                       print("created at: ")
+                       println("created at: ")
                        createInfo := findfunc(ctx.createPC)
                        if createInfo.valid() {
                                file, line := funcline(createInfo, ctx.createPC)
-                               print(funcname(createInfo), "\n")
-                               print("\t", file, ":", line, "\n")
+                               print("  ", funcname(createInfo), "()\n")
+                               print("      ", file, ":", line, " +", hex(ctx.createPC-createInfo.entry()), "\n")
                        } else {
-                               print("<bad pc ", hex(ctx.createPC), ">\n")
+                               print("  <bad pc ", hex(ctx.createPC), ">\n")
                        }
 
                        lastPtr = r.ptr
                }
                println()
                if more {
-                       println("runtime: too many errors")
+                       println("... too many potential issues ...")
                }
-               throw("runtime: detected possible cleanup and/or finalizer leaks")
+               throw("detected possible issues with cleanups and/or finalizers")
        }
 }
 
index 66ec06e3b88905c21621aeaee63f08805fff7da1..b5cfd113d0c42c69d98b985eac0cf9d2bfef902b 100644 (file)
@@ -218,6 +218,7 @@ type mheap struct {
        specialfinalizeralloc      fixalloc // allocator for specialfinalizer*
        specialCleanupAlloc        fixalloc // allocator for specialCleanup*
        specialCheckFinalizerAlloc fixalloc // allocator for specialCheckFinalizer*
+       specialTinyBlockAlloc      fixalloc // allocator for specialTinyBlock*
        specialprofilealloc        fixalloc // allocator for specialprofile*
        specialReachableAlloc      fixalloc // allocator for specialReachable
        specialPinCounterAlloc     fixalloc // allocator for specialPinCounter
@@ -793,6 +794,7 @@ func (h *mheap) init() {
        h.specialfinalizeralloc.init(unsafe.Sizeof(specialfinalizer{}), nil, nil, &memstats.other_sys)
        h.specialCleanupAlloc.init(unsafe.Sizeof(specialCleanup{}), nil, nil, &memstats.other_sys)
        h.specialCheckFinalizerAlloc.init(unsafe.Sizeof(specialCheckFinalizer{}), nil, nil, &memstats.other_sys)
+       h.specialTinyBlockAlloc.init(unsafe.Sizeof(specialTinyBlock{}), nil, nil, &memstats.other_sys)
        h.specialprofilealloc.init(unsafe.Sizeof(specialprofile{}), nil, nil, &memstats.other_sys)
        h.specialReachableAlloc.init(unsafe.Sizeof(specialReachable{}), nil, nil, &memstats.other_sys)
        h.specialPinCounterAlloc.init(unsafe.Sizeof(specialPinCounter{}), nil, nil, &memstats.other_sys)
@@ -1967,23 +1969,28 @@ func (q *mSpanQueue) popN(n int) mSpanQueue {
 }
 
 const (
+       // _KindSpecialTinyBlock indicates that a given allocation is a tiny block.
+       // Ordered before KindSpecialFinalizer and KindSpecialCleanup so that it
+       // always appears first in the specials list.
+       // Used only if debug.checkfinalizers != 0.
+       _KindSpecialTinyBlock = 1
        // _KindSpecialFinalizer is for tracking finalizers.
-       _KindSpecialFinalizer = 1
+       _KindSpecialFinalizer = 2
        // _KindSpecialWeakHandle is used for creating weak pointers.
-       _KindSpecialWeakHandle = 2
+       _KindSpecialWeakHandle = 3
        // _KindSpecialProfile is for memory profiling.
-       _KindSpecialProfile = 3
+       _KindSpecialProfile = 4
        // _KindSpecialReachable is a special used for tracking
        // reachability during testing.
-       _KindSpecialReachable = 4
+       _KindSpecialReachable = 5
        // _KindSpecialPinCounter is a special used for objects that are pinned
        // multiple times
-       _KindSpecialPinCounter = 5
+       _KindSpecialPinCounter = 6
        // _KindSpecialCleanup is for tracking cleanups.
-       _KindSpecialCleanup = 6
+       _KindSpecialCleanup = 7
        // _KindSpecialCheckFinalizer adds additional context to a finalizer or cleanup.
        // Used only if debug.checkfinalizers != 0.
-       _KindSpecialCheckFinalizer = 7
+       _KindSpecialCheckFinalizer = 8
 )
 
 type special struct {
@@ -2347,6 +2354,45 @@ func clearCleanupContext(ptr uintptr, cleanupID uint64) {
        unlock(&mheap_.speciallock)
 }
 
+// Indicates that an allocation is a tiny block.
+// Used only if debug.checkfinalizers != 0.
+type specialTinyBlock struct {
+       _       sys.NotInHeap
+       special special
+}
+
+// setTinyBlockContext marks an allocation as a tiny block to diagnostics like
+// checkfinalizer.
+//
+// A tiny block is only marked if it actually contains more than one distinct
+// value, since we're using this for debugging.
+func setTinyBlockContext(ptr unsafe.Pointer) {
+       lock(&mheap_.speciallock)
+       s := (*specialTinyBlock)(mheap_.specialTinyBlockAlloc.alloc())
+       unlock(&mheap_.speciallock)
+       s.special.kind = _KindSpecialTinyBlock
+
+       mp := acquirem()
+       addspecial(ptr, &s.special, false)
+       releasem(mp)
+       KeepAlive(ptr)
+}
+
+// inTinyBlock returns whether ptr is in a tiny alloc block, at one point grouped
+// with other distinct values.
+func inTinyBlock(ptr uintptr) bool {
+       assertWorldStopped()
+
+       ptr = alignDown(ptr, maxTinySize)
+       span := spanOfHeap(ptr)
+       if span == nil {
+               return false
+       }
+       offset := ptr - span.base()
+       _, exists := span.specialFindSplicePoint(offset, _KindSpecialTinyBlock)
+       return exists
+}
+
 // The described object has a weak pointer.
 //
 // Weak pointers in the GC have the following invariants:
@@ -2766,6 +2812,11 @@ func freeSpecial(s *special, p unsafe.Pointer, size uintptr) {
                lock(&mheap_.speciallock)
                mheap_.specialCheckFinalizerAlloc.free(unsafe.Pointer(sc))
                unlock(&mheap_.speciallock)
+       case _KindSpecialTinyBlock:
+               st := (*specialTinyBlock)(unsafe.Pointer(s))
+               lock(&mheap_.speciallock)
+               mheap_.specialTinyBlockAlloc.free(unsafe.Pointer(st))
+               unlock(&mheap_.speciallock)
        default:
                throw("bad special kind")
                panic("not reached")
index 9a4e15c95e1e70e1611f9edce2eedcc526be12d1..975d4016943582199cc8eea88e06265643bec5a0 100644 (file)
@@ -333,14 +333,14 @@ var debug struct {
        traceCheckStackOwnership int32
        profstackdepth           int32
        dataindependenttiming    int32
-       checkfinalizers          int32
 
        // debug.malloc is used as a combined debug check
        // in the malloc function and should be set
        // if any of the below debug options is != 0.
-       malloc    bool
-       inittrace int32
-       sbrk      int32
+       malloc          bool
+       inittrace       int32
+       sbrk            int32
+       checkfinalizers int32
        // traceallocfree controls whether execution traces contain
        // detailed trace data about memory allocation. This value
        // affects debug.malloc only if it is != 0 and the execution
@@ -440,7 +440,7 @@ func parsedebugvars() {
        // apply environment settings
        parsegodebug(godebug, nil)
 
-       debug.malloc = (debug.inittrace | debug.sbrk) != 0
+       debug.malloc = (debug.inittrace | debug.sbrk | debug.checkfinalizers) != 0
        debug.profstackdepth = min(debug.profstackdepth, maxProfStackDepth)
 
        // Disable async preemption in checkmark mode. The following situation is
index 410a0f6a23294000cefad0cdd98e5b120744d379..b542f575fe34e6e018c5e86a878bcc9b648a08f4 100644 (file)
@@ -13,6 +13,10 @@ func init() {
        register("DetectFinalizerAndCleanupLeaks", DetectFinalizerAndCleanupLeaks)
 }
 
+type tiny uint8
+
+var tinySink *tiny
+
 // Intended to be run only with `GODEBUG=checkfinalizers=1`.
 func DetectFinalizerAndCleanupLeaks() {
        type T *int
@@ -34,6 +38,15 @@ func DetectFinalizerAndCleanupLeaks() {
                **cNoLeak = x
        }, int(0)).Stop()
 
+       // Ensure we create an allocation into a tiny block that shares space among several values.
+       var ctLeak *tiny
+       for i := 0; i < 18; i++ {
+               tinySink = ctLeak
+               ctLeak = new(tiny)
+               *ctLeak = tiny(i)
+       }
+       runtime.AddCleanup(ctLeak, func(_ struct{}) {}, struct{}{})
+
        // Leak a finalizer.
        fLeak := new(T)
        runtime.SetFinalizer(fLeak, func(_ *T) {
@@ -49,10 +62,4 @@ func DetectFinalizerAndCleanupLeaks() {
        // runtime.GC here should crash.
        runtime.GC()
        println("OK")
-
-       // Keep everything alive.
-       runtime.KeepAlive(cLeak)
-       runtime.KeepAlive(cNoLeak)
-       runtime.KeepAlive(fLeak)
-       runtime.KeepAlive(fNoLeak)
 }