Bug 132714 - Crash when icon deleting table-row used for diagram generation.
Summary: Crash when icon deleting table-row used for diagram generation.
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
5.1.0.3 release
Hardware: All All
: high critical
Assignee: Not Assigned
URL:
Whiteboard:
Keywords: bibisected, bisected, haveBacktrace, regression
Depends on:
Blocks: Chart Crash
  Show dependency treegraph
 
Reported: 2020-05-05 12:20 UTC by Sion
Modified: 2022-08-11 12:52 UTC (History)
9 users (show)

See Also:
Crash report or crash signature: ["SwTableBox::GetSttIdx()"]
Regression By:


Attachments
bt with debug symbols (14.05 KB, text/plain)
2021-03-27 21:34 UTC, Julien Nabet
Details
Valgrind trace (44.49 KB, application/x-bzip)
2021-03-28 15:35 UTC, Julien Nabet
Details
bt with debug symbols (7.34 KB, text/plain)
2021-11-26 16:43 UTC, Julien Nabet
Details
Valgrind trace (59.13 KB, text/x-log)
2021-11-26 17:00 UTC, Julien Nabet
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Sion 2020-05-05 12:20:47 UTC
Description:
First, this is the bug report:
https://crashreport.libreoffice.org/stats/crash_details/f22c0a1f-e712-41b7-8d9a-0414f1101036

(I couldn't find any way to add recreation details to the automatic crash report (consider adding a field for user text before sending the automatic crash report, so I don't have to post several crash reports again)).

Here are recreation details:
#1: Make a table (in my case: X=3 & Y=7).
#2: Fill in some random numbers in the table.
#3: Select the table & go to: "Insert > Diagram".
#4: Generate a diagram from the data in the table.
#4.1: When done & the diagram works as expected.
#5: Go to the table & delete 1 row.
#6: Double click the diagram, so it updates the values. BOOOOM!!!
#7: Crash!

Additional data:
#1.1 In my case I used X-row-1 for axis-titles, set to heading-2
I also left the 2'nd row empty (because I tried to make the diagram not look weird (didn't work)).
#3.1: In my case I used a line diagram with data points.

Crash Cause assumption:
I assume that the diagram isn't updated properly & tries to read the last row that no longer exist & therefore returns some undefined- or null-value that isn't captured & handled as it should.

Hope this is helpful.
Good luck!

Steps to Reproduce:
see description.

Copy from description:

Here are recreation details:
#1: Make a table (in my case: X=3 & Y=7).
#2: Fill in some random numbers in the table.
#3: Select the table & go to: "Insert > Diagram".
#4: Generate a diagram from the data in the table.
#4.1: When done & the diagram works as expected.
#5: Go to the table & delete 1 row.
#6: Double click the diagram, so it updates the values. BOOOOM!!!
#7: Crash!

Additional data:
#1.1 In my case I used X-row-1 for axis-titles, set to heading-2
I also left the 2'nd row empty (because I tried to make the diagram not look weird (didn't work)).
#3.1: In my case I used a line diagram with data points.


Actual Results:
Crash to desktop & recreation of the document with possibly some loss of data.

Expected Results:
A working diagram.


Reproducible: Always


User Profile Reset: No



Additional Info:
LibreOffice version: 6.2.8.2 An older version?

see description.

Crash Cause assumption:
I assume that the diagram isn't updated properly & tries to read the last row that no longer exist & therefore returns some undefined- or null-value that isn't captured & handled as it should.
Comment 1 Telesto 2020-05-05 17:21:50 UTC
1. Open attachment 160394 [details]
2. Select A1 until C2
3. Right click above cell C2 (containing 2)
4. Delete rows
5. Double click the chart -> Crash

