reflect: derive correct Value method in panic messages

methodName was brittle in that it assumed exactly where
in the call stack the exported Value method is.
This broke since recent inlining optimizations changed
exactly which frame the exported method was located.
Instead, iterate through a sufficient number of stack entries
and dynamically determined the exported Value method name.

This is more maintainable, but slightly slower.
The slowdown is acceptable since panics are not the common case.

Change-Id: I9fc939627007d7bae004b4969516ad44be09c270
Reviewed-on: https://go-review.googlesource.com/c/go/+/403494
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>

diff --git a/src/reflect/all_test.go b/src/reflect/all_test.go
index a625a1d2f11a185594a9a51e139eef945863b5f0..febbd5f5a78491e056aa1d013255a1d61c427c93 100644 (file)
--- a/src/reflect/all_test.go
+++ b/src/reflect/all_test.go
@@ -3977,6 +3977,51 @@ func TestCallPanic(t *testing.T) {
        badCall(func() { call(v.Field(7).Field(1).Elem().Method(0)) }) // .namedT2.t0.W
 }
 
+func TestValuePanic(t *testing.T) {
+       vo := ValueOf
+       shouldPanic("reflect.Value.Addr of unaddressable value", func() { vo(0).Addr() })
+       shouldPanic("call of reflect.Value.Bool on float64 Value", func() { vo(0.0).Bool() })
+       shouldPanic("call of reflect.Value.Bytes on string Value", func() { vo("").Bytes() })
+       shouldPanic("call of reflect.Value.Call on bool Value", func() { vo(true).Call(nil) })
+       shouldPanic("call of reflect.Value.CallSlice on int Value", func() { vo(0).CallSlice(nil) })
+       shouldPanic("call of reflect.Value.Close on string Value", func() { vo("").Close() })
+       shouldPanic("call of reflect.Value.Complex on float64 Value", func() { vo(0.0).Complex() })
+       shouldPanic("call of reflect.Value.Elem on bool Value", func() { vo(false).Elem() })
+       shouldPanic("call of reflect.Value.Field on int Value", func() { vo(0).Field(0) })
+       shouldPanic("call of reflect.Value.Float on string Value", func() { vo("").Float() })
+       shouldPanic("call of reflect.Value.Index on float64 Value", func() { vo(0.0).Index(0) })
+       shouldPanic("call of reflect.Value.Int on bool Value", func() { vo(false).Int() })
+       shouldPanic("call of reflect.Value.IsNil on int Value", func() { vo(0).IsNil() })
+       shouldPanic("call of reflect.Value.Len on bool Value", func() { vo(false).Len() })
+       shouldPanic("call of reflect.Value.MapIndex on float64 Value", func() { vo(0.0).MapIndex(vo(0.0)) })
+       shouldPanic("call of reflect.Value.MapKeys on string Value", func() { vo("").MapKeys() })
+       shouldPanic("call of reflect.Value.MapRange on int Value", func() { vo(0).MapRange() })
+       shouldPanic("call of reflect.Value.Method on zero Value", func() { vo(nil).Method(0) })
+       shouldPanic("call of reflect.Value.NumField on string Value", func() { vo("").NumField() })
+       shouldPanic("call of reflect.Value.NumMethod on zero Value", func() { vo(nil).NumMethod() })
+       shouldPanic("call of reflect.Value.OverflowComplex on float64 Value", func() { vo(float64(0)).OverflowComplex(0) })
+       shouldPanic("call of reflect.Value.OverflowFloat on int64 Value", func() { vo(int64(0)).OverflowFloat(0) })
+       shouldPanic("call of reflect.Value.OverflowInt on uint64 Value", func() { vo(uint64(0)).OverflowInt(0) })
+       shouldPanic("call of reflect.Value.OverflowUint on complex64 Value", func() { vo(complex64(0)).OverflowUint(0) })
+       shouldPanic("call of reflect.Value.Recv on string Value", func() { vo("").Recv() })
+       shouldPanic("call of reflect.Value.Send on bool Value", func() { vo(true).Send(vo(true)) })
+       shouldPanic("value of type string is not assignable to type bool", func() { vo(new(bool)).Elem().Set(vo("")) })
+       shouldPanic("call of reflect.Value.SetBool on string Value", func() { vo(new(string)).Elem().SetBool(false) })
+       shouldPanic("reflect.Value.SetBytes using unaddressable value", func() { vo("").SetBytes(nil) })
+       shouldPanic("call of reflect.Value.SetCap on string Value", func() { vo(new(string)).Elem().SetCap(0) })
+       shouldPanic("call of reflect.Value.SetComplex on string Value", func() { vo(new(string)).Elem().SetComplex(0) })
+       shouldPanic("call of reflect.Value.SetFloat on string Value", func() { vo(new(string)).Elem().SetFloat(0) })
+       shouldPanic("call of reflect.Value.SetInt on string Value", func() { vo(new(string)).Elem().SetInt(0) })
+       shouldPanic("call of reflect.Value.SetLen on string Value", func() { vo(new(string)).Elem().SetLen(0) })
+       shouldPanic("call of reflect.Value.SetString on int Value", func() { vo(new(int)).Elem().SetString("") })
+       shouldPanic("reflect.Value.SetUint using unaddressable value", func() { vo(0.0).SetUint(0) })
+       shouldPanic("call of reflect.Value.Slice on bool Value", func() { vo(true).Slice(1, 2) })
+       shouldPanic("call of reflect.Value.Slice3 on int Value", func() { vo(0).Slice3(1, 2, 3) })
+       shouldPanic("call of reflect.Value.TryRecv on bool Value", func() { vo(true).TryRecv() })
+       shouldPanic("call of reflect.Value.TrySend on string Value", func() { vo("").TrySend(vo("")) })
+       shouldPanic("call of reflect.Value.Uint on float64 Value", func() { vo(0.0).Uint() })
+}
+
 func shouldPanic(expect string, f func()) {
        defer func() {
                r := recover()

diff --git a/src/reflect/value.go b/src/reflect/value.go
index 76ae5f8c816206a6b30cd4d6cdeb07dbed63aa04..d68f7ad2e642b9616f541faa14ba760cb0401c0a 100644 (file)
--- a/src/reflect/value.go
+++ b/src/reflect/value.go
@@ -170,26 +170,24 @@ func (e *ValueError) Error() string {
        return "reflect: call of " + e.Method + " on " + e.Kind.String() + " Value"
 }
 
-// methodName returns the name of the calling method,
-// assumed to be two stack frames above.
-func methodName() string {
-       pc, _, _, _ := runtime.Caller(2)
-       f := runtime.FuncForPC(pc)
-       if f == nil {
-               return "unknown method"
-       }
-       return f.Name()
-}
-
-// methodNameSkip is like methodName, but skips another stack frame.
-// This is a separate function so that reflect.flag.mustBe will be inlined.
-func methodNameSkip() string {
-       pc, _, _, _ := runtime.Caller(3)
-       f := runtime.FuncForPC(pc)
-       if f == nil {
-               return "unknown method"
+// valueMethodName returns the name of the exported calling method on Value.
+func valueMethodName() string {
+       var pc [5]uintptr
+       n := runtime.Callers(1, pc[:])
+       frames := runtime.CallersFrames(pc[:n])
+       var frame runtime.Frame
+       for more := true; more; {
+               const prefix = "reflect.Value."
+               frame, more = frames.Next()
+               name := frame.Function
+               if len(name) > len(prefix) && name[:len(prefix)] == prefix {
+                       methodName := name[len(prefix):]
+                       if len(methodName) > 0 && 'A' <= methodName[0] && methodName[0] <= 'Z' {
+                               return name
+                       }
+               }
        }
-       return f.Name()
+       return "unknown method"
 }
 
 // emptyInterface is the header for an interface{} value.
@@ -220,7 +218,7 @@ type nonEmptyInterface struct {
 func (f flag) mustBe(expected Kind) {
        // TODO(mvdan): use f.kind() again once mid-stack inlining gets better
        if Kind(f&flagKindMask) != expected {
-               panic(&ValueError{methodName(), f.kind()})
+               panic(&ValueError{valueMethodName(), f.kind()})
        }
 }
 
@@ -234,10 +232,10 @@ func (f flag) mustBeExported() {
 
 func (f flag) mustBeExportedSlow() {
        if f == 0 {
-               panic(&ValueError{methodNameSkip(), Invalid})
+               panic(&ValueError{valueMethodName(), Invalid})
        }
        if f&flagRO != 0 {
-               panic("reflect: " + methodNameSkip() + " using value obtained using unexported field")
+               panic("reflect: " + valueMethodName() + " using value obtained using unexported field")
        }
 }
 
@@ -252,14 +250,14 @@ func (f flag) mustBeAssignable() {
 
 func (f flag) mustBeAssignableSlow() {
        if f == 0 {
-               panic(&ValueError{methodNameSkip(), Invalid})
+               panic(&ValueError{valueMethodName(), Invalid})
        }
        // Assignable if addressable and not read-only.
        if f&flagRO != 0 {
-               panic("reflect: " + methodNameSkip() + " using value obtained using unexported field")
+               panic("reflect: " + valueMethodName() + " using value obtained using unexported field")
        }
        if f&flagAddr == 0 {
-               panic("reflect: " + methodNameSkip() + " using unaddressable value")
+               panic("reflect: " + valueMethodName() + " using unaddressable value")
        }
 }