From c160ba2a8dec033b4223758b0d30a7021f09fb59 Mon Sep 17 00:00:00 2001 From: yuyi Date: Tue, 21 Jun 2022 18:23:21 +0800 Subject: [PATCH] checker: stricter mutable reference check (fix #14803) (#14805) --- vlib/dlmalloc/dlmalloc.v | 16 ++++++++-------- vlib/net/address.v | 2 +- vlib/term/ui/input_windows.c.v | 2 +- vlib/term/ui/termios_nix.c.v | 8 ++++---- vlib/v/ast/scope.v | 4 ++-- vlib/v/checker/assign.v | 7 +++++++ vlib/v/checker/checker.v | 2 +- vlib/v/checker/fn.v | 4 ++-- .../tests/assign_immutable_reference_var_err.out | 7 +++++++ .../tests/assign_immutable_reference_var_err.vv | 16 ++++++++++++++++ vlib/v/parser/containers.v | 4 ++-- 11 files changed, 51 insertions(+), 21 deletions(-) create mode 100644 vlib/v/checker/tests/assign_immutable_reference_var_err.out create mode 100644 vlib/v/checker/tests/assign_immutable_reference_var_err.vv diff --git a/vlib/dlmalloc/dlmalloc.v b/vlib/dlmalloc/dlmalloc.v index 364413aa10..d421f4b62f 100644 --- a/vlib/dlmalloc/dlmalloc.v +++ b/vlib/dlmalloc/dlmalloc.v @@ -357,7 +357,7 @@ fn (mut c Chunk) set_size_and_pinuse_of_free_chunk(size usize) { } fn (mut c Chunk) set_free_with_pinuse(size usize, n_ &Chunk) { - mut n := n_ + mut n := unsafe { n_ } n.clear_pinuse() c.set_size_and_pinuse_of_free_chunk(size) } @@ -480,7 +480,7 @@ fn (mut dl Dlmalloc) unlink_chunk(chunk &Chunk, size usize) { [unsafe] fn (mut dl Dlmalloc) unlink_small_chunk(chunk_ &Chunk, size usize) { - mut chunk := chunk_ + mut chunk := unsafe { chunk_ } mut f := chunk.prev mut b := chunk.next idx := small_index(size) @@ -563,8 +563,8 @@ fn (mut dl Dlmalloc) unlink_large_chunk(chunk_ &TreeChunk) { [unsafe] fn (mut dl Dlmalloc) unlink_first_small_chunk(head_ &Chunk, next_ &Chunk, idx u32) { - mut next := next_ - mut head := head_ + mut next := unsafe { next_ } + mut head := unsafe { head_ } println('Unlink first small') mut ptr := next.prev if voidptr(head) == voidptr(ptr) { @@ -815,7 +815,7 @@ fn (mut dl Dlmalloc) insert_chunk(chunk &Chunk, size usize) { [unsafe] fn (mut dl Dlmalloc) insert_small_chunk(chunk_ &Chunk, size usize) { - mut chunk := chunk_ + mut chunk := unsafe { chunk_ } idx := small_index(size) unsafe { mut head := dl.smallbin_at(idx) @@ -837,8 +837,8 @@ fn (mut dl Dlmalloc) insert_small_chunk(chunk_ &Chunk, size usize) { [unsafe] fn (mut dl Dlmalloc) insert_large_chunk(chunk_ &TreeChunk, size usize) { - mut chunk := chunk_ unsafe { + mut chunk := chunk_ idx := dl.compute_tree_index(size) mut h := dl.treebin_at(idx) @@ -1527,7 +1527,7 @@ fn (mut dl Dlmalloc) try_realloc_chunk(p_ &Chunk, nb usize, can_move bool) &Chun [unsafe] fn (mut dl Dlmalloc) mmap_resize(oldp_ &Chunk, nb usize, can_move bool) &Chunk { - mut oldp := oldp_ + mut oldp := unsafe { oldp_ } oldsize := oldp.size() if is_small(nb) { return voidptr(0) @@ -1568,7 +1568,7 @@ fn (dl &Dlmalloc) mmap_align(a usize) usize { [unsafe] fn (mut dl Dlmalloc) dispose_chunk(p_ &Chunk, psize_ usize) { - mut p := p_ + mut p := unsafe { p_ } mut psize := psize_ unsafe { mut next := p.plus_offset(psize) diff --git a/vlib/net/address.v b/vlib/net/address.v index 41467c86be..079352d99d 100644 --- a/vlib/net/address.v +++ b/vlib/net/address.v @@ -205,7 +205,7 @@ pub fn resolve_ipaddrs(addr string, family AddrFamily, typ SocketType) ?[]Addr { // convert them into an array mut addresses := []Addr{} - for result := results; !isnil(result); result = result.ai_next { + for result := unsafe { results }; !isnil(result); result = result.ai_next { match AddrFamily(result.ai_family) { .ip { new_addr := Addr{ diff --git a/vlib/term/ui/input_windows.c.v b/vlib/term/ui/input_windows.c.v index cafb62fbba..c37f7dbbdd 100644 --- a/vlib/term/ui/input_windows.c.v +++ b/vlib/term/ui/input_windows.c.v @@ -86,7 +86,7 @@ pub fn init(cfg Config) &Context { C.atexit(restore_terminal_state) for code in ctx.cfg.reset { os.signal_opt(code, fn (_ os.Signal) { - mut c := ui.ctx_ptr + mut c := unsafe { ui.ctx_ptr } if unsafe { c != 0 } { c.cleanup() } diff --git a/vlib/term/ui/termios_nix.c.v b/vlib/term/ui/termios_nix.c.v index 67fc714180..19bb58b14d 100644 --- a/vlib/term/ui/termios_nix.c.v +++ b/vlib/term/ui/termios_nix.c.v @@ -43,7 +43,7 @@ fn restore_terminal_state_signal(_ os.Signal) { fn restore_terminal_state() { termios_reset() - mut c := ctx_ptr + mut c := unsafe { ctx_ptr } if unsafe { c != 0 } { c.paused = true load_title() @@ -125,7 +125,7 @@ fn (mut ctx Context) termios_setup() ? { C.atexit(restore_terminal_state) os.signal_opt(.tstp, restore_terminal_state_signal) or {} os.signal_opt(.cont, fn (_ os.Signal) { - mut c := ctx_ptr + mut c := unsafe { ctx_ptr } if unsafe { c != 0 } { c.termios_setup() or { panic(err) } c.window_height, c.window_width = get_terminal_size() @@ -140,7 +140,7 @@ fn (mut ctx Context) termios_setup() ? { }) or {} for code in ctx.cfg.reset { os.signal_opt(code, fn (_ os.Signal) { - mut c := ctx_ptr + mut c := unsafe { ctx_ptr } if unsafe { c != 0 } { c.cleanup() } @@ -149,7 +149,7 @@ fn (mut ctx Context) termios_setup() ? { } os.signal_opt(.winch, fn (_ os.Signal) { - mut c := ctx_ptr + mut c := unsafe { ctx_ptr } if unsafe { c != 0 } { c.window_height, c.window_width = get_terminal_size() diff --git a/vlib/v/ast/scope.v b/vlib/v/ast/scope.v index 5f94dd82f8..7e072f74f9 100644 --- a/vlib/v/ast/scope.v +++ b/vlib/v/ast/scope.v @@ -42,7 +42,7 @@ fn (s &Scope) dont_lookup_parent() bool { } pub fn (s &Scope) find(name string) ?ScopeObject { - for sc := s; true; sc = sc.parent { + for sc := unsafe { s }; true; sc = sc.parent { if name in sc.objects { return unsafe { sc.objects[name] } } @@ -55,7 +55,7 @@ pub fn (s &Scope) find(name string) ?ScopeObject { // selector_expr: name.field_name pub fn (s &Scope) find_struct_field(name string, struct_type Type, field_name string) ?ScopeStructField { - for sc := s; true; sc = sc.parent { + for sc := unsafe { s }; true; sc = sc.parent { if field := sc.struct_fields[name] { if field.struct_type == struct_type && field.name == field_name { return field diff --git a/vlib/v/checker/assign.v b/vlib/v/checker/assign.v index 9e0df64dc7..d68ba66f75 100644 --- a/vlib/v/checker/assign.v +++ b/vlib/v/checker/assign.v @@ -157,6 +157,13 @@ pub fn (mut c Checker) assign_stmt(mut node ast.AssignStmt) { } } } + if mut left is ast.Ident && mut right is ast.Ident { + if !c.inside_unsafe && left_type.is_ptr() && left.is_mut() && right_type.is_ptr() + && !right.is_mut() { + c.error('`$right.name` is immutable, cannot have a mutable reference to an immutable object', + right.pos) + } + } } else { // Make sure the variable is mutable c.fail_if_immutable(left) diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index 0754d30862..46334c1052 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -174,7 +174,7 @@ fn (mut c Checker) reset_checker_state_at_start_of_new_file() { } pub fn (mut c Checker) check(ast_file_ &ast.File) { - mut ast_file := ast_file_ + mut ast_file := unsafe { ast_file_ } c.reset_checker_state_at_start_of_new_file() c.change_current_file(ast_file) for i, ast_import in ast_file.imports { diff --git a/vlib/v/checker/fn.v b/vlib/v/checker/fn.v index 37b298f5bd..7dd30c4960 100644 --- a/vlib/v/checker/fn.v +++ b/vlib/v/checker/fn.v @@ -877,7 +877,7 @@ pub fn (mut c Checker) fn_call(mut node ast.CallExpr, mut continue_check &bool) c.fail_if_unreadable(call_arg.expr, arg_typ, 'argument') } } - mut final_param_sym := param_typ_sym + mut final_param_sym := unsafe { param_typ_sym } mut final_param_typ := param.typ if func.is_variadic && param_typ_sym.info is ast.Array { final_param_typ = param_typ_sym.info.elem_type @@ -1349,7 +1349,7 @@ pub fn (mut c Checker) method_call(mut node ast.CallExpr) ast.Type { c.error('when forwarding a variadic variable, it must be the final argument', arg.pos) } - mut final_arg_sym := exp_arg_sym + mut final_arg_sym := unsafe { exp_arg_sym } mut final_arg_typ := exp_arg_typ if method.is_variadic && exp_arg_sym.info is ast.Array { final_arg_typ = exp_arg_sym.info.elem_type diff --git a/vlib/v/checker/tests/assign_immutable_reference_var_err.out b/vlib/v/checker/tests/assign_immutable_reference_var_err.out new file mode 100644 index 0000000000..fa690a6c87 --- /dev/null +++ b/vlib/v/checker/tests/assign_immutable_reference_var_err.out @@ -0,0 +1,7 @@ +vlib/v/checker/tests/assign_immutable_reference_var_err.vv:8:11: error: `x` is immutable, cannot have a mutable reference to an immutable object + 6 | + 7 | fn y(x &Foo) { + 8 | mut m := x + | ^ + 9 | m.value = 42 + 10 | } diff --git a/vlib/v/checker/tests/assign_immutable_reference_var_err.vv b/vlib/v/checker/tests/assign_immutable_reference_var_err.vv new file mode 100644 index 0000000000..e525417112 --- /dev/null +++ b/vlib/v/checker/tests/assign_immutable_reference_var_err.vv @@ -0,0 +1,16 @@ +[heap] +struct Foo { +mut: + value int +} + +fn y(x &Foo) { + mut m := x + m.value = 42 +} + +fn main() { + x := Foo{123} + y(x) + println(x) +} diff --git a/vlib/v/parser/containers.v b/vlib/v/parser/containers.v index ec9e7ea0bf..29579d0cf9 100644 --- a/vlib/v/parser/containers.v +++ b/vlib/v/parser/containers.v @@ -89,7 +89,7 @@ fn (mut p Parser) array_init() ast.ArrayInit { p.scope_register_it_as_index() default_expr = p.expr(0) has_it = if var := p.scope.find_var('it') { - mut variable := var + mut variable := unsafe { var } is_used := variable.is_used variable.is_used = true is_used @@ -148,7 +148,7 @@ fn (mut p Parser) array_init() ast.ArrayInit { p.scope_register_it_as_index() default_expr = p.expr(0) has_it = if var := p.scope.find_var('it') { - mut variable := var + mut variable := unsafe { var } is_used := variable.is_used variable.is_used = true is_used