798234 – Cut Transaction discards the reference to the description/memo strings so that Paste Transaction will paste uninitialised data (or other strings)

Bug 798234 - Cut Transaction discards the reference to the description/memo strings so that Paste Transaction will paste uninitialised data (or other strings)

Summary: Cut Transaction discards the reference to the description/memo strings so tha...

Status:	RESOLVED FIXED

Alias:	None

Product:	GnuCash
Classification:	Unclassified
Component:	Register (show other bugs)
Version:	4.6
Hardware:	PC Linux

Importance:	Normal normal
Target Milestone:	---
Assignee:	ui
QA Contact:	ui

URL:
Whiteboard:
Keywords:

Depends on:
Blocks:

Reported:	2021-07-07 15:20 EDT by Simon Arlott
Modified:	2021-09-03 14:11 EDT (History)
CC List:	3 users (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Description Simon Arlott 2021-07-07 15:20:05 EDT

xaccFreeTransaction() on the transaction being cut is freeing the associated "orig" transaction (which is the new dupe_trans() transaction) too. The result is that the string reference counts are incremented by 1 and then decremented by 2.


This is a cut operation only and the description/memo strings are globally unique:

Thread 1 "gnucash" hit Breakpoint 2, qof_string_cache_insert (
    key=0x55a38445ea80 "Example Description") at /home/simon/src/gnucash/libgnucash/engine/qof-string-cache.cpp:111
111	{
(gdb) bt
#0  qof_string_cache_insert(char const*)
    (key=0x55a38445ea80 "Example Description") at /home/simon/src/gnucash/libgnucash/engine/qof-string-cache.cpp:111
#1  0x00007fdfd3d2939a in dupe_trans.lto_priv.1621 (from=0x55a3868c1120)
    at /home/simon/src/gnucash/libgnucash/engine/Transaction.c:606
#2  0x00007fdfd3d2b8f6 in xaccTransBeginEdit(Transaction*) (trans=0x55a3868c1120)
    at /home/simon/src/gnucash/libgnucash/engine/Transaction.c:1455
#3  0x00007fdfd3d2b94b in xaccTransDestroy(Transaction*) (trans=0x55a3868c1120)
    at /home/simon/src/gnucash/libgnucash/engine/Transaction.c:1469
#4  0x00007fdfcfde8aa1 in gnc_split_register_delete_current_trans (reg=0x55a382b07f70)
    at /home/simon/src/gnucash/gnucash/register/ledger-core/split-register.c:1250
#5  0x00007fdfcfde7b2e in gnc_split_register_cut_current (reg=0x55a382b07f70)
    at /home/simon/src/gnucash/gnucash/register/ledger-core/split-register.c:889
#6  0x00007fdfd5ddeea1 in gsr_default_cut_txn_handler (gsr=0x55a38841a8a0, data=0x0)
    at /home/simon/src/gnucash/gnucash/gnome/gnc-split-reg.c:1021
#7  0x00007fdfd5df59a4 in gnc_plugin_page_register_cmd_cut_transaction.lto_priv.1444
    (action=0x55a382ad8550, plugin_page=0x55a388525830)
    at /home/simon/src/gnucash/gnucash/gnome/gnc-plugin-page-register.c:3923

Thread 1 "gnucash" hit Breakpoint 2, qof_string_cache_insert (
    key=0x55a386894ce0 "Example Memo")
    at /home/simon/src/gnucash/libgnucash/engine/qof-string-cache.cpp:111
111	{
(gdb) bt
#0  qof_string_cache_insert(char const*)
    (key=0x55a386894ce0 "Example Memo")
    at /home/simon/src/gnucash/libgnucash/engine/qof-string-cache.cpp:111
#1  0x00007fdfd3d22fb6 in xaccDupeSplit(Split const*) (s=0x55a3868c20a0)
    at /home/simon/src/gnucash/libgnucash/engine/Split.c:567
#2  0x00007fdfd3d293dd in dupe_trans.lto_priv.1621 (from=0x55a3868c1120)
    at /home/simon/src/gnucash/libgnucash/engine/Transaction.c:611
#3  0x00007fdfd3d2b8f6 in xaccTransBeginEdit(Transaction*) (trans=0x55a3868c1120)
    at /home/simon/src/gnucash/libgnucash/engine/Transaction.c:1455
#4  0x00007fdfd3d2b94b in xaccTransDestroy(Transaction*) (trans=0x55a3868c1120)
    at /home/simon/src/gnucash/libgnucash/engine/Transaction.c:1469
#5  0x00007fdfcfde8aa1 in gnc_split_register_delete_current_trans (reg=0x55a382b07f70)
    at /home/simon/src/gnucash/gnucash/register/ledger-core/split-register.c:1250
#6  0x00007fdfcfde7b2e in gnc_split_register_cut_current (reg=0x55a382b07f70)
    at /home/simon/src/gnucash/gnucash/register/ledger-core/split-register.c:889
#7  0x00007fdfd5ddeea1 in gsr_default_cut_txn_handler (gsr=0x55a38841a8a0, data=0x0)
    at /home/simon/src/gnucash/gnucash/gnome/gnc-split-reg.c:1021
#8  0x00007fdfd5df59a4 in gnc_plugin_page_register_cmd_cut_transaction.lto_priv.1444
    (action=0x55a382ad8550, plugin_page=0x55a388525830)
    at /home/simon/src/gnucash/gnucash/gnome/gnc-plugin-page-register.c:3923

Thread 1 "gnucash" hit Breakpoint 1, qof_string_cache_remove (
    key=0x55a386894ce0 "Example Memo")
    at /home/simon/src/gnucash/libgnucash/engine/qof-string-cache.cpp:86
86	{
(gdb) bt
#0  qof_string_cache_remove(char const*)
    (key=0x55a386894ce0 "Example Memo")
    at /home/simon/src/gnucash/libgnucash/engine/qof-string-cache.cpp:86
#1  0x00007fdfd3d23423 in xaccFreeSplit(Split*) (split=0x55a3868c20a0)
    at /home/simon/src/gnucash/libgnucash/engine/Split.c:704
#2  0x00007fdfd3c55372 in qof_commit_edit_part2(QofInstance*, void (*)(QofInstance*, QofBackendError), void (*)(QofInstance*), void (*)(QofInstance*)) (inst=0x55a3868c20a0, on_error=
    0x7fdfd3d23fd2 <commit_err(QofInstance*, QofBackendError)>, on_done=0x0, on_free=0x7fdfd3d233b2 <xaccFreeSplit(Split*)>) at /home/simon/src/gnucash/libgnucash/engine/qofinstance.cpp:1040
#3  0x00007fdfd3d2435f in xaccSplitCommitEdit(Split*) (s=0x55a3868c20a0)
    at /home/simon/src/gnucash/libgnucash/engine/Split.c:1025
#4  0x00007fdfd3d2bb26 in do_destroy.lto_priv.1617 (trans=0x55a3868c1120)
    at /home/simon/src/gnucash/libgnucash/engine/Transaction.c:1533
#5  0x00007fdfd3c55372 in qof_commit_edit_part2(QofInstance*, void (*)(QofInstance*, QofBackendError), void (*)(QofInstance*), void (*)(QofInstance*)) (inst=0x55a3868c1120, on_error=
    0x7fdfd3d2bbed <trans_on_error.lto_priv.1619>, on_done=0x7fdfd3d2bc4c <trans_cleanup_commit.lto_priv.1620>, on_free=0x7fdfd3d2ba25 <do_destroy.lto_priv.1617>)
    at /home/simon/src/gnucash/libgnucash/engine/qofinstance.cpp:1040
#6  0x00007fdfd3d2c0e3 in xaccTransCommitEdit(Transaction*) (trans=0x55a3868c1120)
    at /home/simon/src/gnucash/libgnucash/engine/Transaction.c:1694
#7  0x00007fdfd3d2b968 in xaccTransDestroy(Transaction*) (trans=0x55a3868c1120)
    at /home/simon/src/gnucash/libgnucash/engine/Transaction.c:1471
#8  0x00007fdfcfde8aa1 in gnc_split_register_delete_current_trans (reg=0x55a382b07f70)
    at /home/simon/src/gnucash/gnucash/register/ledger-core/split-register.c:1250
#9  0x00007fdfcfde7b2e in gnc_split_register_cut_current (reg=0x55a382b07f70)
    at /home/simon/src/gnucash/gnucash/register/ledger-core/split-register.c:889
#10 0x00007fdfd5ddeea1 in gsr_default_cut_txn_handler (gsr=0x55a38841a8a0, data=0x0)
    at /home/simon/src/gnucash/gnucash/gnome/gnc-split-reg.c:1021
#11 0x00007fdfd5df59a4 in gnc_plugin_page_register_cmd_cut_transaction.lto_priv.1444
    (action=0x55a382ad8550, plugin_page=0x55a388525830)
    at /home/simon/src/gnucash/gnucash/gnome/gnc-plugin-page-register.c:3923

Thread 1 "gnucash" hit Breakpoint 1, qof_string_cache_remove (
    key=0x55a38445ea80 "Example Description") at /home/simon/src/gnucash/libgnucash/engine/qof-string-cache.cpp:86
86	{
(gdb) bt
#0  qof_string_cache_remove(char const*)
    (key=0x55a38445ea80 "Example Description") at /home/simon/src/gnucash/libgnucash/engine/qof-string-cache.cpp:86
#1  0x00007fdfd3d29c73 in xaccFreeTransaction.lto_priv.1615 (trans=0x55a3868c1120)
    at /home/simon/src/gnucash/libgnucash/engine/Transaction.c:814
#2  0x00007fdfd3d2bb61 in do_destroy.lto_priv.1617 (trans=0x55a3868c1120)
    at /home/simon/src/gnucash/libgnucash/engine/Transaction.c:1538
#3  0x00007fdfd3c55372 in qof_commit_edit_part2(QofInstance*, void (*)(QofInstance*, QofBackendError), void (*)(QofInstance*), void (*)(QofInstance*)) (inst=0x55a3868c1120, on_error=
    0x7fdfd3d2bbed <trans_on_error.lto_priv.1619>, on_done=0x7fdfd3d2bc4c <trans_cleanup_commit.lto_priv.1620>, on_free=0x7fdfd3d2ba25 <do_destroy.lto_priv.1617>)
    at /home/simon/src/gnucash/libgnucash/engine/qofinstance.cpp:1040
#4  0x00007fdfd3d2c0e3 in xaccTransCommitEdit(Transaction*) (trans=0x55a3868c1120)
    at /home/simon/src/gnucash/libgnucash/engine/Transaction.c:1694
#5  0x00007fdfd3d2b968 in xaccTransDestroy(Transaction*) (trans=0x55a3868c1120)
    at /home/simon/src/gnucash/libgnucash/engine/Transaction.c:1471
#6  0x00007fdfcfde8aa1 in gnc_split_register_delete_current_trans (reg=0x55a382b07f70)
    at /home/simon/src/gnucash/gnucash/register/ledger-core/split-register.c:1250
#7  0x00007fdfcfde7b2e in gnc_split_register_cut_current (reg=0x55a382b07f70)
    at /home/simon/src/gnucash/gnucash/register/ledger-core/split-register.c:889
#8  0x00007fdfd5ddeea1 in gsr_default_cut_txn_handler (gsr=0x55a38841a8a0, data=0x0)
    at /home/simon/src/gnucash/gnucash/gnome/gnc-split-reg.c:1021
#9  0x00007fdfd5df59a4 in gnc_plugin_page_register_cmd_cut_transaction.lto_priv.1444
    (action=0x55a382ad8550, plugin_page=0x55a388525830)
    at /home/simon/src/gnucash/gnucash/gnome/gnc-plugin-page-register.c:3923

Thread 1 "gnucash" hit Breakpoint 1, qof_string_cache_remove (
    key=0x55a386894ce0 "Example Memo")
    at /home/simon/src/gnucash/libgnucash/engine/qof-string-cache.cpp:86
86	{
(gdb) bt
#0  qof_string_cache_remove(char const*)
    (key=0x55a386894ce0 "Example Memo")
    at /home/simon/src/gnucash/libgnucash/engine/qof-string-cache.cpp:86
#1  0x00007fdfd3d23423 in xaccFreeSplit(Split*) (split=0x55a387ad0d70)
    at /home/simon/src/gnucash/libgnucash/engine/Split.c:704
#2  0x00007fdfd3d29c24 in xaccFreeTransaction.lto_priv.1615 (trans=0x55a393005250)
    at /home/simon/src/gnucash/libgnucash/engine/Transaction.c:808
#3  0x00007fdfd3d29ce7 in xaccFreeTransaction.lto_priv.1615 (trans=0x55a3868c1120)
    at /home/simon/src/gnucash/libgnucash/engine/Transaction.c:826
#4  0x00007fdfd3d2bb61 in do_destroy.lto_priv.1617 (trans=0x55a3868c1120)
    at /home/simon/src/gnucash/libgnucash/engine/Transaction.c:1538
#5  0x00007fdfd3c55372 in qof_commit_edit_part2(QofInstance*, void (*)(QofInstance*, QofBackendError), void (*)(QofInstance*), void (*)(QofInstance*)) (inst=0x55a3868c1120, on_error=
    0x7fdfd3d2bbed <trans_on_error.lto_priv.1619>, on_done=0x7fdfd3d2bc4c <trans_cleanup_commit.lto_priv.1620>, on_free=0x7fdfd3d2ba25 <do_destroy.lto_priv.1617>)
    at /home/simon/src/gnucash/libgnucash/engine/qofinstance.cpp:1040
#6  0x00007fdfd3d2c0e3 in xaccTransCommitEdit(Transaction*) (trans=0x55a3868c1120)
    at /home/simon/src/gnucash/libgnucash/engine/Transaction.c:1694
#7  0x00007fdfd3d2b968 in xaccTransDestroy(Transaction*) (trans=0x55a3868c1120)
    at /home/simon/src/gnucash/libgnucash/engine/Transaction.c:1471
#8  0x00007fdfcfde8aa1 in gnc_split_register_delete_current_trans (reg=0x55a382b07f70)
    at /home/simon/src/gnucash/gnucash/register/ledger-core/split-register.c:1250
#9  0x00007fdfcfde7b2e in gnc_split_register_cut_current (reg=0x55a382b07f70)
    at /home/simon/src/gnucash/gnucash/register/ledger-core/split-register.c:889
#10 0x00007fdfd5ddeea1 in gsr_default_cut_txn_handler (gsr=0x55a38841a8a0, data=0x0)
    at /home/simon/src/gnucash/gnucash/gnome/gnc-split-reg.c:1021
#11 0x00007fdfd5df59a4 in gnc_plugin_page_register_cmd_cut_transaction.lto_priv.1444
    (action=0x55a382ad8550, plugin_page=0x55a388525830)
    at /home/simon/src/gnucash/gnucash/gnome/gnc-plugin-page-register.c:3923

Thread 1 "gnucash" hit Breakpoint 1, qof_string_cache_remove (
    key=0x55a38445ea80 "Example Description") at /home/simon/src/gnucash/libgnucash/engine/qof-string-cache.cpp:86
86	{
(gdb) bt
#0  qof_string_cache_remove(char const*)
    (key=0x55a38445ea80 "Example Description") at /home/simon/src/gnucash/libgnucash/engine/qof-string-cache.cpp:86
#1  0x00007fdfd3d29c73 in xaccFreeTransaction.lto_priv.1615 (trans=0x55a393005250)
    at /home/simon/src/gnucash/libgnucash/engine/Transaction.c:814
#2  0x00007fdfd3d29ce7 in xaccFreeTransaction.lto_priv.1615 (trans=0x55a3868c1120)
    at /home/simon/src/gnucash/libgnucash/engine/Transaction.c:826
#3  0x00007fdfd3d2bb61 in do_destroy.lto_priv.1617 (trans=0x55a3868c1120)
    at /home/simon/src/gnucash/libgnucash/engine/Transaction.c:1538
#4  0x00007fdfd3c55372 in qof_commit_edit_part2(QofInstance*, void (*)(QofInstance*, QofBackendError), void (*)(QofInstance*), void (*)(QofInstance*)) (inst=0x55a3868c1120, on_error=
    0x7fdfd3d2bbed <trans_on_error.lto_priv.1619>, on_done=0x7fdfd3d2bc4c <trans_cleanup_commit.lto_priv.1620>, on_free=0x7fdfd3d2ba25 <do_destroy.lto_priv.1617>)
    at /home/simon/src/gnucash/libgnucash/engine/qofinstance.cpp:1040
#5  0x00007fdfd3d2c0e3 in xaccTransCommitEdit(Transaction*) (trans=0x55a3868c1120)
    at /home/simon/src/gnucash/libgnucash/engine/Transaction.c:1694
#6  0x00007fdfd3d2b968 in xaccTransDestroy(Transaction*) (trans=0x55a3868c1120)
    at /home/simon/src/gnucash/libgnucash/engine/Transaction.c:1471
#7  0x00007fdfcfde8aa1 in gnc_split_register_delete_current_trans (reg=0x55a382b07f70)
    at /home/simon/src/gnucash/gnucash/register/ledger-core/split-register.c:1250
#8  0x00007fdfcfde7b2e in gnc_split_register_cut_current (reg=0x55a382b07f70)
    at /home/simon/src/gnucash/gnucash/register/ledger-core/split-register.c:889
#9  0x00007fdfd5ddeea1 in gsr_default_cut_txn_handler (gsr=0x55a38841a8a0, data=0x0)
    at /home/simon/src/gnucash/gnucash/gnome/gnc-split-reg.c:1021
#10 0x00007fdfd5df59a4 in gnc_plugin_page_register_cmd_cut_transaction.lto_priv.1444
    (action=0x55a382ad8550, plugin_page=0x55a388525830)
    at /home/simon/src/gnucash/gnucash/gnome/gnc-plugin-page-register.c:3923

Comment 1 Simon Arlott 2021-07-07 15:24:36 EDT

Looks like the problem is in gnc_txn_to_float_txn() where the string is copied without taking a new reference.

Comment 2 Simon Arlott 2021-07-07 15:58:46 EDT

Take a new reference to the strings when using them in floating transactions/splits:
https://github.com/Gnucash/gnucash/pull/1071

Comment 3 Simon Arlott 2021-07-07 16:11:37 EDT

Assuming this is a correct fix (and the Transaction being cut is supposed to be freed completely) then the unit test for cutting could free the transaction and check that the strings still work but that will require unique strings and the ability to inspect the string cache.

Comment 4 John Ralls 2021-09-03 11:59:07 EDT

Simon,

The PR is merged, can this be closed?

Comment 5 Simon Arlott 2021-09-03 12:51:39 EDT

Yes

Note You need to log in before you can comment on or make changes to this bug.