Windows (?) Table Crash

[Richard Kimberly Heck]

On 6/2/24 13:18, Udicoudco wrote:
> On Sun, Jun 2, 2024 at 7:53 PM Richard Kimberly Heck <rikiheck at gmail.com> wrote:
>> On 6/2/24 12:35, Udicoudco wrote:
>>> On Sun, Jun 2, 2024 at 7:32 PM Richard Kimberly Heck <rikiheck at gmail.com> wrote:
>>>> We've had a report of the following sort of crash, or maybe assertion.
>>>>
>>>> Create a table. Mark more than half the rows or columns. Delete those
>>>> (using the toolbar button, but I doubt that matters). Boom.
>>>>
>>>> I cannot reproduce on Linux, but Eugene was able to reproduce on
>>>> Windows. Anyone else?
>>> I can reproduce on Windows as well.
>> Can you get a backtrace?
> Here is the output when running with -dbg any from the point of
> clicking the delete rows button:
>
> Undo.cpp (23b): +++++++ Creating new group c for buffer 000002A164FFF720
> frontends\qt\GuiApplication.cpp (6d8): cmd:  action: 362
> [tabular-feature]  arg: 'delete-row' x: 0 y: 0
> BufferView.cpp (556): BufferView::dispatch: cmd:  action: 362
> [tabular-feature]  arg: 'delete-row' x: 0 y: 0
> Buffer.cpp (b55): Buffer::dispatch: cmd:  action: 362
> [tabular-feature]  arg: 'delete-row' x: 0 y: 0
> Cursor.cpp (30d): Cursor::dispatch: cmd:  action: 362
> [tabular-feature]  arg: 'delete-row' x: 0 y: 0
>
>   cursor:                                | anchor:
>   inset: 000002A163DE44B0 idx: 0 par: 0 pos: 0 | inset:
> 000002A163DE44B0 idx: 0 par: 0 pos: 0
>   inset: 000002A1640B77B0 idx: 12 par: 0 pos: 0 | inset:
> 000002A1640B77B0 idx: 5 par: 0 pos: 0
>   selection: 1 boundary: 0
>
> Cursor.cpp (32c): Cursor::dispatch: cmd:  action: 362
> [tabular-feature]  arg: 'delete-row' x: 0 y: 0
>
>   cursor:                                | anchor:
>   inset: 000002A163DE44B0 idx: 0 par: 0 pos: 0 | inset:
> 000002A163DE44B0 idx: 0 par: 0 pos: 0
>   inset: 000002A1640B77B0 idx: 12 par: 0 pos: 0 | inset:
> 000002A1640B77B0 idx: 5 par: 0 pos: 0
>   selection: 1 boundary: 0
>
> insets\InsetTabular.cpp (13bd): # InsetTabular::doDispatch: cmd:
> action: 362 [tabular-feature]  arg: 'delete-row' x: 0 y: 0
>    cur:
>   cursor:                                | anchor:
>   inset: 000002A163DE44B0 idx: 0 par: 0 pos: 0 | inset:
> 000002A163DE44B0 idx: 0 par: 0 pos: 0
>   inset: 000002A1640B77B0 idx: 12 par: 0 pos: 0 | inset:
> 000002A1640B77B0 idx: 5 par: 0 pos: 0
>   selection: 1 boundary: 0
>
> Undo.cpp (152): Create undo element of group c
> insets\InsetTabular.cpp (19d9): Feature: delete-row             value:
>   C:/Users/pc1/newfile14.lyx.emergency
> support/os_win32.cpp (149): <Win32 path correction>
> [~/newfile14.lyx.emergency]->>[~\newfile14.lyx.emergency]
>
> Is this what you meant (or is there a better way to backtrace)?

That's helpful, but what I had in mind is the call stack at the time of 
the crash.

Still, I think I see the problem now. This change:

          for (row_type r = sel_row_start; r <= sel_row_end; ++r)
-            tabular.deleteRow(sel_row_start);
+            tabular.deleteRow(r);
          if (sel_row_start >= tabular.nrows())
              --sel_row_start;
          cur.idx() = tabular.cellIndex(sel_row_start, column);

in InsetTabular, l. 6783, is the culprit. As rows are deleted, the 
variable r can point to an invalid row (and, in the recipe, will do so 
if we ask to delete more than half the rows, from the top). It would 
appear that, on my system, a request to delete beyond the end of the 
vector deletes the last element, so it 'just works', even though it 
should crash.

The old code didn't have this problem: It just kept deleting the 'same' 
row number, as they got moved up. It got changed to handle 
change-tracking. I think the solution is to do this in reverse:

for (row_type r = sel_row_end; r >= sel_row_start; --r)
     tabular.deleteRow(r);

And maybe put an assertion into Tabular::deleteRow to prevent this kind 
of crash.

The same issue exists in Tabular::DELETE_COLUMN.

Riki