From 958e9a4d6fadc631e0257e40a23539ce26e0f58e Mon Sep 17 00:00:00 2001 From: Richard Cohen Date: Thu, 15 Jun 2023 20:09:14 +0100 Subject: [PATCH 1/2] Valgrind: fix "Invalid read" in test-exp-parser The pointer passed to the gfec exception handler is not valid after the exception handler ends ==382525== Invalid read of size 1 ==382525== at 0x484B050: memcpy@GLIBC_2.2.5 (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==382525== by 0x531AB67: __printf_buffer_write (Xprintf_buffer_write.c:39) ==382525== by 0x5322CD4: __printf_buffer (vfprintf-process-arg.c:501) ==382525== by 0x53465F3: __vasprintf_internal (vasprintf.c:102) ==382525== by 0x499C8C1: g_vasprintf (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7600.1) ==382525== by 0x4969BE0: g_strdup_vprintf (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7600.1) ==382525== by 0x494B2F6: g_logv (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7600.1) ==382525== by 0x494B7A2: g_log (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7600.1) ==382525== by 0x4862862: func_op (gnc-exp-parser.c:342) ==382525== by 0x485D381: primary_exp (expression_parser.c:1222) ==382525== by 0x485CE0F: multiply_divide_op (expression_parser.c:1028) ==382525== by 0x485CC33: add_sub_op (expression_parser.c:968) ==382525== Address 0xad387ec is 764 bytes inside a block of size 766 free'd ==382525== at 0x484620F: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==382525== by 0x4861D9B: gfec_apply (gfec.c:145) ==382525== by 0x4862813: func_op (gnc-exp-parser.c:339) ==382525== by 0x485D381: primary_exp (expression_parser.c:1222) ==382525== by 0x485CE0F: multiply_divide_op (expression_parser.c:1028) ==382525== by 0x485CC33: add_sub_op (expression_parser.c:968) ==382525== by 0x485C9DE: assignment_op (expression_parser.c:886) ==382525== by 0x485C101: parse_string (expression_parser.c:605) ==382525== by 0x4862F07: gnc_exp_parser_parse_separate_vars (gnc-exp-parser.c:535) ==382525== by 0x4862D60: gnc_exp_parser_parse (gnc-exp-parser.c:475) ==382525== by 0x10A963: run_parser_test (test-exp-parser.c:94) ==382525== by 0x10AB99: run_parser_tests (test-exp-parser.c:144) ==382525== Block was alloc'd at ==382525== at 0x4843738: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==382525== by 0x4A81473: scm_realloc (in /usr/lib/x86_64-linux-gnu/libguile-3.0.so.1.5.0) ==382525== by 0x4AEEF70: scm_to_utf8_stringn (in /usr/lib/x86_64-linux-gnu/libguile-3.0.so.1.5.0) ==382525== by 0x55AA71F: gnc_scm_to_utf8_string (gnc-guile-utils.c:42) ==382525== by 0x4861D4E: gfec_apply (gfec.c:136) ==382525== by 0x4862813: func_op (gnc-exp-parser.c:339) ==382525== by 0x485D381: primary_exp (expression_parser.c:1222) ==382525== by 0x485CE0F: multiply_divide_op (expression_parser.c:1028) ==382525== by 0x485CC33: add_sub_op (expression_parser.c:968) ==382525== by 0x485C9DE: assignment_op (expression_parser.c:886) ==382525== by 0x485C101: parse_string (expression_parser.c:605) ==382525== by 0x4862F07: gnc_exp_parser_parse_separate_vars (gnc-exp-parser.c:535) --- libgnucash/app-utils/gnc-exp-parser.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/libgnucash/app-utils/gnc-exp-parser.c b/libgnucash/app-utils/gnc-exp-parser.c index dcb9cb80ed..ebca57a74a 100644 --- a/libgnucash/app-utils/gnc-exp-parser.c +++ b/libgnucash/app-utils/gnc-exp-parser.c @@ -280,11 +280,10 @@ update_variables (var_store_ptr vars) } } -static char* _function_evaluation_error_msg = NULL; static void _exception_handler(const char *error_message) { - _function_evaluation_error_msg = (char*)error_message; + PERR("function eval error: [%s]\n", error_message); } static @@ -335,14 +334,9 @@ func_op(const char *fname, int argc, void **argv) scmArgs = scm_cons( scmTmp, scmArgs ); } - //scmTmp = scm_apply(scmFn, scmArgs , SCM_EOL); scmTmp = gfec_apply(scmFn, scmArgs, _exception_handler); - if (_function_evaluation_error_msg != NULL) - { - PERR("function eval error: [%s]\n", _function_evaluation_error_msg); - _function_evaluation_error_msg = NULL; + if (scmTmp == SCM_UNDEFINED) return NULL; - } if (!scm_is_number (scmTmp)) { From 3c0b051ab0f9855cf4650801c6d57d8e593a4dd4 Mon Sep 17 00:00:00 2001 From: Richard Cohen Date: Mon, 19 Jun 2023 16:51:22 +0100 Subject: [PATCH 2/2] Valgrind: fix "Mismatched free/delete" for gnc_print_time64, qof_print_date ... when called from guile e.g. test-transaction and lots of other tests - gchar will also get the char* typemap, by typedef reduction ==119964== Mismatched free() / delete / delete [] ==119964== at 0x4847A1F: operator delete[](void*) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==119964== by 0x669F3E4: _wrap_gnc_print_time64(scm_unused_struct*, scm_unused_struct*) (swig-engine.cpp:38533) ... ==119948== Mismatched free() / delete / delete [] ==119948== at 0x4847A1F: operator delete[](void*) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==119948== by 0x6F6B431: _wrap_qof_print_date(scm_unused_struct*) (swig-engine.cpp:39124) ... --- bindings/engine.i | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bindings/engine.i b/bindings/engine.i index 186a0860a8..7e849af8bb 100644 --- a/bindings/engine.i +++ b/bindings/engine.i @@ -74,7 +74,7 @@ GLIST_HELPER_INOUT(PriceList, SWIGTYPE_p_GNCPrice); // TODO: free PriceList? GLIST_HELPER_INOUT(CommodityList, SWIGTYPE_p_gnc_commodity); -%typemap(newfree) gchar * "g_free($1);" +%typemap(newfree) char * "g_free($1);" /* These need to be here so that they are *before* the function declarations in the header files, some of which are included by