gnucash stable: Revert "Fix two use-after-free issues found by address sanitizer."

[John Ralls]

Updated	 via  https://github.com/Gnucash/gnucash/commit/8546aa97 (commit)
	from  https://github.com/Gnucash/gnucash/commit/4dbf8030 (commit)



commit 8546aa975ef590c245c9654395d841ea140c057f
Author: John Ralls <jralls at ceridwen.us>
Date:   Mon Feb 19 21:11:54 2024 -0800

    Revert "Fix two use-after-free issues found by address sanitizer."
    
    This reverts commit 4dbf8030411a1e2287be6e939336668e10f4a03a.
    
    The use-after free errors are caused by the compiler reordering the
    steps in xaccSplitFree and Transaction's do_destroy. Unfortunately the
    corrections here caused trouble in other places, leading to test failures.

diff --git a/libgnucash/engine/Split.cpp b/libgnucash/engine/Split.cpp
index f481a57df0..2da0c51b9f 100644
--- a/libgnucash/engine/Split.cpp
+++ b/libgnucash/engine/Split.cpp
@@ -706,13 +706,7 @@ xaccFreeSplit (Split *split)
     }
     CACHE_REMOVE(split->memo);
     CACHE_REMOVE(split->action);
-    if (GNC_IS_ACCOUNT (split->acc) && !qof_instance_get_destroying (QOF_INSTANCE (split->acc)))
-        gnc_account_remove_split (split->acc, split);
-    if (GNC_IS_LOT (split->lot) && !qof_instance_get_destroying (QOF_INSTANCE (split->lot)))
-        gnc_lot_remove_split (split->lot, split);
-/* We should do the same for split->parent but we might be getting
- * called from xaccFreeTransactiob abd tgat would cause trouble.
- */
+
     /* Just in case someone looks up freed memory ... */
     split->memo        = (char *) 1;
     split->action      = NULL;
diff --git a/libgnucash/engine/Transaction.cpp b/libgnucash/engine/Transaction.cpp
index 0ac8b346c4..899adaa9d0 100644
--- a/libgnucash/engine/Transaction.cpp
+++ b/libgnucash/engine/Transaction.cpp
@@ -1509,11 +1509,7 @@ do_destroy (Transaction *trans)
        done for the next split, then a split will still be on the split list after it
        has been freed.  This can cause other parts of the code (e.g. in xaccSplitDestroy())
        to reference the split after it has been freed. */
-
-    auto splits = trans->splits;
-    trans->splits = NULL;
-
-    for (node = splits; node; node = node->next)
+    for (node = trans->splits; node; node = node->next)
     {
         Split *s = GNC_SPLIT(node->data);
         if (s && s->parent == trans)
@@ -1521,7 +1517,7 @@ do_destroy (Transaction *trans)
             xaccSplitDestroy(s);
         }
     }
-    for (node = splits; node; node = node->next)
+    for (node = trans->splits; node; node = node->next)
     {
         Split *s = GNC_SPLIT(node->data);
         if (s && s->parent == trans)
@@ -1530,6 +1526,7 @@ do_destroy (Transaction *trans)
         }
     }
     g_list_free (trans->splits);
+    trans->splits = NULL;
     xaccFreeTransaction (trans);
 }
 



Summary of changes:
 libgnucash/engine/Split.cpp       | 8 +-------
 libgnucash/engine/Transaction.cpp | 9 +++------
 2 files changed, 4 insertions(+), 13 deletions(-)