From 8b389eb2be2d66563669c74f6515b5f537542ce9 Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Mon, 18 Apr 2022 10:05:24 -0700 Subject: [PATCH] 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 Reviewed-by: Russ Cox Reviewed-by: Ian Lance Taylor Run-TryBot: Russ Cox Auto-Submit: Russ Cox --- src/reflect/all_test.go | 45 ++++++++++++++++++++++++++++++++++++++ src/reflect/value.go | 48 ++++++++++++++++++++--------------------- 2 files changed, 68 insertions(+), 25 deletions(-) diff --git a/src/reflect/all_test.go b/src/reflect/all_test.go index a625a1d2f1..febbd5f5a7 100644 --- 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 76ae5f8c81..d68f7ad2e6 100644 --- 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") } } -- 2.48.1