From: doujiang24 <doujiang24@gmail.com>
Date: Wed, 13 Sep 2023 09:44:19 +0000 (+0000)
Subject: runtime: silently allow pinning of non-Go pointers in runtime.Pinner.Pin
X-Git-Tag: go1.22rc1~856
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=bfa8a8586f6ba62265c31d4edc2027f5b15f7ed3;p=gostls13.git

runtime: silently allow pinning of non-Go pointers in runtime.Pinner.Pin

People may not know the details of a pointer, this makes the
runtime.Pinner.Pin API easier to use.

Fixes #62356

Change-Id: I071df44e01320648a6df5e2a1e65afd6ea52e274
GitHub-Last-Rev: 560a581b63c6e7fe6c8829437e7ab3acd3b31514
GitHub-Pull-Request: golang/go#62549
Reviewed-on: https://go-review.googlesource.com/c/go/+/527156
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
---

diff --git a/src/runtime/pinner.go b/src/runtime/pinner.go
index 8bb351eb8f..eb62a0fd53 100644
--- a/src/runtime/pinner.go
+++ b/src/runtime/pinner.go
@@ -25,10 +25,8 @@ type Pinner struct {
 // objects, these objects must be pinned separately if they are going to be
 // accessed from C code.
 //
-// The argument must be a pointer of any type or an
-// unsafe.Pointer. It must be the result of calling new,
-// taking the address of a composite literal, or taking the address of a
-// local variable. If one of these conditions is not met, Pin will panic.
+// The argument must be a pointer of any type or an unsafe.Pointer.
+// It's safe to call Pin on non-Go pointers, in which case Pin will do nothing.
 func (p *Pinner) Pin(pointer any) {
 	if p.pinner == nil {
 		// Check the pinner cache first.
@@ -59,8 +57,9 @@ func (p *Pinner) Pin(pointer any) {
 		}
 	}
 	ptr := pinnerGetPtr(&pointer)
-	setPinned(ptr, true)
-	p.refs = append(p.refs, ptr)
+	if setPinned(ptr, true) {
+		p.refs = append(p.refs, ptr)
+	}
 }
 
 // Unpin unpins all pinned objects of the Pinner.
@@ -143,15 +142,19 @@ func isPinned(ptr unsafe.Pointer) bool {
 	return pinState.isPinned()
 }
 
-// setPinned marks or unmarks a Go pointer as pinned.
-func setPinned(ptr unsafe.Pointer, pin bool) {
+// setPinned marks or unmarks a Go pointer as pinned, when the ptr is a Go pointer.
+// It will be ignored while try to pin a non-Go pointer,
+// and it will be panic while try to unpin a non-Go pointer,
+// which should not happen in normal usage.
+func setPinned(ptr unsafe.Pointer, pin bool) bool {
 	span := spanOfHeap(uintptr(ptr))
 	if span == nil {
-		if isGoPointerWithoutSpan(ptr) {
-			// this is a linker-allocated or zero size object, nothing to do.
-			return
+		if !pin {
+			panic(errorString("tried to unpin non-Go pointer"))
 		}
-		panic(errorString("runtime.Pinner.Pin: argument is not a Go pointer"))
+		// This is a linker-allocated, zero size object or other object,
+		// nothing to do, silently ignore it.
+		return false
 	}
 
 	// ensure that the span is swept, b/c sweeping accesses the specials list
@@ -209,7 +212,7 @@ func setPinned(ptr unsafe.Pointer, pin bool) {
 	}
 	unlock(&span.speciallock)
 	releasem(mp)
-	return
+	return true
 }
 
 type pinState struct {
diff --git a/src/runtime/pinner_test.go b/src/runtime/pinner_test.go
index 88ead7c946..ef8500cd35 100644
--- a/src/runtime/pinner_test.go
+++ b/src/runtime/pinner_test.go
@@ -522,3 +522,19 @@ func BenchmarkPinnerIsPinnedOnUnpinnedParallel(b *testing.B) {
 		}
 	})
 }
+
+// const string data is not in span.
+func TestPinnerConstStringData(t *testing.T) {
+	var pinner runtime.Pinner
+	str := "test-const-string"
+	p := unsafe.StringData(str)
+	addr := unsafe.Pointer(p)
+	if !runtime.IsPinned(addr) {
+		t.Fatal("not marked as pinned")
+	}
+	pinner.Pin(p)
+	pinner.Unpin()
+	if !runtime.IsPinned(addr) {
+		t.Fatal("not marked as pinned")
+	}
+}