From 14f4b38fb1c7c14251433a2f0c53e786b4f6f587 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 7 Jun 2022 11:04:25 +0100 Subject: [PATCH 1/5] tests: NULL-initialise some variables to help scan-build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dynamically, these will only ever be used after they’ve been initialised due to correct checking of `use_udp` throughout the test. However, that’s a global variable and the static analyser is assuming it might change value. So help it out by NULL-initialising the variables so they can never be used uninitialised. Signed-off-by: Philip Withnall --- gio/tests/socket-client.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/gio/tests/socket-client.c b/gio/tests/socket-client.c index 92bf41fb9..025632767 100644 --- a/gio/tests/socket-client.c +++ b/gio/tests/socket-client.c @@ -249,14 +249,14 @@ int main (int argc, char *argv[]) { - GSocket *socket; - GSocketAddress *address; + GSocket *socket = NULL; + GSocketAddress *address = NULL; GError *error = NULL; GOptionContext *context; GCancellable *cancellable; - GIOStream *connection; - GInputStream *istream; - GOutputStream *ostream; + GIOStream *connection = NULL; + GInputStream *istream = NULL; + GOutputStream *ostream = NULL; GSocketAddress *src_address = NULL; GTlsCertificate *certificate = NULL; gint i; From 423bcab9f4e7456006099e65c9d5ea505b7e6d53 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 7 Jun 2022 11:05:26 +0100 Subject: [PATCH 2/5] garray: Change free/unref semantics under static analysis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Recent changes to `GPtrArray` and/or Coverity mean that Coverity is now assuming that `g_ptr_array_free (my_array, TRUE)` can leak memory. This is true in the case that `g_ptr_array_ref (my_array)` has been called elsewhere, but Coverity never actually verifies that. Very little (or no?) GLib code mixes `g_ptr_array_free()` with `g_ptr_array_{ref,unref}()`, so this isn’t a problem in practice. However, it has created a hundred or more false positives in Coverity (as pointer arrays are widely used within GLib and GIO), which is a complete pain. Before taking the dramatic step of ditching Coverity due to its atrocious false positive rate, let’s try changing the semantics of `g_ptr_array_free()` only when running under Coverity. Signed-off-by: Philip Withnall --- glib/garray.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/glib/garray.c b/glib/garray.c index 920a40258..f77ef1990 100644 --- a/glib/garray.c +++ b/glib/garray.c @@ -1555,9 +1555,14 @@ g_ptr_array_free (GPtrArray *array, /* if others are holding a reference, preserve the wrapper but * do free/return the data + * + * Coverity doesn’t understand this and assumes it’s a leak, so comment this + * out. */ +#ifndef __COVERITY__ if (!g_atomic_ref_count_dec (&rarray->ref_count)) flags |= PRESERVE_WRAPPER; +#endif return ptr_array_free (array, flags); } From 504727c31750bbd88e375011e5596e06a4b2694a Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 7 Jun 2022 11:08:14 +0100 Subject: [PATCH 3/5] gvariant: Zero-initialise GVariantBuilder children under static analysis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit scan-build can’t link the types used in `g_variant_builder_init()` with the (same) types used in `g_variant_builder_end()`, so ends up assuming that the children have not been initialised. At runtime, this is prevented by the precondition checks on `GVSB()->offset` in `g_variant_builder_end()`. scan-build doesn’t notice that though. Avoid a scan-build warning by zero-initialising the children array when running static analysis. Doing this unconditionally would be an unnecessary performance hit. Signed-off-by: Philip Withnall --- glib/gvariant.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/glib/gvariant.c b/glib/gvariant.c index e9399710e..062c2582e 100644 --- a/glib/gvariant.c +++ b/glib/gvariant.c @@ -3484,8 +3484,19 @@ g_variant_builder_init (GVariantBuilder *builder, g_assert_not_reached (); } +#ifdef G_ANALYZER_ANALYZING + /* Static analysers can’t couple the code in g_variant_builder_init() to the + * code in g_variant_builder_end() by GVariantType, so end up assuming that + * @offset and @children mismatch and that uninitialised memory is accessed + * from @children. At runtime, this is caught by the preconditions at the top + * of g_variant_builder_end(). Help the analyser by zero-initialising the + * memory to avoid a false positive. */ + GVSB(builder)->children = g_new0 (GVariant *, + GVSB(builder)->allocated_children); +#else GVSB(builder)->children = g_new (GVariant *, GVSB(builder)->allocated_children); +#endif } static void From 6c0bde8aa4c0b4471d117d50f2afca1db5836f1f Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 7 Jun 2022 11:09:39 +0100 Subject: [PATCH 4/5] tests: Fix a scan-build warning about uninitialised threads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It seems that scan-build assumes `n_threads > 0`, but doesn’t assume a tighter condition than that, and hence assumes that the two loops to initialise and join the threads have different numbers of iterations. That’s obviously not the case. Try and help scan-build out here by marking `n_threads` as `const`. I don’t know if this will work, but it’s correct regardless. Signed-off-by: Philip Withnall --- glib/tests/mutex.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/glib/tests/mutex.c b/glib/tests/mutex.c index a5ba2ea95..120229e42 100644 --- a/glib/tests/mutex.c +++ b/glib/tests/mutex.c @@ -186,7 +186,7 @@ addition_thread (gpointer value) static void test_mutex_perf (gconstpointer data) { - guint n_threads = GPOINTER_TO_UINT (data); + const guint n_threads = GPOINTER_TO_UINT (data); GThread *threads[THREADS]; gint64 start_time; gdouble rate; From 99267034941e1f6e8c39ee19c570404fcec121ce Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 7 Jun 2022 11:11:04 +0100 Subject: [PATCH 5/5] tests: Avoid an uninitialised variable warning in slice-memchunk test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dynamically, all the right elements of `ps` are initialised before they are used. However, scan-build doesn’t think so. It (probably) thinks that `number_of_blocks` could change value between the different loops over `ps`. Try and avoid that by marking `number_of_blocks` (and related variables) as `const`. Signed-off-by: Philip Withnall --- glib/tests/slice-memchunk.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/glib/tests/slice-memchunk.c b/glib/tests/slice-memchunk.c index 0938eb96f..ea8bee0c0 100644 --- a/glib/tests/slice-memchunk.c +++ b/glib/tests/slice-memchunk.c @@ -25,11 +25,11 @@ #define quick_rand32() \ (rand_accu = 1664525 * rand_accu + 1013904223, rand_accu) -static guint prime_size = 1021; /* 769; 509 */ -static gboolean clean_memchunks = FALSE; -static guint number_of_blocks = 10000; /* total number of blocks allocated */ -static guint number_of_repetitions = 10000; /* number of alloc+free repetitions */ -static gboolean want_corruption = FALSE; +static const guint prime_size = 1021; /* 769; 509 */ +static const gboolean clean_memchunks = FALSE; +static const guint number_of_blocks = 10000; /* total number of blocks allocated */ +static const guint number_of_repetitions = 10000; /* number of alloc+free repetitions */ +static const gboolean want_corruption = FALSE; /* --- old memchunk prototypes (memchunks.c) --- */ GMemChunk* old_mem_chunk_new (const gchar *name,