From: Michael Anthony Knyszek <mknyszek@google.com>
Date: Mon, 9 Dec 2024 19:21:48 +0000 (+0000)
Subject: runtime: make special offset a uintptr
X-Git-Tag: go1.24rc1~3^2~13
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=d87878c62b2db318a12e5bd2126a82c117961156;p=gostls13.git

runtime: make special offset a uintptr

Currently specials try to save on space by only encoding the offset from
the base of the span in a uint16. This worked fine up until Go 1.24.
- Most specials have an offset of 0 (mem profile, finalizers, etc.)
- Cleanups do not care about the offset at all, so even if it's wrong,
  it's OK.
- Weak pointers *do* care, but the unique package always makes a new
  allocation, so the weak pointer handle offset it makes is always zero.

With Go 1.24 and general weak pointers now available, nothing is
stopping someone from just creating a weak pointer that is >64 KiB
offset from the start of an object, and this weak pointer must be
distinct from others.

Fix this problem by just increasing the size of a special and making the
offset a uintptr, to capture all possible offsets. Since we're in the
freeze, this is the safest thing to do. Specials aren't so common that I
expect a substantial memory increase from this change. In a future
release (or if there is a problem) we can almost certainly pack the
special's kind and offset together. There was already a bunch of wasted
space due to padding, so this would bring us back to the same memory
footprint before this change.

Also, add tests for equality of basic weak interior pointers. This
works, but we really should've had tests for it.

Fixes #70739.

Change-Id: Ib49a7f8f0f1ec3db4571a7afb0f4d94c8a93aa40
Reviewed-on: https://go-review.googlesource.com/c/go/+/634598
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Commit-Queue: Michael Knyszek <mknyszek@google.com>
---

diff --git a/src/runtime/mheap.go b/src/runtime/mheap.go
index 4fcfbeca84..e058dd8489 100644
--- a/src/runtime/mheap.go
+++ b/src/runtime/mheap.go
@@ -1839,7 +1839,7 @@ const (
 type special struct {
 	_      sys.NotInHeap
 	next   *special // linked list in span
-	offset uint16   // span offset of object
+	offset uintptr  // span offset of object
 	kind   byte     // kind of special
 }
 
@@ -1886,7 +1886,7 @@ func addspecial(p unsafe.Pointer, s *special, force bool) bool {
 	iter, exists := span.specialFindSplicePoint(offset, kind)
 	if !exists || force {
 		// Splice in record, fill in offset.
-		s.offset = uint16(offset)
+		s.offset = offset
 		s.next = *iter
 		*iter = s
 		spanHasSpecials(span)
diff --git a/src/runtime/pinner.go b/src/runtime/pinner.go
index 7a9c381580..543bfdb7a4 100644
--- a/src/runtime/pinner.go
+++ b/src/runtime/pinner.go
@@ -331,7 +331,7 @@ func (span *mspan) incPinCounter(offset uintptr) {
 		rec = (*specialPinCounter)(mheap_.specialPinCounterAlloc.alloc())
 		unlock(&mheap_.speciallock)
 		// splice in record, fill in offset.
-		rec.special.offset = uint16(offset)
+		rec.special.offset = offset
 		rec.special.kind = _KindSpecialPinCounter
 		rec.special.next = *ref
 		*ref = (*special)(unsafe.Pointer(rec))
diff --git a/src/weak/pointer_test.go b/src/weak/pointer_test.go
index 213dde8c40..002b4130f0 100644
--- a/src/weak/pointer_test.go
+++ b/src/weak/pointer_test.go
@@ -43,9 +43,11 @@ func TestPointer(t *testing.T) {
 func TestPointerEquality(t *testing.T) {
 	bt := make([]*T, 10)
 	wt := make([]weak.Pointer[T], 10)
+	wo := make([]weak.Pointer[int], 10)
 	for i := range bt {
 		bt[i] = new(T)
 		wt[i] = weak.Make(bt[i])
+		wo[i] = weak.Make(&bt[i].a)
 	}
 	for i := range bt {
 		st := wt[i].Value()
@@ -55,6 +57,9 @@ func TestPointerEquality(t *testing.T) {
 		if wp := weak.Make(st); wp != wt[i] {
 			t.Fatalf("new weak pointer not equal to existing weak pointer: %v vs. %v", wp, wt[i])
 		}
+		if wp := weak.Make(&st.a); wp != wo[i] {
+			t.Fatalf("new weak pointer not equal to existing weak pointer: %v vs. %v", wp, wo[i])
+		}
 		if i == 0 {
 			continue
 		}
@@ -72,6 +77,9 @@ func TestPointerEquality(t *testing.T) {
 		if wp := weak.Make(st); wp != wt[i] {
 			t.Fatalf("new weak pointer not equal to existing weak pointer: %v vs. %v", wp, wt[i])
 		}
+		if wp := weak.Make(&st.a); wp != wo[i] {
+			t.Fatalf("new weak pointer not equal to existing weak pointer: %v vs. %v", wp, wo[i])
+		}
 		if i == 0 {
 			continue
 		}
@@ -210,3 +218,12 @@ func TestIssue69210(t *testing.T) {
 	}
 	wg.Wait()
 }
+
+func TestIssue70739(t *testing.T) {
+	x := make([]*int, 4<<16)
+	wx1 := weak.Make(&x[1<<16])
+	wx2 := weak.Make(&x[1<<16])
+	if wx1 != wx2 {
+		t.Fatal("failed to look up special and made duplicate weak handle; see issue #70739")
+	}
+}