From c4212c46492e577a15e8d9e524af7a190a3c7bdc Mon Sep 17 00:00:00 2001 From: Christoffer Lerno Date: Mon, 10 Feb 2025 00:39:02 +0100 Subject: [PATCH] - Test runner will also check for leaks. - `write` of qoi would leak memory. - Issue when having an empty `Path` or just "." - `set_env` would leak memory. --- lib/std/compression/qoi.c3 | 2 +- lib/std/core/runtime_test.c3 | 28 ++++++++++++----------- lib/std/io/path.c3 | 8 +++++-- lib/std/os/env.c3 | 2 +- releasenotes.md | 4 ++++ test/unit/stdlib/collections/copy_map.c3 | 6 ++++- test/unit/stdlib/compression/qoi.c3 | 9 ++++---- test/unit/stdlib/core/dstring.c3 | 3 +++ test/unit/stdlib/core/string.c3 | 6 +++++ test/unit/stdlib/io/bytebuffer.c3 | 2 +- test/unit/stdlib/io/bytestream.c3 | 1 + test/unit/stdlib/io/dstringstream.c3 | 2 ++ test/unit/stdlib/io/path.c3 | 23 ++++++++++++------- test/unit/stdlib/io/scanner.c3 | 1 + test/unit/stdlib/io/stream.c3 | 2 ++ test/unit/stdlib/sort/countingsort.c3 | 2 ++ test/unit/stdlib/string.c3 | 1 + test/unit/stdlib/threads/mutex.c3 | 4 ++-- test/unit/stdlib/threads/pool.c3 | 6 ++--- test/unit/stdlib/threads/simple_thread.c3 | 10 ++++---- 20 files changed, 81 insertions(+), 41 deletions(-) diff --git a/lib/std/compression/qoi.c3 b/lib/std/compression/qoi.c3 index f02847d90..4d55d6290 100644 --- a/lib/std/compression/qoi.c3 +++ b/lib/std/compression/qoi.c3 @@ -74,7 +74,7 @@ import std::io; fn usz! write(String filename, char[] input, QOIDesc* desc) => @pool() { // encode data - char[] output = new_encode(input, desc)!; + char[] output = new_encode(input, desc, allocator: allocator::temp())!; // open file File! f = file::open(filename, "wb"); diff --git a/lib/std/core/runtime_test.c3 b/lib/std/core/runtime_test.c3 index cc0b23bad..9bb110232 100644 --- a/lib/std/core/runtime_test.c3 +++ b/lib/std/core/runtime_test.c3 @@ -256,13 +256,12 @@ fn bool run_tests(String[] args, TestUnit[] tests) @private name.appendf("Testing %s ", unit.name); name.append_repeat('.', max_name - unit.name.len + 2); io::printf("%s ", name.str_view()); - + TrackingAllocator mem; + mem.init(allocator::heap()); if (libc::setjmp(&context.buf) == 0) { mute_output(); - TrackingAllocator mem; - mem.init(allocator::heap()); - bool has_leaks; + mem.clear(); mem::@scoped(&mem) { $if(!$$OLD_TEST): @@ -274,22 +273,25 @@ fn bool run_tests(String[] args, TestUnit[] tests) @private continue; } $endif - has_leaks = mem.has_leaks(); }; - mem.free(); unmute_output(false); // all good, discard output - - io::printf(test_context.has_ansi_codes ? "[\e[0;32mPASS\e[0m]" : "[PASS]"); - tests_passed++; - - if (has_leaks) io::print(" LEAKS!"); - io::printn(); - + if (mem.has_leaks()) + { + io::print(test_context.has_ansi_codes ? "[\e[0;31mFAIL\e[0m]" : "[FAIL]"); + io::printn(" LEAKS DETECTED!"); + mem.print_report(); + } + else + { + io::printfn(test_context.has_ansi_codes ? "[\e[0;32mPASS\e[0m]" : "[PASS]"); + tests_passed++; + } if (test_context.teardown_fn) { test_context.teardown_fn(); } } + mem.free(); } io::printfn("\n%d test%s run.\n", test_count-tests_skipped, test_count > 1 ? "s" : ""); diff --git a/lib/std/io/path.c3 b/lib/std/io/path.c3 index 0b7af3aa3..aa76d9a3e 100644 --- a/lib/std/io/path.c3 +++ b/lib/std/io/path.c3 @@ -395,7 +395,7 @@ fn Path! Path.parent(self) fn String! normalize(String path_str, PathEnv path_env = DEFAULT_PATH_ENV) { - if (!path_str.len) return ""; + if (!path_str.len) return path_str; usz path_start = volume_name_len(path_str, path_env)!; if (path_start > 0 && path_env == PathEnv.WIN32) { @@ -506,7 +506,11 @@ fn String! normalize(String path_str, PathEnv path_env = DEFAULT_PATH_ENV) if (len > path_start + 1 && is_separator(path_str[len - 1], path_env)) len--; if (path_str.len > len) path_str.ptr[len] = 0; // Empty path after normalization -> "." - if (!len) return "."; + if (!len) + { + path_str[0] = '.'; + return path_str[:1]; + } return path_str[:len]; } diff --git a/lib/std/os/env.c3 b/lib/std/os/env.c3 index 31e7e5323..c4b23e88d 100644 --- a/lib/std/os/env.c3 +++ b/lib/std/os/env.c3 @@ -57,7 +57,7 @@ fn bool set_var(String name, String value, bool overwrite = true) => @pool() // https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-setenvironmentvariable return (win32::setEnvironmentVariableW(wname, value.to_temp_wstring()) ?? 1) == 0; $case env::LIBC && !env::WIN32: - return libc::setenv(name.zstr_tcopy(), value.zstr_copy(), (int)overwrite) == 0; + return libc::setenv(name.zstr_tcopy(), value.zstr_tcopy(), (int)overwrite) == 0; $default: return false; $endswitch diff --git a/releasenotes.md b/releasenotes.md index c8fe1018b..e5083a2c2 100644 --- a/releasenotes.md +++ b/releasenotes.md @@ -20,6 +20,7 @@ - Cleaner error message when missing comma in struct initializer #1941. - Distinct inline void causes unexpected error if used in slice #1946. - Allow `fn int test() => @pool() { return 1; }` short function syntax usage #1906. +- Test runner will also check for leaks. ### Fixes - Fix issue requiring prefix on a generic interface declaration. @@ -54,6 +55,9 @@ - Assert when using optional as init or inc part in a for loop #1942. - Fix bigint hex parsing #1945. - `bigint::from_int(0)` throws assertion #1944. +- `write` of qoi would leak memory. +- Issue when having an empty `Path` or just "." +- `set_env` would leak memory. ### Stdlib changes - Added '%h' and '%H' for printing out binary data in hexadecimal using the formatter. diff --git a/test/unit/stdlib/collections/copy_map.c3 b/test/unit/stdlib/collections/copy_map.c3 index 9a50f28e7..abb70709b 100644 --- a/test/unit/stdlib/collections/copy_map.c3 +++ b/test/unit/stdlib/collections/copy_map.c3 @@ -7,6 +7,7 @@ fn void copy_map() @test { TrackingAllocator alloc; alloc.init(allocator::heap()); + defer alloc.free(); assert(alloc.allocated() == 0); mem::@scoped(&alloc) { @@ -39,7 +40,8 @@ fn void copy_map() @test fn void copy_keys() @test { String[] y; - @pool() { + @pool() + { IntMap x; x.temp_init(); x.set("hello", 0); // keys copied into temp hashmap @@ -47,4 +49,6 @@ fn void copy_keys() @test // end of pool: hashmap and its copied-in keys dropped }; assert(y == {"hello"}); + foreach(key : y) free(key); + free(y); } diff --git a/test/unit/stdlib/compression/qoi.c3 b/test/unit/stdlib/compression/qoi.c3 index c617edc87..a3f8a8a9a 100644 --- a/test/unit/stdlib/compression/qoi.c3 +++ b/test/unit/stdlib/compression/qoi.c3 @@ -9,18 +9,21 @@ const char[?] TEST_QOI_DATA = b64"cW9pZgAAAVQAAACpBABV/f39/f39/f39/f39/f39/f39/f fn void test_qoi_all() { - @pool() { + @pool() + { // create a test descriptor QOIDesc test_desc; // decode the test data char[] decoded = qoi::new_decode(TEST_QOI_DATA[..], &test_desc)!!; + defer free(decoded); + assert(test_desc.width == 340 && test_desc.height == 169, "Expected resolution of 340x169"); // encode the decoded data char[] encoded = qoi::new_encode(decoded, &test_desc)!!; assert(encoded == TEST_QOI_DATA[..], "Encoder output should match the test data"); - + defer free(encoded); // encode and write the decoded data to a file usz written = qoi::write("unittest.qoi", decoded, &test_desc)!!; @@ -30,8 +33,6 @@ fn void test_qoi_all() // cleanup file::delete("unittest.qoi")!!; - free(encoded); - free(decoded); free(read); }; } diff --git a/test/unit/stdlib/core/dstring.c3 b/test/unit/stdlib/core/dstring.c3 index 654ff0afc..9c42c60c5 100644 --- a/test/unit/stdlib/core/dstring.c3 +++ b/test/unit/stdlib/core/dstring.c3 @@ -22,6 +22,7 @@ fn void test_reverse() assert(a.str_view() == "abcde"); a.reverse(); assert(a.str_view() == "edcba"); + a.free(); } fn void test_delete() @@ -31,6 +32,7 @@ fn void test_delete() d.append("Hello cruel world."); d.delete_range(5, 10); assert(d.str_view() == "Hello world."); + d.free(); } DString d; d.append("Hello cruel world."); @@ -44,6 +46,7 @@ fn void test_delete() assert(d.str_view() == "llo crul worl"); d.delete(0, 1); assert(d.str_view() == "lo crul worl"); + d.free(); } fn void test_append() diff --git a/test/unit/stdlib/core/string.c3 b/test/unit/stdlib/core/string.c3 index 396ebf8d4..a7805e2aa 100644 --- a/test/unit/stdlib/core/string.c3 +++ b/test/unit/stdlib/core/string.c3 @@ -103,10 +103,12 @@ fn void test_split() assert(strings[2] == ""); assert(strings[3] == "c"); assert(strings[4] == ""); + free(strings); strings = test.split("|", 2); assert(strings.len == 2); assert(strings[0] == "abc"); assert(strings[1] == "b||c|"); + free(strings); } fn void test_split_skip_empty() @@ -117,10 +119,12 @@ fn void test_split_skip_empty() assert(strings[0] == "abc"); assert(strings[1] == "b"); assert(strings[2] == "c"); + free(strings); strings = test.split("|", 2, skip_empty: true); assert(strings.len == 2); assert(strings[0] == "abc"); assert(strings[1] == "b||c|"); + free(strings); } fn void test_split_to_buffer_skip_empty() @@ -136,6 +140,7 @@ fn void test_split_to_buffer_skip_empty() assert(strings.len == 2); assert(strings[0] == "abc"); assert(strings[1] == "b||c|"); + free(strings); } fn void test_split_to_buffer() @@ -155,6 +160,7 @@ fn void test_split_to_buffer() assert(strings.len == 2); assert(strings[0] == "abc"); assert(strings[1] == "b||c|"); + free(strings); } fn void test_index_of() diff --git a/test/unit/stdlib/io/bytebuffer.c3 b/test/unit/stdlib/io/bytebuffer.c3 index cc84ed0b5..0f8abe2ea 100644 --- a/test/unit/stdlib/io/bytebuffer.c3 +++ b/test/unit/stdlib/io/bytebuffer.c3 @@ -5,7 +5,7 @@ fn void write_read() { ByteBuffer buffer; buffer.new_init(0)!!; - + defer buffer.free(); buffer.write("hello")!!; char[8] bytes; diff --git a/test/unit/stdlib/io/bytestream.c3 b/test/unit/stdlib/io/bytestream.c3 index 64cc5ca43..3c9384090 100644 --- a/test/unit/stdlib/io/bytestream.c3 +++ b/test/unit/stdlib/io/bytestream.c3 @@ -13,6 +13,7 @@ fn void bytestream() assert((String)buffer[:len] == "abc"); ByteWriter w; w.new_init(); + defer (void)w.destroy(); OutStream ws = &w; ws.write("helloworld")!!; assert(w.str_view() == "helloworld"); diff --git a/test/unit/stdlib/io/dstringstream.c3 b/test/unit/stdlib/io/dstringstream.c3 index b8f095c2e..adc101bc1 100644 --- a/test/unit/stdlib/io/dstringstream.c3 +++ b/test/unit/stdlib/io/dstringstream.c3 @@ -4,10 +4,12 @@ fn void test_writing() { DString foo; foo.new_init(); + defer foo.free(); OutStream s = &foo; s.write("hello")!!; s.write_byte('-')!!; s.write("what?-------------------------------------------------------")!!; + ByteReader r; String test_str = "2134"; io::copy_to(r.init(test_str), s)!!; diff --git a/test/unit/stdlib/io/path.c3 b/test/unit/stdlib/io/path.c3 index 2edada696..7bb8d85d5 100644 --- a/test/unit/stdlib/io/path.c3 +++ b/test/unit/stdlib/io/path.c3 @@ -3,9 +3,11 @@ module std::io::path @test; fn void test_dot() { Path p = path::new(".")!!; + defer p.free(); assert(@catch(p.parent())); // It must be possible to form the absolute version. Path p2 = p.new_absolute()!!; + p2.free(); p2 = p.new_append("/hello/world")!!; if (p2.env == POSIX) { @@ -15,21 +17,26 @@ fn void test_dot() { assert(p2.str_view() == `hello\world`); } + p2.free(); } fn void test_parent() { Path p = path::new("")!!; assert(@catch(p.parent())); + p.free(); p = path::new("/", path_env: PathEnv.POSIX)!!; assert(@catch(p.parent())); + p.free(); p = path::new("/a/b/c", path_env: PathEnv.POSIX)!!; assert(p.parent().str_view()!! == "/a/b"); + p.free(); p = path::new("/a/b/c", path_env: PathEnv.WIN32)!!; assert(p.parent().str_view()!! == `\a\b`); + p.free(); } -fn void test_path_normalized() +fn void test_path_normalized() => mem::@scoped(allocator::temp()) { assert(path::new("", path_env: PathEnv.WIN32).str_view()!! == ""); assert(@catch(path::new("1:\\a\\b\\c.txt", path_env: PathEnv.WIN32))); @@ -205,7 +212,7 @@ fn void test_path_normalized() } -fn void test_extension() +fn void test_extension() => mem::@scoped(allocator::temp()) { assert(@catch(path::new(`C:`, path_env: PathEnv.WIN32).extension())); assert(@catch(path::new(`C:`, path_env: PathEnv.POSIX).extension())); @@ -239,7 +246,7 @@ fn void test_extension() } -fn void test_has_extension() +fn void test_has_extension() => mem::@scoped(allocator::temp()) { assert(!path::new(`C:\temp\foo.bar\README`, path_env: PathEnv.WIN32)!!.has_extension(`bar\README`)); @@ -269,7 +276,7 @@ fn void test_has_extension() } -fn void test_basename() +fn void test_basename() => mem::@scoped(allocator::temp()) { assert(path::new_windows("file.txt").basename()!! == "file.txt"); assert(path::new_posix("file.txt").basename()!! == "file.txt"); @@ -296,7 +303,7 @@ fn void test_basename() assert(path::new_posix(`\\server\abc`).basename()!! == `\\server\abc`); } -fn void test_dirname() +fn void test_dirname() => mem::@scoped(allocator::temp()) { assert(path::new_posix("").dirname()!! == "."); assert(path::new_posix("/file").dirname()!! == "/"); @@ -336,7 +343,7 @@ fn void test_dirname() assert(path::new_posix(`\\server\`).dirname()!! == `.`); } -fn void test_path_volume() +fn void test_path_volume() => mem::@scoped(allocator::temp()) { assert(path::new_windows(`C:\abs`).volume_name()!! == `C:`); assert(path::new_windows(`C:abs`).volume_name()!! == `C:`); @@ -346,7 +353,7 @@ fn void test_path_volume() assert(path::new_windows(`\\server\foo\abc`).volume_name()!! == `\\server\foo`); } -fn void test_path_is_absolute() +fn void test_path_is_absolute() => mem::@scoped(allocator::temp()) { assert(!path::new_posix("").is_absolute()!!); assert(path::new_posix("/").is_absolute()!!); @@ -360,7 +367,7 @@ fn void test_path_is_absolute() assert(path::new_windows(`\\server\foo\abc`).is_absolute()!!); } -fn void test_path_absolute() +fn void test_path_absolute() => mem::@scoped(allocator::temp()) { $if env::WIN32: assert(path::new_windows(`C:\abs`).new_absolute()!!.str_view() == `C:\abs`); diff --git a/test/unit/stdlib/io/scanner.c3 b/test/unit/stdlib/io/scanner.c3 index 62fed9402..726a3a7a7 100644 --- a/test/unit/stdlib/io/scanner.c3 +++ b/test/unit/stdlib/io/scanner.c3 @@ -27,6 +27,7 @@ fn void scanner() sc.init(&br, buffer[..]); Results results; + defer results.free(); while LOOP: (true) { char[]! res = sc.scan(",,"); diff --git a/test/unit/stdlib/io/stream.c3 b/test/unit/stdlib/io/stream.c3 index 63f2219e8..14e960d66 100644 --- a/test/unit/stdlib/io/stream.c3 +++ b/test/unit/stdlib/io/stream.c3 @@ -77,6 +77,7 @@ fn void read_tiny_bytearray_test() ByteReader reader = io::wrap_bytes(&&x'07aabbcc00112233'); char[] read = io::read_tiny_bytearray(&reader, allocator: allocator::heap())!!; assert(read == &&x'aabbcc00112233'); + free(read); } fn void read_short_bytearray_test() @@ -84,4 +85,5 @@ fn void read_short_bytearray_test() ByteReader reader = io::wrap_bytes(&&x'0007aabbcc00112233'); char[] read = io::read_short_bytearray(&reader, allocator: allocator::heap())!!; assert(read == &&x'aabbcc00112233'); + free(read); } diff --git a/test/unit/stdlib/sort/countingsort.c3 b/test/unit/stdlib/sort/countingsort.c3 index 8da53d9d1..a43997918 100644 --- a/test/unit/stdlib/sort/countingsort.c3 +++ b/test/unit/stdlib/sort/countingsort.c3 @@ -95,4 +95,6 @@ module sort_test @test; sort::countingsort(list, &sort::key_int_value); assert(check::int_ascending_sort(list.array_view())); + list.free(); + } \ No newline at end of file diff --git a/test/unit/stdlib/string.c3 b/test/unit/stdlib/string.c3 index 59fa311e8..d5582d087 100644 --- a/test/unit/stdlib/string.c3 +++ b/test/unit/stdlib/string.c3 @@ -3,6 +3,7 @@ module string_test; fn void test_clear() @test { DString s = dstring::new_with_capacity(32); + defer s.free(); assert(s.len() == 0); assert(s.capacity() == 32); s.append_repeat('x', 63); diff --git a/test/unit/stdlib/threads/mutex.c3 b/test/unit/stdlib/threads/mutex.c3 index f677cc964..ebe3e66a0 100644 --- a/test/unit/stdlib/threads/mutex.c3 +++ b/test/unit/stdlib/threads/mutex.c3 @@ -77,7 +77,7 @@ fn void shared_mutex_decrement(ArgsWrapper1* args) args.m.unlock()!!; } -fn void shared_mutex() @test +fn void shared_mutex() @test => mem::@scoped(&allocator::LIBC_ALLOCATOR) { Mutex m; m.init()!!; @@ -127,7 +127,7 @@ fn void acquire_recursively(RecursiveMutex* m) } } -fn void test_recursive_mutex() @test +fn void test_recursive_mutex() @test => mem::@scoped(&allocator::LIBC_ALLOCATOR) { RecursiveMutex m; m.init()!!; diff --git a/test/unit/stdlib/threads/pool.c3 b/test/unit/stdlib/threads/pool.c3 index decd54044..80b06f939 100644 --- a/test/unit/stdlib/threads/pool.c3 +++ b/test/unit/stdlib/threads/pool.c3 @@ -5,7 +5,7 @@ import std::thread::pool; def Pool = ThreadPool(<4>); -fn void init_destroy() @test +fn void init_destroy() @test => mem::@scoped(&allocator::LIBC_ALLOCATOR) { for (usz i = 0; i < 20; i++) { @@ -15,7 +15,7 @@ fn void init_destroy() @test } } -fn void push_destroy() @test +fn void push_destroy() @test => mem::@scoped(&allocator::LIBC_ALLOCATOR) { for FOO: (usz i = 0; i < 20; i++) { @@ -42,7 +42,7 @@ fn void push_destroy() @test } } -fn void push_stop() @test +fn void push_stop() @test => mem::@scoped(&allocator::LIBC_ALLOCATOR) { for (usz i = 0; i < 20; i++) { diff --git a/test/unit/stdlib/threads/simple_thread.c3 b/test/unit/stdlib/threads/simple_thread.c3 index 02820d279..b391281bc 100644 --- a/test/unit/stdlib/threads/simple_thread.c3 +++ b/test/unit/stdlib/threads/simple_thread.c3 @@ -3,7 +3,7 @@ import std::io; int a; -fn void testrun() @test +fn void testrun() @test => mem::@scoped(&allocator::LIBC_ALLOCATOR) { Thread t; a = 0; @@ -18,7 +18,7 @@ fn void testrun() @test Mutex m_global; -fn void testrun_mutex() @test +fn void testrun_mutex() @test => mem::@scoped(&allocator::LIBC_ALLOCATOR) { Thread[20] ts; a = 0; @@ -48,7 +48,7 @@ fn void testrun_mutex() @test m_global.destroy()!!; } -fn void testrun_mutex_try() @test +fn void testrun_mutex_try() @test => mem::@scoped(&allocator::LIBC_ALLOCATOR) { Mutex m; m.init()!!; @@ -59,7 +59,7 @@ fn void testrun_mutex_try() @test m.unlock()!!; } -fn void testrun_mutex_timeout() @test +fn void testrun_mutex_timeout() @test => mem::@scoped(&allocator::LIBC_ALLOCATOR) { TimedMutex m; m.init()!!; @@ -80,7 +80,7 @@ fn void call_once() x_once += 100; } -fn void testrun_once() @test +fn void testrun_once() @test => mem::@scoped(&allocator::LIBC_ALLOCATOR) { OnceFlag once; once.call(&call_once);