From dc62959553f59a0a88dcc803eeac6d0eb6a2b77b Mon Sep 17 00:00:00 2001 From: Richard Cohen Date: Mon, 15 May 2023 18:58:10 +0100 Subject: [PATCH 1/3] Fix crash in test-engine on Arch Thanks, Valgrind: ==515314== Invalid read of size 8 ==515314== at 0x4ED46F3: gncInvoiceRemoveEntries (gncInvoice.c:767) ==515314== by 0x142B35: teardown_with_invoice (utest-Invoice.c:274) ... ==515314== Address 0x8557b98 is 8 bytes inside a block of size 24 free'd ==515314== at 0x484620F: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==515314== by 0x51B565D: g_list_remove (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7600.1) ==515314== by 0x4ED42EF: gncInvoiceRemoveEntry (gncInvoice.c:688) ==515314== by 0x4ED46A2: gncInvoiceRemoveEntries (gncInvoice.c:781) ==515314== by 0x142B35: teardown_with_invoice (utest-Invoice.c:274) ... ==515314== Block was alloc'd at ==515314== at 0x4843828: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==515314== by 0x51BD948: g_malloc (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7600.1) ==515314== by 0x51B1CB9: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7600.1) ==515314== by 0x4ED4271: gncInvoiceAddEntry (gncInvoice.c:676) ==515314== by 0x142401: setup_with_invoice (utest-Invoice.c:142) ... ok 57 /engine/gncInvoice/post trans - vendor bill --- libgnucash/engine/gncInvoice.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/libgnucash/engine/gncInvoice.c b/libgnucash/engine/gncInvoice.c index f4ad3cb66b..fc5220795d 100644 --- a/libgnucash/engine/gncInvoice.c +++ b/libgnucash/engine/gncInvoice.c @@ -760,12 +760,13 @@ void gncInvoiceSortEntries (GncInvoice *invoice) void gncInvoiceRemoveEntries (GncInvoice *invoice) { - GList *node; - if (!invoice) return; - for (node = invoice->entries; node; node = node->next) + // gnc{Bill,Invoice}RemoveEntry free the "entry" node. + // Make sure to save "next" first. + for (GList *next, *node = invoice->entries; node; node = next) { + next = node->next; GncEntry *entry = node->data; switch (gncInvoiceGetOwnerType (invoice)) From bed43c7f3b8a2dce20aa241800b26f3ae65f0b8d Mon Sep 17 00:00:00 2001 From: Richard Cohen Date: Mon, 15 May 2023 19:12:33 +0100 Subject: [PATCH 2/3] Fix potential crash in test-engine ==515314== Invalid read of size 1 ==515314== at 0x484AD67: __strcmp_sse42 (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==515314== by 0x171D36: do_test_list_handler (unittest-support.c:181) ==515314== by 0x171DCE: test_list_handler (unittest-support.c:197) ==515314== by 0x51BD4C1: g_logv (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7600.1) ==515314== by 0x51BD7A2: g_log (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7600.1) ==515314== by 0x4D5D0D9: xaccSplitEqualCheckBal (Split.c:753) ==515314== by 0x4D5D841: xaccSplitEqual (Split.c:869) ==515314== by 0x4D647A5: xaccTransEqual (Transaction.c:981) ==515314== by 0x15C0E8: test_xaccTransEqual(Fixture*, void const*) (utest-Transaction.cpp:901) ... ==515314== Address 0x8725260 is 0 bytes inside a block of size 59 free'd ==515314== at 0x484620F: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==515314== by 0x15BDB1: test_xaccTransEqual(Fixture*, void const*) (utest-Transaction.cpp:883) ... ==515314== Block was alloc'd at ==515314== at 0x4843828: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==515314== by 0x5618677: __vasprintf_internal (vasprintf.c:116) ==515314== by 0x520E8C1: g_vasprintf (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7600.1) ==515314== by 0x51DBBE0: g_strdup_vprintf (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7600.1) ==515314== by 0x51DBC9C: g_strdup_printf (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7600.1) ==515314== by 0x15BBAC: test_xaccTransEqual(Fixture*, void const*) (utest-Transaction.cpp:879) ... ok 78 /engine/Transaction/xaccTransEqual --- libgnucash/engine/test/utest-Transaction.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/libgnucash/engine/test/utest-Transaction.cpp b/libgnucash/engine/test/utest-Transaction.cpp index 0a0f676ae9..3c86a780b4 100644 --- a/libgnucash/engine/test/utest-Transaction.cpp +++ b/libgnucash/engine/test/utest-Transaction.cpp @@ -881,6 +881,7 @@ test_xaccTransEqual (Fixture *fixture, gconstpointer pData) frame->set({"qux", "quux", "corge"}, new KvpValue(123.456)); xaccTransCommitEdit (clone); g_free (cleanup->msg); + cleanup->msg = NULL; g_free (check->msg); check->msg = g_strdup ("[xaccSplitEqual] GUIDs differ"); auto split1 = xaccTransGetSplit (clone, 0); From 81fba56bbb5266cc58417236418175ddb257cebd Mon Sep 17 00:00:00 2001 From: Richard Cohen Date: Mon, 15 May 2023 19:36:30 +0100 Subject: [PATCH 3/3] Initialise guids in test-engine-kvp-properties.c Valgrind: Conditional jump or move depends on uninitialised value(s) --- .../engine/test/test-engine-kvp-properties.c | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/libgnucash/engine/test/test-engine-kvp-properties.c b/libgnucash/engine/test/test-engine-kvp-properties.c index 7a11dfd050..6491f36e79 100644 --- a/libgnucash/engine/test/test-engine-kvp-properties.c +++ b/libgnucash/engine/test/test-engine-kvp-properties.c @@ -142,7 +142,7 @@ test_account_kvp_properties (Fixture *fixture, gconstpointer pData) gchar *ab_acct_id = "1234-5678-9087"; gchar *ab_bank_code = "0032340"; gchar *online_id_r, *ab_acct_id_r, *ab_bank_code_r; - GncGUID *ofx_income_acct = guid_malloc (); + GncGUID *ofx_income_acct = guid_new (); GncGUID *ofx_income_acct_r; Time64 trans_retr = {gnc_time(NULL)}; Time64 *trans_retr_r; @@ -186,8 +186,8 @@ test_account_kvp_properties (Fixture *fixture, gconstpointer pData) static void test_trans_kvp_properties (Fixture *fixture, gconstpointer pData) { - GncGUID *invoice = guid_malloc (); - GncGUID *from_sx = guid_malloc (); + GncGUID *invoice = guid_new (); + GncGUID *from_sx = guid_new (); GncGUID *invoice_r, *from_sx_r; gchar *online_id = "my online id"; gchar *online_id_r; @@ -227,7 +227,7 @@ test_split_kvp_properties (Fixture *fixture, gconstpointer pData) gchar *online_id = "my_online_id"; gchar *debit_formula_r, *credit_formula_r, *sx_shares_r; gchar *online_id_r; - GncGUID *sx_account = guid_malloc (); + GncGUID *sx_account = guid_new (); GncGUID *sx_account_r; gnc_numeric debit_numeric = gnc_numeric_create (123, 456); gnc_numeric credit_numeric = gnc_numeric_create (789, 456); @@ -285,11 +285,11 @@ test_split_kvp_properties (Fixture *fixture, gconstpointer pData) static void test_lot_kvp_properties (Fixture *fixture, gconstpointer pData) { - GncGUID *invoice = guid_malloc (); + GncGUID *invoice = guid_new (); GncGUID *invoice_r; gint64 owner_type = 47; gint64 owner_type_r; - GncGUID *owner = guid_malloc (); + GncGUID *owner = guid_new (); GncGUID *owner_r; qof_begin_edit (QOF_INSTANCE (fixture->lot)); @@ -322,8 +322,8 @@ test_customer_kvp_properties (Fixture *fixture, gconstpointer pData) { gchar *pdf_dir = "/foo/bar/baz"; gchar *pdf_dir_r; - GncGUID *inv_acct = guid_malloc (); - GncGUID *pmt_acct = guid_malloc (); + GncGUID *inv_acct = guid_new (); + GncGUID *pmt_acct = guid_new (); GncGUID *inv_acct_r, *pmt_acct_r; qof_begin_edit (QOF_INSTANCE (fixture->cust)); @@ -358,8 +358,8 @@ test_employee_kvp_properties (Fixture *fixture, gconstpointer pData) { gchar *pdf_dir = "/foo/bar/baz"; gchar *pdf_dir_r; - GncGUID *inv_acct = guid_malloc (); - GncGUID *pmt_acct = guid_malloc (); + GncGUID *inv_acct = guid_new (); + GncGUID *pmt_acct = guid_new (); GncGUID *inv_acct_r, *pmt_acct_r; qof_begin_edit (QOF_INSTANCE (fixture->emp)); @@ -417,8 +417,8 @@ test_vendor_kvp_properties (Fixture *fixture, gconstpointer pData) { gchar *pdf_dir = "/foo/bar/baz"; gchar *pdf_dir_r; - GncGUID *inv_acct = guid_malloc (); - GncGUID *pmt_acct = guid_malloc (); + GncGUID *inv_acct = guid_new (); + GncGUID *pmt_acct = guid_new (); GncGUID *inv_acct_r, *pmt_acct_r; qof_begin_edit (QOF_INSTANCE (fixture->vend));