From 09bb0af59cf1c1e78d4aa8410475105724242b6c Mon Sep 17 00:00:00 2001
From: Naoki MATSUMOTO <naoki@pibvt.net>
Date: Tue, 1 Feb 2022 21:02:00 +0900
Subject: [PATCH] checker: Fix array,map builtin methods can be called without
 lock (#13329)

---
 vlib/v/checker/fn.v                  | 18 ++++++++++++++++++
 vlib/v/gen/c/array.v                 |  8 ++++++--
 vlib/v/gen/c/cgen.v                  |  4 +++-
 vlib/v/tests/shared_lock_expr_test.v | 11 +++++++++++
 4 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/vlib/v/checker/fn.v b/vlib/v/checker/fn.v
index 3735bc43ac..ddb4f4fb3a 100644
--- a/vlib/v/checker/fn.v
+++ b/vlib/v/checker/fn.v
@@ -1045,9 +1045,13 @@ pub fn (mut c Checker) method_call(mut node ast.CallExpr) ast.Type {
 	// TODO: remove this for actual methods, use only for compiler magic
 	// FIXME: Argument count != 1 will break these
 	if left_sym.kind == .array && method_name in array_builtin_methods {
+		// check if shared variable is locked
+		c.fail_if_unreadable(node.left, left_type, 'receiver')
 		return c.array_builtin_method_call(mut node, left_type, c.table.sym(left_type))
 	} else if (left_sym.kind == .map || final_left_sym.kind == .map)
 		&& method_name in ['clone', 'keys', 'move', 'delete'] {
+		// check if shared variable is locked
+		c.fail_if_unreadable(node.left, left_type, 'receiver')
 		if left_sym.kind == .map {
 			return c.map_builtin_method_call(mut node, left_type, left_sym)
 		} else {
@@ -1055,6 +1059,8 @@ pub fn (mut c Checker) method_call(mut node ast.CallExpr) ast.Type {
 			return c.map_builtin_method_call(mut node, parent_type, final_left_sym)
 		}
 	} else if left_sym.kind == .array && method_name in ['insert', 'prepend'] {
+		// check if shared variable is locked
+		c.fail_if_unreadable(node.left, left_type, 'receiver')
 		if method_name == 'insert' {
 			if node.args.len != 2 {
 				c.error('`array.insert()` should have 2 arguments, e.g. `insert(1, val)`',
@@ -1083,6 +1089,8 @@ pub fn (mut c Checker) method_call(mut node ast.CallExpr) ast.Type {
 			c.error('cannot $method_name `$arg_sym.name` to `$left_sym.name`', arg_expr.pos())
 		}
 	} else if final_left_sym.kind == .array && method_name in ['first', 'last', 'pop'] {
+		// check if shared variable is locked
+		c.fail_if_unreadable(node.left, left_type, 'receiver')
 		if final_left_sym.info is ast.Array {
 			node.return_type = final_left_sym.info.elem_type
 			return node.return_type
@@ -1647,6 +1655,7 @@ fn (mut c Checker) map_builtin_method_call(mut node ast.CallExpr, left_type ast.
 			} else {
 				ret_type = left_type
 			}
+			ret_type = ret_type.clear_flag(.shared_f)
 		}
 		'keys' {
 			info := left_sym.info as ast.Map
@@ -1667,6 +1676,10 @@ fn (mut c Checker) map_builtin_method_call(mut node ast.CallExpr, left_type ast.
 		else {}
 	}
 	node.receiver_type = left_type.ref()
+
+	// all methods does not take a lock inside them.
+	node.receiver_type = node.receiver_type.clear_flag(.shared_f)
+
 	node.return_type = ret_type
 	return node.return_type
 }
@@ -1775,6 +1788,7 @@ fn (mut c Checker) array_builtin_method_call(mut node ast.CallExpr, left_type as
 		} else {
 			node.return_type = node.receiver_type.set_nr_muls(0)
 		}
+		node.return_type = node.return_type.clear_flag(.shared_f)
 	} else if method_name == 'sort' {
 		node.return_type = ast.void_type
 	} else if method_name == 'contains' {
@@ -1791,6 +1805,10 @@ fn (mut c Checker) array_builtin_method_call(mut node ast.CallExpr, left_type as
 			node.receiver_type = left_type
 		}
 	}
+
+	// all methods does not take a lock inside them.
+	node.receiver_type = node.receiver_type.clear_flag(.shared_f)
+
 	return node.return_type
 }
 
diff --git a/vlib/v/gen/c/array.v b/vlib/v/gen/c/array.v
index 39eb7b524d..826faa3dfd 100644
--- a/vlib/v/gen/c/array.v
+++ b/vlib/v/gen/c/array.v
@@ -404,7 +404,9 @@ fn (mut g Gen) gen_array_sort(node ast.CallExpr) {
 	if node.args.len == 0 {
 		comparison_type = g.unwrap(info.elem_type.set_nr_muls(0))
 		shared a := g.array_sort_fn
-		array_sort_fn := a.clone()
+		array_sort_fn := rlock a {
+			a.clone()
+		}
 		if compare_fn in array_sort_fn {
 			g.gen_array_sort_call(node, compare_fn)
 			return
@@ -425,7 +427,9 @@ fn (mut g Gen) gen_array_sort(node ast.CallExpr) {
 			compare_fn += '_reverse'
 		}
 		shared a := g.array_sort_fn
-		array_sort_fn := a.clone()
+		array_sort_fn := rlock a {
+			a.clone()
+		}
 		if compare_fn in array_sort_fn {
 			g.gen_array_sort_call(node, compare_fn)
 			return
diff --git a/vlib/v/gen/c/cgen.v b/vlib/v/gen/c/cgen.v
index add0b4d121..a0be79588c 100644
--- a/vlib/v/gen/c/cgen.v
+++ b/vlib/v/gen/c/cgen.v
@@ -958,7 +958,9 @@ fn (mut g Gen) register_optional(t ast.Type) string {
 }
 
 fn (mut g Gen) write_optionals() {
-	mut done := g.done_optionals.clone()
+	mut done := rlock g.done_optionals {
+		g.done_optionals.clone()
+	}
 	for base, styp in g.optionals {
 		if base in done {
 			continue
diff --git a/vlib/v/tests/shared_lock_expr_test.v b/vlib/v/tests/shared_lock_expr_test.v
index 390cc9197a..68cf048bab 100644
--- a/vlib/v/tests/shared_lock_expr_test.v
+++ b/vlib/v/tests/shared_lock_expr_test.v
@@ -121,3 +121,14 @@ fn test_shared_lock_chan_rec_expr() {
 		}
 	}
 }
+
+fn test_shared_array_clone() {
+	shared a := []string{}
+	lock a {
+		a << "test"
+	}
+	b := rlock a {
+		a.clone()
+	}
+	assert b[0] == "test"
+}
\ No newline at end of file