From 8925290cf701fc8f7ec95e4df3d6a8d423b26780 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Fri, 18 Sep 2020 14:19:22 -0700 Subject: reflect: use zero buffer to back the Value returned by Zero MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the common case (<1KB types), no allocation is required by reflect.Zero. Also use memclr instead of memmove in Set when the source is known to be zero. Fixes #33136 Change-Id: Ic66871930fbb53328032e587153ebd12995ccf55 Reviewed-on: https://go-review.googlesource.com/c/go/+/192331 Trust: Keith Randall Run-TryBot: Keith Randall TryBot-Result: Go Bot Reviewed-by: Martin Möhrmann --- src/reflect/value.go | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) (limited to 'src/reflect/value.go') diff --git a/src/reflect/value.go b/src/reflect/value.go index c6f24a5609..a14131e1f8 100644 --- a/src/reflect/value.go +++ b/src/reflect/value.go @@ -1553,7 +1553,11 @@ func (v Value) Set(x Value) { } x = x.assignTo("reflect.Set", v.typ, target) if x.flag&flagIndir != 0 { - typedmemmove(v.typ, v.ptr, x.ptr) + if x.ptr == unsafe.Pointer(&zeroVal[0]) { + typedmemclr(v.typ, v.ptr) + } else { + typedmemmove(v.typ, v.ptr, x.ptr) + } } else { *(*unsafe.Pointer)(v.ptr) = x.ptr } @@ -2360,11 +2364,23 @@ func Zero(typ Type) Value { t := typ.(*rtype) fl := flag(t.Kind()) if ifaceIndir(t) { - return Value{t, unsafe_New(t), fl | flagIndir} + var p unsafe.Pointer + if t.size <= maxZero { + p = unsafe.Pointer(&zeroVal[0]) + } else { + p = unsafe_New(t) + } + return Value{t, p, fl | flagIndir} } return Value{t, nil, fl} } +// must match declarations in runtime/map.go. +const maxZero = 1024 + +//go:linkname zeroVal runtime.zeroVal +var zeroVal [maxZero]byte + // New returns a Value representing a pointer to a new zero value // for the specified type. That is, the returned Value's Type is PtrTo(typ). func New(typ Type) Value { -- cgit v1.3 From 8f26b57f9afc238bdecb9b7030bc2f4364093885 Mon Sep 17 00:00:00 2001 From: Cuong Manh Le Date: Sat, 3 Oct 2020 01:23:47 +0700 Subject: cmd/compile: split exported/non-exported methods for interface type Currently, mhdr/methods is emitted with the same len/cap. There's no way to distinguish between exported and non-exported methods statically. This CL splits mhdr/methods into two parts, use "len" for number of exported methods, and "cap" for all methods. This fixes the bug in issue #22075, which intends to return the number of exported methods but currently return all methods. Note that with this encoding, we still can access either all/exported-only/non-exported-only methods: mhdr[:cap(mhdr)] // all methods mhdr // exported methods mhdr[len(mhdr):cap(mhdr)] // non-exported methods Thank to Matthew Dempsky (@mdempsky) for suggesting this encoding. Fixes #22075 Change-Id: If662adb03ccff27407d55a5578a0ed05a15e7cdd Reviewed-on: https://go-review.googlesource.com/c/go/+/259237 Trust: Cuong Manh Le Run-TryBot: Cuong Manh Le TryBot-Result: Go Bot Reviewed-by: Cherry Zhang Reviewed-by: Matthew Dempsky --- doc/go1.16.html | 8 +++++ src/cmd/compile/internal/gc/reflect.go | 3 +- src/internal/reflectlite/type.go | 35 +++++++++++++++------- src/internal/reflectlite/value.go | 4 +-- src/reflect/all_test.go | 18 +++++++---- src/reflect/type.go | 55 ++++++++++++++++++++-------------- src/reflect/value.go | 21 +++++++------ src/runtime/alg.go | 2 +- src/runtime/iface.go | 12 ++++---- src/runtime/mfinal.go | 4 +-- src/runtime/type.go | 26 ++++++++++++---- 11 files changed, 124 insertions(+), 64 deletions(-) (limited to 'src/reflect/value.go') diff --git a/doc/go1.16.html b/doc/go1.16.html index 720acc757a..509956fbf2 100644 --- a/doc/go1.16.html +++ b/doc/go1.16.html @@ -213,6 +213,14 @@ Do not send CLs removing the interior tags from such phrases. with "use of closed network connection".

+

reflect

+ +

+ For interface types and values, Method, + MethodByName, and + NumMethod now + operate on the interface's exported method set, rather than its full method set. +

text/template/parse

