From 635f8d43ebe6f2611c59fbb93143e0bebd4f15e7 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 16 Jan 2024 09:59:13 +0000 Subject: [PATCH 1/2] =?UTF-8?q?gvariant-core:=20Don=E2=80=99t=20call=20pos?= =?UTF-8?q?ix=5Fmemalign()=20with=20size=3D=3D0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While glibc is fine with it (and returns a `NULL` pointer), technically it’s implementation-defined behaviour according to POSIX, so it’s best avoided. See https://pubs.opengroup.org/onlinepubs/9699919799/functions/posix_memalign.html. In particular, valgrind will warn about it, which is causing failures of the gdbus-codegen tests when valgrind is enabled. For example, https://gitlab.gnome.org/GNOME/glib/-/jobs/3460673 gives ``` ==15276== posix_memalign() invalid size value: 0 ==15276== at 0x484B7BC: posix_memalign (vg_replace_malloc.c:2099) ==15276== by 0x49320B2: g_variant_new_from_bytes (gvariant-core.c:629) ==15276== by 0x4931853: g_variant_new_from_data (gvariant.c:6226) ==15276== by 0x4B9A951: g_dbus_gvalue_to_gvariant (gdbusutils.c:708) ==15276== by 0x41BD15: _foo_igen_bat_skeleton_handle_get_property (gdbus-test-codegen-generated.c:13503) ==15276== by 0x41BFAF: foo_igen_bat_skeleton_dbus_interface_get_properties (gdbus-test-codegen-generated.c:13582) … ``` Signed-off-by: Philip Withnall Helps: #3228 --- glib/gvariant-core.c | 10 ++++++++-- glib/tests/gvariant.c | 15 ++++++++++++--- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/glib/gvariant-core.c b/glib/gvariant-core.c index d22299eab..eb25d768d 100644 --- a/glib/gvariant-core.c +++ b/glib/gvariant-core.c @@ -625,8 +625,14 @@ g_variant_new_from_bytes (const GVariantType *type, /* posix_memalign() requires the alignment to be a multiple of * sizeof(void*), and a power of 2. See g_variant_type_info_query() for - * details on the alignment format. */ - if (posix_memalign (&aligned_data, MAX (sizeof (void *), alignment + 1), + * details on the alignment format. + * + * While calling posix_memalign() with aligned_size==0 is safe on glibc, + * POSIX specifies that the behaviour is implementation-defined, so avoid + * that and leave aligned_data==NULL in that case. + * See https://pubs.opengroup.org/onlinepubs/9699919799/functions/posix_memalign.html */ + if (aligned_size != 0 && + posix_memalign (&aligned_data, MAX (sizeof (void *), alignment + 1), aligned_size) != 0) g_error ("posix_memalign failed"); diff --git a/glib/tests/gvariant.c b/glib/tests/gvariant.c index c8f13360c..110f5c33b 100644 --- a/glib/tests/gvariant.c +++ b/glib/tests/gvariant.c @@ -1337,12 +1337,15 @@ flavoured_free (gpointer data, static gpointer align_malloc (gsize size) { - gpointer mem; + gpointer mem = NULL; #ifdef HAVE_POSIX_MEMALIGN /* posix_memalign() requires the alignment to be a multiple of - * sizeof(void*), and a power of 2. */ - if (posix_memalign (&mem, MAX (sizeof (void *), 8), size)) + * sizeof(void*), and a power of 2. + * Calling it with size==0 leads to implementation-defined behaviour, so avoid + * that and guarantee to return NULL. */ + if (size != 0 && + posix_memalign (&mem, MAX (sizeof (void *), 8), size)) g_error ("posix_memalign failed"); #else /* NOTE: there may be platforms that lack posix_memalign() and also @@ -4959,6 +4962,12 @@ test_gbytes (void) g_bytes_unref (bytes2); g_variant_unref (a); g_variant_unref (tuple); + + bytes = g_bytes_new (NULL, 0); + a = g_variant_new_from_bytes (G_VARIANT_TYPE ("as"), bytes, TRUE); + g_bytes_unref (bytes); + g_assert_cmpuint (g_variant_n_children (a), ==, 0); + g_variant_unref (a); } typedef struct { From 1e0cec289ce6bf2548b09b677d8067c9051944ca Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 16 Jan 2024 16:55:08 +0000 Subject: [PATCH 2/2] tests: Avoid calling malloc(0) in gvariant tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Its behaviour is implementation-defined according to POSIX (https://pubs.opengroup.org/onlinepubs/9699919799/functions/malloc.html), and we’d quite like it to consistently return `NULL` for a zero-size allocation. Signed-off-by: Philip Withnall --- glib/tests/gvariant.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/glib/tests/gvariant.c b/glib/tests/gvariant.c index 110f5c33b..c24cd2f3e 100644 --- a/glib/tests/gvariant.c +++ b/glib/tests/gvariant.c @@ -1352,7 +1352,7 @@ align_malloc (gsize size) * have malloc() that returns non-8-aligned. if so, we need to try * harder here. */ - mem = malloc (size); + mem = (size > 0) ? malloc (size) : NULL; #endif return mem;