From 48e8569b20b0105f17adbaef8588138600b21a5f Mon Sep 17 00:00:00 2001 From: Kezhu Wang Date: Tue, 25 Jun 2019 21:20:34 +0800 Subject: [PATCH 1/3] reflect: error if Value.Addr().Elem.CanSet() diff from Value.CanSet() --- src/reflect/all_test.go | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/src/reflect/all_test.go b/src/reflect/all_test.go index 0dbf4c5e877e50..da03b4d998e57e 100644 --- a/src/reflect/all_test.go +++ b/src/reflect/all_test.go @@ -350,6 +350,7 @@ func TestCanSetField(t *testing.T) { } type testCase struct { + // -1 means Addr().Elem() of current value index []int canSet bool } @@ -360,17 +361,33 @@ func TestCanSetField(t *testing.T) { val: ValueOf(&S1{}), cases: []testCase{ {[]int{0}, false}, + {[]int{0, -1}, false}, {[]int{0, 0}, false}, + {[]int{0, 0, -1}, false}, + {[]int{0, -1, 0}, false}, + {[]int{0, -1, 0, -1}, false}, {[]int{0, 1}, true}, + {[]int{0, 1, -1}, true}, + {[]int{0, -1, 1}, true}, + {[]int{0, -1, 1, -1}, true}, {[]int{1}, false}, + {[]int{1, -1}, false}, {[]int{2}, true}, + {[]int{2, -1}, true}, }, }, { val: ValueOf(&S2{embed: &embed{}}), cases: []testCase{ {[]int{0}, false}, + {[]int{0, -1}, false}, {[]int{0, 0}, false}, + {[]int{0, 0, -1}, false}, + {[]int{0, -1, 0}, false}, + {[]int{0, -1, 0, -1}, false}, {[]int{0, 1}, true}, + {[]int{0, 1, -1}, true}, + {[]int{0, -1, 1}, true}, + {[]int{0, -1, 1, -1}, true}, {[]int{1}, false}, {[]int{2}, true}, }, @@ -378,8 +395,15 @@ func TestCanSetField(t *testing.T) { val: ValueOf(&S3{}), cases: []testCase{ {[]int{0}, true}, + {[]int{0, -1}, true}, {[]int{0, 0}, false}, + {[]int{0, 0, -1}, false}, + {[]int{0, -1, 0}, false}, + {[]int{0, -1, 0, -1}, false}, {[]int{0, 1}, true}, + {[]int{0, 1, -1}, true}, + {[]int{0, -1, 1}, true}, + {[]int{0, -1, 1, -1}, true}, {[]int{1}, false}, {[]int{2}, true}, }, @@ -387,8 +411,15 @@ func TestCanSetField(t *testing.T) { val: ValueOf(&S4{Embed: &Embed{}}), cases: []testCase{ {[]int{0}, true}, + {[]int{0, -1}, true}, {[]int{0, 0}, false}, + {[]int{0, 0, -1}, false}, + {[]int{0, -1, 0}, false}, + {[]int{0, -1, 0, -1}, false}, {[]int{0, 1}, true}, + {[]int{0, 1, -1}, true}, + {[]int{0, -1, 1}, true}, + {[]int{0, -1, 1, -1}, true}, {[]int{1}, false}, {[]int{2}, true}, }, @@ -402,7 +433,11 @@ func TestCanSetField(t *testing.T) { if f.Kind() == Ptr { f = f.Elem() } - f = f.Field(i) + if i == -1 { + f = f.Addr().Elem() + } else { + f = f.Field(i) + } } if got := f.CanSet(); got != tc.canSet { t.Errorf("CanSet() = %v, want %v", got, tc.canSet) From 808aaeaddbe3b89f325fa9f90070aeb770d6af26 Mon Sep 17 00:00:00 2001 From: Kezhu Wang Date: Tue, 25 Jun 2019 22:02:41 +0800 Subject: [PATCH 2/3] reflect: inherit all bits of flagRO in Value.Addr() Currently, Value.Addr() collapses flagRO, which is a combination of flagEmbedRO and flagStickyRO, to flagStickyRO. This causes exported fields of unexported anonymous field from Value.Addr().Elem() read only. This commit fix this by inheriting all bits of flagRO from origin value in Value.Addr(). This should be safe due to following reasons: * Result of Value.Addr() is not CanSet() because of it is not CanAddr() but not flagRO. * Addr().Elem() get same flagRO as origin, so it should behave same as origin in CanSet(). Fixes #32772. --- src/reflect/value.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/reflect/value.go b/src/reflect/value.go index 218b4d25cc223f..198ada50caca2d 100644 --- a/src/reflect/value.go +++ b/src/reflect/value.go @@ -257,7 +257,10 @@ func (v Value) Addr() Value { if v.flag&flagAddr == 0 { panic("reflect.Value.Addr of unaddressable value") } - return Value{v.typ.ptrTo(), v.ptr, v.flag.ro() | flag(Ptr)} + // Inherits possible read only flags from this value, + // so that later Elem() will restore equivalent value back. + fl := v.flag & flagRO + return Value{v.typ.ptrTo(), v.ptr, fl | flag(Ptr)} } // Bool returns v's underlying value. From 78e280e6d06865661b5835def74c252c94a92800 Mon Sep 17 00:00:00 2001 From: Kezhu Wang Date: Mon, 4 May 2020 10:27:46 +0800 Subject: [PATCH 3/3] fixup: Change code comment according to code reviewer request --- src/reflect/value.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/reflect/value.go b/src/reflect/value.go index 198ada50caca2d..142f71b4771624 100644 --- a/src/reflect/value.go +++ b/src/reflect/value.go @@ -257,8 +257,8 @@ func (v Value) Addr() Value { if v.flag&flagAddr == 0 { panic("reflect.Value.Addr of unaddressable value") } - // Inherits possible read only flags from this value, - // so that later Elem() will restore equivalent value back. + // Preserve flagRO instead of using v.flag.ro() so that + // v.Addr().Elem() is equivalent to v (#32772) fl := v.flag & flagRO return Value{v.typ.ptrTo(), v.ptr, fl | flag(Ptr)} }