reflect: keep RO flags unchanged 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 keeping 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.

Change-Id: I79e086628c0fb6569a50ce63f3b95916f997eda1
GitHub-Last-Rev: 78e280e6d06865661b5835def74c252c94a92800
GitHub-Pull-Request: golang/go#32787
Reviewed-on: https://go-review.googlesource.com/c/go/+/183937
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/reflect/all_test.go b/src/reflect/all_test.go
index b95f74354f9a1fcd03328d609bb701352fb4bf4e..e87d1d27cd0683f089f266a2ce83dd2129f2f4a4 100644 (file)
--- 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)

diff --git a/src/reflect/value.go b/src/reflect/value.go
index abddd1774f753cd81d7137b3378f1c801a66f62f..8ce495a33bf4d7c09e8134ebd7983c72540d1164 100644 (file)
--- a/src/reflect/value.go
+++ b/src/reflect/value.go
@@ -269,7 +269,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)}
+       // 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)}
 }
 
 // Bool returns v's underlying value.