From 15daeaeafa59fa70857c4ec1bc22a47b0ffca795 Mon Sep 17 00:00:00 2001 From: Nick Treleaven Date: Mon, 22 Feb 2021 12:54:24 +0000 Subject: [PATCH] cgen: add fixed array bounds checking for non-literal index (#8832) --- TESTS.md | 8 +++++++- vlib/builtin/builtin.c.v | 11 +++++++++++ vlib/v/gen/c/cgen.v | 11 ++++++++++- vlib/v/markused/markused.v | 1 + vlib/v/tests/inout/compiler_test.v | 11 +++++++++-- vlib/v/tests/inout/fixed_array_index.out | 6 ++++++ vlib/v/tests/inout/fixed_array_index.vv | 4 ++++ vlib/v/tests/inout/fixed_array_slice.out | 5 +++++ vlib/v/tests/inout/fixed_array_slice.vv | 3 +++ 9 files changed, 56 insertions(+), 4 deletions(-) create mode 100644 vlib/v/tests/inout/fixed_array_index.out create mode 100644 vlib/v/tests/inout/fixed_array_index.vv create mode 100644 vlib/v/tests/inout/fixed_array_slice.out create mode 100644 vlib/v/tests/inout/fixed_array_slice.vv diff --git a/TESTS.md b/TESTS.md index ff39c5f7ed..0ba073a5ba 100644 --- a/TESTS.md +++ b/TESTS.md @@ -18,6 +18,11 @@ Tip: use `v -cc tcc` when compiling tests for speed. General runnable tests for different features of the V compiler. +* `vlib/v/tests/inout/compiler_test.v` + +Test output of running a V program matches an expected .out file. +Check the source for how to test panics. + ## Test building of actual V programs (examples, tools, V itself) * `v build-tools` @@ -41,11 +46,12 @@ This verifies that `_keep.v` files would be unchanged by `vfmt -w`. This checks all source files are formatted and prints a summary. This is not required. -## Other * `v test-fmt` Test all files in the current directory are formatted. +## Markdown + * `v check-md -hide-warnings .` Ensure that all .md files in the project are formatted properly, diff --git a/vlib/builtin/builtin.c.v b/vlib/builtin/builtin.c.v index 361e97c848..1a59ef180a 100644 --- a/vlib/builtin/builtin.c.v +++ b/vlib/builtin/builtin.c.v @@ -274,3 +274,14 @@ pub fn is_atty(fd int) int { return C.isatty(fd) } } + +[inline] +fn v_fixed_index(i int, len int) int { + $if !no_bounds_checking ? { + if i < 0 || i >= len { + s := 'fixed array index out of range (index: $i, len: $len)' + panic(s) + } + } + return i +} diff --git a/vlib/v/gen/c/cgen.v b/vlib/v/gen/c/cgen.v index 00999f63d8..29e7c1bed0 100644 --- a/vlib/v/gen/c/cgen.v +++ b/vlib/v/gen/c/cgen.v @@ -4217,6 +4217,7 @@ fn (mut g Gen) index_expr(node ast.IndexExpr) { g.write(')') } else { + // indexing sym := g.table.get_final_type_symbol(node.left_type) left_is_ptr := node.left_type.is_ptr() if sym.kind == .array { @@ -4397,7 +4398,15 @@ fn (mut g Gen) index_expr(node ast.IndexExpr) { g.expr(node.left) } g.write('[') - g.expr(node.index) + direct := g.fn_decl != 0 && g.fn_decl.is_direct_arr + if direct || node.index is ast.IntegerLiteral { + g.expr(node.index) + } else { + // bounds check + g.write('v_fixed_index(') + g.expr(node.index) + g.write(', $info.size)') + } g.write(']') if is_fn_index_call { g.write(')') diff --git a/vlib/v/markused/markused.v b/vlib/v/markused/markused.v index 500417431c..cefe3c2226 100644 --- a/vlib/v/markused/markused.v +++ b/vlib/v/markused/markused.v @@ -22,6 +22,7 @@ pub fn mark_used(mut the_table table.Table, pref &pref.Preferences, ast_files [] '__new_array_with_default', '__new_array_with_array_default', 'new_array_from_c_array', + 'v_fixed_index', 'memdup', 'vstrlen', '__as_cast', diff --git a/vlib/v/tests/inout/compiler_test.v b/vlib/v/tests/inout/compiler_test.v index e5ba13b536..86277527b6 100644 --- a/vlib/v/tests/inout/compiler_test.v +++ b/vlib/v/tests/inout/compiler_test.v @@ -1,3 +1,7 @@ +// .out file: +// To test a panic, remove everything after the long `===` line +// You can also remove the line with 'line:' e.g. for a builtin fn + import os import term import v.util @@ -52,7 +56,7 @@ fn test_all() { n_found := normalize_panic_message(found, vroot) n_expected := normalize_panic_message(expected, vroot) if found.contains('================ V panic ================') { - if n_found.contains(n_expected) { + if n_found.starts_with(n_expected) { println(term.green('OK (panic)')) continue } else { @@ -86,7 +90,10 @@ fn test_all() { fn normalize_panic_message(message string, vroot string) string { mut msg := message.all_before('=========================================') - msg = msg.replace(vroot + os.path_separator, '') + // change windows to nix path + s := vroot.replace(os.path_separator, '/') + // remove vroot + msg = msg.replace(s + '/', '') msg = msg.trim_space() return msg } diff --git a/vlib/v/tests/inout/fixed_array_index.out b/vlib/v/tests/inout/fixed_array_index.out new file mode 100644 index 0000000000..6c2b2820e2 --- /dev/null +++ b/vlib/v/tests/inout/fixed_array_index.out @@ -0,0 +1,6 @@ +1 +================ V panic ================ + module: builtin + function: v_fixed_index() + message: fixed array index out of range (index: 2, len: 2) + file: vlib/builtin/builtin.c.v diff --git a/vlib/v/tests/inout/fixed_array_index.vv b/vlib/v/tests/inout/fixed_array_index.vv new file mode 100644 index 0000000000..15fef607b6 --- /dev/null +++ b/vlib/v/tests/inout/fixed_array_index.vv @@ -0,0 +1,4 @@ +a := [1,2]! +println(a[0]) +i := 2 +_ = a[i] diff --git a/vlib/v/tests/inout/fixed_array_slice.out b/vlib/v/tests/inout/fixed_array_slice.out new file mode 100644 index 0000000000..d98ba7af1f --- /dev/null +++ b/vlib/v/tests/inout/fixed_array_slice.out @@ -0,0 +1,5 @@ +================ V panic ================ + module: builtin + function: slice() + message: array.slice: slice bounds out of range (3 >= 2) + file: vlib/builtin/array.v diff --git a/vlib/v/tests/inout/fixed_array_slice.vv b/vlib/v/tests/inout/fixed_array_slice.vv new file mode 100644 index 0000000000..56dc0e9f88 --- /dev/null +++ b/vlib/v/tests/inout/fixed_array_slice.vv @@ -0,0 +1,3 @@ +a := [1,2]! +i := 3 +_ = a[i..i]