From 9d168895ede3d7a03e3a37839201a4b000dfbcf6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Uwe=20Kr=C3=BCger?=
 <45282134+UweKrueger@users.noreply.github.com>
Date: Mon, 15 Mar 2021 14:54:06 +0100
Subject: [PATCH] checker: check write access to shared elements of `struct`
 and `array` (#9314)

---
 vlib/v/checker/check_types.v                 |  5 ++-
 vlib/v/checker/checker.v                     | 24 ++++++++++
 vlib/v/checker/tests/shared_element_lock.out | 27 ++++++++++++
 vlib/v/checker/tests/shared_element_lock.vv  | 46 ++++++++++++++++++++
 vlib/v/tests/shared_elem_test.v              | 11 +++--
 5 files changed, 108 insertions(+), 5 deletions(-)
 create mode 100644 vlib/v/checker/tests/shared_element_lock.out
 create mode 100644 vlib/v/checker/tests/shared_element_lock.vv

diff --git a/vlib/v/checker/check_types.v b/vlib/v/checker/check_types.v
index 1a9ac09391..36170201d0 100644
--- a/vlib/v/checker/check_types.v
+++ b/vlib/v/checker/check_types.v
@@ -349,6 +349,7 @@ pub fn (c &Checker) get_default_fmt(ftyp table.Type, typ table.Type) byte {
 }
 
 pub fn (mut c Checker) fail_if_unreadable(expr ast.Expr, typ table.Type, what string) {
+	mut pos := token.Position{}
 	match expr {
 		ast.Ident {
 			if typ.has_flag(.shared_f) {
@@ -361,16 +362,18 @@ pub fn (mut c Checker) fail_if_unreadable(expr ast.Expr, typ table.Type, what st
 			return
 		}
 		ast.SelectorExpr {
+			pos = expr.pos
 			c.fail_if_unreadable(expr.expr, expr.expr_type, what)
 		}
 		ast.IndexExpr {
+			pos = expr.left.position().extend(expr.pos)
 			c.fail_if_unreadable(expr.left, expr.left_type, what)
 		}
 		else {}
 	}
 	if typ.has_flag(.shared_f) {
 		c.error('you have to create a handle and `rlock` it to use a `shared` element as non-mut $what',
-			expr.position())
+			pos)
 	}
 }
 
diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v
index 48f547688f..5f6d63cd9a 100644
--- a/vlib/v/checker/checker.v
+++ b/vlib/v/checker/checker.v
@@ -1084,6 +1084,25 @@ fn (mut c Checker) fail_if_immutable(expr ast.Expr) (string, token.Position) {
 			}
 		}
 		ast.IndexExpr {
+			left_sym := c.table.get_type_symbol(expr.left_type)
+			mut elem_type := table.Type(0)
+			mut kind := ''
+			match left_sym.info {
+				table.Array {
+					elem_type, kind = left_sym.info.elem_type, 'array'
+				}
+				table.ArrayFixed {
+					elem_type, kind = left_sym.info.elem_type, 'fixed array'
+				}
+				table.Map {
+					elem_type, kind = left_sym.info.value_type, 'map'
+				}
+				else {}
+			}
+			if elem_type.has_flag(.shared_f) {
+				c.error('you have to create a handle and `lock` it to modify `shared` $kind element',
+					expr.left.position().extend(expr.pos))
+			}
 			to_lock, pos = c.fail_if_immutable(expr.left)
 		}
 		ast.ParExpr {
@@ -1128,6 +1147,11 @@ fn (mut c Checker) fail_if_immutable(expr ast.Expr) (string, token.Position) {
 						c.error('field `$expr.field_name` of struct `$type_str` is immutable',
 							expr.pos)
 					}
+					if field_info.typ.has_flag(.shared_f) {
+						type_str := c.table.type_to_str(expr.expr_type)
+						c.error('you have to create a handle and `lock` it to modify `shared` field `$expr.field_name` of struct `$type_str`',
+							expr.pos)
+					}
 					to_lock, pos = c.fail_if_immutable(expr.expr)
 					if to_lock != '' {
 						// No automatic lock for struct access
diff --git a/vlib/v/checker/tests/shared_element_lock.out b/vlib/v/checker/tests/shared_element_lock.out
new file mode 100644
index 0000000000..81b15dde70
--- /dev/null
+++ b/vlib/v/checker/tests/shared_element_lock.out
@@ -0,0 +1,27 @@
+vlib/v/checker/tests/shared_element_lock.vv:36:5: error: you have to create a handle and `lock` it to modify `shared` field `pe` of struct `Programmer`
+   34 |         }
+   35 |     }
+   36 |     pr.pe.color = 3
+      |        ~~
+   37 |     shared y := pr.pe
+   38 |     rlock y {
+vlib/v/checker/tests/shared_element_lock.vv:42:2: error: `g` is `shared` and needs explicit lock for `v.ast.SelectorExpr`
+   40 |     }
+   41 |     shared g := Pro{}
+   42 |     g.pers.age = 42
+      |     ^
+   43 |     mut h := []shared Pro{len: 3}
+   44 |     h[2].pers.age = 42
+vlib/v/checker/tests/shared_element_lock.vv:44:2: error: you have to create a handle and `lock` it to modify `shared` array element
+   42 |     g.pers.age = 42
+   43 |     mut h := []shared Pro{len: 3}
+   44 |     h[2].pers.age = 42
+      |     ~~~~
+   45 |     println(h[2].pers.age)
+   46 | }
+vlib/v/checker/tests/shared_element_lock.vv:45:10: error: you have to create a handle and `rlock` it to use a `shared` element as non-mut argument to print
+   43 |     mut h := []shared Pro{len: 3}
+   44 |     h[2].pers.age = 42
+   45 |     println(h[2].pers.age)
+      |             ~~~~
+   46 | }
diff --git a/vlib/v/checker/tests/shared_element_lock.vv b/vlib/v/checker/tests/shared_element_lock.vv
new file mode 100644
index 0000000000..de61ad9b96
--- /dev/null
+++ b/vlib/v/checker/tests/shared_element_lock.vv
@@ -0,0 +1,46 @@
+struct Person {
+mut:
+    name string
+    age int
+}
+
+struct Pet {
+mut:
+    name string
+    color int
+}
+
+struct Programmer {
+mut:
+	pers Person
+	pe shared Pet
+}
+
+struct Pro {
+mut:
+	pers Person
+	pe Pet
+}
+
+fn main() {
+	mut pr := Programmer{
+		pers: Person{
+			name: 'Qwe'
+			age: 44
+		}
+		pe: Pet{
+			name: 'Ghj'
+			color: 7
+		}
+	}
+	pr.pe.color = 3
+	shared y := pr.pe
+	rlock y {
+		println(y.color)
+	}
+	shared g := Pro{}
+	g.pers.age = 42
+	mut h := []shared Pro{len: 3}
+	h[2].pers.age = 42
+	println(h[2].pers.age)
+}
diff --git a/vlib/v/tests/shared_elem_test.v b/vlib/v/tests/shared_elem_test.v
index fc57bb4671..3f62c161d9 100644
--- a/vlib/v/tests/shared_elem_test.v
+++ b/vlib/v/tests/shared_elem_test.v
@@ -78,10 +78,13 @@ fn test_shared_map_in_struct() {
 }
 
 fn test_array_of_shared() {
-	mut a := []shared Xyz{len: 3}
-	a[0] = Xyz{ n: 3 }
-	a[1] = Xyz{ n: 7 }
-	a[2] = Xyz{ n: 13 }
+	mut a := []shared Xyz{cap: 3}
+	a0 := Xyz{ n: 3 }
+	a << a0
+	a1 := Xyz{ n: 7 }
+	a << a1
+	a2 := Xyz{ n: 13 }
+	a << a2
 	shared p := a[0]
 	shared q := a[2]
 	lock q {