From 34d39ccb6496cdbc77b276c116b11ed71cbc33d7 Mon Sep 17 00:00:00 2001 From: Delyan Angelov Date: Fri, 13 Aug 2021 18:37:34 +0300 Subject: [PATCH] builtin: fix leak in rune.str(), fix leaks in most assert x == y statements in tests (#11091) --- vlib/builtin/builtin.v | 17 ++++++ vlib/builtin/utf8.v | 25 ++++---- vlib/clipboard/clipboard_darwin.c.v | 2 +- vlib/encoding/base64/base64_memory_test.v | 8 ++- vlib/term/term.v | 6 +- vlib/v/ast/str.v | 9 +-- vlib/v/gen/c/assert.v | 23 ++++++++ vlib/v/gen/c/cgen.v | 2 +- vlib/v/gen/c/fn.v | 4 +- vlib/v/markused/markused.v | 2 + vlib/v/preludes/tests_assertions.v | 2 + vlib/v/tests/valgrind/base64.v | 59 +++++++++++++++++++ .../buffer_passed_in_fn_that_uses_tos_on_it.v | 7 +++ vlib/v/tests/valgrind/rune_str_method.v | 4 ++ vlib/v/tests/valgrind/string_str_method.v | 4 ++ 15 files changed, 152 insertions(+), 22 deletions(-) create mode 100644 vlib/v/tests/valgrind/base64.v create mode 100644 vlib/v/tests/valgrind/buffer_passed_in_fn_that_uses_tos_on_it.v create mode 100644 vlib/v/tests/valgrind/rune_str_method.v create mode 100644 vlib/v/tests/valgrind/string_str_method.v diff --git a/vlib/builtin/builtin.v b/vlib/builtin/builtin.v index beeb20ed71..188e28dbb7 100644 --- a/vlib/builtin/builtin.v +++ b/vlib/builtin/builtin.v @@ -55,6 +55,23 @@ pub: rvalue string // the stringified *actual value* of the right side of a failed assertion } +// free is used to free the memory occupied by the assertion meta data. +// It is called by cb_assertion_failed, and cb_assertion_ok in the preludes, +// once they are done with reporting/formatting the meta data. +[manualfree; unsafe] +pub fn (ami &VAssertMetaInfo) free() { + unsafe { + ami.fpath.free() + ami.fn_name.free() + ami.src.free() + ami.op.free() + ami.llabel.free() + ami.rlabel.free() + ami.lvalue.free() + ami.rvalue.free() + } +} + fn __print_assert_failure(i &VAssertMetaInfo) { eprintln('$i.fpath:${i.line_nr + 1}: FAIL: fn $i.fn_name: assert $i.src') if i.op.len > 0 && i.op != 'call' { diff --git a/vlib/builtin/utf8.v b/vlib/builtin/utf8.v index 6f3745a1a1..839c249d26 100644 --- a/vlib/builtin/utf8.v +++ b/vlib/builtin/utf8.v @@ -12,34 +12,38 @@ pub fn utf8_char_len(b byte) int { pub fn utf32_to_str(code u32) string { unsafe { mut buffer := malloc_noscan(5) - return utf32_to_str_no_malloc(code, buffer) + res := utf32_to_str_no_malloc(code, buffer) + if res.len == 0 { + // the buffer was not used at all + free(buffer) + } + return res } } -[unsafe] -pub fn utf32_to_str_no_malloc(code u32, buf voidptr) string { - icode := int(code) // Prevents doing casts everywhere - mut res := '' +[manualfree; unsafe] +pub fn utf32_to_str_no_malloc(code u32, buf &byte) string { unsafe { + icode := int(code) // Prevents doing casts everywhere mut buffer := &byte(buf) if icode <= 127 { // 0x7F buffer[0] = byte(icode) buffer[1] = 0 - res = tos(buffer, 1) + return tos(buffer, 1) } else if icode <= 2047 { // 0x7FF buffer[0] = 192 | byte(icode >> 6) // 0xC0 - 110xxxxx buffer[1] = 128 | byte(icode & 63) // 0x80 - 0x3F - 10xxxxxx buffer[2] = 0 - res = tos(buffer, 2) + return tos(buffer, 2) } else if icode <= 65535 { // 0xFFFF buffer[0] = 224 | byte(icode >> 12) // 0xE0 - 1110xxxx buffer[1] = 128 | (byte(icode >> 6) & 63) // 0x80 - 0x3F - 10xxxxxx buffer[2] = 128 | byte(icode & 63) // 0x80 - 0x3F - 10xxxxxx buffer[3] = 0 - res = tos(buffer, 3) + return tos(buffer, 3) } // 0x10FFFF else if icode <= 1114111 { @@ -48,11 +52,10 @@ pub fn utf32_to_str_no_malloc(code u32, buf voidptr) string { buffer[2] = 128 | (byte(icode >> 6) & 63) // 0x80 - 0x3F - 10xxxxxx buffer[3] = 128 | byte(icode & 63) // 0x80 - 0x3F - 10xxxxxx buffer[4] = 0 - res = tos(buffer, 4) + return tos(buffer, 4) } } - res.is_lit = 1 // let autofree know this string doesn't have to be freed - return res + return '' } // Convert utf8 to utf32 diff --git a/vlib/clipboard/clipboard_darwin.c.v b/vlib/clipboard/clipboard_darwin.c.v index eff5d3b2ad..13c9ee83cf 100644 --- a/vlib/clipboard/clipboard_darwin.c.v +++ b/vlib/clipboard/clipboard_darwin.c.v @@ -60,7 +60,7 @@ pub fn (mut cb Clipboard) get_text() string { return '' } utf8_clip := C.darwin_get_pasteboard_text(cb.pb) - return unsafe { utf8_clip.vstring() } + return unsafe { tos_clone(&byte(utf8_clip)) } } // new_primary returns a new X11 `PRIMARY` type `Clipboard` instance allocated on the heap. diff --git a/vlib/encoding/base64/base64_memory_test.v b/vlib/encoding/base64/base64_memory_test.v index 6bd9f99faf..be543af486 100644 --- a/vlib/encoding/base64/base64_memory_test.v +++ b/vlib/encoding/base64/base64_memory_test.v @@ -6,6 +6,7 @@ fn test_long_encoding() { s_original := []byte{len: input_size, init: `a`} s_encoded := base64.encode(s_original) + s_encoded_bytes := s_encoded.bytes() s_decoded := base64.decode(s_encoded) assert s_encoded.len > s_original.len @@ -13,6 +14,10 @@ fn test_long_encoding() { ebuffer := unsafe { malloc(s_encoded.len) } dbuffer := unsafe { malloc(s_decoded.len) } + defer { + unsafe { free(ebuffer) } + unsafe { free(dbuffer) } + } // encoded_size := base64.encode_in_buffer(s_original, ebuffer) mut encoded_in_buf := []byte{len: encoded_size} @@ -27,7 +32,8 @@ fn test_long_encoding() { assert encoded_in_buf[encoded_size - 3] == `W` assert encoded_in_buf[encoded_size - 2] == `F` assert encoded_in_buf[encoded_size - 1] == `h` - assert encoded_in_buf == s_encoded.bytes() + + assert encoded_in_buf == s_encoded_bytes decoded_size := base64.decode_in_buffer(s_encoded, dbuffer) assert decoded_size == input_size diff --git a/vlib/term/term.v b/vlib/term/term.v index 390b3be31a..460c0b17aa 100644 --- a/vlib/term/term.v +++ b/vlib/term/term.v @@ -181,11 +181,13 @@ fn supports_escape_sequences(fd int) bool { if vcolors_override == 'never' { return false } - if os.getenv('TERM') == 'dumb' { + env_term := os.getenv('TERM') + if env_term == 'dumb' { return false } $if windows { - if os.getenv('ConEmuANSI') == 'ON' { + env_conemu := os.getenv('ConEmuANSI') + if env_conemu == 'ON' { return true } // 4 is enable_virtual_terminal_processing diff --git a/vlib/v/ast/str.v b/vlib/v/ast/str.v index 6364dd5137..b0a16641be 100644 --- a/vlib/v/ast/str.v +++ b/vlib/v/ast/str.v @@ -290,16 +290,17 @@ pub fn (x Expr) str() string { } CallExpr { sargs := args2str(x.args) + propagate_suffix := if x.or_block.kind == .propagate { ' ?' } else { '' } if x.is_method { - return '${x.left.str()}.${x.name}($sargs)' + return '${x.left.str()}.${x.name}($sargs)$propagate_suffix' } if x.name.starts_with('${x.mod}.') { - return util.strip_main_name('${x.name}($sargs)') + return util.strip_main_name('${x.name}($sargs)$propagate_suffix') } if x.mod == '' && x.name == '' { - return x.left.str() + '($sargs)' + return x.left.str() + '($sargs)$propagate_suffix' } - return '${x.mod}.${x.name}($sargs)' + return '${x.mod}.${x.name}($sargs)$propagate_suffix' } CharLiteral { return '`$x.val`' diff --git a/vlib/v/gen/c/assert.v b/vlib/v/gen/c/assert.v index bc20b789f5..fa6f74a393 100644 --- a/vlib/v/gen/c/assert.v +++ b/vlib/v/gen/c/assert.v @@ -101,6 +101,7 @@ fn (mut g Gen) gen_assert_metainfo(node ast.AssertStmt) string { } fn (mut g Gen) gen_assert_single_expr(expr ast.Expr, typ ast.Type) { + // eprintln('> gen_assert_single_expr typ: $typ | expr: $expr | typeof(expr): ${typeof(expr)}') unknown_value := '*unknown value*' match expr { ast.CastExpr, ast.IndexExpr, ast.MatchExpr { @@ -121,7 +122,29 @@ fn (mut g Gen) gen_assert_single_expr(expr ast.Expr, typ ast.Type) { g.write(ctoslit('$sym.name')) } else { + mut should_clone := true + if typ == ast.string_type && expr is ast.StringLiteral { + should_clone = false + } + if expr is ast.CTempVar { + if expr.orig is ast.CallExpr { + should_clone = false + if expr.orig.or_block.kind == .propagate { + should_clone = true + } + if expr.orig.is_method && expr.orig.args.len == 0 + && expr.orig.name == 'type_name' { + should_clone = true + } + } + } + if should_clone { + g.write('string_clone(') + } g.gen_expr_to_string(expr, typ) + if should_clone { + g.write(')') + } } } g.write(' /* typeof: ' + expr.type_name() + ' type: ' + typ.str() + ' */ ') diff --git a/vlib/v/gen/c/cgen.v b/vlib/v/gen/c/cgen.v index 920af7ae82..7d417025cd 100644 --- a/vlib/v/gen/c/cgen.v +++ b/vlib/v/gen/c/cgen.v @@ -3625,7 +3625,7 @@ fn (mut g Gen) typeof_expr(node ast.TypeOf) { if sym.kind == .sum_type { // When encountering a .sum_type, typeof() should be done at runtime, // because the subtype of the expression may change: - g.write('tos3( /* $sym.name */ v_typeof_sumtype_${sym.cname}( (') + g.write('charptr_vstring_literal( /* $sym.name */ v_typeof_sumtype_${sym.cname}( (') g.expr(node.expr) g.write(')._typ ))') } else if sym.kind == .array_fixed { diff --git a/vlib/v/gen/c/fn.v b/vlib/v/gen/c/fn.v index ad724e8d3c..52df03a41e 100644 --- a/vlib/v/gen/c/fn.v +++ b/vlib/v/gen/c/fn.v @@ -745,14 +745,14 @@ fn (mut g Gen) method_call(node ast.CallExpr) { } if left_sym.kind == .sum_type && node.name == 'type_name' { - g.write('tos3( /* $left_sym.name */ v_typeof_sumtype_${typ_sym.cname}( (') + g.write('charptr_vstring_literal( /* $left_sym.name */ v_typeof_sumtype_${typ_sym.cname}( (') g.expr(node.left) dot := if node.left_type.is_ptr() { '->' } else { '.' } g.write(')${dot}_typ ))') return } if left_sym.kind == .interface_ && node.name == 'type_name' { - g.write('tos3( /* $left_sym.name */ v_typeof_interface_${typ_sym.cname}( (') + g.write('charptr_vstring_literal( /* $left_sym.name */ v_typeof_interface_${typ_sym.cname}( (') g.expr(node.left) dot := if node.left_type.is_ptr() { '->' } else { '.' } g.write(')${dot}_typ ))') diff --git a/vlib/v/markused/markused.v b/vlib/v/markused/markused.v index bad2df95a5..f75b0b4c70 100644 --- a/vlib/v/markused/markused.v +++ b/vlib/v/markused/markused.v @@ -49,8 +49,10 @@ pub fn mark_used(mut table ast.Table, pref &pref.Preferences, ast_files []&ast.F // byteptr and charptr '3.vstring', '3.vstring_with_len', + '3.vstring_literal', '4.vstring', '4.vstring_with_len', + '4.vstring_literal', // byte. methods '9.str_escaped', // string. methods diff --git a/vlib/v/preludes/tests_assertions.v b/vlib/v/preludes/tests_assertions.v index 2675cf1fed..254c1c66ff 100644 --- a/vlib/v/preludes/tests_assertions.v +++ b/vlib/v/preludes/tests_assertions.v @@ -62,6 +62,7 @@ fn cb_assertion_failed(i &VAssertMetaInfo) { eprintln(' $final_src') } eprintln('') + unsafe { i.free() } } fn cb_assertion_ok(i &VAssertMetaInfo) { @@ -85,6 +86,7 @@ fn cb_assertion_ok(i &VAssertMetaInfo) { } println('$final_funcname ($final_filepath)') */ + unsafe { i.free() } } fn cb_propagate_test_error(line_nr int, file string, mod string, fn_name string, errmsg string) { diff --git a/vlib/v/tests/valgrind/base64.v b/vlib/v/tests/valgrind/base64.v new file mode 100644 index 0000000000..14acce26a4 --- /dev/null +++ b/vlib/v/tests/valgrind/base64.v @@ -0,0 +1,59 @@ +import encoding.base64 + +fn main() { + repeats := 1000 + input_size := 3000 + + s_original := []byte{len: input_size, init: `a`} + s_encoded := base64.encode(s_original) + s_encoded_bytes := s_encoded.bytes() + s_decoded := base64.decode(s_encoded) + + assert s_encoded.len > s_original.len + assert s_original == s_decoded + + ebuffer := unsafe { malloc(s_encoded.len) } + dbuffer := unsafe { malloc(s_decoded.len) } + defer { + unsafe { free(ebuffer) } + unsafe { free(dbuffer) } + } + // + encoded_size := base64.encode_in_buffer(s_original, ebuffer) + mut encoded_in_buf := []byte{len: encoded_size} + unsafe { C.memcpy(encoded_in_buf.data, ebuffer, encoded_size) } + assert input_size * 4 / 3 == encoded_size + assert encoded_in_buf[0] == `Y` + assert encoded_in_buf[1] == `W` + assert encoded_in_buf[2] == `F` + assert encoded_in_buf[3] == `h` + + assert encoded_in_buf[encoded_size - 4] == `Y` + assert encoded_in_buf[encoded_size - 3] == `W` + assert encoded_in_buf[encoded_size - 2] == `F` + assert encoded_in_buf[encoded_size - 1] == `h` + + assert encoded_in_buf == s_encoded_bytes + + decoded_size := base64.decode_in_buffer(s_encoded, dbuffer) + assert decoded_size == input_size + mut decoded_in_buf := []byte{len: decoded_size} + unsafe { C.memcpy(decoded_in_buf.data, dbuffer, decoded_size) } + assert decoded_in_buf == s_original + + mut s := 0 + for _ in 0 .. repeats { + resultsize := base64.encode_in_buffer(s_original, ebuffer) + s += resultsize + assert resultsize == s_encoded.len + } + + for _ in 0 .. repeats { + resultsize := base64.decode_in_buffer(s_encoded, dbuffer) + s += resultsize + assert resultsize == s_decoded.len + } + + println('Final s: $s') + // assert s == 39147008 +} diff --git a/vlib/v/tests/valgrind/buffer_passed_in_fn_that_uses_tos_on_it.v b/vlib/v/tests/valgrind/buffer_passed_in_fn_that_uses_tos_on_it.v new file mode 100644 index 0000000000..24b5344fd5 --- /dev/null +++ b/vlib/v/tests/valgrind/buffer_passed_in_fn_that_uses_tos_on_it.v @@ -0,0 +1,7 @@ +fn main() { + unsafe { + mut buffer := malloc_noscan(5) + s := utf32_to_str_no_malloc(77, buffer) + println(s) + } +} diff --git a/vlib/v/tests/valgrind/rune_str_method.v b/vlib/v/tests/valgrind/rune_str_method.v new file mode 100644 index 0000000000..e04c78b509 --- /dev/null +++ b/vlib/v/tests/valgrind/rune_str_method.v @@ -0,0 +1,4 @@ +fn main() { + a := `Y`.str() + println(a) +} diff --git a/vlib/v/tests/valgrind/string_str_method.v b/vlib/v/tests/valgrind/string_str_method.v new file mode 100644 index 0000000000..111e6b1316 --- /dev/null +++ b/vlib/v/tests/valgrind/string_str_method.v @@ -0,0 +1,4 @@ +fn main() { + a := 'Y'.str() + println(a) +}