From 843de10442a566b0a045b0c72eeffe0b8ef7ea25 Mon Sep 17 00:00:00 2001 From: Delyan Angelov Date: Tue, 16 Feb 2021 15:08:01 +0200 Subject: [PATCH] parser,gen: fix `arr << map[key] using map_get_and_set_1, leading to double free --- vlib/builtin/array_test.v | 4 +++- vlib/v/ast/ast.v | 17 +++++++++++++++-- vlib/v/checker/checker.v | 18 ++++++++++++++++++ vlib/v/gen/c/cgen.v | 12 ++++++++++-- vlib/v/parser/pratt.v | 6 ++++++ vlib/v/tests/nested_map_test.v | 34 ++++++++++++++++++---------------- 6 files changed, 70 insertions(+), 21 deletions(-) diff --git a/vlib/builtin/array_test.v b/vlib/builtin/array_test.v index 086fb7a783..ab4718cc3a 100644 --- a/vlib/builtin/array_test.v +++ b/vlib/builtin/array_test.v @@ -1092,7 +1092,9 @@ fn test_push_arr_string_free() { mut lines := ['hi'] s := 'a' + 'b' lines << s - s.free() // make sure the data in the array is valid after freeing the string + // make sure the data in the array is valid after freeing the string + unsafe { s.free() } + // println(lines) assert lines.len == 2 assert lines[0] == 'hi' diff --git a/vlib/v/ast/ast.v b/vlib/v/ast/ast.v index 21d2ea293f..2ef61268a9 100644 --- a/vlib/v/ast/ast.v +++ b/vlib/v/ast/ast.v @@ -110,12 +110,12 @@ pub: pub struct SelectorExpr { pub: pos token.Position - expr Expr // expr.field_name field_name string is_mut bool // is used for the case `if mut ident.selector is MyType {`, it indicates if the root ident is mutable mut_pos token.Position next_token token.Kind pub mut: + expr Expr // expr.field_name expr_type table.Type // type of `Foo` in `Foo.bar` typ table.Type // type of the entire thing (`Foo.bar`) name_type table.Type // T in `T.name` or typeof in `typeof(expr).name` @@ -609,12 +609,15 @@ pub mut: pub struct IndexExpr { pub: pos token.Position - left Expr index Expr // [0], RangeExpr [start..end] or map[key] or_expr OrExpr pub mut: + left Expr left_type table.Type // array, map, fixed array is_setter bool + is_map bool + is_array bool + is_farray bool } pub struct IfExpr { @@ -1543,3 +1546,13 @@ pub fn (expr Expr) is_mut_ident() bool { } return false } + +// helper for dealing with `m[k1][k2][k3][k3] = value` +pub fn (mut lx IndexExpr) recursive_mapset_is_setter(val bool) { + lx.is_setter = val + if mut lx.left is IndexExpr { + if lx.left.is_map { + lx.left.recursive_mapset_is_setter(val) + } + } +} diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index 33f16a8e9c..22e778be9d 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -2695,6 +2695,12 @@ pub fn (mut c Checker) assign_stmt(mut assign_stmt ast.AssignStmt) { } } else { + if mut left is ast.IndexExpr { + // eprintln('>>> left.is_setter: ${left.is_setter:10} | left.is_map: ${left.is_map:10} | left.is_array: ${left.is_array:10}') + if left.is_map && left.is_setter { + left.recursive_mapset_is_setter(true) + } + } if is_decl { c.error('non-name `$left` on left side of `:=`', left.position()) } @@ -5223,6 +5229,18 @@ pub fn (mut c Checker) index_expr(mut node ast.IndexExpr) table.Type { mut typ := c.expr(node.left) node.left_type = typ typ_sym := c.table.get_final_type_symbol(typ) + match typ_sym.kind { + .map { + node.is_map = true + } + .array { + node.is_array = true + } + .array_fixed { + node.is_farray = true + } + else {} + } if typ_sym.kind !in [.array, .array_fixed, .string, .map] && !typ.is_ptr() && typ !in [table.byteptr_type, table.charptr_type] && !typ.has_flag(.variadic) { c.error('type `$typ_sym.name` does not support indexing', node.pos) diff --git a/vlib/v/gen/c/cgen.v b/vlib/v/gen/c/cgen.v index 12e46e0962..6c654c631f 100644 --- a/vlib/v/gen/c/cgen.v +++ b/vlib/v/gen/c/cgen.v @@ -4373,7 +4373,11 @@ fn (mut g Gen) index_expr(node ast.IndexExpr) { g.is_array_set = true g.write('map_set_1(') } else { - g.write('(*(($elem_type_str*)map_get_and_set_1(') + if node.is_setter { + g.write('(*(($elem_type_str*)map_get_and_set_1(') + } else { + g.write('(*(($elem_type_str*)map_get_1(') + } } if !left_is_ptr || node.left_type.has_flag(.shared_f) { g.write('&') @@ -4409,7 +4413,11 @@ fn (mut g Gen) index_expr(node ast.IndexExpr) { || g.inside_map_index || (g.is_assign_lhs && !g.is_array_set && get_and_set_types) { zero := g.type_default(info.value_type) - g.write('(*($elem_type_str*)map_get_and_set_1(') + if node.is_setter { + g.write('(*($elem_type_str*)map_get_and_set_1(') + } else { + g.write('(*($elem_type_str*)map_get_1(') + } if !left_is_ptr { g.write('&') } diff --git a/vlib/v/parser/pratt.v b/vlib/v/parser/pratt.v index 7640fc8d47..70afeddfe1 100644 --- a/vlib/v/parser/pratt.v +++ b/vlib/v/parser/pratt.v @@ -348,6 +348,9 @@ pub fn (mut p Parser) expr_with_left(left ast.Expr, precedence int, is_stmt_iden p.next() right := p.expr(precedence - 1) pos.update_last_line(p.prev_tok.line_nr) + if mut node is ast.IndexExpr { + node.recursive_mapset_is_setter(true) + } node = ast.InfixExpr{ left: node right: right @@ -381,6 +384,9 @@ pub fn (mut p Parser) expr_with_left(left ast.Expr, precedence int, is_stmt_iden p.error_with_pos('$p.tok must be on the same line as the previous token', p.tok.position()) } + if mut node is ast.IndexExpr { + node.recursive_mapset_is_setter(true) + } node = ast.PostfixExpr{ op: p.tok.kind expr: node diff --git a/vlib/v/tests/nested_map_test.v b/vlib/v/tests/nested_map_test.v index 99e92881af..3cf12bc1dd 100644 --- a/vlib/v/tests/nested_map_test.v +++ b/vlib/v/tests/nested_map_test.v @@ -1,14 +1,4 @@ -struct Foo { -mut: - name string -} - -fn test_nested_maps() { - if true { - } - // - else { - } +fn test_map_of_map() { mut x := map[string]map[string]int{} x['a'] = map[string]int{} assert x['a']['b'] == 0 @@ -16,6 +6,9 @@ fn test_nested_maps() { assert x['a']['b'] == 5 x['a']['b'] = 7 assert x['a']['b'] == 7 +} + +fn test_map_of_map_of_map() { mut y := map[string]map[string]map[string]int{} y['a'] = map[string]map[string]int{} y['a']['b'] = map[string]int{} @@ -24,9 +17,18 @@ fn test_nested_maps() { assert y['a']['b']['c'] == 5 y['a']['b']['c'] = 7 assert y['a']['b']['c'] == 7 - mut foos := map[string]map[string]Foo{} - foos['a']['b'] = Foo{'bar'} - assert foos['a']['b'].name == 'bar' - foos['a']['b'].name = 'baz' - assert foos['a']['b'].name == 'baz' +} + +struct Foo { +mut: + name string +} + +fn test_map_of_map_to_struct() { + mut foos := map[string]map[string]Foo{} + foos['zza']['zzb'] = Foo{'bar'} + assert foos['zza']['zzb'].name == 'bar' + // + foos['zza']['zzb'].name = 'baz' + assert foos['zza']['zzb'].name == 'baz' }