diff --git a/src/cmd/compile/internal/gc/reflect.go b/src/cmd/compile/internal/gc/reflect.go index 21429af782..229fcfeaee 100644 --- a/src/cmd/compile/internal/gc/reflect.go +++ b/src/cmd/compile/internal/gc/reflect.go @@ -1275,8 +1275,9 @@ func dtypesym(t *types.Type) *obj.LSym { } ot = dgopkgpath(lsym, ot, tpkg) + xcount := sort.Search(n, func(i int) bool { return !types.IsExported(m[i].name.Name) }) ot = dsymptr(lsym, ot, lsym, ot+3*Widthptr+uncommonSize(t)) - ot = duintptr(lsym, ot, uint64(n)) + ot = duintptr(lsym, ot, uint64(xcount)) ot = duintptr(lsym, ot, uint64(n)) dataAdd := imethodSize() * n ot = dextratype(lsym, ot, t, dataAdd) diff --git a/src/internal/reflectlite/type.go b/src/internal/reflectlite/type.go index 15ba30da36..37cf03594f 100644 --- a/src/internal/reflectlite/type.go +++ b/src/internal/reflectlite/type.go @@ -234,10 +234,13 @@ type imethod struct { // interfaceType represents an interface type. type interfaceType struct { rtype - pkgPath name // import path - methods []imethod // sorted by hash + pkgPath name // import path + expMethods []imethod // sorted by name, see runtime/type.go:interfacetype to see how it is encoded. } +func (t *interfaceType) methods() []imethod { return t.expMethods[:cap(t.expMethods)] } +func (t *interfaceType) isEmpty() bool { return cap(t.expMethods) == 0 } + // mapType represents a map type. type mapType struct { rtype @@ -695,7 +698,7 @@ func add(p unsafe.Pointer, x uintptr, whySafe string) unsafe.Pointer { } // NumMethod returns the number of interface methods in the type's method set. -func (t *interfaceType) NumMethod() int { return len(t.methods) } +func (t *interfaceType) NumMethod() int { return len(t.expMethods) } // TypeOf returns the reflection Type that represents the dynamic type of i. // If i is a nil interface value, TypeOf returns nil. @@ -732,9 +735,10 @@ func implements(T, V *rtype) bool { return false } t := (*interfaceType)(unsafe.Pointer(T)) - if len(t.methods) == 0 { + if t.isEmpty() { return true } + tmethods := t.methods() // The same algorithm applies in both cases, but the // method tables for an interface type and a concrete type @@ -751,10 +755,11 @@ func implements(T, V *rtype) bool { if V.Kind() == Interface { v := (*interfaceType)(unsafe.Pointer(V)) i := 0 - for j := 0; j < len(v.methods); j++ { - tm := &t.methods[i] + vmethods := v.methods() + for j := 0; j < len(vmethods); j++ { + tm := &tmethods[i] tmName := t.nameOff(tm.name) - vm := &v.methods[j] + vm := &vmethods[j] vmName := V.nameOff(vm.name) if vmName.name() == tmName.name() && V.typeOff(vm.typ) == t.typeOff(tm.typ) { if !tmName.isExported() { @@ -770,7 +775,7 @@ func implements(T, V *rtype) bool { continue } } - if i++; i >= len(t.methods) { + if i++; i >= len(tmethods) { return true } } @@ -785,7 +790,7 @@ func implements(T, V *rtype) bool { i := 0 vmethods := v.methods() for j := 0; j < int(v.mcount); j++ { - tm := &t.methods[i] + tm := &tmethods[i] tmName := t.nameOff(tm.name) vm := vmethods[j] vmName := V.nameOff(vm.name) @@ -803,7 +808,7 @@ func implements(T, V *rtype) bool { continue } } - if i++; i >= len(t.methods) { + if i++; i >= len(tmethods) { return true } } @@ -897,7 +902,7 @@ func haveIdenticalUnderlyingType(T, V *rtype, cmpTags bool) bool { case Interface: t := (*interfaceType)(unsafe.Pointer(T)) v := (*interfaceType)(unsafe.Pointer(V)) - if len(t.methods) == 0 && len(v.methods) == 0 { + if t.isEmpty() && v.isEmpty() { return true } // Might have the same methods but still @@ -962,3 +967,11 @@ func toType(t *rtype) Type { func ifaceIndir(t *rtype) bool { return t.kind&kindDirectIface == 0 } + +func isEmptyIface(t *rtype) bool { + if t.Kind() != Interface { + return false + } + tt := (*interfaceType)(unsafe.Pointer(t)) + return tt.isEmpty() +} diff --git a/src/internal/reflectlite/value.go b/src/internal/reflectlite/value.go index 0365eeeabf..fb0ec77b58 100644 --- a/src/internal/reflectlite/value.go +++ b/src/internal/reflectlite/value.go @@ -228,7 +228,7 @@ func (v Value) Elem() Value { switch k { case Interface: var eface interface{} - if v.typ.NumMethod() == 0 { + if isEmptyIface(v.typ) { eface = *(*interface{})(v.ptr) } else { eface = (interface{})(*(*interface { @@ -433,7 +433,7 @@ func (v Value) assignTo(context string, dst *rtype, target unsafe.Pointer) Value return Value{dst, nil, flag(Interface)} } x := valueInterface(v) - if dst.NumMethod() == 0 { + if isEmptyIface(dst) { *(*interface{})(target) = x } else { ifaceE2I(dst, x, target) diff --git a/src/reflect/all_test.go b/src/reflect/all_test.go index a12712d254..be15362aae 100644 --- a/src/reflect/all_test.go +++ b/src/reflect/all_test.go @@ -2995,6 +2995,14 @@ func TestUnexportedMethods(t *testing.T) { if got := typ.NumMethod(); got != 0 { t.Errorf("NumMethod=%d, want 0 satisfied methods", got) } + + var i unexpI + if got := TypeOf(&i).Elem().NumMethod(); got != 0 { + t.Errorf("NumMethod=%d, want 0 satisfied methods", got) + } + if got := ValueOf(&i).Elem().NumMethod(); got != 0 { + t.Errorf("NumMethod=%d, want 0 satisfied methods", got) + } } type InnerInt struct { @@ -3648,21 +3656,21 @@ func TestCallPanic(t *testing.T) { v := ValueOf(T{i, i, i, i, T2{i, i}, i, i, T2{i, i}}) badCall(func() { call(v.Field(0).Method(0)) }) // .t0.W badCall(func() { call(v.Field(0).Elem().Method(0)) }) // .t0.W - badCall(func() { call(v.Field(0).Method(1)) }) // .t0.w + badMethod(func() { call(v.Field(0).Method(1)) }) // .t0.w badMethod(func() { call(v.Field(0).Elem().Method(2)) }) // .t0.w ok(func() { call(v.Field(1).Method(0)) }) // .T1.Y ok(func() { call(v.Field(1).Elem().Method(0)) }) // .T1.Y - badCall(func() { call(v.Field(1).Method(1)) }) // .T1.y + badMethod(func() { call(v.Field(1).Method(1)) }) // .T1.y badMethod(func() { call(v.Field(1).Elem().Method(2)) }) // .T1.y ok(func() { call(v.Field(2).Method(0)) }) // .NamedT0.W ok(func() { call(v.Field(2).Elem().Method(0)) }) // .NamedT0.W - badCall(func() { call(v.Field(2).Method(1)) }) // .NamedT0.w + badMethod(func() { call(v.Field(2).Method(1)) }) // .NamedT0.w badMethod(func() { call(v.Field(2).Elem().Method(2)) }) // .NamedT0.w ok(func() { call(v.Field(3).Method(0)) }) // .NamedT1.Y ok(func() { call(v.Field(3).Elem().Method(0)) }) // .NamedT1.Y - badCall(func() { call(v.Field(3).Method(1)) }) // .NamedT1.y + badMethod(func() { call(v.Field(3).Method(1)) }) // .NamedT1.y badMethod(func() { call(v.Field(3).Elem().Method(3)) }) // .NamedT1.y ok(func() { call(v.Field(4).Field(0).Method(0)) }) // .NamedT2.T1.Y @@ -3672,7 +3680,7 @@ func TestCallPanic(t *testing.T) { badCall(func() { call(v.Field(5).Method(0)) }) // .namedT0.W badCall(func() { call(v.Field(5).Elem().Method(0)) }) // .namedT0.W - badCall(func() { call(v.Field(5).Method(1)) }) // .namedT0.w + badMethod(func() { call(v.Field(5).Method(1)) }) // .namedT0.w badMethod(func() { call(v.Field(5).Elem().Method(2)) }) // .namedT0.w badCall(func() { call(v.Field(6).Method(0)) }) // .namedT1.Y diff --git a/src/reflect/type.go b/src/reflect/type.go index a3a616701b..0b34ca0c94 100644 --- a/src/reflect/type.go +++ b/src/reflect/type.go @@ -386,10 +386,14 @@ type imethod struct { // interfaceType represents an interface type. type interfaceType struct { rtype - pkgPath name // import path - methods []imethod // sorted by hash + pkgPath name // import path + expMethods []imethod // sorted by name, see runtime/type.go:interfacetype to see how it is encoded. } +// methods returns t's full method set, both exported and non-exported. +func (t *interfaceType) methods() []imethod { return t.expMethods[:cap(t.expMethods)] } +func (t *interfaceType) isEmpty() bool { return cap(t.expMethods) == 0 } + // mapType represents a map type. type mapType struct { rtype @@ -1049,25 +1053,22 @@ func (d ChanDir) String() string { // Method returns the i'th method in the type's method set. func (t *interfaceType) Method(i int) (m Method) { - if i < 0 || i >= len(t.methods) { - return + if i < 0 || i >= len(t.expMethods) { + panic("reflect: Method index out of range") } - p := &t.methods[i] + p := &t.expMethods[i] pname := t.nameOff(p.name) m.Name = pname.name() if !pname.isExported() { - m.PkgPath = pname.pkgPath() - if m.PkgPath == "" { - m.PkgPath = t.pkgPath.name() - } + panic("reflect: unexported method: " + pname.name()) } m.Type = toType(t.typeOff(p.typ)) m.Index = i return } -// NumMethod returns the number of interface methods in the type's method set. -func (t *interfaceType) NumMethod() int { return len(t.methods) } +// NumMethod returns the number of exported interface methods in the type's method set. +func (t *interfaceType) NumMethod() int { return len(t.expMethods) } // MethodByName method with the given name in the type's method set. func (t *interfaceType) MethodByName(name string) (m Method, ok bool) { @@ -1075,8 +1076,8 @@ func (t *interfaceType) MethodByName(name string) (m Method, ok bool) { return } var p *imethod - for i := range t.methods { - p = &t.methods[i] + for i := range t.expMethods { + p = &t.expMethods[i] if t.nameOff(p.name).name() == name { return t.Method(i), true } @@ -1485,9 +1486,10 @@ func implements(T, V *rtype) bool { return false } t := (*interfaceType)(unsafe.Pointer(T)) - if len(t.methods) == 0 { + if t.isEmpty() { return true } + tmethods := t.methods() // The same algorithm applies in both cases, but the // method tables for an interface type and a concrete type @@ -1504,10 +1506,11 @@ func implements(T, V *rtype) bool { if V.Kind() == Interface { v := (*interfaceType)(unsafe.Pointer(V)) i := 0 - for j := 0; j < len(v.methods); j++ { - tm := &t.methods[i] + vmethods := v.methods() + for j := 0; j < len(vmethods); j++ { + tm := &tmethods[i] tmName := t.nameOff(tm.name) - vm := &v.methods[j] + vm := &vmethods[j] vmName := V.nameOff(vm.name) if vmName.name() == tmName.name() && V.typeOff(vm.typ) == t.typeOff(tm.typ) { if !tmName.isExported() { @@ -1523,7 +1526,7 @@ func implements(T, V *rtype) bool { continue } } - if i++; i >= len(t.methods) { + if i++; i >= len(tmethods) { return true } } @@ -1538,7 +1541,7 @@ func implements(T, V *rtype) bool { i := 0 vmethods := v.methods() for j := 0; j < int(v.mcount); j++ { - tm := &t.methods[i] + tm := &tmethods[i] tmName := t.nameOff(tm.name) vm := vmethods[j] vmName := V.nameOff(vm.name) @@ -1556,7 +1559,7 @@ func implements(T, V *rtype) bool { continue } } - if i++; i >= len(t.methods) { + if i++; i >= len(tmethods) { return true } } @@ -1658,7 +1661,7 @@ func haveIdenticalUnderlyingType(T, V *rtype, cmpTags bool) bool { case Interface: t := (*interfaceType)(unsafe.Pointer(T)) v := (*interfaceType)(unsafe.Pointer(V)) - if len(t.methods) == 0 && len(v.methods) == 0 { + if t.isEmpty() && v.isEmpty() { return true } // Might have the same methods but still @@ -2442,7 +2445,7 @@ func StructOf(fields []StructField) Type { switch f.typ.Kind() { case Interface: ift := (*interfaceType)(unsafe.Pointer(ft)) - for im, m := range ift.methods { + for im, m := range ift.methods() { if ift.nameOff(m.name).pkgPath() != "" { // TODO(sbinet). Issue 15924. panic("reflect: embedded interface with unexported method(s) not implemented") @@ -3149,3 +3152,11 @@ func addTypeBits(bv *bitVector, offset uintptr, t *rtype) { } } } + +func isEmptyIface(rt *rtype) bool { + if rt.Kind() != Interface { + return false + } + tt := (*interfaceType)(unsafe.Pointer(rt)) + return len(tt.methods()) == 0 +} diff --git a/src/reflect/value.go b/src/reflect/value.go index a14131e1f8..bb6371b867 100644 --- a/src/reflect/value.go +++ b/src/reflect/value.go @@ -635,10 +635,11 @@ func methodReceiver(op string, v Value, methodIndex int) (rcvrtype *rtype, t *fu i := methodIndex if v.typ.Kind() == Interface { tt := (*interfaceType)(unsafe.Pointer(v.typ)) - if uint(i) >= uint(len(tt.methods)) { + ttmethods := tt.methods() + if uint(i) >= uint(len(ttmethods)) { panic("reflect: internal error: invalid method index") } - m := &tt.methods[i] + m := &ttmethods[i] if !tt.nameOff(m.name).isExported() { panic("reflect: " + op + " of unexported method") } @@ -812,7 +813,7 @@ func (v Value) Elem() Value { switch k { case Interface: var eface interface{} - if v.typ.NumMethod() == 0 { + if isEmptyIface(v.typ) { eface = *(*interface{})(v.ptr) } else { eface = (interface{})(*(*interface { @@ -1033,7 +1034,7 @@ func valueInterface(v Value, safe bool) interface{} { // Special case: return the element inside the interface. // Empty interface has one layout, all interfaces with // methods have a second layout. - if v.NumMethod() == 0 { + if isEmptyIface(v.typ) { return *(*interface{})(v.ptr) } return *(*interface { @@ -1908,10 +1909,11 @@ func (v Value) Type() Type { if v.typ.Kind() == Interface { // Method on interface. tt := (*interfaceType)(unsafe.Pointer(v.typ)) - if uint(i) >= uint(len(tt.methods)) { + ttmethods := tt.methods() + if uint(i) >= uint(len(ttmethods)) { panic("reflect: internal error: invalid method index") } - m := &tt.methods[i] + m := &ttmethods[i] return v.typ.typeOff(m.typ) } // Method on concrete type. @@ -2429,7 +2431,7 @@ func (v Value) assignTo(context string, dst *rtype, target unsafe.Pointer) Value return Value{dst, nil, flag(Interface)} } x := valueInterface(v, false) - if dst.NumMethod() == 0 { + if isEmptyIface(dst) { *(*interface{})(target) = x } else { ifaceE2I(dst, x, target) @@ -2718,10 +2720,11 @@ func cvtDirect(v Value, typ Type) Value { func cvtT2I(v Value, typ Type) Value { target := unsafe_New(typ.common()) x := valueInterface(v, false) - if typ.NumMethod() == 0 { + rt := typ.(*rtype) + if isEmptyIface(rt) { *(*interface{})(target) = x } else { - ifaceE2I(typ.(*rtype), x, target) + ifaceE2I(rt, x, target) } return Value{typ.common(), target, v.flag.ro() | flagIndir | flag(Interface)} } diff --git a/src/runtime/alg.go b/src/runtime/alg.go index 0af48ab25c..4a98b84e4a 100644 --- a/src/runtime/alg.go +++ b/src/runtime/alg.go @@ -185,7 +185,7 @@ func typehash(t *_type, p unsafe.Pointer, h uintptr) uintptr { return strhash(p, h) case kindInterface: i := (*interfacetype)(unsafe.Pointer(t)) - if len(i.mhdr) == 0 { + if i.isEmpty() { return nilinterhash(p, h) } return interhash(p, h) diff --git a/src/runtime/iface.go b/src/runtime/iface.go index 0504b89363..f8b7d429a3 100644 --- a/src/runtime/iface.go +++ b/src/runtime/iface.go @@ -31,16 +31,17 @@ func itabHashFunc(inter *interfacetype, typ *_type) uintptr { } func getitab(inter *interfacetype, typ *_type, canfail bool) *itab { - if len(inter.mhdr) == 0 { + if inter.isEmpty() { throw("internal error - misuse of itab") } + imethods := inter.methods() // easy case if typ.tflag&tflagUncommon == 0 { if canfail { return nil } - name := inter.typ.nameOff(inter.mhdr[0].name) + name := inter.typ.nameOff(imethods[0].name) panic(&TypeAssertionError{nil, typ, &inter.typ, name.name()}) } @@ -63,7 +64,7 @@ func getitab(inter *interfacetype, typ *_type, canfail bool) *itab { } // Entry doesn't exist yet. Make a new entry & add it. - m = (*itab)(persistentalloc(unsafe.Sizeof(itab{})+uintptr(len(inter.mhdr)-1)*sys.PtrSize, 0, &memstats.other_sys)) + m = (*itab)(persistentalloc(unsafe.Sizeof(itab{})+uintptr(len(imethods)-1)*sys.PtrSize, 0, &memstats.other_sys)) m.inter = inter m._type = typ // The hash is used in type switches. However, compiler statically generates itab's @@ -197,7 +198,8 @@ func (m *itab) init() string { // and interface names are unique, // so can iterate over both in lock step; // the loop is O(ni+nt) not O(ni*nt). - ni := len(inter.mhdr) + imethods := inter.methods() + ni := len(imethods) nt := int(x.mcount) xmhdr := (*[1 << 16]method)(add(unsafe.Pointer(x), uintptr(x.moff)))[:nt:nt] j := 0 @@ -205,7 +207,7 @@ func (m *itab) init() string { var fun0 unsafe.Pointer imethods: for k := 0; k < ni; k++ { - i := &inter.mhdr[k] + i := &imethods[k] itype := inter.typ.typeOff(i.ityp) name := inter.typ.nameOff(i.name) iname := name.name() diff --git a/src/runtime/mfinal.go b/src/runtime/mfinal.go index cd6196dcab..6676ae6736 100644 --- a/src/runtime/mfinal.go +++ b/src/runtime/mfinal.go @@ -210,7 +210,7 @@ func runfinq() { // set up with empty interface (*eface)(frame)._type = &f.ot.typ (*eface)(frame).data = f.arg - if len(ityp.mhdr) != 0 { + if !ityp.isEmpty() { // convert to interface with methods // this conversion is guaranteed to succeed - we checked in SetFinalizer *(*iface)(frame) = assertE2I(ityp, *(*eface)(frame)) @@ -394,7 +394,7 @@ func SetFinalizer(obj interface{}, finalizer interface{}) { } case fint.kind&kindMask == kindInterface: ityp := (*interfacetype)(unsafe.Pointer(fint)) - if len(ityp.mhdr) == 0 { + if ityp.isEmpty() { // ok - satisfies empty interface goto okarg } diff --git a/src/runtime/type.go b/src/runtime/type.go index 81455f3532..36492619e1 100644 --- a/src/runtime/type.go +++ b/src/runtime/type.go @@ -366,7 +366,19 @@ type imethod struct { type interfacetype struct { typ _type pkgpath name - mhdr []imethod + // expMethods contains all interface methods. + // + // - len(expMethods) returns number of exported methods. + // - cap(expMethods) returns all interface methods, including both exported/non-exported methods. + expMethods []imethod +} + +func (it *interfacetype) methods() []imethod { + return it.expMethods[:cap(it.expMethods)] +} + +func (it *interfacetype) isEmpty() bool { + return cap(it.expMethods) == 0 } type maptype struct { @@ -664,13 +676,15 @@ func typesEqual(t, v *_type, seen map[_typePair]struct{}) bool { if it.pkgpath.name() != iv.pkgpath.name() { return false } - if len(it.mhdr) != len(iv.mhdr) { + itmethods := it.methods() + ivmethods := iv.methods() + if len(itmethods) != len(ivmethods) { return false } - for i := range it.mhdr { - tm := &it.mhdr[i] - vm := &iv.mhdr[i] - // Note the mhdr array can be relocated from + for i := range itmethods { + tm := &itmethods[i] + vm := &ivmethods[i] + // Note the expMethods array can be relocated from // another module. See #17724. tname := resolveNameOff(unsafe.Pointer(tm), tm.name) vname := resolveNameOff(unsafe.Pointer(vm), vm.name) -- cgit v1.3 From 009d71409821a6ac4f1b32aaae2c856c20a29f92 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Thu, 22 Oct 2020 16:37:19 -0700 Subject: cmd/compile, runtime: store pointers to go:notinheap types indirectly pointers to go:notinheap types should be treated as scalars. That means they shouldn't be stored directly in interfaces, or directly in reflect.Value.ptr. Also be sure to use uintpr to compare such pointers in reflect.DeepEqual. Fixes #42076 Change-Id: I53735f6d434e9c3108d4940bd1bae14c61ef2a74 Reviewed-on: https://go-review.googlesource.com/c/go/+/264480 Trust: Keith Randall Reviewed-by: Ian Lance Taylor --- src/cmd/compile/internal/gc/subr.go | 6 ++-- src/cmd/compile/internal/gc/typecheck.go | 2 +- src/reflect/deepequal.go | 12 +++++++- src/reflect/value.go | 12 +++++++- src/runtime/netpoll.go | 47 ++++++++++++++++++++++---------- test/fixedbugs/issue42076.go | 21 ++++++++++++++ 6 files changed, 81 insertions(+), 19 deletions(-) create mode 100644 test/fixedbugs/issue42076.go (limited to 'src/reflect/value.go') diff --git a/src/cmd/compile/internal/gc/subr.go b/src/cmd/compile/internal/gc/subr.go index eccd6f8a74..defefd76b3 100644 --- a/src/cmd/compile/internal/gc/subr.go +++ b/src/cmd/compile/internal/gc/subr.go @@ -1854,8 +1854,10 @@ func isdirectiface(t *types.Type) bool { } switch t.Etype { - case TPTR, - TCHAN, + case TPTR: + // Pointers to notinheap types must be stored indirectly. See issue 42076. + return !t.Elem().NotInHeap() + case TCHAN, TMAP, TFUNC, TUNSAFEPTR: diff --git a/src/cmd/compile/internal/gc/typecheck.go b/src/cmd/compile/internal/gc/typecheck.go index ce817db446..8ebeaf1330 100644 --- a/src/cmd/compile/internal/gc/typecheck.go +++ b/src/cmd/compile/internal/gc/typecheck.go @@ -2516,7 +2516,7 @@ func lookdot(n *Node, t *types.Type, dostrcmp int) *types.Field { n.Left = nod(OADDR, n.Left, nil) n.Left.SetImplicit(true) n.Left = typecheck(n.Left, ctxType|ctxExpr) - } else if tt.IsPtr() && !rcvr.IsPtr() && types.Identical(tt.Elem(), rcvr) { + } else if tt.IsPtr() && (!rcvr.IsPtr() || rcvr.IsPtr() && rcvr.Elem().NotInHeap()) && types.Identical(tt.Elem(), rcvr) { n.Left = nod(ODEREF, n.Left, nil) n.Left.SetImplicit(true) n.Left = typecheck(n.Left, ctxType|ctxExpr) diff --git a/src/reflect/deepequal.go b/src/reflect/deepequal.go index be66464129..d951d8d999 100644 --- a/src/reflect/deepequal.go +++ b/src/reflect/deepequal.go @@ -35,7 +35,17 @@ func deepValueEqual(v1, v2 Value, visited map[visit]bool) bool { // and it's safe and valid to get Value's internal pointer. hard := func(v1, v2 Value) bool { switch v1.Kind() { - case Map, Slice, Ptr, Interface: + case Ptr: + if v1.typ.ptrdata == 0 { + // go:notinheap pointers can't be cyclic. + // At least, all of our current uses of go:notinheap have + // that property. The runtime ones aren't cyclic (and we don't use + // DeepEqual on them anyway), and the cgo-generated ones are + // all empty structs. + return false + } + fallthrough + case Map, Slice, Interface: // Nil pointers cannot be cyclic. Avoid putting them in the visited map. return !v1.IsNil() && !v2.IsNil() } diff --git a/src/reflect/value.go b/src/reflect/value.go index bb6371b867..24eab6a2c6 100644 --- a/src/reflect/value.go +++ b/src/reflect/value.go @@ -90,6 +90,7 @@ func (f flag) ro() flag { // pointer returns the underlying pointer represented by v. // v.Kind() must be Ptr, Map, Chan, Func, or UnsafePointer +// if v.Kind() == Ptr, the base type must not be go:notinheap. func (v Value) pointer() unsafe.Pointer { if v.typ.size != ptrSize || !v.typ.pointers() { panic("can't call pointer on a non-pointer Value") @@ -1453,7 +1454,16 @@ func (v Value) Pointer() uintptr { // TODO: deprecate k := v.kind() switch k { - case Chan, Map, Ptr, UnsafePointer: + 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 { diff --git a/src/runtime/netpoll.go b/src/runtime/netpoll.go index 34ea82a7fa..77eb3aa4c6 100644 --- a/src/runtime/netpoll.go +++ b/src/runtime/netpoll.go @@ -79,16 +79,17 @@ type pollDesc struct { lock mutex // protects the following fields fd uintptr closing bool - everr bool // marks event scanning error happened - user uint32 // user settable cookie - rseq uintptr // protects from stale read timers - rg uintptr // pdReady, pdWait, G waiting for read or nil - rt timer // read deadline timer (set if rt.f != nil) - rd int64 // read deadline - wseq uintptr // protects from stale write timers - wg uintptr // pdReady, pdWait, G waiting for write or nil - wt timer // write deadline timer - wd int64 // write deadline + everr bool // marks event scanning error happened + user uint32 // user settable cookie + rseq uintptr // protects from stale read timers + rg uintptr // pdReady, pdWait, G waiting for read or nil + rt timer // read deadline timer (set if rt.f != nil) + rd int64 // read deadline + wseq uintptr // protects from stale write timers + wg uintptr // pdReady, pdWait, G waiting for write or nil + wt timer // write deadline timer + wd int64 // write deadline + self *pollDesc // storage for indirect interface. See (*pollDesc).makeArg. } type pollCache struct { @@ -157,6 +158,7 @@ func poll_runtime_pollOpen(fd uintptr) (*pollDesc, int) { pd.wseq++ pd.wg = 0 pd.wd = 0 + pd.self = pd unlock(&pd.lock) var errno int32 @@ -271,14 +273,14 @@ func poll_runtime_pollSetDeadline(pd *pollDesc, d int64, mode int) { // Copy current seq into the timer arg. // Timer func will check the seq against current descriptor seq, // if they differ the descriptor was reused or timers were reset. - pd.rt.arg = pd + pd.rt.arg = pd.makeArg() pd.rt.seq = pd.rseq resettimer(&pd.rt, pd.rd) } } else if pd.rd != rd0 || combo != combo0 { pd.rseq++ // invalidate current timers if pd.rd > 0 { - modtimer(&pd.rt, pd.rd, 0, rtf, pd, pd.rseq) + modtimer(&pd.rt, pd.rd, 0, rtf, pd.makeArg(), pd.rseq) } else { deltimer(&pd.rt) pd.rt.f = nil @@ -287,14 +289,14 @@ func poll_runtime_pollSetDeadline(pd *pollDesc, d int64, mode int) { if pd.wt.f == nil { if pd.wd > 0 && !combo { pd.wt.f = netpollWriteDeadline - pd.wt.arg = pd + pd.wt.arg = pd.makeArg() pd.wt.seq = pd.wseq resettimer(&pd.wt, pd.wd) } } else if pd.wd != wd0 || combo != combo0 { pd.wseq++ // invalidate current timers if pd.wd > 0 && !combo { - modtimer(&pd.wt, pd.wd, 0, netpollWriteDeadline, pd, pd.wseq) + modtimer(&pd.wt, pd.wd, 0, netpollWriteDeadline, pd.makeArg(), pd.wseq) } else { deltimer(&pd.wt) pd.wt.f = nil @@ -547,3 +549,20 @@ func (c *pollCache) alloc() *pollDesc { unlock(&c.lock) return pd } + +// makeArg converts pd to an interface{}. +// makeArg does not do any allocation. Normally, such +// a conversion requires an allocation because pointers to +// go:notinheap types (which pollDesc is) must be stored +// in interfaces indirectly. See issue 42076. +func (pd *pollDesc) makeArg() (i interface{}) { + x := (*eface)(unsafe.Pointer(&i)) + x._type = pdType + x.data = unsafe.Pointer(&pd.self) + return +} + +var ( + pdEface interface{} = (*pollDesc)(nil) + pdType *_type = efaceOf(&pdEface)._type +) diff --git a/test/fixedbugs/issue42076.go b/test/fixedbugs/issue42076.go new file mode 100644 index 0000000000..3e954813c9 --- /dev/null +++ b/test/fixedbugs/issue42076.go @@ -0,0 +1,21 @@ +// run + +// Copyright 2020 The Go Authors. All rights reserved. Use of this +// source code is governed by a BSD-style license that can be found in +// the LICENSE file. + +package main + +import "reflect" + +//go:notinheap +type NIH struct { +} + +var x, y NIH + +func main() { + if reflect.DeepEqual(&x, &y) != true { + panic("should report true") + } +} -- cgit v1.3 From 642329fdd55aabafc67b3a7c50902e29125621ab Mon Sep 17 00:00:00 2001 From: Cuong Manh Le Date: Thu, 22 Oct 2020 00:25:17 +0700 Subject: Revert "cmd/compile: split exported/non-exported methods for interface type" This reverts commit 8f26b57f9afc238bdecb9b7030bc2f4364093885. Reason for revert: break a bunch of code, include standard library. Fixes #42123 Change-Id: Ife90ecbafd2cb395623d1db555fbfc9c1b0098e0 Reviewed-on: https://go-review.googlesource.com/c/go/+/264026 Trust: Cuong Manh Le Run-TryBot: Cuong Manh Le TryBot-Result: Go Bot Reviewed-by: Russ Cox --- doc/go1.16.html | 9 ------ src/cmd/compile/internal/gc/reflect.go | 3 +- src/internal/reflectlite/type.go | 35 +++++++--------------- src/internal/reflectlite/value.go | 4 +-- src/reflect/all_test.go | 18 ++++------- src/reflect/type.go | 55 ++++++++++++++-------------------- src/reflect/value.go | 21 ++++++------- src/runtime/alg.go | 2 +- src/runtime/iface.go | 12 ++++---- src/runtime/mfinal.go | 4 +-- src/runtime/type.go | 26 ++++------------ 11 files changed, 64 insertions(+), 125 deletions(-) (limited to 'src/reflect/value.go') diff --git a/doc/go1.16.html b/doc/go1.16.html index ba2f80f95e..3592d0b663 100644 --- a/doc/go1.16.html +++ b/doc/go1.16.html @@ -264,15 +264,6 @@ Do not send CLs removing the interior tags from such phrases. On Linux kernel version 4.1 and above, the maximum is now 4294967295.

-

reflect

- -

- For interface types and values, Method, - MethodByName, and - NumMethod now - operate on the interface's exported method set, rather than its full method set. -

-

text/template/parse

diff --git a/src/cmd/compile/internal/gc/reflect.go b/src/cmd/compile/internal/gc/reflect.go index 229fcfeaee..21429af782 100644 --- a/src/cmd/compile/internal/gc/reflect.go +++ b/src/cmd/compile/internal/gc/reflect.go @@ -1275,9 +1275,8 @@ func dtypesym(t *types.Type) *obj.LSym { } ot = dgopkgpath(lsym, ot, tpkg) - xcount := sort.Search(n, func(i int) bool { return !types.IsExported(m[i].name.Name) }) ot = dsymptr(lsym, ot, lsym, ot+3*Widthptr+uncommonSize(t)) - ot = duintptr(lsym, ot, uint64(xcount)) + ot = duintptr(lsym, ot, uint64(n)) ot = duintptr(lsym, ot, uint64(n)) dataAdd := imethodSize() * n ot = dextratype(lsym, ot, t, dataAdd) diff --git a/src/internal/reflectlite/type.go b/src/internal/reflectlite/type.go index 37cf03594f..15ba30da36 100644 --- a/src/internal/reflectlite/type.go +++ b/src/internal/reflectlite/type.go @@ -234,13 +234,10 @@ type imethod struct { // interfaceType represents an interface type. type interfaceType struct { rtype - pkgPath name // import path - expMethods []imethod // sorted by name, see runtime/type.go:interfacetype to see how it is encoded. + pkgPath name // import path + methods []imethod // sorted by hash } -func (t *interfaceType) methods() []imethod { return t.expMethods[:cap(t.expMethods)] } -func (t *interfaceType) isEmpty() bool { return cap(t.expMethods) == 0 } - // mapType represents a map type. type mapType struct { rtype @@ -698,7 +695,7 @@ func add(p unsafe.Pointer, x uintptr, whySafe string) unsafe.Pointer { } // NumMethod returns the number of interface methods in the type's method set. -func (t *interfaceType) NumMethod() int { return len(t.expMethods) } +func (t *interfaceType) NumMethod() int { return len(t.methods) } // TypeOf returns the reflection Type that represents the dynamic type of i. // If i is a nil interface value, TypeOf returns nil. @@ -735,10 +732,9 @@ func implements(T, V *rtype) bool { return false } t := (*interfaceType)(unsafe.Pointer(T)) - if t.isEmpty() { + if len(t.methods) == 0 { return true } - tmethods := t.methods() // The same algorithm applies in both cases, but the // method tables for an interface type and a concrete type @@ -755,11 +751,10 @@ func implements(T, V *rtype) bool { if V.Kind() == Interface { v := (*interfaceType)(unsafe.Pointer(V)) i := 0 - vmethods := v.methods() - for j := 0; j < len(vmethods); j++ { - tm := &tmethods[i] + for j := 0; j < len(v.methods); j++ { + tm := &t.methods[i] tmName := t.nameOff(tm.name) - vm := &vmethods[j] + vm := &v.methods[j] vmName := V.nameOff(vm.name) if vmName.name() == tmName.name() && V.typeOff(vm.typ) == t.typeOff(tm.typ) { if !tmName.isExported() { @@ -775,7 +770,7 @@ func implements(T, V *rtype) bool { continue } } - if i++; i >= len(tmethods) { + if i++; i >= len(t.methods) { return true } } @@ -790,7 +785,7 @@ func implements(T, V *rtype) bool { i := 0 vmethods := v.methods() for j := 0; j < int(v.mcount); j++ { - tm := &tmethods[i] + tm := &t.methods[i] tmName := t.nameOff(tm.name) vm := vmethods[j] vmName := V.nameOff(vm.name) @@ -808,7 +803,7 @@ func implements(T, V *rtype) bool { continue } } - if i++; i >= len(tmethods) { + if i++; i >= len(t.methods) { return true } } @@ -902,7 +897,7 @@ func haveIdenticalUnderlyingType(T, V *rtype, cmpTags bool) bool { case Interface: t := (*interfaceType)(unsafe.Pointer(T)) v := (*interfaceType)(unsafe.Pointer(V)) - if t.isEmpty() && v.isEmpty() { + if len(t.methods) == 0 && len(v.methods) == 0 { return true } // Might have the same methods but still @@ -967,11 +962,3 @@ func toType(t *rtype) Type { func ifaceIndir(t *rtype) bool { return t.kind&kindDirectIface == 0 } - -func isEmptyIface(t *rtype) bool { - if t.Kind() != Interface { - return false - } - tt := (*interfaceType)(unsafe.Pointer(t)) - return tt.isEmpty() -} diff --git a/src/internal/reflectlite/value.go b/src/internal/reflectlite/value.go index fb0ec77b58..0365eeeabf 100644 --- a/src/internal/reflectlite/value.go +++ b/src/internal/reflectlite/value.go @@ -228,7 +228,7 @@ func (v Value) Elem() Value { switch k { case Interface: var eface interface{} - if isEmptyIface(v.typ) { + if v.typ.NumMethod() == 0 { eface = *(*interface{})(v.ptr) } else { eface = (interface{})(*(*interface { @@ -433,7 +433,7 @@ func (v Value) assignTo(context string, dst *rtype, target unsafe.Pointer) Value return Value{dst, nil, flag(Interface)} } x := valueInterface(v) - if isEmptyIface(dst) { + if dst.NumMethod() == 0 { *(*interface{})(target) = x } else { ifaceE2I(dst, x, target) diff --git a/src/reflect/all_test.go b/src/reflect/all_test.go index be15362aae..a12712d254 100644 --- a/src/reflect/all_test.go +++ b/src/reflect/all_test.go @@ -2995,14 +2995,6 @@ func TestUnexportedMethods(t *testing.T) { if got := typ.NumMethod(); got != 0 { t.Errorf("NumMethod=%d, want 0 satisfied methods", got) } - - var i unexpI - if got := TypeOf(&i).Elem().NumMethod(); got != 0 { - t.Errorf("NumMethod=%d, want 0 satisfied methods", got) - } - if got := ValueOf(&i).Elem().NumMethod(); got != 0 { - t.Errorf("NumMethod=%d, want 0 satisfied methods", got) - } } type InnerInt struct { @@ -3656,21 +3648,21 @@ func TestCallPanic(t *testing.T) { v := ValueOf(T{i, i, i, i, T2{i, i}, i, i, T2{i, i}}) badCall(func() { call(v.Field(0).Method(0)) }) // .t0.W badCall(func() { call(v.Field(0).Elem().Method(0)) }) // .t0.W - badMethod(func() { call(v.Field(0).Method(1)) }) // .t0.w + badCall(func() { call(v.Field(0).Method(1)) }) // .t0.w badMethod(func() { call(v.Field(0).Elem().Method(2)) }) // .t0.w ok(func() { call(v.Field(1).Method(0)) }) // .T1.Y ok(func() { call(v.Field(1).Elem().Method(0)) }) // .T1.Y - badMethod(func() { call(v.Field(1).Method(1)) }) // .T1.y + badCall(func() { call(v.Field(1).Method(1)) }) // .T1.y badMethod(func() { call(v.Field(1).Elem().Method(2)) }) // .T1.y ok(func() { call(v.Field(2).Method(0)) }) // .NamedT0.W ok(func() { call(v.Field(2).Elem().Method(0)) }) // .NamedT0.W - badMethod(func() { call(v.Field(2).Method(1)) }) // .NamedT0.w + badCall(func() { call(v.Field(2).Method(1)) }) // .NamedT0.w badMethod(func() { call(v.Field(2).Elem().Method(2)) }) // .NamedT0.w ok(func() { call(v.Field(3).Method(0)) }) // .NamedT1.Y ok(func() { call(v.Field(3).Elem().Method(0)) }) // .NamedT1.Y - badMethod(func() { call(v.Field(3).Method(1)) }) // .NamedT1.y + badCall(func() { call(v.Field(3).Method(1)) }) // .NamedT1.y badMethod(func() { call(v.Field(3).Elem().Method(3)) }) // .NamedT1.y ok(func() { call(v.Field(4).Field(0).Method(0)) }) // .NamedT2.T1.Y @@ -3680,7 +3672,7 @@ func TestCallPanic(t *testing.T) { badCall(func() { call(v.Field(5).Method(0)) }) // .namedT0.W badCall(func() { call(v.Field(5).Elem().Method(0)) }) // .namedT0.W - badMethod(func() { call(v.Field(5).Method(1)) }) // .namedT0.w + badCall(func() { call(v.Field(5).Method(1)) }) // .namedT0.w badMethod(func() { call(v.Field(5).Elem().Method(2)) }) // .namedT0.w badCall(func() { call(v.Field(6).Method(0)) }) // .namedT1.Y diff --git a/src/reflect/type.go b/src/reflect/type.go index 0b34ca0c94..a3a616701b 100644 --- a/src/reflect/type.go +++ b/src/reflect/type.go @@ -386,14 +386,10 @@ type imethod struct { // interfaceType represents an interface type. type interfaceType struct { rtype - pkgPath name // import path - expMethods []imethod // sorted by name, see runtime/type.go:interfacetype to see how it is encoded. + pkgPath name // import path + methods []imethod // sorted by hash } -// methods returns t's full method set, both exported and non-exported. -func (t *interfaceType) methods() []imethod { return t.expMethods[:cap(t.expMethods)] } -func (t *interfaceType) isEmpty() bool { return cap(t.expMethods) == 0 } - // mapType represents a map type. type mapType struct { rtype @@ -1053,22 +1049,25 @@ func (d ChanDir) String() string { // Method returns the i'th method in the type's method set. func (t *interfaceType) Method(i int) (m Method) { - if i < 0 || i >= len(t.expMethods) { - panic("reflect: Method index out of range") + if i < 0 || i >= len(t.methods) { + return } - p := &t.expMethods[i] + p := &t.methods[i] pname := t.nameOff(p.name) m.Name = pname.name() if !pname.isExported() { - panic("reflect: unexported method: " + pname.name()) + m.PkgPath = pname.pkgPath() + if m.PkgPath == "" { + m.PkgPath = t.pkgPath.name() + } } m.Type = toType(t.typeOff(p.typ)) m.Index = i return } -// NumMethod returns the number of exported interface methods in the type's method set. -func (t *interfaceType) NumMethod() int { return len(t.expMethods) } +// NumMethod returns the number of interface methods in the type's method set. +func (t *interfaceType) NumMethod() int { return len(t.methods) } // MethodByName method with the given name in the type's method set. func (t *interfaceType) MethodByName(name string) (m Method, ok bool) { @@ -1076,8 +1075,8 @@ func (t *interfaceType) MethodByName(name string) (m Method, ok bool) { return } var p *imethod - for i := range t.expMethods { - p = &t.expMethods[i] + for i := range t.methods { + p = &t.methods[i] if t.nameOff(p.name).name() == name { return t.Method(i), true } @@ -1486,10 +1485,9 @@ func implements(T, V *rtype) bool { return false } t := (*interfaceType)(unsafe.Pointer(T)) - if t.isEmpty() { + if len(t.methods) == 0 { return true } - tmethods := t.methods() // The same algorithm applies in both cases, but the // method tables for an interface type and a concrete type @@ -1506,11 +1504,10 @@ func implements(T, V *rtype) bool { if V.Kind() == Interface { v := (*interfaceType)(unsafe.Pointer(V)) i := 0 - vmethods := v.methods() - for j := 0; j < len(vmethods); j++ { - tm := &tmethods[i] + for j := 0; j < len(v.methods); j++ { + tm := &t.methods[i] tmName := t.nameOff(tm.name) - vm := &vmethods[j] + vm := &v.methods[j] vmName := V.nameOff(vm.name) if vmName.name() == tmName.name() && V.typeOff(vm.typ) == t.typeOff(tm.typ) { if !tmName.isExported() { @@ -1526,7 +1523,7 @@ func implements(T, V *rtype) bool { continue } } - if i++; i >= len(tmethods) { + if i++; i >= len(t.methods) { return true } } @@ -1541,7 +1538,7 @@ func implements(T, V *rtype) bool { i := 0 vmethods := v.methods() for j := 0; j < int(v.mcount); j++ { - tm := &tmethods[i] + tm := &t.methods[i] tmName := t.nameOff(tm.name) vm := vmethods[j] vmName := V.nameOff(vm.name) @@ -1559,7 +1556,7 @@ func implements(T, V *rtype) bool { continue } } - if i++; i >= len(tmethods) { + if i++; i >= len(t.methods) { return true } } @@ -1661,7 +1658,7 @@ func haveIdenticalUnderlyingType(T, V *rtype, cmpTags bool) bool { case Interface: t := (*interfaceType)(unsafe.Pointer(T)) v := (*interfaceType)(unsafe.Pointer(V)) - if t.isEmpty() && v.isEmpty() { + if len(t.methods) == 0 && len(v.methods) == 0 { return true } // Might have the same methods but still @@ -2445,7 +2442,7 @@ func StructOf(fields []StructField) Type { switch f.typ.Kind() { case Interface: ift := (*interfaceType)(unsafe.Pointer(ft)) - for im, m := range ift.methods() { + for im, m := range ift.methods { if ift.nameOff(m.name).pkgPath() != "" { // TODO(sbinet). Issue 15924. panic("reflect: embedded interface with unexported method(s) not implemented") @@ -3152,11 +3149,3 @@ func addTypeBits(bv *bitVector, offset uintptr, t *rtype) { } } } - -func isEmptyIface(rt *rtype) bool { - if rt.Kind() != Interface { - return false - } - tt := (*interfaceType)(unsafe.Pointer(rt)) - return len(tt.methods()) == 0 -} diff --git a/src/reflect/value.go b/src/reflect/value.go index 24eab6a2c6..bf926a7453 100644 --- a/src/reflect/value.go +++ b/src/reflect/value.go @@ -636,11 +636,10 @@ func methodReceiver(op string, v Value, methodIndex int) (rcvrtype *rtype, t *fu i := methodIndex if v.typ.Kind() == Interface { tt := (*interfaceType)(unsafe.Pointer(v.typ)) - ttmethods := tt.methods() - if uint(i) >= uint(len(ttmethods)) { + if uint(i) >= uint(len(tt.methods)) { panic("reflect: internal error: invalid method index") } - m := &ttmethods[i] + m := &tt.methods[i] if !tt.nameOff(m.name).isExported() { panic("reflect: " + op + " of unexported method") } @@ -814,7 +813,7 @@ func (v Value) Elem() Value { switch k { case Interface: var eface interface{} - if isEmptyIface(v.typ) { + if v.typ.NumMethod() == 0 { eface = *(*interface{})(v.ptr) } else { eface = (interface{})(*(*interface { @@ -1035,7 +1034,7 @@ func valueInterface(v Value, safe bool) interface{} { // Special case: return the element inside the interface. // Empty interface has one layout, all interfaces with // methods have a second layout. - if isEmptyIface(v.typ) { + if v.NumMethod() == 0 { return *(*interface{})(v.ptr) } return *(*interface { @@ -1919,11 +1918,10 @@ func (v Value) Type() Type { if v.typ.Kind() == Interface { // Method on interface. tt := (*interfaceType)(unsafe.Pointer(v.typ)) - ttmethods := tt.methods() - if uint(i) >= uint(len(ttmethods)) { + if uint(i) >= uint(len(tt.methods)) { panic("reflect: internal error: invalid method index") } - m := &ttmethods[i] + m := &tt.methods[i] return v.typ.typeOff(m.typ) } // Method on concrete type. @@ -2441,7 +2439,7 @@ func (v Value) assignTo(context string, dst *rtype, target unsafe.Pointer) Value return Value{dst, nil, flag(Interface)} } x := valueInterface(v, false) - if isEmptyIface(dst) { + if dst.NumMethod() == 0 { *(*interface{})(target) = x } else { ifaceE2I(dst, x, target) @@ -2730,11 +2728,10 @@ func cvtDirect(v Value, typ Type) Value { func cvtT2I(v Value, typ Type) Value { target := unsafe_New(typ.common()) x := valueInterface(v, false) - rt := typ.(*rtype) - if isEmptyIface(rt) { + if typ.NumMethod() == 0 { *(*interface{})(target) = x } else { - ifaceE2I(rt, x, target) + ifaceE2I(typ.(*rtype), x, target) } return Value{typ.common(), target, v.flag.ro() | flagIndir | flag(Interface)} } diff --git a/src/runtime/alg.go b/src/runtime/alg.go index 2ec3fc3658..1b3bf1180d 100644 --- a/src/runtime/alg.go +++ b/src/runtime/alg.go @@ -166,7 +166,7 @@ func typehash(t *_type, p unsafe.Pointer, h uintptr) uintptr { return strhash(p, h) case kindInterface: i := (*interfacetype)(unsafe.Pointer(t)) - if i.isEmpty() { + if len(i.mhdr) == 0 { return nilinterhash(p, h) } return interhash(p, h) diff --git a/src/runtime/iface.go b/src/runtime/iface.go index f8b7d429a3..0504b89363 100644 --- a/src/runtime/iface.go +++ b/src/runtime/iface.go @@ -31,17 +31,16 @@ func itabHashFunc(inter *interfacetype, typ *_type) uintptr { } func getitab(inter *interfacetype, typ *_type, canfail bool) *itab { - if inter.isEmpty() { + if len(inter.mhdr) == 0 { throw("internal error - misuse of itab") } - imethods := inter.methods() // easy case if typ.tflag&tflagUncommon == 0 { if canfail { return nil } - name := inter.typ.nameOff(imethods[0].name) + name := inter.typ.nameOff(inter.mhdr[0].name) panic(&TypeAssertionError{nil, typ, &inter.typ, name.name()}) } @@ -64,7 +63,7 @@ func getitab(inter *interfacetype, typ *_type, canfail bool) *itab { } // Entry doesn't exist yet. Make a new entry & add it. - m = (*itab)(persistentalloc(unsafe.Sizeof(itab{})+uintptr(len(imethods)-1)*sys.PtrSize, 0, &memstats.other_sys)) + m = (*itab)(persistentalloc(unsafe.Sizeof(itab{})+uintptr(len(inter.mhdr)-1)*sys.PtrSize, 0, &memstats.other_sys)) m.inter = inter m._type = typ // The hash is used in type switches. However, compiler statically generates itab's @@ -198,8 +197,7 @@ func (m *itab) init() string { // and interface names are unique, // so can iterate over both in lock step; // the loop is O(ni+nt) not O(ni*nt). - imethods := inter.methods() - ni := len(imethods) + ni := len(inter.mhdr) nt := int(x.mcount) xmhdr := (*[1 << 16]method)(add(unsafe.Pointer(x), uintptr(x.moff)))[:nt:nt] j := 0 @@ -207,7 +205,7 @@ func (m *itab) init() string { var fun0 unsafe.Pointer imethods: for k := 0; k < ni; k++ { - i := &imethods[k] + i := &inter.mhdr[k] itype := inter.typ.typeOff(i.ityp) name := inter.typ.nameOff(i.name) iname := name.name() diff --git a/src/runtime/mfinal.go b/src/runtime/mfinal.go index 6ec5133be0..f4dbd77252 100644 --- a/src/runtime/mfinal.go +++ b/src/runtime/mfinal.go @@ -210,7 +210,7 @@ func runfinq() { // set up with empty interface (*eface)(frame)._type = &f.ot.typ (*eface)(frame).data = f.arg - if !ityp.isEmpty() { + if len(ityp.mhdr) != 0 { // convert to interface with methods // this conversion is guaranteed to succeed - we checked in SetFinalizer *(*iface)(frame) = assertE2I(ityp, *(*eface)(frame)) @@ -394,7 +394,7 @@ func SetFinalizer(obj interface{}, finalizer interface{}) { } case fint.kind&kindMask == kindInterface: ityp := (*interfacetype)(unsafe.Pointer(fint)) - if ityp.isEmpty() { + if len(ityp.mhdr) == 0 { // ok - satisfies empty interface goto okarg } diff --git a/src/runtime/type.go b/src/runtime/type.go index 36492619e1..81455f3532 100644 --- a/src/runtime/type.go +++ b/src/runtime/type.go @@ -366,19 +366,7 @@ type imethod struct { type interfacetype struct { typ _type pkgpath name - // expMethods contains all interface methods. - // - // - len(expMethods) returns number of exported methods. - // - cap(expMethods) returns all interface methods, including both exported/non-exported methods. - expMethods []imethod -} - -func (it *interfacetype) methods() []imethod { - return it.expMethods[:cap(it.expMethods)] -} - -func (it *interfacetype) isEmpty() bool { - return cap(it.expMethods) == 0 + mhdr []imethod } type maptype struct { @@ -676,15 +664,13 @@ func typesEqual(t, v *_type, seen map[_typePair]struct{}) bool { if it.pkgpath.name() != iv.pkgpath.name() { return false } - itmethods := it.methods() - ivmethods := iv.methods() - if len(itmethods) != len(ivmethods) { + if len(it.mhdr) != len(iv.mhdr) { return false } - for i := range itmethods { - tm := &itmethods[i] - vm := &ivmethods[i] - // Note the expMethods array can be relocated from + for i := range it.mhdr { + tm := &it.mhdr[i] + vm := &iv.mhdr[i] + // Note the mhdr array can be relocated from // another module. See #17724. tname := resolveNameOff(unsafe.Pointer(tm), tm.name) vname := resolveNameOff(unsafe.Pointer(vm), vm.name) -- cgit v1.3 From f0ff6d4a67ec9a956aa655d487543da034cf576b Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Wed, 25 Nov 2020 13:47:42 -0800 Subject: reflect: fix Value.Convert for int-to-string conversions (regression) The bug was introduced by https://golang.org/cl/220844. Updates #42792. Fixes #42835. Change-Id: I03065c7526488aded35ef2f800b7162e1606877a Reviewed-on: https://go-review.googlesource.com/c/go/+/273326 Trust: Robert Griesemer Run-TryBot: Robert Griesemer TryBot-Result: Go Bot Reviewed-by: Ian Lance Taylor --- src/reflect/all_test.go | 3 +++ src/reflect/value.go | 12 ++++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) (limited to 'src/reflect/value.go') diff --git a/src/reflect/all_test.go b/src/reflect/all_test.go index a12712d254..b01158635f 100644 --- a/src/reflect/all_test.go +++ b/src/reflect/all_test.go @@ -4007,9 +4007,12 @@ var convertTests = []struct { {V(int16(-3)), V(string("\uFFFD"))}, {V(int32(-4)), V(string("\uFFFD"))}, {V(int64(-5)), V(string("\uFFFD"))}, + {V(int64(-1 << 32)), V(string("\uFFFD"))}, + {V(int64(1 << 32)), V(string("\uFFFD"))}, {V(uint(0x110001)), V(string("\uFFFD"))}, {V(uint32(0x110002)), V(string("\uFFFD"))}, {V(uint64(0x110003)), V(string("\uFFFD"))}, + {V(uint64(1 << 32)), V(string("\uFFFD"))}, {V(uintptr(0x110004)), V(string("\uFFFD"))}, // named string diff --git a/src/reflect/value.go b/src/reflect/value.go index bf926a7453..1f185b52e4 100644 --- a/src/reflect/value.go +++ b/src/reflect/value.go @@ -2681,12 +2681,20 @@ func cvtComplex(v Value, t Type) Value { // convertOp: intXX -> string func cvtIntString(v Value, t Type) Value { - return makeString(v.flag.ro(), string(rune(v.Int())), t) + s := "\uFFFD" + if x := v.Int(); int64(rune(x)) == x { + s = string(rune(x)) + } + return makeString(v.flag.ro(), s, t) } // convertOp: uintXX -> string func cvtUintString(v Value, t Type) Value { - return makeString(v.flag.ro(), string(rune(v.Uint())), t) + s := "\uFFFD" + if x := v.Uint(); uint64(rune(x)) == x { + s = string(rune(x)) + } + return makeString(v.flag.ro(), s, t) } // convertOp: []byte -> string -- cgit v1.3