Skip to content

Commit 8b45a3f

Browse files
committed
runtime: guarantee checkfinalizers test allocates in a shared tiny block
Currently the checkfinalizers test (TestDetectCleanupOrFinalizerLeak) only *tries* to ensure the tiny alloc with a cleanup attached shares a block with other objects. However, what it does is insufficient, because it could get unlucky and have the last object allocated be the first object of a new block. This change changes the test to guarantee that a tiny object is not at the start of a fresh block by looking at the alignment of the object's pointer. If the object's pointer is odd, then that's good enough to know that it shares a block with something else, since the blocks themselves are aligned to a much higher power of two. This fixes a failure I've seen on the builders. Fixes #73810. Change-Id: Ieafdbb9cccb0d2dc3659a9a5d9d9233718461635 Reviewed-on: https://go-review.googlesource.com/c/go/+/674655 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com>
1 parent 8960970 commit 8b45a3f

File tree

2 files changed

+13
-3
lines changed

2 files changed

+13
-3
lines changed

src/runtime/malloc.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1672,7 +1672,10 @@ func postMallocgcDebug(x unsafe.Pointer, elemsize uintptr, typ *_type) {
16721672
}
16731673

16741674
// N.B. elemsize == 0 indicates a tiny allocation, since no new slot was
1675-
// allocated to fulfill this call to mallocgc.
1675+
// allocated to fulfill this call to mallocgc. This means checkfinalizer
1676+
// will only flag an error if there is actually any risk. If an allocation
1677+
// has the tiny block to itself, it will not get flagged, because we won't
1678+
// mark the block as a tiny block.
16761679
if debug.checkfinalizers != 0 && elemsize == 0 {
16771680
setTinyBlockContext(unsafe.Pointer(alignDown(uintptr(x), maxTinySize)))
16781681
}

src/runtime/testdata/testprog/checkfinalizers.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package main
77
import (
88
"runtime"
99
"runtime/debug"
10+
"unsafe"
1011
)
1112

1213
func init() {
@@ -40,10 +41,16 @@ func DetectFinalizerAndCleanupLeaks() {
4041

4142
// Ensure we create an allocation into a tiny block that shares space among several values.
4243
var ctLeak *tiny
43-
for i := 0; i < 18; i++ {
44+
for {
4445
tinySink = ctLeak
4546
ctLeak = new(tiny)
46-
*ctLeak = tiny(i)
47+
*ctLeak = tiny(55)
48+
// Make sure the address is an odd value. This is sufficient to
49+
// be certain that we're sharing a block with another value and
50+
// trip the detector.
51+
if uintptr(unsafe.Pointer(ctLeak))%2 != 0 {
52+
break
53+
}
4754
}
4855
runtime.AddCleanup(ctLeak, func(_ struct{}) {}, struct{}{})
4956

0 commit comments

Comments
 (0)