From 105a0e015ef9133b73dd27495c632d47ab2fdc32 Mon Sep 17 00:00:00 2001 From: Nick Treleaven Date: Fri, 17 Jul 2020 18:14:12 +0100 Subject: [PATCH] checker: warn if unsafe method called outside unsafe block (#5863) --- vlib/builtin/array.v | 16 ++++++++++++---- vlib/builtin/map.v | 2 +- vlib/builtin/string.v | 5 ++++- vlib/v/builder/compile.v | 9 +++++++-- vlib/v/checker/checker.v | 4 ++++ vlib/v/parser/fn.v | 5 ++++- vlib/v/table/table.v | 1 + vlib/v/tests/unsafe.out | 34 ++++++++++++++++++++++++++++++++++ vlib/v/tests/unsafe.v | 25 +++++++++++++++++++++++++ 9 files changed, 92 insertions(+), 9 deletions(-) create mode 100644 vlib/v/tests/unsafe.out create mode 100644 vlib/v/tests/unsafe.v diff --git a/vlib/builtin/array.v b/vlib/builtin/array.v index 8f3e54928a..2925cb1fbb 100644 --- a/vlib/builtin/array.v +++ b/vlib/builtin/array.v @@ -331,12 +331,18 @@ pub fn (a &array) clone() array { if a.element_size == sizeof(array) { for i in 0..a.len { ar := array{} - C.memcpy(&ar, a.get_unsafe(i), sizeof(array)) + unsafe { + C.memcpy(&ar, a.get_unsafe(i), sizeof(array)) + } ar_clone := ar.clone() - arr.set_unsafe(i, &ar_clone) + unsafe { + arr.set_unsafe(i, &ar_clone) + } } } else { - C.memcpy(byteptr(arr.data), a.data, a.cap * a.element_size) + unsafe { + C.memcpy(byteptr(arr.data), a.data, a.cap * a.element_size) + } } return arr } @@ -681,7 +687,9 @@ pub fn compare_f32(a, b &f32) int { pub fn (a array) pointers() []voidptr { mut res := []voidptr{} for i in 0..a.len { - res << a.get_unsafe(i) + unsafe { + res << a.get_unsafe(i) + } } return res } diff --git a/vlib/builtin/map.v b/vlib/builtin/map.v index f125bb7c45..3b65e4f4df 100644 --- a/vlib/builtin/map.v +++ b/vlib/builtin/map.v @@ -517,7 +517,7 @@ pub fn (m map) clone() map { cap: m.cap cached_hashbits: m.cached_hashbits shift: m.shift - key_values: m.key_values.clone() + key_values: unsafe {m.key_values.clone()} metas: &u32(malloc(int(metas_size))) extra_metas: m.extra_metas len: m.len diff --git a/vlib/builtin/string.v b/vlib/builtin/string.v index 57ff7a589d..1e2bdb7131 100644 --- a/vlib/builtin/string.v +++ b/vlib/builtin/string.v @@ -1201,11 +1201,14 @@ pub fn (u ustring) at(idx int) string { return u.substr(idx, idx + 1) } +[unsafe_fn] fn (u &ustring) free() { $if prealloc { return } - u.runes.free() + unsafe { + u.runes.free() + } } pub fn (c byte) is_digit() bool { diff --git a/vlib/v/builder/compile.v b/vlib/v/builder/compile.v index 7449a6566f..ebafec3223 100644 --- a/vlib/v/builder/compile.v +++ b/vlib/v/builder/compile.v @@ -42,17 +42,22 @@ pub fn compile(command string, pref &pref.Preferences) { println('compilation took: $sw.elapsed().milliseconds() ms') } // running does not require the parsers anymore - b.myfree() + unsafe { + b.myfree() + } if pref.is_test || pref.is_run { b.run_compiled_executable_and_exit() } } // Temporary, will be done by -autofree +[unsafe_fn] fn (mut b Builder) myfree() { // for file in b.parsed_files { // } - b.parsed_files.free() + unsafe { + b.parsed_files.free() + } } fn (mut b Builder) run_compiled_executable_and_exit() { diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index e082dcd25b..0c51a90ae7 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -954,6 +954,10 @@ pub fn (mut c Checker) call_method(mut call_expr ast.CallExpr) table.Type { } } } + if method.is_unsafe && !c.inside_unsafe { + c.warn('method `${left_type_sym.name}.$method_name` must be called from an `unsafe` block', + call_expr.pos) + } // TODO: typ optimize.. this node can get processed more than once if call_expr.expected_arg_types.len == 0 { for i in 1 .. method.args.len { diff --git a/vlib/v/parser/fn.v b/vlib/v/parser/fn.v index 040080db08..2b7954abdc 100644 --- a/vlib/v/parser/fn.v +++ b/vlib/v/parser/fn.v @@ -123,6 +123,7 @@ fn (mut p Parser) fn_decl() ast.FnDecl { p.top_level_statement_start() start_pos := p.tok.position() is_deprecated := 'deprecated' in p.attrs + is_unsafe := 'unsafe_fn' in p.attrs is_pub := p.tok.kind == .key_pub if is_pub { p.next() @@ -250,6 +251,7 @@ fn (mut p Parser) fn_decl() ast.FnDecl { is_generic: is_generic is_pub: is_pub is_deprecated: is_deprecated + is_unsafe: is_unsafe ctdefine: ctdefine mod: p.mod attrs: p.attrs @@ -271,12 +273,13 @@ fn (mut p Parser) fn_decl() ast.FnDecl { args: args return_type: return_type is_variadic: is_variadic - language: language is_generic: is_generic is_pub: is_pub is_deprecated: is_deprecated + is_unsafe: is_unsafe ctdefine: ctdefine mod: p.mod + language: language }) } // Body diff --git a/vlib/v/table/table.v b/vlib/v/table/table.v index c2b8f8bb66..3f3dbe1eb7 100644 --- a/vlib/v/table/table.v +++ b/vlib/v/table/table.v @@ -29,6 +29,7 @@ pub: is_generic bool is_pub bool is_deprecated bool + is_unsafe bool mod string ctdefine string // compile time define. myflag, when [if myflag] tag attrs []string diff --git a/vlib/v/tests/unsafe.out b/vlib/v/tests/unsafe.out new file mode 100644 index 0000000000..c7a93b2900 --- /dev/null +++ b/vlib/v/tests/unsafe.out @@ -0,0 +1,34 @@ +unsafe.v:4:6: warning: pointer arithmetic is only allowed in `unsafe` blocks + 2 | v := 5 + 3 | mut p := &v + 4 | p++ + | ~~ + 5 | p += 2 + 6 | _ := v +unsafe.v:5:7: warning: pointer arithmetic is only allowed in `unsafe` blocks + 3 | mut p := &v + 4 | p++ + 5 | p += 2 + | ~~ + 6 | _ := v + 7 | } +unsafe.v:11:14: warning: pointer arithmetic is only allowed in `unsafe` blocks + 9 | fn test_ptr_infix() { + 10 | v := 4 + 11 | mut q := &v - 1 + | ^ + 12 | q = q + 3 + 13 | _ := q +unsafe.v:12:9: warning: pointer arithmetic is only allowed in `unsafe` blocks + 10 | v := 4 + 11 | mut q := &v - 1 + 12 | q = q + 3 + | ^ + 13 | _ := q + 14 | _ := v +unsafe.v:24:7: warning: method `S1.f` must be called from an `unsafe` block + 22 | fn test_funcs() { + 23 | s := S1{} + 24 | s.f() + | ~~~ + 25 | } diff --git a/vlib/v/tests/unsafe.v b/vlib/v/tests/unsafe.v new file mode 100644 index 0000000000..917a2f0fec --- /dev/null +++ b/vlib/v/tests/unsafe.v @@ -0,0 +1,25 @@ +fn test_ptr_assign() { + v := 5 + mut p := &v + p++ + p += 2 + _ := v +} + +fn test_ptr_infix() { + v := 4 + mut q := &v - 1 + q = q + 3 + _ := q + _ := v +} + +struct S1 {} + +[unsafe_fn] +fn (s S1) f(){} + +fn test_funcs() { + s := S1{} + s.f() +}