From 4d550727f8b85e9f8866f22c8a02b8f56fa64159 Mon Sep 17 00:00:00 2001 From: Cuong Manh Le Date: Sat, 18 Sep 2021 10:32:22 +0700 Subject: [PATCH] reflect: add Value.UnsafePointer Allowing eliminates a class of possible misuse of unsafe.Pointer, and allow callers to migrate from Value.Addr and Value.Pointer, thus they can be now deprecated. Fixes #40592 Change-Id: I798e507c748922cac5cc1c1971c1b2cc7095a068 Reviewed-on: https://go-review.googlesource.com/c/go/+/350691 Trust: Cuong Manh Le Run-TryBot: Cuong Manh Le TryBot-Result: Go Bot Reviewed-by: Keith Randall Reviewed-by: Ian Lance Taylor --- src/reflect/all_test.go | 22 +++++----- src/reflect/deepequal.go | 6 +-- src/reflect/set_test.go | 6 +-- src/reflect/type.go | 4 +- src/reflect/value.go | 94 ++++++++++++++++++++++++---------------- 5 files changed, 75 insertions(+), 57 deletions(-) diff --git a/src/reflect/all_test.go b/src/reflect/all_test.go index 0370906f7d..141cc8f73d 100644 --- a/src/reflect/all_test.go +++ b/src/reflect/all_test.go @@ -3223,11 +3223,11 @@ func (*outer) M() {} func TestNestedMethods(t *testing.T) { typ := TypeOf((*outer)(nil)) - if typ.NumMethod() != 1 || typ.Method(0).Func.Pointer() != ValueOf((*outer).M).Pointer() { + if typ.NumMethod() != 1 || typ.Method(0).Func.UnsafePointer() != ValueOf((*outer).M).UnsafePointer() { t.Errorf("Wrong method table for outer: (M=%p)", (*outer).M) for i := 0; i < typ.NumMethod(); i++ { m := typ.Method(i) - t.Errorf("\t%d: %s %#x\n", i, m.Name, m.Func.Pointer()) + t.Errorf("\t%d: %s %p\n", i, m.Name, m.Func.UnsafePointer()) } } } @@ -3266,11 +3266,11 @@ func (i *InnerInt) M() int { func TestEmbeddedMethods(t *testing.T) { typ := TypeOf((*OuterInt)(nil)) - if typ.NumMethod() != 1 || typ.Method(0).Func.Pointer() != ValueOf((*OuterInt).M).Pointer() { + if typ.NumMethod() != 1 || typ.Method(0).Func.UnsafePointer() != ValueOf((*OuterInt).M).UnsafePointer() { t.Errorf("Wrong method table for OuterInt: (m=%p)", (*OuterInt).M) for i := 0; i < typ.NumMethod(); i++ { m := typ.Method(i) - t.Errorf("\t%d: %s %#x\n", i, m.Name, m.Func.Pointer()) + t.Errorf("\t%d: %s %p\n", i, m.Name, m.Func.UnsafePointer()) } } @@ -3528,11 +3528,11 @@ func TestSlice(t *testing.T) { rv := ValueOf(&xs).Elem() rv = rv.Slice(3, 4) - ptr2 := rv.Pointer() + ptr2 := rv.UnsafePointer() rv = rv.Slice(5, 5) - ptr3 := rv.Pointer() + ptr3 := rv.UnsafePointer() if ptr3 != ptr2 { - t.Errorf("xs.Slice(3,4).Slice3(5,5).Pointer() = %#x, want %#x", ptr3, ptr2) + t.Errorf("xs.Slice(3,4).Slice3(5,5).UnsafePointer() = %p, want %p", ptr3, ptr2) } } @@ -3575,11 +3575,11 @@ func TestSlice3(t *testing.T) { rv = ValueOf(&xs).Elem() rv = rv.Slice3(3, 5, 7) - ptr2 := rv.Pointer() + ptr2 := rv.UnsafePointer() rv = rv.Slice3(4, 4, 4) - ptr3 := rv.Pointer() + ptr3 := rv.UnsafePointer() if ptr3 != ptr2 { - t.Errorf("xs.Slice3(3,5,7).Slice3(4,4,4).Pointer() = %#x, want %#x", ptr3, ptr2) + t.Errorf("xs.Slice3(3,5,7).Slice3(4,4,4).UnsafePointer() = %p, want %p", ptr3, ptr2) } } @@ -6818,7 +6818,7 @@ func verifyGCBitsSlice(t *testing.T, typ Type, cap int, bits []byte) { // repeat a bitmap for a small array or executing a repeat in // a GC program. val := MakeSlice(typ, 0, cap) - data := NewAt(ArrayOf(cap, typ), unsafe.Pointer(val.Pointer())) + data := NewAt(ArrayOf(cap, typ), val.UnsafePointer()) heapBits := GCBits(data.Interface()) // Repeat the bitmap for the slice size, trimming scalars in // the last element. diff --git a/src/reflect/deepequal.go b/src/reflect/deepequal.go index 94174dec04..7f1ecb2809 100644 --- a/src/reflect/deepequal.go +++ b/src/reflect/deepequal.go @@ -102,7 +102,7 @@ func deepValueEqual(v1, v2 Value, visited map[visit]bool) bool { if v1.Len() != v2.Len() { return false } - if v1.Pointer() == v2.Pointer() { + if v1.UnsafePointer() == v2.UnsafePointer() { return true } // Special case for []byte, which is common. @@ -121,7 +121,7 @@ func deepValueEqual(v1, v2 Value, visited map[visit]bool) bool { } return deepValueEqual(v1.Elem(), v2.Elem(), visited) case Ptr: - if v1.Pointer() == v2.Pointer() { + if v1.UnsafePointer() == v2.UnsafePointer() { return true } return deepValueEqual(v1.Elem(), v2.Elem(), visited) @@ -139,7 +139,7 @@ func deepValueEqual(v1, v2 Value, visited map[visit]bool) bool { if v1.Len() != v2.Len() { return false } - if v1.Pointer() == v2.Pointer() { + if v1.UnsafePointer() == v2.UnsafePointer() { return true } for _, k := range v1.MapKeys() { diff --git a/src/reflect/set_test.go b/src/reflect/set_test.go index a633e6eee2..566dc7fb65 100644 --- a/src/reflect/set_test.go +++ b/src/reflect/set_test.go @@ -79,7 +79,7 @@ func TestImplicitMapConversion(t *testing.T) { if x != b2 { t.Errorf("#5 after SetMapIndex(b1, b2): %p (!= %p), %t (map=%v)", x, b2, ok, m) } - if p := mv.MapIndex(ValueOf(b1)).Elem().Pointer(); p != uintptr(unsafe.Pointer(b2)) { + if p := mv.MapIndex(ValueOf(b1)).Elem().UnsafePointer(); p != unsafe.Pointer(b2) { t.Errorf("#5 MapIndex(b1) = %#x want %p", p, b2) } } @@ -94,7 +94,7 @@ func TestImplicitMapConversion(t *testing.T) { if x != c2 { t.Errorf("#6 after SetMapIndex(c1, c2): %p (!= %p), %t (map=%v)", x, c2, ok, m) } - if p := mv.MapIndex(ValueOf(c1)).Pointer(); p != ValueOf(c2).Pointer() { + if p := mv.MapIndex(ValueOf(c1)).UnsafePointer(); p != ValueOf(c2).UnsafePointer() { t.Errorf("#6 MapIndex(c1) = %#x want %p", p, c2) } } @@ -110,7 +110,7 @@ func TestImplicitMapConversion(t *testing.T) { if x != b2 { t.Errorf("#7 after SetMapIndex(b1, b2): %p (!= %p), %t (map=%v)", x, b2, ok, m) } - if p := mv.MapIndex(ValueOf(b1)).Pointer(); p != uintptr(unsafe.Pointer(b2)) { + if p := mv.MapIndex(ValueOf(b1)).UnsafePointer(); p != unsafe.Pointer(b2) { t.Errorf("#7 MapIndex(b1) = %#x want %p", p, b2) } } diff --git a/src/reflect/type.go b/src/reflect/type.go index afb802e641..96f589ca9c 100644 --- a/src/reflect/type.go +++ b/src/reflect/type.go @@ -2666,8 +2666,8 @@ func StructOf(fields []StructField) Type { {Name: "M", Type: ArrayOf(len(methods), TypeOf(methods[0]))}, })) - typ = (*structType)(unsafe.Pointer(tt.Elem().Field(0).UnsafeAddr())) - ut = (*uncommonType)(unsafe.Pointer(tt.Elem().Field(1).UnsafeAddr())) + typ = (*structType)(tt.Elem().Field(0).Addr().UnsafePointer()) + ut = (*uncommonType)(tt.Elem().Field(1).Addr().UnsafePointer()) copy(tt.Elem().Field(2).Slice(0, len(methods)).Interface().([]method), methods) } diff --git a/src/reflect/value.go b/src/reflect/value.go index 6bc02c1c8c..a272714ac9 100644 --- a/src/reflect/value.go +++ b/src/reflect/value.go @@ -1933,44 +1933,10 @@ func (v Value) OverflowUint(x uint64) bool { // If v's Kind is Slice, the returned pointer is to the first // element of the slice. If the slice is nil the returned value // is 0. If the slice is empty but non-nil the return value is non-zero. +// +// Deprecated: use uintptr(Value.UnsafePointer()) to get the equivalent result. func (v Value) Pointer() uintptr { - // TODO: deprecate - k := v.kind() - switch k { - case Ptr: - if v.typ.ptrdata == 0 { - // Handle pointers to go:notinheap types directly, - // so we never materialize such pointers as an - // unsafe.Pointer. (Such pointers are always indirect.) - // See issue 42076. - return *(*uintptr)(v.ptr) - } - fallthrough - case Chan, Map, UnsafePointer: - return uintptr(v.pointer()) - case Func: - if v.flag&flagMethod != 0 { - // As the doc comment says, the returned pointer is an - // underlying code pointer but not necessarily enough to - // identify a single function uniquely. All method expressions - // created via reflect have the same underlying code pointer, - // so their Pointers are equal. The function used here must - // match the one used in makeMethodValue. - f := methodValueCall - return **(**uintptr)(unsafe.Pointer(&f)) - } - p := v.pointer() - // Non-nil func value points at data block. - // First word of data block is actual code. - if p != nil { - p = *(*unsafe.Pointer)(p) - } - return uintptr(p) - - case Slice: - return (*SliceHeader)(v.ptr).Data - } - panic(&ValueError{"reflect.Value.Pointer", v.kind()}) + return uintptr(v.UnsafePointer()) } // Recv receives and returns a value from the channel v. @@ -2476,8 +2442,9 @@ func (v Value) Uint() uint64 { // UnsafeAddr returns a pointer to v's data, as a uintptr. // It is for advanced clients that also import the "unsafe" package. // It panics if v is not addressable. +// +// Deprecated: use uintptr(Value.Addr().UnsafePointer()) to get the equivalent result. func (v Value) UnsafeAddr() uintptr { - // TODO: deprecate if v.typ == nil { panic(&ValueError{"reflect.Value.UnsafeAddr", Invalid}) } @@ -2487,6 +2454,57 @@ func (v Value) UnsafeAddr() uintptr { return uintptr(v.ptr) } +// UnsafePointer returns v's value as a unsafe.Pointer. +// It panics if v's Kind is not Chan, Func, Map, Ptr, Slice, or UnsafePointer. +// +// If v's Kind is Func, the returned pointer is an underlying +// code pointer, but not necessarily enough to identify a +// single function uniquely. The only guarantee is that the +// result is zero if and only if v is a nil func Value. +// +// If v's Kind is Slice, the returned pointer is to the first +// element of the slice. If the slice is nil the returned value +// is nil. If the slice is empty but non-nil the return value is non-nil. +func (v Value) UnsafePointer() unsafe.Pointer { + k := v.kind() + switch k { + case Ptr: + if v.typ.ptrdata == 0 { + // Since it is a not-in-heap pointer, all pointers to the heap are + // forbidden! See comment in Value.Elem and issue #48399. + if !verifyNotInHeapPtr(*(*uintptr)(v.ptr)) { + panic("reflect: reflect.Value.UnsafePointer on an invalid notinheap pointer") + } + return *(*unsafe.Pointer)(v.ptr) + } + fallthrough + case Chan, Map, UnsafePointer: + return v.pointer() + case Func: + if v.flag&flagMethod != 0 { + // As the doc comment says, the returned pointer is an + // underlying code pointer but not necessarily enough to + // identify a single function uniquely. All method expressions + // created via reflect have the same underlying code pointer, + // so their Pointers are equal. The function used here must + // match the one used in makeMethodValue. + f := methodValueCall + return **(**unsafe.Pointer)(unsafe.Pointer(&f)) + } + p := v.pointer() + // Non-nil func value points at data block. + // First word of data block is actual code. + if p != nil { + p = *(*unsafe.Pointer)(p) + } + return p + + case Slice: + return (*unsafeheader.Slice)(v.ptr).Data + } + panic(&ValueError{"reflect.Value.UnsafePointer", v.kind()}) +} + // StringHeader is the runtime representation of a string. // It cannot be used safely or portably and its representation may // change in a later release.