Not in
Versie: 4.4.7.2 
Build ID: f3153a8b245191196a4b6b9abd1d0da16eead600
Locale: nl_NL
Comment 2 Aron Budea 2020-05-09 02:59:32 UTC
Already crashes with Telesto's sample in oldest commit of bibisect-linux-64-5.2, but fine in latest commit of bibisect-50max. Needs to be bibisected in Windows.
Comment 3 Buovjaga 2020-05-11 14:40:16 UTC
(In reply to Aron Budea from comment #2)
> Already crashes with Telesto's sample in oldest commit of
> bibisect-linux-64-5.2, but fine in latest commit of bibisect-50max. Needs to
> be bibisected in Windows.

Not sure why on Windows? Did you test with 5.1 repo as well?

I was able to repro the crash exactly once on Windows, with master. Modification to Telesto's step 3 was that I did not click above the cell containing 2, but on it. However, I was then no longer able to repro with the same version or master of Win repos 5.2, 5.3, 5.4, 6.2, 6.3, 6.4, 6.5.

It seems either Telesto needs to bibisect or I need some help with reliability of crashing.

Version: 7.0.0.0.alpha0+ (x64)
Build ID: 00db5933ded1884b2ac453552badae20fa943478
CPU threads: 4; OS: Windows 10.0 Build 18362; UI render: default; VCL: win; 
Locale: fi-FI (fi_FI); UI-Language: en-US
Calc: threaded
Comment 4 Aron Budea 2020-05-12 04:23:07 UTC
(In reply to Buovjaga from comment #3)
> (In reply to Aron Budea from comment #2)
> > Already crashes with Telesto's sample in oldest commit of
> > bibisect-linux-64-5.2, but fine in latest commit of bibisect-50max. Needs to
> > be bibisected in Windows.
> 
> Not sure why on Windows? Did you test with 5.1 repo as well?
There's no 5.1 bibisect repo on Linux, yet.
What's strange though is that it doesn't crash with release versions until later... In Linux it's fine with 5.4.0.3, but crashes with 6.0.0.3. In Windows it's fine with 6.0.0.3, and crashes with 6.1.0.3.

Crash report:
- Win / 7.0 alpha1: https://crashreport.libreoffice.org/stats/crash_details/4de191f8-31c1-4508-9e64-e7e959847a52
- Win / 6.1.0.3: https://crashreport.libreoffice.org/stats/crash_details/0f78ca77-4a6f-4ea5-a711-090fad019d41
- Linux / 6.0.0.3: https://crashreport.libreoffice.org/stats/crash_details/0cb5ffd6-252d-4bf6-bc4c-651fd7790cc6

> I was able to repro the crash exactly once on Windows, with master.
> Modification to Telesto's step 3 was that I did not click above the cell
> containing 2, but on it. However, I was then no longer able to repro with
> the same version or master of Win repos 5.2, 5.3, 5.4, 6.2, 6.3, 6.4, 6.5.
Where you click in the table doesn't matter, you have a 3x2 cell selection ranging from 'a' to '2', the two affected rows will be deleted (in fact it's enough to select a range covering a single cell in each row, eg. 'a' and 'b').

There were a few times when I couldn't repro the crash, but generally I can.
Comment 5 Julien Nabet 2021-03-27 21:34:59 UTC
Created attachment 170788 [details]
bt with debug symbols

On pc Debian x86-64 with master sources updated today, I could reproduce this.
Comment 6 Julien Nabet 2021-03-28 15:35:37 UTC
Created attachment 170795 [details]
Valgrind trace

Here's a Valgrind trace retrieve on pc Debian x86-64 with master sources updated today (without enable-dbgutil).
Comment 7 Matt K 2021-05-18 03:49:39 UTC Comment hidden (obsolete)
Comment 8 Buovjaga 2021-05-18 05:31:15 UTC
(In reply to Matt K from comment #7)
> I don't see the option for "Insert > Diagram".  Is there another way to
> insert it?

Insert > Chart.

"Diagram" probably comes from free-form translation of whatever the UI language of the reporter is.
Comment 9 raal 2021-09-29 06:29:20 UTC
No crash with Version: 7.3.0.0.alpha0+ (x64) / LibreOffice Community
Build ID: 41ccb5c63c94e54b07707eca62bed761c87b0d10
CPU threads: 4; OS: Windows 10.0 Build 17763; UI render: Skia/Vulkan; VCL: win
Locale: cs-CZ (cs_CZ); UI: en-US
Calc: CL
Followed the steps in comment 1.
Please, can you retest it?
Comment 10 Aron Budea 2021-11-06 02:17:48 UTC
I can still repro in LO Version: 7.3.0.0.alpha0+ (ca74611acfef50280a2c1f785448d9a09cca5a0d) / Ubuntu. The commit is ~2 weeks old.
Comment 11 Julien Nabet 2021-11-26 16:43:12 UTC
Created attachment 176528 [details]
bt with debug symbols

On pc Debian x86-64 with master sources updated today + gen rendering, I got an assertion.
Comment 12 Julien Nabet 2021-11-26 17:00:31 UTC
Created attachment 176529 [details]
Valgrind trace

Here's an updated Valgrind trace (retrieved with gen rendering)
Comment 13 Julien Nabet 2021-11-27 12:36:32 UTC
I wanted to declare m_pStartNode in sw/inc/swtable.hxx like this:
std::unique_ptr<SwStartNode> m_pStartNode
instead of:
const SwStartNode * m_pStartNode

but I got stuck since I don't know which element must own the object and which one just wants to have some info from it.
Comment 14 Timur 2022-04-26 12:31:25 UTC
I reproduced steps from Comment 1 in 7.1. 
From 7.2 it is worse, crash already after rows select and delete (I used icon).
 e9ead9066c796b61750ac96729843daa8d848f2e is the first bad commit
    source sha:eea37aa26932d06ed8e93d001862bf45175c4446
    prev sha:7fd980a607eead2c6cf6557c07a9c25cb5b1a5d4

author	Armin Le Grand (Allotropia) <Armin.Le.Grand@me.com>	2022-01-18
tdf#122995 Trigger Chart refresh directly in UpdateCharts for SW

CC Armin, please see.
Comment 15 Armin Le Grand 2022-05-13 09:43:19 UTC
Made a quick check. Seems to depend on how the row is deleted:
- Clicked in table field, context menu, Delete/Delete_rows -> OK
- Other ways (e.g. toolbar command) -> Crash

Definitely *not all ways* to delete a row lead to that crash.

I would have guessed that all ways to delete trigger the same (and that may be the case), so seems as if some side-effect (selection? current selection?) does cause the trouble (?)

If it crashes I get in SwTableBox::GetSttIdx() a bad ptr in m_pStartNode (in my case not nullptr, but 0x10000000000000 -> crash in call to m_pStartNode->GetIndex(), bad this-ptr).

Unfortunately I have no knowledge about SwTableBox and what m_pStartNode is used for, but seems as if SwStartNode* in m_pStartNode gets invalid but used (deleted?)
Comment 16 Matt K 2022-08-04 01:41:21 UTC
I bibisected the crash 3 times in the win64-7.0 repo and got 3 different results (following the bibisecting guide) and I couldn't determine if the changes were related or not.

A note about the repro: sometimes you have to double-click twice on the chart to see the crash
Comment 17 Noel Grandin 2022-08-11 09:40:04 UTC
So the problem(s) here is that writer does not appear to update charts when a table structure happens (or when a table is deleted).

The code that manages table structure changes (e.g rows added/deleted), will need to be updated to check if any charts depend on that table, and if so, force that chart to change it's pointers to table and cell nodes.
Comment 18 Noel Grandin 2022-08-11 12:52:03 UTC
Specifically,

SwChartDataSequence which has a field
  sw::UnoCursorPointer m_pTableCursor
which points at a
    SwUnoTableCursor
which extends
    SwTableCursor
which has a field
    SwSelBoxes m_SelectedBoxes
which is an array, each element of which has a field
    SwStartNode* m_pStartNode
which are pointing at the cells in the table.

When those cells are deleted, those pointers are now invalid, but we still try and dereference them.