From 365411ea327a5f126896b106fe81a113bc0cc7df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Wed, 8 May 2024 01:56:02 +0200 Subject: [PATCH 01/16] gio/tests/cancellable: Explain failure on GCancellableSource tests on valgrind It looks like that finally also valgrind notices the same leaks as address sanitizer does. It does it more randomly but it still happens, so better to inform about until #2309 is resolved. --- gio/tests/cancellable.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/gio/tests/cancellable.c b/gio/tests/cancellable.c index be7017dcb7..f12a339cb1 100644 --- a/gio/tests/cancellable.c +++ b/gio/tests/cancellable.c @@ -25,6 +25,7 @@ #include #include "glib/glib-private.h" +#include "glib/gvalgrind.h" /* How long to wait in ms for each iteration */ #define WAIT_ITERATION (10) @@ -276,6 +277,11 @@ test_cancellable_source_threaded_dispose (void) "to (in another thread)"); g_test_bug ("https://gitlab.gnome.org/GNOME/glib/issues/1841"); +#ifdef ENABLE_VALGRIND + if (RUNNING_ON_VALGRIND) + g_test_incomplete ("FIXME: Leaks lots of GCancellableSource objects, see glib#2309"); +#endif + /* Create a new thread and wait until it’s ready to execute. Each iteration of * the test will pass it a new #GCancellableSource. */ g_cond_init (&data.cond); -- GitLab From ddfc8e29188e48b55c402248b26f227fc6cfc108 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Wed, 8 May 2024 22:59:44 +0200 Subject: [PATCH 02/16] glocalfile: Trash, free values when done avoiding leaks on early returns In glocalfile we're allocating some temporary strings but we don't free them on early returns, so free them once done and unset the variables to prevent them being used incorrectly. --- gio/glocalfile.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/gio/glocalfile.c b/gio/glocalfile.c index 69dd60f81e..bc33af6449 100644 --- a/gio/glocalfile.c +++ b/gio/glocalfile.c @@ -2284,6 +2284,7 @@ g_local_file_trash (GFile *file, data = g_strdup_printf ("[Trash Info]\nPath=%s\nDeletionDate=%s\n", original_name_escaped, delete_time); g_free (delete_time); + g_clear_pointer (&original_name_escaped, g_free); if (!g_file_set_contents_full (infofile, data, -1, G_FILE_SET_CONTENTS_CONSISTENT | G_FILE_SET_CONTENTS_ONLY_EXISTING, @@ -2291,6 +2292,7 @@ g_local_file_trash (GFile *file, { g_unlink (infofile); + g_free (data); g_free (filesdir); g_free (trashname); g_free (infofile); @@ -2298,6 +2300,8 @@ g_local_file_trash (GFile *file, return FALSE; } + g_clear_pointer (&data, g_free); + /* TODO: Maybe we should verify that you can delete the file from the trash * before moving it? OTOH, that is hard, as it needs a recursive scan */ @@ -2341,9 +2345,6 @@ g_local_file_trash (GFile *file, /* TODO: Do we need to update mtime/atime here after the move? */ g_free (infofile); - g_free (data); - - g_free (original_name_escaped); g_free (trashname); return TRUE; -- GitLab From 5c848233422880ef10eeade9777245cd2b892880 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Wed, 8 May 2024 23:02:39 +0200 Subject: [PATCH 03/16] gio/tests/gsettings: Cleanup allocated new locales --- gio/tests/gsettings.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/gio/tests/gsettings.c b/gio/tests/gsettings.c index 4c25b31646..cb98dd3221 100644 --- a/gio/tests/gsettings.c +++ b/gio/tests/gsettings.c @@ -924,6 +924,8 @@ test_l10n_time (void) g_assert_true (original_locale != (locale_t) 0); new_locale = duplocale (original_locale); g_assert_true (new_locale != (locale_t) 0); + g_clear_pointer (&new_locale, freelocale); + new_locale = newlocale (LC_TIME_MASK, "C", new_locale); g_assert_true (new_locale != (locale_t) 0); result = uselocale (new_locale); @@ -936,6 +938,7 @@ test_l10n_time (void) g_assert_cmpstr (str, ==, "12:00 AM"); g_free (str); + g_clear_pointer (&new_locale, freelocale); str = NULL; new_locale = newlocale (LC_TIME_MASK, "de_DE.UTF-8", new_locale); @@ -964,7 +967,7 @@ test_l10n_time (void) result = uselocale (original_locale); g_assert_true (result == new_locale); - freelocale (new_locale); + g_clear_pointer (&new_locale, freelocale); g_object_unref (settings); #endif -- GitLab From 19d704004746bac1b2bb04d2051b820b7438d30c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Wed, 8 May 2024 23:03:03 +0200 Subject: [PATCH 04/16] glib/tests/unicode: Cleanup allocated old locale if tests is skipped --- glib/tests/unicode.c | 1 + 1 file changed, 1 insertion(+) diff --git a/glib/tests/unicode.c b/glib/tests/unicode.c index e91425aa75..cb74068a51 100644 --- a/glib/tests/unicode.c +++ b/glib/tests/unicode.c @@ -525,6 +525,7 @@ test_turkish_strupdown (void) if (oldlocale == NULL) { g_test_skip ("locale tr_TR not available"); + g_free (old_lang); return; } -- GitLab From 84259c46fa34e2069f9e758ed8336d1297828391 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Wed, 8 May 2024 23:03:31 +0200 Subject: [PATCH 05/16] gio/tests/subprocess: Skip the trapped test under ASAN since it uses ptrace And as the sanitizer tells us, it doesn't support it --- gio/tests/gsubprocess.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/gio/tests/gsubprocess.c b/gio/tests/gsubprocess.c index 5baff93a40..2ba28d693e 100644 --- a/gio/tests/gsubprocess.c +++ b/gio/tests/gsubprocess.c @@ -1,3 +1,4 @@ +#include "glib-private.h" #include #include @@ -2073,6 +2074,8 @@ trace_children (pid_t main_child) static void test_exit_status_trapped (void) { +#ifndef _GLIB_ADDRESS_SANITIZER + #ifdef __linux__ GPtrArray *args = NULL; pid_t test_child; @@ -2103,6 +2106,13 @@ test_exit_status_trapped (void) #else g_test_skip ("ptrace() support for this test is only tested on Linux"); #endif + +#else /* if defined (_GLIB_ADDRESS_SANITIZER) */ + +g_test_skip ("LeakSanitizer does not work under ptrace"); +(void) trace_children; + +#endif /* _GLIB_ADDRESS_SANITIZER */ } #endif /* G_OS_UNIX */ -- GitLab From d544d409cb9fd5da6278023d2603eff7c6f6cc0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 9 May 2024 16:59:57 +0200 Subject: [PATCH 06/16] glib/tests/gutils-user-database: Add test dependency on preload library We do preload the library but that's not set as test dependency and so it may not be built --- glib/tests/meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/glib/tests/meson.build b/glib/tests/meson.build index cbce66fafe..abf053fce3 100644 --- a/glib/tests/meson.build +++ b/glib/tests/meson.build @@ -265,7 +265,7 @@ else glib_tests += { 'gutils-user-database' : { - 'depends' : [], + 'depends' : getpwuid_preload, 'env' : { var_preload: getpwuid_preload.full_path() }, -- GitLab From c78d0ad51499f6229002e357b42625456e12e0b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 9 May 2024 17:47:04 +0200 Subject: [PATCH 07/16] girepository/introspection: Properly check for sanitizer value The sanitizer option is set to 'none' when not used --- girepository/introspection/meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/girepository/introspection/meson.build b/girepository/introspection/meson.build index 862ca61cc1..ee2bd1b412 100644 --- a/girepository/introspection/meson.build +++ b/girepository/introspection/meson.build @@ -9,7 +9,7 @@ gi_gen_shared_sources = [ gi_gen_env_variables = environment() -if get_option('b_sanitize') != '' +if get_option('b_sanitize') != 'none' gi_gen_env_variables.append( 'ASAN_OPTIONS', 'verify_asan_link_order=0', separator: ',') endif -- GitLab From 0eb6c856063e52fa01f280592c52e3e9dc5555ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Wed, 8 May 2024 23:07:48 +0200 Subject: [PATCH 08/16] build: Define glib_sanitizers variable to easily get the sanitizers in use It's an array containing the list of sanitizers in use, normally it contains a value, but in some cases may have more than one (e.g. 'address' and 'undefined'). And so use it to avoid repeated checks --- gio/tests/meson.build | 2 +- girepository/introspection/meson.build | 2 +- meson.build | 5 +++++ 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/gio/tests/meson.build b/gio/tests/meson.build index d6b7f26d51..1821a121ef 100644 --- a/gio/tests/meson.build +++ b/gio/tests/meson.build @@ -310,7 +310,7 @@ if host_machine.system() != 'windows' } # LD_PRELOAD modules don't work so well with AddressSanitizer - if have_rtld_next and glib_build_shared and get_option('b_sanitize') == 'none' + if have_rtld_next and glib_build_shared and glib_sanitizers.length() == 0 gio_tests += { 'gsocketclient-slow' : { 'depends' : [ diff --git a/girepository/introspection/meson.build b/girepository/introspection/meson.build index ee2bd1b412..6ed8d6b597 100644 --- a/girepository/introspection/meson.build +++ b/girepository/introspection/meson.build @@ -9,7 +9,7 @@ gi_gen_shared_sources = [ gi_gen_env_variables = environment() -if get_option('b_sanitize') != 'none' +if glib_sanitizers.length() > 0 gi_gen_env_variables.append( 'ASAN_OPTIONS', 'verify_asan_link_order=0', separator: ',') endif diff --git a/meson.build b/meson.build index 8b42940558..aad78f5741 100644 --- a/meson.build +++ b/meson.build @@ -635,6 +635,11 @@ endif # improve this. glib_link_flags = cc.get_supported_link_arguments(warning_c_link_args) +glib_sanitizers = get_option('b_sanitize').split(',') +if glib_sanitizers == ['none'] + glib_sanitizers = [] +endif + # Windows SDK requirements and checks if host_system == 'windows' # Check whether we're building for UWP apps -- GitLab From 8032ba88d685359c2836cce791de6e72bd934e3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Wed, 8 May 2024 23:04:38 +0200 Subject: [PATCH 09/16] glib/tests/gutils-user-database: Ensure the test run under ASAN It uses LD_PRELOAD that may break asan, so let's ignore asan load order for now, even though that implies a partial test --- glib/tests/meson.build | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/glib/tests/meson.build b/glib/tests/meson.build index abf053fce3..21492ac5ff 100644 --- a/glib/tests/meson.build +++ b/glib/tests/meson.build @@ -263,15 +263,20 @@ else var_preload = 'DYLD_INSERT_LIBRARIES' endif + asan_env = {} + if 'address' in glib_sanitizers + asan_env = {'ASAN_OPTIONS': 'verify_asan_link_order=0'} + endif + glib_tests += { 'gutils-user-database' : { 'depends' : getpwuid_preload, 'env' : { var_preload: getpwuid_preload.full_path() - }, + } + asan_env, 'installed_tests_env' : { var_preload: installed_tests_execdir / fs.name(getpwuid_preload.full_path()) - }, + } + asan_env, }, } endif -- GitLab From 486ad655359d5092f90973e1851317ddce619fcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 9 May 2024 16:07:09 +0200 Subject: [PATCH 10/16] gio/tests: Enable gsocketclient-slow under address santizer The test can run properly, even though we won't do the right checks on the preloaded library --- gio/tests/meson.build | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/gio/tests/meson.build b/gio/tests/meson.build index 1821a121ef..1c16619e5b 100644 --- a/gio/tests/meson.build +++ b/gio/tests/meson.build @@ -309,8 +309,12 @@ if host_machine.system() != 'windows' }, } - # LD_PRELOAD modules don't work so well with AddressSanitizer - if have_rtld_next and glib_build_shared and glib_sanitizers.length() == 0 + if have_rtld_next and glib_build_shared + asan_env = {} + if 'address' in glib_sanitizers + asan_env = {'ASAN_OPTIONS': 'verify_asan_link_order=0'} + endif + gio_tests += { 'gsocketclient-slow' : { 'depends' : [ @@ -325,10 +329,10 @@ if host_machine.system() != 'windows' ], 'env' : { 'LD_PRELOAD': '@0@/slow-connect-preload.so'.format(meson.current_build_dir()) - }, + } + asan_env, 'installed_tests_env' : { 'LD_PRELOAD': '@0@/slow-connect-preload.so'.format(installed_tests_execdir), - }, + } + asan_env, }, } endif -- GitLab From aab0ff201b60879fbaa2ed94ea94e69e37ec11a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 9 May 2024 16:45:46 +0200 Subject: [PATCH 11/16] glib/tests/build: Support setting env variables for python tests --- glib/tests/meson.build | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/glib/tests/meson.build b/glib/tests/meson.build index 21492ac5ff..68cf43fa69 100644 --- a/glib/tests/meson.build +++ b/glib/tests/meson.build @@ -508,6 +508,11 @@ foreach test_name, extra_args : python_tests suite += 'failing' endif + local_test_env = test_env + foreach var, value : extra_args.get('env', {}) + local_test_env.append(var, value) + endforeach + foreach program : extra_args.get('extra_programs', []) depends += test_extra_programs_targets[program] endforeach @@ -518,7 +523,7 @@ foreach test_name, extra_args : python_tests protocol : extra_args.get('protocol', test_protocol), depends: depends, args: ['-B', files(test_name)], - env: test_env, + env: local_test_env, suite: suite, ) -- GitLab From d22e96aa722ea6874cb4231320ba073668b1af15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Wed, 8 May 2024 23:09:31 +0200 Subject: [PATCH 12/16] glib/tests/assert-msg-test.py: Skip the GDB test under sanitizers --- glib/tests/assert-msg-test.py | 4 ++++ glib/tests/meson.build | 1 + 2 files changed, 5 insertions(+) diff --git a/glib/tests/assert-msg-test.py b/glib/tests/assert-msg-test.py index 33aa2249e5..5e5293b5b3 100755 --- a/glib/tests/assert-msg-test.py +++ b/glib/tests/assert-msg-test.py @@ -143,6 +143,10 @@ class TestAssertMessage(unittest.TestCase): """Test running g_assert() within gdb and fail the program.""" if self.__gdb is None: self.skipTest("GDB is not installed, skipping this test!") + if {"thread", "address"} & set( + os.getenv("_GLIB_TEST_SANITIZERS", "").split(",") + ): + self.skipTest("GDB can't run under sanitizers") with tempfile.NamedTemporaryFile( prefix="assert-msg-test-", suffix=".gdb", mode="w", delete=False diff --git a/glib/tests/meson.build b/glib/tests/meson.build index 68cf43fa69..9d16bc4ea6 100644 --- a/glib/tests/meson.build +++ b/glib/tests/meson.build @@ -489,6 +489,7 @@ python_tests = { 'assert-msg-test.py' : { 'can_fail' : host_system == 'windows', 'extra_programs': ['assert-msg-test'], + 'env': {'_GLIB_TEST_SANITIZERS': ','.join(glib_sanitizers)}, }, } -- GitLab From f7b1ed1bf35bc9a0dd692f5ab6fc0e4ecdca67d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Wed, 8 May 2024 23:09:58 +0200 Subject: [PATCH 13/16] glib/tests/1bit-emufutex: Mark it as failing under ASAN --- glib/tests/meson.build | 2 ++ 1 file changed, 2 insertions(+) diff --git a/glib/tests/meson.build b/glib/tests/meson.build index 9d16bc4ea6..5fbaa854b1 100644 --- a/glib/tests/meson.build +++ b/glib/tests/meson.build @@ -186,6 +186,8 @@ glib_tests = { '1bit-emufutex' : { 'source' : '1bit-mutex.c', 'c_args' : ['-DTEST_EMULATED_FUTEX'], + # FIXME: https://gitlab.gnome.org/GNOME/glib/-/issues/3359 + 'can_fail': 'undefined' in glib_sanitizers, 'install' : false, 'suite' : ['slow'], }, -- GitLab From 1d2d865f47e10b6574ce295ebc37739619d761d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 10 May 2024 01:17:15 +0200 Subject: [PATCH 14/16] glib/tests/mapping: Check the exit status of the child process In this way if it fails for some memory error we can track it --- glib/tests/mapping.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/glib/tests/mapping.c b/glib/tests/mapping.c index 99cb8eee02..5fcde83ae7 100644 --- a/glib/tests/mapping.c +++ b/glib/tests/mapping.c @@ -22,6 +22,7 @@ #ifdef G_OS_UNIX #include +#include #endif #ifdef G_OS_WIN32 #include @@ -201,8 +202,10 @@ test_child_private (void) gsize len; gchar *child_argv[4]; GPid child_pid; + GSpawnFlags spawn_flags = G_SPAWN_DEFAULT; #ifndef G_OS_WIN32 GMainLoop *loop; + int wait_status; #endif gchar pid[100]; gchar *dir, *global_filename, *childname; @@ -221,6 +224,7 @@ test_child_private (void) #ifndef G_OS_WIN32 signal (SIGUSR1, handle_usr1); + spawn_flags |= G_SPAWN_DO_NOT_REAP_CHILD; #endif g_snprintf (pid, sizeof(pid), "%d", getpid ()); @@ -230,7 +234,7 @@ test_child_private (void) child_argv[3] = NULL; result = g_spawn_async (dir, child_argv, NULL, - 0, NULL, NULL, &child_pid, &error); + spawn_flags, NULL, NULL, &child_pid, &error); g_assert_no_error (error); g_assert_true (result); g_test_message ("test_child_private: child spawned"); @@ -261,6 +265,10 @@ test_child_private (void) #ifndef G_OS_WIN32 g_idle_add (check_stop, loop); g_main_loop_run (loop); + waitpid (child_pid, &wait_status, 0); + g_test_message ("Child exited with status %d", wait_status); + g_spawn_check_wait_status (wait_status, &error); + g_assert_no_error (error); #else g_usleep (2000000); #endif -- GitLab From f9cb8d59de744ab642b4d478c121b5e0fef8d104 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 10 May 2024 01:17:41 +0200 Subject: [PATCH 15/16] glib/tests/mapping: Unref the mapped file on exit --- glib/tests/mapping.c | 1 + 1 file changed, 1 insertion(+) diff --git a/glib/tests/mapping.c b/glib/tests/mapping.c index 5fcde83ae7..bbd4de8b00 100644 --- a/glib/tests/mapping.c +++ b/glib/tests/mapping.c @@ -125,6 +125,7 @@ child_main (void) g_free (childname); g_free (global_filename); g_free (dir); + g_mapped_file_unref (map); signal_parent (NULL); } -- GitLab From caf2f2fbda97aeb325c7993bf7922f8734b78135 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 10 May 2024 02:42:42 +0200 Subject: [PATCH 16/16] girepository/introspection: Set asan option only on address sanitizer It's the only sanitizer failing when generating the introspection data --- girepository/introspection/meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/girepository/introspection/meson.build b/girepository/introspection/meson.build index 6ed8d6b597..0a217e75f3 100644 --- a/girepository/introspection/meson.build +++ b/girepository/introspection/meson.build @@ -9,7 +9,7 @@ gi_gen_shared_sources = [ gi_gen_env_variables = environment() -if glib_sanitizers.length() > 0 +if 'address' in glib_sanitizers gi_gen_env_variables.append( 'ASAN_OPTIONS', 'verify_asan_link_order=0', separator: ',') endif -- GitLab