From: Kezhu Wang <kezhuw@gmail.com>
Date: Mon, 4 May 2020 02:28:39 +0000 (+0000)
Subject: reflect: keep RO flags unchanged in Value.Addr
X-Git-Tag: go1.15beta1~237
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=4c003f6b780b471afbf032438eb6c7519458855b;p=gostls13.git

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 b95f74354f..e87d1d27cd 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)
diff --git a/src/reflect/value.go b/src/reflect/value.go
index abddd1774f..8ce495a33b 100644
--- 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.