diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 574ec61..69bc1e2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -26,7 +26,7 @@ jobs: sudo apt-get install -y libasound2-dev libgl1-mesa-dev libglu1-mesa-dev libwayland-dev libxkbcommon-dev libegl-dev - wayland-protocols pkg-config + wayland-protocols pkg-config valgrind - name: Cleanup cache folders continue-on-error: true run: | @@ -38,6 +38,16 @@ jobs: set +x - name: Unit tests run: zig build -j1 test + - name: Build unit test binary + run: zig build -j1 -Dvalgrind=true + - name: Valgrind on unit tests + run: | + valgrind --leak-check=full \ + --show-leak-kinds=all \ + --track-origins=yes \ + --error-exitcode=1 \ + --suppressions=valgrind/swindings.supp \ + zig-out/bin/unit_tests - name: Build project run: zig build --summary all -j1 --color off -freference-trace=0 -Doptimize=ReleaseSafe --verbose diff --git a/README.md b/README.md index 6e6525f..4326895 100644 --- a/README.md +++ b/README.md @@ -103,6 +103,44 @@ git submodule update --init --recursive zig build test ``` +### Memory checking + +`WITH_VALGRIND` is injected by passing `-Dvalgrind=true` to the build system. + +Build the binary with `-Dvalgrind=true`: + +```bash +zig build -j1 -Dvalgrind=true +``` + +Then run valgrind against it: + +**On unit tests** + +```bash +valgrind --leak-check=full \ + --show-leak-kinds=all \ + --track-origins=yes \ + --error-exitcode=1 \ + --suppressions=valgrind/swindings.supp \ + zig-out/bin/unit_tests +``` + +**On executable** + +Requires a live Wayland compositor (cannot run in CI). Build with `-Dvalgrind=true`: + +```bash +valgrind --leak-check=full \ + --show-leak-kinds=all \ + --track-origins=yes \ + --error-exitcode=1 \ + --suppressions=valgrind/swindings.supp \ + --log-file=raw.log \ + zig-out/bin/swindings +``` + + ### Generating compile_commands.json ```bash diff --git a/build.zig b/build.zig index f51acfe..ea95aca 100644 --- a/build.zig +++ b/build.zig @@ -8,6 +8,20 @@ const c_flags = [_][]const u8{ "-Wextra", "-pedantic", "-D_POSIX_C_SOURCE=200809L", + "-Wshadow", + "-Wvla", + "-Wfloat-equal", + "-Wdouble-promotion", + "-Wformat=2", + "-Wformat-truncation", + "-Wundef", + "-Wconversion", + "-Wsign-conversion", + "-Wnull-dereference", + "-Wuninitialized", + "-Winit-self", + "-Wstrict-prototypes", + "-Wold-style-definition", }; const src_files = [_][]const u8{ @@ -19,12 +33,29 @@ const src_files = [_][]const u8{ "src/structures.c", "src/theme.c", "src/cli.c", +}; + +const deps_files = [_][]const u8{ "subprojects/tomlc17/src/tomlc17.c", "subprojects/cargs/src/cargs.c", "subprojects/fzy/src/match.c", "subprojects/asprintf/asprintf.c", }; +const debug_sanitizer_flags = [_][]const u8{ + "-fno-omit-frame-pointer", + "-fsanitize=undefined", + "-g3", + // "-fsanitize=leak", # NOTE: valgrind is used instead +}; + +fn combineFlags(allocator: std.mem.Allocator, a: []const []const u8, b: []const []const u8) ![]const []const u8 { + var list: std.ArrayList([]const u8) = .empty; + try list.appendSlice(allocator, a); + try list.appendSlice(allocator, b); + return list.toOwnedSlice(allocator); +} + fn getGitVersion(b: *std.Build, io: std.Io) ![]const u8 { const tag_result = std.process.run( b.allocator, @@ -76,7 +107,7 @@ fn addSystemLibraryPaths(mod: *std.Build.Module, io: std.Io) void { } } -fn copyConfigHeader(b: *std.Build) !std.ArrayList(std.Build.LazyPath) { +fn getIncludePaths(b: *std.Build) !std.ArrayList(std.Build.LazyPath) { const wf = b.addWriteFiles(); _ = wf.addCopyFile( @@ -120,6 +151,7 @@ pub fn build(b: *std.Build) !void { // --- Step 2: Target & Optimization --- const target = b.standardTargetOptions(.{}); const optimize = b.standardOptimizeOption(.{}); + const valgrind = b.option(bool, "valgrind", "Build with WITH_VALGRIND defined") orelse false; // --- Step 3: Dependencies --- const raylib_dep = b.dependency("raylib", .{ @@ -137,6 +169,7 @@ pub fn build(b: *std.Build) !void { .target = target, .optimize = optimize, .link_libc = true, + .sanitize_c = if (optimize == .Debug) .full else .off, }), }); @@ -151,13 +184,18 @@ pub fn build(b: *std.Build) !void { ); // --- Step 5: Config & Include Paths --- - const include_paths = try copyConfigHeader(b); + const include_paths = try getIncludePaths(b); exe.root_module.addCMacro("GIT_VERSION", b.fmt("\"{s}\"", .{version})); + if (valgrind) exe.root_module.addCMacro("WITH_VALGRIND", "1"); addSystemLibraryPaths(exe.root_module, io); addCIncludePaths(exe.root_module, include_paths); + const exe_flags = try combineFlags(b.allocator, &c_flags, if (optimize == .Debug) &debug_sanitizer_flags else &.{}); + defer b.allocator.free(exe_flags); + // --- Step 6: Source Files --- - addCSourceFiles(exe.root_module, &src_files, &c_flags); + addCSourceFiles(exe.root_module, &src_files, exe_flags); + addCSourceFiles(exe.root_module, &deps_files, &c_flags); // --- Step 7: Linking --- exe.root_module.linkLibrary(raylib); @@ -172,6 +210,7 @@ pub fn build(b: *std.Build) !void { .target = target, .optimize = optimize, .link_libc = true, + .sanitize_c = if (optimize == .Debug) .full else .off, }), }); @@ -185,17 +224,23 @@ pub fn build(b: *std.Build) !void { addCIncludePaths(unit_test.root_module, test_include_paths); + const test_flags = try combineFlags(b.allocator, &c_flags, if (optimize == .Debug) &debug_sanitizer_flags else &.{}); + defer b.allocator.free(test_flags); + addCSourceFiles(unit_test.root_module, &[_][]const u8{ "tests/all_test.c", "tests/structures_test.c", "tests/theme_test.c", - "subprojects/unity/src/unity.c", "src/structures.c", "src/theme.c", + }, test_flags); + addCSourceFiles(unit_test.root_module, &[_][]const u8{ + "subprojects/unity/src/unity.c", "subprojects/tomlc17/src/tomlc17.c", "subprojects/asprintf/asprintf.c", }, &c_flags); b.installArtifact(unit_test); + if (valgrind) unit_test.root_module.addCMacro("WITH_VALGRIND", "1"); const run_unit_tests = b.addRunArtifact(unit_test); const test_step = b.step("test", "Run unit tests with Unity"); diff --git a/src/display.c b/src/display.c index 70308f3..0e57c2d 100644 --- a/src/display.c +++ b/src/display.c @@ -8,6 +8,9 @@ #include #include #include +#ifdef WITH_VALGRIND +#include +#endif #define WINDOW_WIDTH 800 #define WINDOW_HEIGHT 600 @@ -396,10 +399,15 @@ void display(const KeyMapList *kml, const theme_t *theme) { theme->bottom.font.size, spacing, "", labels); } EndDrawing(); +#ifdef WITH_VALGRIND + if (RUNNING_ON_VALGRIND) + break; +#endif } free(visible); free_search_result(&search_result); UnloadFont(top_font); UnloadFont(body_font); + UnloadFont(bottom_font); CloseWindow(); } diff --git a/src/theme.c b/src/theme.c index 3884f51..ff7797a 100644 --- a/src/theme.c +++ b/src/theme.c @@ -138,6 +138,8 @@ static theme_color_t color_new_with_defaults(void) { static void font_init_with_defaults(theme_font_t *font) { font->file = NULL; font->size = 16; + font->has_size = false; + font->has_color = false; font->color = (theme_color_t){ .r = 130, .g = 130, .b = 130, .a = 255, .has_alpha = true}; } @@ -256,6 +258,7 @@ theme_toml_parse(const toml_result_t *toml, theme_layer_t *layer, // Parse font file if (font_file.type == TOML_STRING) { + free(font.file); if (strlen(font_file.u.str.ptr) > 0) { font.file = strdup(font_file.u.str.ptr); if (!font.file) diff --git a/tests/theme_test.c b/tests/theme_test.c index 0519191..e30d756 100644 --- a/tests/theme_test.c +++ b/tests/theme_test.c @@ -96,6 +96,7 @@ void test_theme_load_nonexistent_file_creates_it_and_loads_defaults(void) { TEST_ASSERT_EQUAL(16, theme.highlight.font.size); TEST_ASSERT_TRUE(ends_with(theme.highlight.font.file, DEFAULT_FONT_SUFFIX)); + theme_free(&theme); } void test_theme_load_empty_file_returns_all_defaults(void) { @@ -155,6 +156,7 @@ void test_theme_load_empty_file_returns_all_defaults(void) { TEST_ASSERT_EQUAL(16, res.theme.highlight.font.size); TEST_ASSERT_TRUE( ends_with(res.theme.highlight.font.file, DEFAULT_FONT_SUFFIX)); + theme_free(&res.theme); } void test_global_overrides_section_if_not_set(void) { @@ -168,6 +170,7 @@ void test_global_overrides_section_if_not_set(void) { TEST_ASSERT_EQUAL( 20, res.theme.top.font.size); // Should NOT be overridden by global TEST_ASSERT_EQUAL(0x11, res.theme.top.background.color.r); + theme_free(&res.theme); } void test_section_overrides_global(void) { @@ -179,6 +182,7 @@ void test_section_overrides_global(void) { TEST_ASSERT_EQUAL(0xaa, res.theme.top.background.color.r); TEST_ASSERT_NOT_EQUAL(0x11, res.theme.top.background.color.r); + theme_free(&res.theme); } void test_global_alpha_propagates_to_sections(void) { @@ -190,6 +194,8 @@ void test_global_alpha_propagates_to_sections(void) { TEST_ASSERT_EQUAL(128, res.theme.top.background.color.a); TEST_ASSERT_EQUAL(128, res.theme.body.background.color.a); TEST_ASSERT_EQUAL(128, res.theme.bottom.background.color.a); + + theme_free(&res.theme); } void test_section_alpha_propagates_to_body(void) { @@ -200,6 +206,7 @@ void test_section_alpha_propagates_to_body(void) { // Should NOT propagate to body unless explicitly implemented TEST_ASSERT_EQUAL(180, res.theme.body.background.color.a); + theme_free(&res.theme); } typedef struct { @@ -323,6 +330,7 @@ void test_theme_load_various_configs(void) { TEST_ASSERT_TRUE( ends_with(res.theme.top.font.file, DEFAULT_FONT_SUFFIX)); } + theme_free(&res.theme); } } @@ -376,6 +384,7 @@ void test_alpha_applies_to_color_when_missing_alpha(void) { TEST_ASSERT_EQUAL(THEME_SUCCESS, res.error); TEST_ASSERT_EQUAL(128, res.theme.body.background.color.a); TEST_ASSERT_TRUE(res.theme.body.background.color.has_alpha); + theme_free(&res.theme); } void test_font_file_is_duplicated_between_layers(void) { @@ -387,6 +396,7 @@ void test_font_file_is_duplicated_between_layers(void) { TEST_ASSERT_NOT_NULL(res.theme.body.font.file); TEST_ASSERT_NOT_NULL(res.theme.top.font.file); TEST_ASSERT_NOT_EQUAL(res.theme.body.font.file, res.theme.top.font.file); + theme_free(&res.theme); } void test_invalid_type_returns_parsing_error(void) { @@ -395,6 +405,7 @@ void test_invalid_type_returns_parsing_error(void) { delete_temp_file(path); TEST_ASSERT_EQUAL(THEME_PARSING_ERROR, res.error); + theme_free(&res.theme); } void test_invalid_toml_syntax_returns_parsing_error(void) { @@ -404,6 +415,7 @@ void test_invalid_toml_syntax_returns_parsing_error(void) { delete_temp_file(path); TEST_ASSERT_EQUAL(THEME_PARSING_ERROR, res.error); + theme_free(&res.theme); } void test_memory_allocation_failure(void) { @@ -420,6 +432,7 @@ void test_font_file_not_found(void) { TEST_ASSERT_EQUAL(THEME_SUCCESS, res.error); TEST_ASSERT_NOT_NULL(res.theme.body.font.file); + theme_free(&res.theme); } void test_get_config_filepath_with_home(void) { diff --git a/valgrind/swindings.supp b/valgrind/swindings.supp new file mode 100644 index 0000000..b6cff97 --- /dev/null +++ b/valgrind/swindings.supp @@ -0,0 +1,503 @@ +{ + g_thread-reachable + Memcheck:Leak + match-leak-kinds: reachable + ... + fun:g_thread_proxy + fun:start_thread + fun:clone +} + +{ + g_thread-definite + Memcheck:Leak + match-leak-kinds: definite + ... + fun:g_thread_proxy + fun:start_thread + fun:clone +} + +{ + g_thread-possible + Memcheck:Leak + match-leak-kinds: possible + ... + fun:g_thread_proxy + fun:start_thread + fun:clone +} + +{ + g_thread-indirect + Memcheck:Leak + match-leak-kinds: indirect + ... + fun:g_thread_proxy + fun:start_thread + fun:clone +} + +{ + Lost in loss record + Memcheck:Leak + match-leak-kinds: possible + fun:malloc + fun:malloc + fun:allocate_dtv_entry + fun:allocate_and_init + fun:tls_get_addr_tail + fun:_dl_tlsdesc_dynamic_xsave + obj:* + obj:* + obj:/usr/lib64/libstdc++.so* +} + + +{ + glib_all_g_malloc + Memcheck:Leak + match-leak-kinds: reachable + ... + fun:g_malloc +} + +{ + pango-reachable + Memcheck:Leak + match-leak-kinds: reachable + ... + fun:pango_* +} + +{ + pango-reachable + Memcheck:Leak + match-leak-kinds: definite + ... + fun:pango_* +} + +{ + pango-indirect + Memcheck:Leak + match-leak-kinds: indirect + ... + fun:pango_* +} + +{ + pango-indirect + Memcheck:Leak + match-leak-kinds: possible + ... + fun:pango_* +} + +{ + cairo-reachable + Memcheck:Leak + match-leak-kinds: reachable + ... + fun:*cairo_* +} + +{ + cairo-indirect + Memcheck:Leak + match-leak-kinds: indirect + ... + fun:*cairo_* +} + +{ + cairo-possible + Memcheck:Leak + match-leak-kinds: possible + ... + fun:*cairo_* +} + +{ + gdk-reachable + Memcheck:Leak + match-leak-kinds: reachable + ... + fun:*gdk_* +} + +{ + gdk-indirect + Memcheck:Leak + match-leak-kinds: indirect + ... + fun:*gdk_* +} + +{ + gdk-possible + Memcheck:Leak + match-leak-kinds: possible + ... + fun:*gdk_* +} + + +{ + gtk-reachable + Memcheck:Leak + match-leak-kinds: reachable + ... + fun:*gtk_* +} + +{ + gtk-definite + Memcheck:Leak + match-leak-kinds: definite + ... + fun:*gtk_* +} + + +{ + gtk-indirect + Memcheck:Leak + match-leak-kinds: indirect + ... + fun:*gtk_* +} + +{ + gtk-possible + Memcheck:Leak + match-leak-kinds: possible + ... + fun:*gtk_* +} + +{ + dl-reachable + Memcheck:Leak + match-leak-kinds: reachable + ... + fun:_dl_* +} + +{ + dl-indirect + Memcheck:Leak + match-leak-kinds: indirect + ... + fun:_dl_* +} + +{ + dl-possible + Memcheck:Leak + match-leak-kinds: possible + ... + fun:_dl_* +} + +{ + ft-load-reachable + Memcheck:Leak + match-leak-kinds: reachable + ... + fun:FT_Load_* +} + +{ + ft-load-indirect + Memcheck:Leak + match-leak-kinds: indirect + ... + fun:FT_Load_* +} + +{ + ft-load-possible + Memcheck:Leak + match-leak-kinds: possible + ... + fun:FT_Load_* +} + +{ + xmlParse-reachable + Memcheck:Leak + match-leak-kinds: reachable + ... + fun:xmlParse* +} + +{ + xmlParse-indirect + Memcheck:Leak + match-leak-kinds: indirect + ... + fun:xmlParse* +} + +{ + xmlParse-possible + Memcheck:Leak + match-leak-kinds: possible + ... + fun:xmlParse* +} + +{ + *glfw-reachable + Memcheck:Leak + match-leak-kinds: reachable + ... + fun:*glfw* +} + +{ + *glfw-indirect + Memcheck:Leak + match-leak-kinds: indirect + ... + fun:*glfw* +} + +{ + *glfw-possible + Memcheck:Leak + match-leak-kinds: possible + ... + fun:*glfw* +} + + +{ + *glfw-definite + Memcheck:Leak + match-leak-kinds: definite + ... + fun:*glfw* +} + +{ + *rust-reachable + Memcheck:Leak + match-leak-kinds: reachable + ... + fun:*<*::*>* +} + +{ + *rust-indirect + Memcheck:Leak + match-leak-kinds: indirect + ... + fun:*<*::*>* +} + +{ + *rust-possible + Memcheck:Leak + match-leak-kinds: possible + ... + fun:*<*::*>* +} + + +{ + gobj-reachable + Memcheck:Leak + match-leak-kinds: reachable + ... + fun:g_* +} + +{ + gobj-indirect + Memcheck:Leak + match-leak-kinds: indirect + ... + fun:g_* +} + +{ + gobj-possible + Memcheck:Leak + match-leak-kinds: possible + ... + fun:g_* +} +{ + _dbus-reachable + Memcheck:Leak + match-leak-kinds: reachable + ... + fun:_dbus_* +} + +{ + _dbus-indirect + Memcheck:Leak + match-leak-kinds: indirect + ... + fun:_dbus_* +} + +{ + _dbus-possible + Memcheck:Leak + match-leak-kinds: possible + ... + fun:_dbus_* +} + +{ + wl-reachable + Memcheck:Leak + match-leak-kinds: reachable + ... + fun:wl_* +} + +{ + wl-indirect + Memcheck:Leak + match-leak-kinds: indirect + ... + fun:wl_* +} + +{ + wl-possible + Memcheck:Leak + match-leak-kinds: possible + ... + fun:wl_* +} + +{ + af-reachable + Memcheck:Leak + match-leak-kinds: reachable + ... + fun:af_* +} + +{ + af-indirect + Memcheck:Leak + match-leak-kinds: indirect + ... + fun:af_* +} + +{ + wl-possible + Memcheck:Leak + match-leak-kinds: possible + ... + fun:af_* +} + + +{ + fontconfig_values_reachable + Memcheck:Leak + match-leak-kinds: reachable + fun:malloc + ... + fun:FcConfigValues +} + +{ + fontconfig_values_indirect + Memcheck:Leak + match-leak-kinds: indirect + fun:malloc + ... + fun:FcConfigValues +} + +{ + fontconfig_values_possible + Memcheck:Leak + match-leak-kinds: possible + fun:malloc + ... + fun:FcConfigValues +} + +{ + runtime_driver_indirect_malloc + Memcheck:Leak + match-leak-kinds: indirect + fun:malloc + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* +} + +{ + runtime_driver_reachable_tls + Memcheck:Leak + match-leak-kinds: reachable + fun:calloc + fun:pthread_setspecific@@GLIBC_2.34 + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* +} + +{ + runtime_driver_indirect_calloc + Memcheck:Leak + match-leak-kinds: indirect + fun:calloc + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* +} + +{ + runtime_driver_definite_calloc + Memcheck:Leak + match-leak-kinds: definite + fun:calloc + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* +} +