From 0e73497a4ba97048222ae262f7b5a40c281af0b6 Mon Sep 17 00:00:00 2001 From: Todd Wang Date: Wed, 21 Aug 2013 14:41:55 +1000 Subject: [PATCH] reflect: Fix Convert to add indir bit when the value is actually a pointer. An example that triggers the bad behavior on a 64bit machine http://play.golang.org/p/GrNFakAYLN rv1 := reflect.ValueOf(complex128(0)) rt := rv1.Type() rv2 := rv1.Convert(rt) rv3 := reflect.New(rt).Elem() rv3.Set(rv2) Running the code fails with the following: panic: reflect: internal error: storeIword of 16-byte value I've tested on a 64bit machine and verified this fixes the panic. I haven't tested on a 32bit machine so I haven't verified the other cases, but they follow logically. R=golang-dev, r, iant CC=golang-dev https://golang.org/cl/12805045 --- src/pkg/reflect/all_test.go | 32 +++++++++++++++++++++++++------- src/pkg/reflect/value.go | 6 +++--- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/pkg/reflect/all_test.go b/src/pkg/reflect/all_test.go index c169c3594d..23532bdee1 100644 --- a/src/pkg/reflect/all_test.go +++ b/src/pkg/reflect/all_test.go @@ -169,16 +169,20 @@ var typeTests = []pair{ } var valueTests = []pair{ + {new(int), "132"}, {new(int8), "8"}, {new(int16), "16"}, {new(int32), "32"}, {new(int64), "64"}, + {new(uint), "132"}, {new(uint8), "8"}, {new(uint16), "16"}, {new(uint32), "32"}, {new(uint64), "64"}, {new(float32), "256.25"}, {new(float64), "512.125"}, + {new(complex64), "532.125+10i"}, + {new(complex128), "564.25+1i"}, {new(string), "stringy cheese"}, {new(bool), "true"}, {new(*int8), "*int8(0)"}, @@ -2975,17 +2979,28 @@ func TestConvert(t *testing.T) { all[t2] = true canConvert[[2]Type{t1, t2}] = true + // vout1 represents the in value converted to the in type. v1 := tt.in vout1 := v1.Convert(t1) out1 := vout1.Interface() if vout1.Type() != tt.in.Type() || !DeepEqual(out1, tt.in.Interface()) { - t.Errorf("ValueOf(%T(%v)).Convert(%s) = %T(%v), want %T(%v)", tt.in.Interface(), tt.in.Interface(), t1, out1, out1, tt.in.Interface(), tt.in.Interface()) + t.Errorf("ValueOf(%T(%[1]v)).Convert(%s) = %T(%[3]v), want %T(%[4]v)", tt.in.Interface(), t1, out1, tt.in.Interface()) } - vout := v1.Convert(t2) - out := vout.Interface() - if vout.Type() != tt.out.Type() || !DeepEqual(out, tt.out.Interface()) { - t.Errorf("ValueOf(%T(%v)).Convert(%s) = %T(%v), want %T(%v)", tt.in.Interface(), tt.in.Interface(), t2, out, out, tt.out.Interface(), tt.out.Interface()) + // vout2 represents the in value converted to the out type. + vout2 := v1.Convert(t2) + out2 := vout2.Interface() + if vout2.Type() != tt.out.Type() || !DeepEqual(out2, tt.out.Interface()) { + t.Errorf("ValueOf(%T(%[1]v)).Convert(%s) = %T(%[3]v), want %T(%[4]v)", tt.in.Interface(), t2, out2, tt.out.Interface()) + } + + // vout3 represents a new value of the out type, set to vout2. This makes + // sure the converted value vout2 is really usable as a regular value. + vout3 := New(t2).Elem() + vout3.Set(vout2) + out3 := vout3.Interface() + if vout3.Type() != tt.out.Type() || !DeepEqual(out3, tt.out.Interface()) { + t.Errorf("Set(ValueOf(%T(%[1]v)).Convert(%s)) = %T(%[3]v), want %T(%[4]v)", tt.in.Interface(), t2, out3, tt.out.Interface()) } if IsRO(v1) { @@ -2994,8 +3009,11 @@ func TestConvert(t *testing.T) { if IsRO(vout1) { t.Errorf("self-conversion output %v is RO, should not be", vout1) } - if IsRO(vout) { - t.Errorf("conversion output %v is RO, should not be", vout) + if IsRO(vout2) { + t.Errorf("conversion output %v is RO, should not be", vout2) + } + if IsRO(vout3) { + t.Errorf("set(conversion output) %v is RO, should not be", vout3) } if !IsRO(MakeRO(v1).Convert(t1)) { t.Errorf("RO self-conversion output %v is not RO, should be", v1) diff --git a/src/pkg/reflect/value.go b/src/pkg/reflect/value.go index 112e17dff4..dbecc59da8 100644 --- a/src/pkg/reflect/value.go +++ b/src/pkg/reflect/value.go @@ -2298,7 +2298,7 @@ func makeInt(f flag, bits uint64, t Type) Value { // Assume ptrSize >= 4, so this must be uint64. ptr := unsafe_New(typ) *(*uint64)(unsafe.Pointer(ptr)) = bits - return Value{typ, ptr, f | flag(typ.Kind())<= 4, so this must be float64. ptr := unsafe_New(typ) *(*float64)(unsafe.Pointer(ptr)) = v - return Value{typ, ptr, f | flag(typ.Kind())<