Skip to content

Commit 9d21ef3

Browse files
committed
runtime: fix the equality check in AddCleanup
This fixes the check that ensures that arg is not equal to ptr in AddCleanup. This also changes any use of throw to panic in AddCleanup. Fixes #71316 Change-Id: Ie5a3e0163b254dff44b7fefedf75207ba587b771 Reviewed-on: https://go-review.googlesource.com/c/go/+/643655 Reviewed-by: Cherry Mui <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent 5a46b17 commit 9d21ef3

File tree

2 files changed

+34
-7
lines changed

2 files changed

+34
-7
lines changed

src/runtime/mcleanup.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -70,19 +70,19 @@ func AddCleanup[T, S any](ptr *T, cleanup func(S), arg S) Cleanup {
7070

7171
// The pointer to the object must be valid.
7272
if ptr == nil {
73-
throw("runtime.AddCleanup: ptr is nil")
73+
panic("runtime.AddCleanup: ptr is nil")
7474
}
7575
usptr := uintptr(unsafe.Pointer(ptr))
7676

7777
// Check that arg is not equal to ptr.
78-
// TODO(67535) this does not cover the case where T and *S are the same
79-
// type and ptr and arg are equal.
80-
if unsafe.Pointer(&arg) == unsafe.Pointer(ptr) {
81-
throw("runtime.AddCleanup: ptr is equal to arg, cleanup will never run")
78+
if kind := abi.TypeOf(arg).Kind(); kind == abi.Pointer || kind == abi.UnsafePointer {
79+
if unsafe.Pointer(ptr) == *((*unsafe.Pointer)(unsafe.Pointer(&arg))) {
80+
panic("runtime.AddCleanup: ptr is equal to arg, cleanup will never run")
81+
}
8282
}
8383
if inUserArenaChunk(usptr) {
8484
// Arena-allocated objects are not eligible for cleanup.
85-
throw("runtime.AddCleanup: ptr is arena-allocated")
85+
panic("runtime.AddCleanup: ptr is arena-allocated")
8686
}
8787
if debug.sbrk != 0 {
8888
// debug.sbrk never frees memory, so no cleanup will ever run
@@ -105,7 +105,7 @@ func AddCleanup[T, S any](ptr *T, cleanup func(S), arg S) Cleanup {
105105
// Cleanup is a noop.
106106
return Cleanup{}
107107
}
108-
throw("runtime.AddCleanup: ptr not in allocated block")
108+
panic("runtime.AddCleanup: ptr not in allocated block")
109109
}
110110

111111
// Ensure we have a finalizer processing goroutine running.

src/runtime/mcleanup_test.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,3 +269,30 @@ func TestCleanupStopAfterCleanupRuns(t *testing.T) {
269269
<-ch
270270
stop()
271271
}
272+
273+
func TestCleanupPointerEqualsArg(t *testing.T) {
274+
// See go.dev/issue/71316
275+
defer func() {
276+
want := "runtime.AddCleanup: ptr is equal to arg, cleanup will never run"
277+
if r := recover(); r == nil {
278+
t.Error("want panic, test did not panic")
279+
} else if r == want {
280+
// do nothing
281+
} else {
282+
t.Errorf("wrong panic: want=%q, got=%q", want, r)
283+
}
284+
}()
285+
286+
// allocate struct with pointer to avoid hitting tinyalloc.
287+
// Otherwise we can't be sure when the allocation will
288+
// be freed.
289+
type T struct {
290+
v int
291+
p unsafe.Pointer
292+
}
293+
v := &new(T).v
294+
*v = 97531
295+
runtime.AddCleanup(v, func(x *int) {}, v)
296+
v = nil
297+
runtime.GC()
298+
}

0 commit comments

Comments
 (0)