From a412ffe4d3b3775c00703cf31dd302b1200fd0e5 Mon Sep 17 00:00:00 2001 From: Michael Catanzaro Date: Mon, 23 Nov 2020 15:51:16 -0600 Subject: [PATCH 1/2] gatomicarray: suppress valgrind memory leak warnings The problem occurs because we keep a pointer inside the allocated block, instead of a pointer to the start of the block: ``` ==180238== 16 bytes in 1 blocks are possibly lost in loss record 3,086 of 16,075 ==180238== at 0x483980B: malloc (vg_replace_malloc.c:309) ==180238== by 0x548942C: g_malloc (gmem.c:102) ==180238== by 0x54A4748: g_slice_alloc (gslice.c:1025) ==180238== by 0x53D0AAF: freelist_alloc (gatomicarray.c:77) ==180238== by 0x53D0B85: _g_atomic_array_copy (gatomicarray.c:133) ==180238== by 0x53F8E6D: iface_node_set_offset_L (gtype.c:1347) ==180238== by 0x53F91F1: type_node_add_iface_entry_W (gtype.c:1444) ==180238== by 0x53F93DF: type_add_interface_Wm (gtype.c:1477) ==180238== by 0x53FC946: g_type_add_interface_static (gtype.c:2852) ==180238== by 0x4A3D53A: gtk_menu_shell_accessible_get_type_once (gtkmenushellaccessible.c:26) ==180238== by 0x4A3D495: gtk_menu_shell_accessible_get_type (gtkmenushellaccessible.c:26) ==180238== by 0x4C8AC44: gtk_menu_shell_class_init (gtkmenushell.c:424) ``` Note we cannot use VALGRIND_FREELIKE_BLOCK() in freelist_free() because we have not actually freed the FreeListNode and need to dereference it in freelist_alloc() to decide whether to reuse the block. That would result in a use-after-free warning before we would get a chance to call VALGRIND_MALLOCLIKE_BLOCK() in the reuse path. Also note that this free list only ever grows: it never shrinks for the lifetime of the application, so nothing here will ever be truely freed, although unused elements are eligible for reuse. Fix suggested by Philip Withnall Related: #2076 --- gobject/gatomicarray.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/gobject/gatomicarray.c b/gobject/gatomicarray.c index 5ebf6b89b..43111e8c7 100644 --- a/gobject/gatomicarray.c +++ b/gobject/gatomicarray.c @@ -17,6 +17,7 @@ #include "config.h" +#include "../glib/gvalgrind.h" #include #include "gatomicarray.h" @@ -77,6 +78,11 @@ freelist_alloc (gsize size, gboolean reuse) mem = g_slice_alloc (real_size); mem = ((char *) mem) + sizeof (gsize); G_ATOMIC_ARRAY_DATA_SIZE (mem) = size; + +#if ENABLE_VALGRIND + VALGRIND_MALLOCLIKE_BLOCK (mem, real_size - sizeof (gsize), FALSE, FALSE); +#endif + return mem; } From fb6e10c959038d8a83dc9f52eb58763d00ece3e5 Mon Sep 17 00:00:00 2001 From: Michael Catanzaro Date: Mon, 23 Nov 2020 15:54:08 -0600 Subject: [PATCH 2/2] gtype: suppress valgrind memory leak warnings The problem occurs because we keep a pointer inside the allocated block, instead of a pointer to the start of the block. This memory exists for the lifetime of the application, so let's silence it. This is probably abuse of VALGRIND_MALLOCLIKE_BLOCK(), which is really intended for use in memory allocators, but gtype.c already uses it in two other places, and it's a practical solution. I wrote another larger fix for this issue that involves keeping an array of extra pointers when running under valgrind. This is simpler. Fix suggested by Philip Withnall ``` ==180238== 16 bytes in 1 blocks are possibly lost in loss record 3,078 of 16,075 ==180238== at 0x483BB1A: calloc (vg_replace_malloc.c:762) ==180238== by 0x5489495: g_malloc0 (gmem.c:132) ==180238== by 0x5489754: g_malloc0_n (gmem.c:364) ==180238== by 0x53FDBEE: type_set_qdata_W (gtype.c:3722) ==180238== by 0x53FDEE8: type_add_flags_W (gtype.c:3787) ==180238== by 0x53FC348: g_type_register_fundamental (gtype.c:2662) ==180238== by 0x53D969B: _g_enum_types_init (genums.c:124) ==180238== by 0x53FF058: gobject_init (gtype.c:4432) ==180238== by 0x53FF082: gobject_init_ctor (gtype.c:4493) ==180238== by 0x4010F29: call_init.part.0 (dl-init.c:72) ==180238== by 0x4011030: call_init (dl-init.c:30) ==180238== by 0x4011030: _dl_init (dl-init.c:119) ==180238== by 0x4002149: ??? (in /usr/lib64/ld-2.30.so) ``` Fixes #2076 --- gobject/gtype.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/gobject/gtype.c b/gobject/gtype.c index 909faf138..69cd27512 100644 --- a/gobject/gtype.c +++ b/gobject/gtype.c @@ -442,6 +442,10 @@ type_node_any_new_W (TypeNode *pnode, node = G_STRUCT_MEMBER_P (node, SIZEOF_FUNDAMENTAL_INFO); static_fundamental_type_nodes[ftype >> G_TYPE_FUNDAMENTAL_SHIFT] = node; type = ftype; + +#if ENABLE_VALGRIND + VALGRIND_MALLOCLIKE_BLOCK (node, node_size - SIZEOF_FUNDAMENTAL_INFO, FALSE, TRUE); +#endif } else type = (GType) node;