runtime: panic on AddCleanup with self pointer

For #75066

Change-Id: Ifd555586fb448e7510fed16372648bdd7ec0ab4a
Reviewed-on: https://go-review.googlesource.com/c/go/+/697535
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>

diff --git a/src/runtime/mcleanup.go b/src/runtime/mcleanup.go
index fc71af9f3f8ca101b1f37c26d10e12cb8a36730b..d190653213fae6c8467d85552bc7c4f46ff4e2b1 100644 (file)
--- a/src/runtime/mcleanup.go
+++ b/src/runtime/mcleanup.go
@@ -84,7 +84,8 @@ func AddCleanup[T, S any](ptr *T, cleanup func(S), arg S) Cleanup {
 
        // Check that arg is not equal to ptr.
        argType := abi.TypeOf(arg)
-       if kind := argType.Kind(); kind == abi.Pointer || kind == abi.UnsafePointer {
+       kind := argType.Kind()
+       if kind == abi.Pointer || kind == abi.UnsafePointer {
                if unsafe.Pointer(ptr) == *((*unsafe.Pointer)(unsafe.Pointer(&arg))) {
                        panic("runtime.AddCleanup: ptr is equal to arg, cleanup will never run")
                }
@@ -119,7 +120,7 @@ func AddCleanup[T, S any](ptr *T, cleanup func(S), arg S) Cleanup {
        *argv = arg
 
        // Find the containing object.
-       base, _, _ := findObject(usptr, 0, 0)
+       base, span, _ := findObject(usptr, 0, 0)
        if base == 0 {
                if isGoPointerWithoutSpan(unsafe.Pointer(ptr)) {
                        // Cleanup is a noop.
@@ -128,6 +129,22 @@ func AddCleanup[T, S any](ptr *T, cleanup func(S), arg S) Cleanup {
                panic("runtime.AddCleanup: ptr not in allocated block")
        }
 
+       // Check that arg is not within ptr.
+       if kind == abi.Pointer || kind == abi.UnsafePointer {
+               argPtr := uintptr(*(*unsafe.Pointer)(unsafe.Pointer(&arg)))
+               if argPtr >= base && argPtr < base+span.elemsize {
+                       // It's possible that both pointers are separate
+                       // parts of a tiny allocation, which is OK.
+                       // We side-stepped the tiny allocator above for
+                       // the allocation for the cleanup,
+                       // but the argument itself can still overlap
+                       // with the value to which the cleanup is attached.
+                       if span.spanclass != tinySpanClass {
+                               panic("runtime.AddCleanup: ptr is within arg, cleanup will never run")
+                       }
+               }
+       }
+
        // Create another G if necessary.
        if gcCleanups.needG() {
                gcCleanups.createGs()

diff --git a/src/runtime/mcleanup_test.go b/src/runtime/mcleanup_test.go
index 341d30afa7dcbf920099a96eca234a4cae5ccc8a..28fc7f407fbe5ebb1012ad7fcd3491a4f01567ad 100644 (file)
--- a/src/runtime/mcleanup_test.go
+++ b/src/runtime/mcleanup_test.go
@@ -337,6 +337,23 @@ func TestCleanupLost(t *testing.T) {
        }
 }
 
+func TestCleanupUnreachable(t *testing.T) {
+       defer func() {
+               if recover() == nil {
+                       t.Error("AddCleanup failed to detect self-pointer")
+               }
+       }()
+
+       type T struct {
+               p *byte // use *byte to avoid tiny allocator
+               f int
+       }
+       v := &T{}
+       runtime.AddCleanup(v, func(*int) {
+               t.Error("cleanup ran unexpectedly")
+       }, &v.f)
+}
+
 // BenchmarkAddCleanupAndStop benchmarks adding and removing a cleanup
 // from the same allocation.
 //