After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 741197 - Crash on undo-insert-row
Crash on undo-insert-row
Status: RESOLVED FIXED
Product: Gnumeric
Classification: Applications
Component: Main System
git master
Other All
: Normal major
: ---
Assigned To: Jody Goldberg
Jody Goldberg
: 757276 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2014-12-06 15:00 UTC by Morten Welinder
Modified: 2015-10-29 13:52 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Morten Welinder 2014-12-06 15:00:06 UTC
Original report:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=772075

Test file:
https://bugs.debian.org/cgi-bin/bugreport.cgi?msg=5;filename=test.gnumeric;att=1;bug=772075

Instructions:
1. Insert row before row 9
2. Undo


Preliminary analysis:

==3107== Invalid read of size 8
==3107==    at 0x4EEDD91: dependent_set_expr (dependent.c:416)
==3107==    by 0x4EF15F1: dependents_unrelocate (dependent.c:2037)
==3107==    by 0x54F33D7: go_undo_group_undo (go-undo.c:152)
==3107==    by 0x54F33D7: go_undo_group_undo (go-undo.c:152)
==3107==    by 0x4ED807C: cmd_ins_del_colrow_undo (commands.c:1328)
==3107==    by 0x4ED5A45: command_undo (commands.c:396)
==3107==    by 0x500E99E: cb_undo_activated (wbc-gtk.c:3349)
...
==3107==  Address 0x8 is not stack'd, malloc'd or (recently) free'd
Comment 1 Morten Welinder 2014-12-06 18:07:52 UTC
The why is well understood:

When we insert a row, we do a number of things:

	/* 1. Delete all rows (and their cells) that will fall off the end */
(not relevant here)

	/* 2. Fix references to and from the cells which are moving */
(this includes references in all types of dependencies, such as cell references
in the solver setup.  Relevant here is that it includes style dependencies,
such as conditional formats and that we collect undo information to patch
up every change we make.)

	/* 3. Move the rows to their new location (from last to first) */
(change the location of cells -- not a problem)

        [4] Move the style to the right

Step 4 (which isn't numbered in the code) is implemented as several steps.
The ones relevant here are:
4a. Collect styles in the area we move.
4b. Clear styles in the target area.
4c. Re-apply the styles shifted to the right.

Note, that step 4b clears the style dependencies of conditional formats which
we collected in step 2.  When we undo, we find that our undo information from
step 2 points to freed memory.  *Poof*
Comment 2 Morten Welinder 2014-12-06 18:34:48 UTC
This problem has been fixed in our software repository. The fix will go into the next software release. Thank you for your bug report.
Comment 3 Morten Welinder 2014-12-06 21:16:08 UTC
For posterity: the solution is to clear the style dependents at a new step 1.5
It's ok to do that since step 4c reintroduces new style dependents.
Comment 4 Morten Welinder 2015-10-29 13:52:14 UTC
*** Bug 757276 has been marked as a duplicate of this bug. ***