From 0b3ebd29ebf9f531b046743f51ed88d6f498978f Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Wed, 15 Apr 2026 02:49:40 +0300 Subject: [PATCH] checker: prevent mutation of a function result which should be immutable (fixes #13817) --- vlib/v/checker/checker.v | 146 +++++++++++++----- ...all_result_aliases_immutable_value_err.out | 20 +++ ...call_result_aliases_immutable_value_err.vv | 25 +++ 3 files changed, 155 insertions(+), 36 deletions(-) create mode 100644 vlib/v/checker/tests/call_result_aliases_immutable_value_err.out create mode 100644 vlib/v/checker/tests/call_result_aliases_immutable_value_err.vv diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index f814bd19c..ecf6a73d6 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -149,18 +149,19 @@ mut: is_js_backend bool // doing_line_info int // a quick single file run when called with v -line-info (contains line nr to inspect) // doing_line_path string // same, but stores the path being parsed - is_index_assign bool - comptime_call_pos int // needed for correctly checking use before decl for templates - generic_call_positions map[string]token.Pos // map from generic function key to call position - goto_labels map[string]ast.GotoLabel // to check for unused goto labels - enum_data_type ast.Type - field_data_type ast.Type - variant_data_type ast.Type - fn_return_type ast.Type - orm_table_fields map[string][]ast.StructField // known table structs - short_module_names []string // to check for function names colliding with module functions - visible_param_mutation_cache map[string]bool - visible_param_mutation_in_progress map[string]bool + is_index_assign bool + comptime_call_pos int // needed for correctly checking use before decl for templates + generic_call_positions map[string]token.Pos // map from generic function key to call position + goto_labels map[string]ast.GotoLabel // to check for unused goto labels + enum_data_type ast.Type + field_data_type ast.Type + variant_data_type ast.Type + fn_return_type ast.Type + orm_table_fields map[string][]ast.StructField // known table structs + short_module_names []string // to check for function names colliding with module functions + visible_param_mutation_cache map[string]bool + visible_param_mutation_in_progress map[string]bool + immutable_alias_analysis_in_progress map[string]bool v_current_commit_hash string // same as old C.V_CURRENT_COMMIT_HASH assign_stmt_attr string // for `x := [1,2,3] @[freed]` @@ -183,17 +184,18 @@ pub fn new_checker(table &ast.Table, pref_ &pref.Preferences) &Checker { vcurrent_hash() } mut checker := &Checker{ - table: table - pref: pref_ - timers: util.new_timers( + table: table + pref: pref_ + timers: util.new_timers( should_print: timers_should_print label: 'checker' ) - match_exhaustive_cutoff_limit: pref_.checker_match_exhaustive_cutoff_limit - v_current_commit_hash: v_current_commit_hash - checker_transformer: transformer.new_transformer_with_table(table, pref_) - visible_param_mutation_cache: map[string]bool{} - visible_param_mutation_in_progress: map[string]bool{} + match_exhaustive_cutoff_limit: pref_.checker_match_exhaustive_cutoff_limit + v_current_commit_hash: v_current_commit_hash + checker_transformer: transformer.new_transformer_with_table(table, pref_) + visible_param_mutation_cache: map[string]bool{} + visible_param_mutation_in_progress: map[string]bool{} + immutable_alias_analysis_in_progress: map[string]bool{} } checker.checker_transformer.skip_array_transform = true checker.type_resolver = type_resolver.TypeResolver.new(table, checker) @@ -1399,17 +1401,32 @@ fn (mut c Checker) type_contains_mutable_aliasing(typ ast.Type, mut checked_type return false } +fn (mut c Checker) type_has_mutable_aliasing(typ ast.Type) bool { + mut checked_types := []ast.Type{} + return c.type_contains_mutable_aliasing(typ, mut checked_types) +} + fn (mut c Checker) expr_is_mutable_alias_of_immutable_source(expr ast.Expr) bool { match expr { ast.Ident { - if expr.obj is ast.Var && expr.obj.is_mut && expr.obj.expr is ast.Ident { - mut checked_types := []ast.Type{} - if c.type_contains_mutable_aliasing(expr.obj.typ, mut checked_types) { - return c.expr_is_immutable_source(expr.obj.expr) - } + if expr.obj is ast.Var && expr.obj.is_mut + && expr.obj.expr in [ast.Ident, ast.CallExpr, ast.CastExpr, ast.AsCast, ast.ParExpr, ast.UnsafeExpr] + && c.type_has_mutable_aliasing(expr.obj.typ) { + return c.expr_is_immutable_source(expr.obj.expr) + || c.expr_is_mutable_alias_of_immutable_source(expr.obj.expr) } return false } + ast.CastExpr { + return c.expr_is_mutable_alias_of_immutable_source(expr.expr) + } + ast.CallExpr { + return c.call_expr_immutable_alias_source(expr) !is ast.EmptyExpr + } + ast.IndexExpr { + return c.type_has_mutable_aliasing(expr.typ) && (c.expr_is_immutable_source(expr.left) + || c.expr_is_mutable_alias_of_immutable_source(expr.left)) + } ast.SelectorExpr { if expr.expr_type == 0 { return false @@ -1446,9 +1463,15 @@ fn (mut c Checker) expr_is_mutable_alias_of_immutable_source(expr ast.Expr) bool } return false } + ast.AsCast { + return c.expr_is_mutable_alias_of_immutable_source(expr.expr) + } ast.ParExpr { return c.expr_is_mutable_alias_of_immutable_source(expr.expr) } + ast.UnsafeExpr { + return c.expr_is_mutable_alias_of_immutable_source(expr.expr) + } else { return false } @@ -1474,9 +1497,15 @@ fn (mut c Checker) call_arg_expr_for_param(node ast.CallExpr, func ast.Fn, param fn (mut c Checker) return_expr_immutable_alias_source(expr ast.Expr, func ast.Fn, call ast.CallExpr, allow_non_ptr bool) ast.Expr { match expr { + ast.AsCast { + return c.return_expr_immutable_alias_source(expr.expr, func, call, allow_non_ptr) + } ast.CastExpr { return c.return_expr_immutable_alias_source(expr.expr, func, call, allow_non_ptr) } + ast.CallExpr { + return c.call_expr_immutable_alias_source(expr) + } ast.Ident { if expr.obj is ast.Var && expr.obj.is_arg { arg_expr := c.call_arg_expr_for_param(call, func, expr.name) @@ -1486,7 +1515,20 @@ fn (mut c Checker) return_expr_immutable_alias_source(expr ast.Expr, func ast.Fn } } if expr.obj is ast.Var && (allow_non_ptr || expr.obj.typ.is_ptr()) { - return c.return_expr_immutable_alias_source(expr.obj.expr, func, call, false) + return c.return_expr_immutable_alias_source(expr.obj.expr, func, call, + allow_non_ptr) + } + return ast.empty_expr + } + ast.IndexExpr { + if c.type_has_mutable_aliasing(expr.typ) { + return c.return_expr_immutable_alias_source(expr.left, func, call, true) + } + return ast.empty_expr + } + ast.SelectorExpr { + if c.type_has_mutable_aliasing(expr.typ) { + return c.return_expr_immutable_alias_source(expr.expr, func, call, true) } return ast.empty_expr } @@ -1520,8 +1562,10 @@ fn (mut c Checker) node_immutable_alias_source(node ast.Node, func ast.Fn, call return ast.empty_expr } if node is ast.Return { + allow_non_ptr := !call.return_type.is_ptr() + && c.type_has_mutable_aliasing(call.return_type) for expr in node.exprs { - source := c.return_expr_immutable_alias_source(expr, func, call, false) + source := c.return_expr_immutable_alias_source(expr, func, call, allow_non_ptr) if source !is ast.EmptyExpr { return source } @@ -1540,11 +1584,26 @@ fn (mut c Checker) node_immutable_alias_source(node ast.Node, func ast.Fn, call return ast.empty_expr } +fn (c &Checker) immutable_alias_call_key(node ast.CallExpr) string { + if node.concrete_types.len > 0 { + return c.build_generic_call_key(node.fkey(), node.concrete_types) + } + return node.fkey() +} + fn (mut c Checker) call_expr_immutable_alias_source(node ast.CallExpr) ast.Expr { - if !node.return_type.is_ptr() { + if !c.type_has_mutable_aliasing(node.return_type) { return ast.empty_expr } func := c.get_fn_from_call_expr(node) or { return ast.empty_expr } + call_key := c.immutable_alias_call_key(node) + if call_key in c.immutable_alias_analysis_in_progress { + return ast.empty_expr + } + c.immutable_alias_analysis_in_progress[call_key] = true + defer { + c.immutable_alias_analysis_in_progress.delete(call_key) + } if func.source_fn == unsafe { nil } { return ast.empty_expr } @@ -1578,15 +1637,18 @@ fn (mut c Checker) fail_if_immutable_to_mutable(left_type ast.Type, right_type a } } ast.CallExpr { - source := c.call_expr_immutable_alias_source(right) - if source !is ast.EmptyExpr { - if source is ast.Ident && c.expr_is_immutable_source(source) { - c.note('`${source.name}` is immutable, cannot have a mutable reference to an immutable object', - source.pos) - } else { - c.note('call result aliases mutable data from an immutable value', right.pos) + if right_type.is_ptr() { + source := c.call_expr_immutable_alias_source(right) + if source !is ast.EmptyExpr { + if source is ast.Ident && c.expr_is_immutable_source(source) { + c.note('`${source.name}` is immutable, cannot have a mutable reference to an immutable object', + source.pos) + } else { + c.note('call result aliases mutable data from an immutable value', + right.pos) + } + return false } - return false } } ast.Ident { @@ -1883,6 +1945,18 @@ fn (mut c Checker) fail_if_immutable(mut expr ast.Expr) (string, token.Pos) { explicit_lock_needed = true } } + if !c.inside_unsafe { + source := c.call_expr_immutable_alias_source(expr) + if source !is ast.EmptyExpr { + if source is ast.Ident && c.expr_is_immutable_source(source) { + c.error('`${source.name}` is immutable, cannot have a mutable reference to an immutable object', + source.pos) + } else { + c.error('`${expr}` aliases mutable data from an immutable value', expr.pos) + } + return '', expr.pos + } + } } ast.ArrayInit { c.error('array literal can not be modified', expr.pos) diff --git a/vlib/v/checker/tests/call_result_aliases_immutable_value_err.out b/vlib/v/checker/tests/call_result_aliases_immutable_value_err.out new file mode 100644 index 000000000..b678d79cf --- /dev/null +++ b/vlib/v/checker/tests/call_result_aliases_immutable_value_err.out @@ -0,0 +1,20 @@ +vlib/v/checker/tests/call_result_aliases_immutable_value_err.vv:18:2: error: `s` aliases mutable data from an immutable value, clone it first (or use `unsafe`) + 16 | arr := [1, 2, 3, 4, 5] + 17 | mut s := id(arr) + 18 | s[0] = 0 + | ^ + 19 | id(arr)[0] = 0 + 20 | +vlib/v/checker/tests/call_result_aliases_immutable_value_err.vv:19:2: error: `id(arr)` aliases mutable data from an immutable value, clone it first (or use `unsafe`) + 17 | mut s := id(arr) + 18 | s[0] = 0 + 19 | id(arr)[0] = 0 + | ~~~~~~~ + 20 | + 21 | ja := User{ +vlib/v/checker/tests/call_result_aliases_immutable_value_err.vv:24:7: error: `ja` is immutable, cannot have a mutable reference to an immutable object + 22 | name: 'foo' + 23 | } + 24 | rere(ja).name = 'bar' + | ~~ + 25 | } diff --git a/vlib/v/checker/tests/call_result_aliases_immutable_value_err.vv b/vlib/v/checker/tests/call_result_aliases_immutable_value_err.vv new file mode 100644 index 000000000..af6fb9c3b --- /dev/null +++ b/vlib/v/checker/tests/call_result_aliases_immutable_value_err.vv @@ -0,0 +1,25 @@ +@[heap] +struct User { +mut: + name string +} + +fn id(arr []int) []int { + return arr[1..2] +} + +fn rere(a &User) &User { + return a +} + +fn main() { + arr := [1, 2, 3, 4, 5] + mut s := id(arr) + s[0] = 0 + id(arr)[0] = 0 + + ja := User{ + name: 'foo' + } + rere(ja).name = 'bar' +} -- 2.39.5