From 416bab52eb59bfbd612f72f88ee73837dd976a04 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 11 Feb 2025 11:40:29 +0000 Subject: [PATCH 01/15] tests: Fix a memory leak if a callable-info test is skipped Signed-off-by: Philip Withnall --- girepository/tests/callable-info.c | 1 + 1 file changed, 1 insertion(+) diff --git a/girepository/tests/callable-info.c b/girepository/tests/callable-info.c index ee23d2e62..69fd5e940 100644 --- a/girepository/tests/callable-info.c +++ b/girepository/tests/callable-info.c @@ -199,6 +199,7 @@ test_callable_info_static_vfunc (RepositoryFixture *fx, if (!vfunc_info) { g_test_skip ("g-ir-scanner is not new enough"); + gi_base_info_unref (info); return; } g_assert_nonnull (vfunc_info); From cc070e1a7dd2689fbbb8e11ee2e8d9f1cb5c65b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 24 May 2024 16:58:53 +0200 Subject: [PATCH 02/15] girepository/compiler: Cleanup the parser on write failures --- girepository/compiler/compiler.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/girepository/compiler/compiler.c b/girepository/compiler/compiler.c index e9b1b1563..31d855787 100644 --- a/girepository/compiler/compiler.c +++ b/girepository/compiler/compiler.c @@ -229,6 +229,7 @@ main (int argc, char **argv) { GITypelib *typelib = NULL; + int write_successful; if (shlibs) { @@ -246,10 +247,14 @@ main (int argc, char **argv) g_error (_("Invalid typelib for module ‘%s’: %s"), module->name, error->message); - if (!write_out_typelib (NULL, typelib)) - return 1; - + write_successful = write_out_typelib (NULL, typelib); g_clear_pointer (&typelib, gi_typelib_unref); + + if (!write_successful) + { + gi_ir_parser_free (parser); + return 1; + } } g_debug ("[building] done"); From 98de5367d3cca19fe72822b5df35e0074d393d7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 24 May 2024 17:00:33 +0200 Subject: [PATCH 03/15] girepository/compiler: Cleanup the parser error on failure --- girepository/compiler/compiler.c | 1 + 1 file changed, 1 insertion(+) diff --git a/girepository/compiler/compiler.c b/girepository/compiler/compiler.c index 31d855787..dcbbbe66f 100644 --- a/girepository/compiler/compiler.c +++ b/girepository/compiler/compiler.c @@ -219,6 +219,7 @@ main (int argc, char **argv) g_fprintf (stderr, "%s\n", message); g_free (message); gi_ir_parser_free (parser); + g_error_free (error); return 1; } From 0a47b69b461ec9c34d4a504908ec62cc626d0a53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 24 May 2024 17:01:25 +0200 Subject: [PATCH 04/15] girepository/parser: Clarify ownership of nodetype nodes We may had free'd a list of items without freeing the items in there. So now, properly steal the data instead and free everything. --- girepository/girparser.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/girepository/girparser.c b/girepository/girparser.c index 0cf341533..adb9db7f9 100644 --- a/girepository/girparser.c +++ b/girepository/girparser.c @@ -2309,7 +2309,7 @@ end_type_top (ParseContext *ctx) if (!ctx->type_parameters) goto out; - typenode = (GIIrNodeType*)ctx->type_parameters->data; + typenode = (GIIrNodeType *) g_steal_pointer (&ctx->type_parameters->data); /* Default to pointer for unspecified containers */ if (typenode->tag == GI_TYPE_TAG_ARRAY || @@ -2333,32 +2333,32 @@ end_type_top (ParseContext *ctx) case GI_IR_NODE_PARAM: { GIIrNodeParam *param = (GIIrNodeParam *)ctx->current_typed; - param->type = typenode; + param->type = g_steal_pointer (&typenode); } break; case GI_IR_NODE_FIELD: { GIIrNodeField *field = (GIIrNodeField *)ctx->current_typed; - field->type = typenode; + field->type = g_steal_pointer (&typenode); } break; case GI_IR_NODE_PROPERTY: { GIIrNodeProperty *property = (GIIrNodeProperty *) ctx->current_typed; - property->type = typenode; + property->type = g_steal_pointer (&typenode); } break; case GI_IR_NODE_CONSTANT: { GIIrNodeConstant *constant = (GIIrNodeConstant *)ctx->current_typed; - constant->type = typenode; + constant->type = g_steal_pointer (&typenode); } break; default: g_printerr("current node is %d\n", CURRENT_NODE (ctx)->type); g_assert_not_reached (); } - g_list_free (ctx->type_parameters); + g_list_free_full (ctx->type_parameters, (GDestroyNotify) gi_ir_node_free); out: ctx->type_depth = 0; @@ -2374,7 +2374,7 @@ end_type_recurse (ParseContext *ctx) parent = (GIIrNodeType *) ((GList*)ctx->type_stack->data)->data; if (ctx->type_parameters) - param = (GIIrNodeType *) ctx->type_parameters->data; + param = (GIIrNodeType *) g_steal_pointer (&ctx->type_parameters->data); if (parent->tag == GI_TYPE_TAG_ARRAY || parent->tag == GI_TYPE_TAG_GLIST || @@ -2383,7 +2383,7 @@ end_type_recurse (ParseContext *ctx) g_assert (param != NULL); if (parent->parameter_type1 == NULL) - parent->parameter_type1 = param; + parent->parameter_type1 = g_steal_pointer (¶m); else g_assert_not_reached (); } @@ -2392,13 +2392,14 @@ end_type_recurse (ParseContext *ctx) g_assert (param != NULL); if (parent->parameter_type1 == NULL) - parent->parameter_type1 = param; + parent->parameter_type1 = g_steal_pointer (¶m); else if (parent->parameter_type2 == NULL) - parent->parameter_type2 = param; + parent->parameter_type2 = g_steal_pointer (¶m); else g_assert_not_reached (); } - g_list_free (ctx->type_parameters); + g_clear_pointer ((GIIrNode **) ¶m, gi_ir_node_free); + g_list_free_full (ctx->type_parameters, (GDestroyNotify) gi_ir_node_free); ctx->type_parameters = (GList *)ctx->type_stack->data; ctx->type_stack = g_list_delete_link (ctx->type_stack, ctx->type_stack); } From a3eccd63162eb20c6cc38be86bbc5fa7f75186e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Mon, 24 Feb 2025 19:22:07 +0100 Subject: [PATCH 05/15] gthread: Clarify when g_private_set_alloc0() suppression happens with valgrind --- glib/gthread.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/glib/gthread.c b/glib/gthread.c index b58718132..bd7d084b0 100644 --- a/glib/gthread.c +++ b/glib/gthread.c @@ -505,7 +505,8 @@ static GPrivate g_thread_specific_private = G_PRIVATE_INIT (g_thread_cleanup * * Sets the thread local variable @key to have a newly-allocated and zero-filled * value of given @size, and returns a pointer to that memory. Allocations made - * using this API will be suppressed in valgrind: it is intended to be used for + * using this API will be suppressed in valgrind (when the GLib default + * suppression file, `glib.supp`, is used): it is intended to be used for * one-time allocations which are known to be leaked, such as those for * per-thread initialisation data. Otherwise, this function behaves the same as * g_private_set(). From 0453908570ce84c0303d2c9b50a3914a92f0b7f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 10 May 2024 01:35:15 +0200 Subject: [PATCH 06/15] gthread: Mark data allocated with g_private_set_alloc0 as leaking This is what we already ignored with valgrind and it's something that we should always ignore since it's leaked by design. So do explicitly mark it as such. --- glib/gthread.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/glib/gthread.c b/glib/gthread.c index bd7d084b0..8354c01e5 100644 --- a/glib/gthread.c +++ b/glib/gthread.c @@ -40,6 +40,7 @@ #include "config.h" +#include "glib-private.h" #include "gthread.h" #include "gthreadprivate.h" @@ -506,10 +507,10 @@ static GPrivate g_thread_specific_private = G_PRIVATE_INIT (g_thread_cleanup * Sets the thread local variable @key to have a newly-allocated and zero-filled * value of given @size, and returns a pointer to that memory. Allocations made * using this API will be suppressed in valgrind (when the GLib default - * suppression file, `glib.supp`, is used): it is intended to be used for - * one-time allocations which are known to be leaked, such as those for - * per-thread initialisation data. Otherwise, this function behaves the same as - * g_private_set(). + * suppression file, `glib.supp`, is used) and leak sanitizer: it is intended to + * be used for one-time allocations which are known to be leaked, such as those + * for per-thread initialisation data. Otherwise, this function behaves the same + * as g_private_set(). * * Returns: (transfer full): new thread-local heap allocation of size @size * Since: 2.60 @@ -521,6 +522,7 @@ g_private_set_alloc0 (GPrivate *key, { gpointer allocated = g_malloc0 (size); + g_ignore_leak (allocated); g_private_set (key, allocated); return g_steal_pointer (&allocated); From b11c1217477c76c9bf23f1e0c673cc452e2c0998 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 10 May 2024 01:37:20 +0200 Subject: [PATCH 07/15] gcharset: Mark cached data as known leak The charset data is never freed but it's something stored per thread so we can safely ignore the leak --- glib/gcharset.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/glib/gcharset.c b/glib/gcharset.c index b0a4b9cc0..939ff596d 100644 --- a/glib/gcharset.c +++ b/glib/gcharset.c @@ -26,6 +26,7 @@ #include "garray.h" #include "genviron.h" #include "ghash.h" +#include "glib-private.h" #include "gmessages.h" #include "gstrfuncs.h" #include "gthread.h" @@ -807,6 +808,7 @@ g_get_language_names_with_category (const gchar *category_name) cache = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, language_names_cache_free); g_private_set (&cache_private, cache); + g_ignore_leak (cache); } languages = guess_category_value (category_name); @@ -835,6 +837,7 @@ g_get_language_names_with_category (const gchar *category_name) name_cache->languages = g_strdup (languages); name_cache->language_names = (gchar **) g_ptr_array_free (array, FALSE); g_hash_table_insert (cache, g_strdup (category_name), name_cache); + g_ignore_leak (name_cache); } return (const gchar * const *) name_cache->language_names; From efe215809b28f88dd89a9c9e90a66c1366fcb281 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 10 May 2024 01:51:22 +0200 Subject: [PATCH 08/15] glib/glib-private: Mark the dynamic LSAN checks as unlikely It's not normal to run programs with leak sanitizer enabled, so let's mark those checks as such, in case the compiler can optimize them --- glib/glib-private.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/glib/glib-private.h b/glib/glib-private.h index 3417dfaf6..4dd0c2020 100644 --- a/glib/glib-private.h +++ b/glib/glib-private.h @@ -100,7 +100,7 @@ g_leak_sanitizer_is_supported (void) #if defined (_GLIB_ADDRESS_SANITIZER) return TRUE; #elif defined (HAS_DYNAMIC_ASAN_LOADING) - return __lsan_enable != NULL && __lsan_ignore_object != NULL; + return G_UNLIKELY (__lsan_enable != NULL && __lsan_ignore_object != NULL); #else return FALSE; #endif @@ -122,7 +122,7 @@ g_ignore_leak (gconstpointer p) if (p != NULL) __lsan_ignore_object (p); #elif defined (HAS_DYNAMIC_ASAN_LOADING) - if (p != NULL && __lsan_ignore_object != NULL) + if (G_LIKELY (p != NULL) && G_UNLIKELY (__lsan_ignore_object != NULL)) __lsan_ignore_object (p); #endif } @@ -166,7 +166,7 @@ g_begin_ignore_leaks (void) #if defined (_GLIB_ADDRESS_SANITIZER) __lsan_disable (); #elif defined (HAS_DYNAMIC_ASAN_LOADING) - if (__lsan_disable != NULL) + if (G_UNLIKELY (__lsan_disable != NULL)) __lsan_disable (); #endif } @@ -183,7 +183,7 @@ g_end_ignore_leaks (void) #if defined (_GLIB_ADDRESS_SANITIZER) __lsan_enable (); #elif defined (HAS_DYNAMIC_ASAN_LOADING) - if (__lsan_enable != NULL) + if (G_UNLIKELY (__lsan_enable != NULL)) __lsan_enable (); #endif } From 6ed59a6f408e6369424be95f1c2871dbcdfeedb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 24 May 2024 18:16:45 +0200 Subject: [PATCH 09/15] gio/tests/gdbus-connection: Fix typo --- gio/tests/gdbus-connection.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gio/tests/gdbus-connection.c b/gio/tests/gdbus-connection.c index 8d37af83b..9f44a2e51 100644 --- a/gio/tests/gdbus-connection.c +++ b/gio/tests/gdbus-connection.c @@ -539,7 +539,7 @@ test_connection_signal_handler (GDBusConnection *connection, /* We defer quitting to a G_PRIORITY_DEFAULT_IDLE function so other queued signal * callbacks have a chance to run first. They get dispatched with a higher priority - * of G_PIORITY_DEFAULT, so as long as the queue is non-empty g_main_loop_quit won't + * of G_PRIORITY_DEFAULT, so as long as the queue is non-empty g_main_loop_quit won't * run */ g_idle_add_once ((GSourceOnceFunc) g_main_loop_quit, loop); From ea52feb658b6a1b3b44b3fd6e9db72676d7832ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 24 May 2024 18:17:01 +0200 Subject: [PATCH 10/15] gio/tests/gdbus-export: Ensure we call all the callbacks on return Ensure we don't do an user-after-free access, as reported by ASAN: ==3704==ERROR: AddressSanitizer: stack-use-after-return on address 0x70a58f8631c0 at pc 0x000000405144 bp 0x7fffff62c7a0 sp 0x7fffff62c798 READ of size 4 at 0x70a58f8631c0 thread T0 #0 0x405143 in on_object_unregistered ../../GNOME/glib/gio/tests/gdbus-export.c:597 #1 0x70a592e858d8 in call_destroy_notify_data_in_idle ../../GNOME/glib/gio/gdbusconnection.c:244 #2 0x70a5940016a4 in g_idle_dispatch ../../GNOME/glib/glib/gmain.c:6221 #3 0x70a59401095b in g_main_dispatch ../../GNOME/glib/glib/gmain.c:3348 #4 0x70a59401095b in g_main_context_dispatch_unlocked ../../GNOME/glib/glib/gmain.c:4197 #5 0x70a59401ba17 in g_main_context_iterate_unlocked ../../GNOME/glib/glib/gmain.c:4262 #6 0x70a59401cc73 in g_main_context_iteration ../../GNOME/glib/glib/gmain.c:4327 #7 0x405658 in test_threaded_unregistration_iteration ../../GNOME/glib/gio/tests/gdbus-export.c:1878 #8 0x405658 in test_threaded_unregistration ../../GNOME/glib/gio/tests/gdbus-export.c:1952 #9 0x70a5940dfb04 in test_case_run ../../GNOME/glib/glib/gtestutils.c:2988 #10 0x70a5940dfb04 in g_test_run_suite_internal ../../GNOME/glib/glib/gtestutils.c:3090 #11 0x70a5940df893 in g_test_run_suite_internal ../../GNOME/glib/glib/gtestutils.c:3109 #12 0x70a5940df893 in g_test_run_suite_internal ../../GNOME/glib/glib/gtestutils.c:3109 #13 0x70a5940e0bc9 in g_test_run_suite ../../GNOME/glib/glib/gtestutils.c:3189 #14 0x70a5940e0d1f in g_test_run ../../GNOME/glib/glib/gtestutils.c:2275 #15 0x40eb72 in session_bus_run ../../GNOME/glib/gio/tests/gdbus-sessionbus.c:69 #16 0x403a2c in main ../../GNOME/glib/gio/tests/gdbus-export.c:1990 #17 0x70a591d9f149 in __libc_start_call_main (/lib64/libc.so.6+0x28149) (BuildId: 0d710e9d9dc10c500b8119c85da75004183618e2) #18 0x70a591d9f20a in __libc_start_main_impl (/lib64/libc.so.6+0x2820a) (BuildId: 0d710e9d9dc10c500b8119c85da75004183618e2) #19 0x403b44 in _start (/tmp/_build/gio/tests/gdbus-export+0x403b44) (BuildId: f6312e919c3d94e4c49270b0dfc5c870e1ba550b) Address 0x70a58f8631c0 is located in stack of thread T0 at offset 192 in frame #0 0x40525f in test_threaded_unregistration ../../GNOME/glib/gio/tests/gdbus-export.c:1936 This frame has 7 object(s): [32, 40) 'local_error' (line 1835) [64, 72) 'unregister_thread' (line 1836) [96, 104) 'value' (line 1838) [128, 136) 'value_str' (line 1839) [160, 168) 'call_result' (line 1840) [192, 204) 'object_registration_data' (line 1834) <== Memory access at offset 192 is inside this variable [224, 240) 'data' (line 1833) --- gio/tests/gdbus-export.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/gio/tests/gdbus-export.c b/gio/tests/gdbus-export.c index 599df5bb5..6acfe5040 100644 --- a/gio/tests/gdbus-export.c +++ b/gio/tests/gdbus-export.c @@ -1928,6 +1928,17 @@ test_threaded_unregistration_iteration (gboolean subtree) g_clear_object (&call_result); g_clear_object (&data.connection); + /* We defer quitting to a G_PRIORITY_DEFAULT_IDLE function so other queued + * signal callbacks have a chance to run first. + * In particular we want to ensure that all calls to on_object_unregistered() + * are delivered here before we end this function, so that there won't be any + * invalid stack access. + * They get dispatched with a higher priority (G_PRIORITY_DEFAULT), so as + * long as the queue is non-empty g_main_loop_quit won't run + */ + g_idle_add_once ((GSourceOnceFunc) g_main_loop_quit, loop); + g_main_loop_run (loop); + return unregistration_was_first; } From aee7b2b0923d5c40ffc733e4d5298e101f6604b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 30 Jan 2025 15:47:18 +0100 Subject: [PATCH 11/15] gbytes: Return early if a NULL or 0-sized GBytes is created In case data is NULL we ended up to call memcpy with NULL parameter which is undefined behavior (see the trace below). So instead of having multiple null checks to do just the same, simplify the NULL or 0-sized cases. ../glib/gbytes.c:140:7: runtime error: null pointer passed as argument 2, which is declared to never be null #0 0x7f56ea7c667e in g_bytes_new ../glib/gbytes.c:140 #1 0x5557c3659f06 in test_null ../glib/tests/bytes.c:453 #2 0x7f56ea9c0f70 in test_case_run ../glib/gtestutils.c:3115 #3 0x7f56ea9c0f70 in g_test_run_suite_internal ../glib/gtestutils.c:3210 #4 0x7f56ea9c0ceb in g_test_run_suite_internal ../glib/gtestutils.c:3229 #5 0x7f56ea9c1f89 in g_test_run_suite ../glib/gtestutils.c:3310 #6 0x7f56ea9c20df in g_test_run ../glib/gtestutils.c:2379 #7 0x5557c36599d2 in main ../glib/tests/bytes.c:536 --- glib/gbytes.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/glib/gbytes.c b/glib/gbytes.c index 1fd3078d9..35fc3829d 100644 --- a/glib/gbytes.c +++ b/glib/gbytes.c @@ -123,12 +123,19 @@ g_bytes_new (gconstpointer data, { g_return_val_if_fail (data != NULL || size == 0, NULL); + if (data == NULL || size == 0) + { + g_assert (data != NULL || size == 0); + + return g_bytes_new_with_free_func (NULL, size, NULL, NULL); + } + if (size <= G_BYTES_MAX_INLINE) { GBytesInline *bytes; bytes = g_malloc (sizeof *bytes + size); - bytes->bytes.data = (data != NULL && size > 0) ? bytes->inline_data : NULL; + bytes->bytes.data = bytes->inline_data; bytes->bytes.size = size; bytes->bytes.free_func = NULL; bytes->bytes.user_data = NULL; From 28037242d5cae639f56b03dde02a71bd13c7abb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 30 Jan 2025 18:42:06 +0100 Subject: [PATCH 12/15] gvariant: Do not byteswap empty GVariant values If a value has no size, we'd end up to use g_variant_store() with a NULL value, and this is not allowed by the API, leading to errors: stderr: ../glib/gvariant-core.c:1429:9: runtime error: null pointer passed as argument 1, which is declared to never be null #0 0x7f9c139ce559 in g_variant_store ../glib/gvariant-core.c:732 #1 0x7f9c13c60b01 in g_variant_byteswap ../glib/gvariant.c:6211 #2 0x564ae412e9b9 in test_byteswap ../glib/tests/gvariant.c:2321 #3 0x564ae412e9b9 in test_byteswaps ../glib/tests/gvariant.c:2374 #4 0x7f9c13bc1000 in test_case_run ../glib/gtestutils.c:3115 #5 0x7f9c13bc1000 in g_test_run_suite_internal ../glib/gtestutils.c:3210 #6 0x7f9c13bc0d7b in g_test_run_suite_internal ../glib/gtestutils.c:3229 #7 0x7f9c13bc0d7b in g_test_run_suite_internal ../glib/gtestutils.c:3229 #8 0x7f9c13bc2019 in g_test_run_suite ../glib/gtestutils.c:3310 #9 0x7f9c13bc216f in g_test_run ../glib/gtestutils.c:2379 #10 0x564ae410f326 in main ../glib/tests/gvariant.c:6045 --- glib/gvariant.c | 8 ++++++-- glib/tests/gvariant.c | 14 ++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/glib/gvariant.c b/glib/gvariant.c index bdcbe412b..c69702965 100644 --- a/glib/gvariant.c +++ b/glib/gvariant.c @@ -6187,12 +6187,16 @@ g_variant_byteswap (GVariant *value) GVariantTypeInfo *type_info; guint alignment; GVariant *new; + gsize size = 0; type_info = g_variant_get_type_info (value); g_variant_type_info_query (type_info, &alignment, NULL); - if (alignment && g_variant_is_normal_form (value)) + if (alignment) + size = g_variant_get_size (value); + + if (size > 0 && g_variant_is_normal_form (value)) { /* (potentially) contains multi-byte numeric data, but is also already in * normal form so we can use a faster byteswapping codepath on the @@ -6201,7 +6205,7 @@ g_variant_byteswap (GVariant *value) GBytes *bytes; serialised.type_info = g_variant_get_type_info (value); - serialised.size = g_variant_get_size (value); + serialised.size = size; serialised.data = g_malloc (serialised.size); serialised.depth = g_variant_get_depth (value); serialised.ordered_offsets_up_to = G_MAXSIZE; /* operating on the normal form */ diff --git a/glib/tests/gvariant.c b/glib/tests/gvariant.c index 12a35dad9..e215e9861 100644 --- a/glib/tests/gvariant.c +++ b/glib/tests/gvariant.c @@ -2367,6 +2367,19 @@ test_byteswaps (void) g_variant_type_info_assert_no_infos (); } +static void +test_byteswap_zero_sized (void) +{ + GVariant *variant; + GVariant *swapped; + + variant = g_variant_new_from_data (G_VARIANT_TYPE_STRING, NULL, 0, TRUE, NULL, NULL); + swapped = g_variant_byteswap (variant); + + g_variant_unref (variant); + g_variant_unref (swapped); +} + static void test_serialiser_children (void) { @@ -5939,6 +5952,7 @@ main (int argc, char **argv) g_test_add_func ("/gvariant/serialiser/variant", test_variants); g_test_add_func ("/gvariant/serialiser/strings", test_strings); g_test_add_func ("/gvariant/serialiser/byteswap", test_byteswaps); + g_test_add_func ("/gvariant/serialiser/byteswap/zero-sized", test_byteswap_zero_sized); g_test_add_func ("/gvariant/serialiser/children", test_serialiser_children); for (i = 1; i <= 20; i += 4) From 39c05b13123b12622ee2c93c170dbf20b573f6ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 30 Jan 2025 18:36:03 +0100 Subject: [PATCH 13/15] gvariant-core: Add pre-condition on variant store data being non-null We define the data not-nullable in docs, but we didn't check this prerequisite programmatically --- glib/gvariant-core.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/glib/gvariant-core.c b/glib/gvariant-core.c index 4ee62889b..69680f45e 100644 --- a/glib/gvariant-core.c +++ b/glib/gvariant-core.c @@ -1415,6 +1415,8 @@ void g_variant_store (GVariant *value, gpointer data) { + g_return_if_fail (data != NULL); + g_variant_lock (value); if (value->state & STATE_SERIALISED) From 31c9dbbc732df58e342a6c10d532eb7fc610bb59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 30 Jan 2025 18:47:53 +0100 Subject: [PATCH 14/15] glib/tests/gvariant: Not try to memcpy NULL data It's undefined behavior --- glib/tests/gvariant.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/glib/tests/gvariant.c b/glib/tests/gvariant.c index e215e9861..8e1713046 100644 --- a/glib/tests/gvariant.c +++ b/glib/tests/gvariant.c @@ -2327,7 +2327,15 @@ test_byteswap (void) * often makes something non-normal but still readable. */ three_size_copy = three.size + 1; three_data_copy = g_malloc (three_size_copy); - memcpy (three_data_copy, three.data, three.size); + if (three.data) + { + g_assert_cmpuint (three.size, !=, 0); + memcpy (three_data_copy, three.data, three.size); + } + else + { + g_assert_cmpuint (three.size, ==, 0); + } three_data_copy[three.size] = '\0'; three_variant = g_variant_new_from_data (G_VARIANT_TYPE (g_variant_type_info_get_type_string (three.type_info)), From 03dcf4cdc3611c86a879800c11bd2a4ddc514c8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 3 Apr 2025 14:01:20 +0200 Subject: [PATCH 15/15] glib/messages: Avoid warning on potentially uninitialized value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In function ‘memmem_with_end_pointer’, inlined from ‘domain_found’ at ../glib/gmessages.c:2695:16, inlined from ‘should_drop_message’ at ../glib/gmessages.c:2802:35: ../glib/gmessages.c:2674:19: warning: ‘log_domain_length’ may be used uninitialized [-Wmaybe-uninitialized] 2674 | #define my_memmem memmem | ^ ../glib/gmessages.c:2683:10: note: in expansion of macro ‘my_memmem’ 2683 | return my_memmem (haystack, (const char *) haystack_end - (const char *) haystack, needle, needle_len); | ^~~~~~~~~ ../glib/gmessages.c: In function ‘should_drop_message’: ../glib/gmessages.c:2760:13: note: ‘log_domain_length’ was declared here 2760 | gsize log_domain_length; | ^~~~~~~~~~~~~~~~~ --- glib/gmessages.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/glib/gmessages.c b/glib/gmessages.c index 839361718..cfea3d784 100644 --- a/glib/gmessages.c +++ b/glib/gmessages.c @@ -2778,6 +2778,8 @@ should_drop_message (GLogLevelFlags log_level, if (log_domain == NULL) { + log_domain_length = 0; + for (i = 0; i < n_fields; i++) { if (g_strcmp0 (fields[i].key, "GLIB_DOMAIN") == 0)