Bug 148143 - CRASH: after pasting twice the whole sheet
Summary: CRASH: after pasting twice the whole sheet
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
7.4.0.0 alpha0+
Hardware: All All
: medium normal
Assignee: Kohei Yoshida
URL:
Whiteboard: target:7.6.0 target:7.5.2
Keywords: bibisected, bisected, regression
Depends on:
Blocks: Calc-large-spreadsheets
  Show dependency treegraph
 
Reported: 2022-03-22 21:25 UTC by Xisco Faulí
Modified: 2023-03-09 00:40 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments
gdb backtrace (37.93 KB, text/plain)
2022-03-23 10:50 UTC, Xisco Faulí
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Xisco Faulí 2022-03-22 21:25:25 UTC
Steps to reproduce:
1. Open attachment 68062 [details] from bug 55181
2. Select All
3. Copy
4. Paste
5. Paste

-> Crash

Reproduced in

Version: 7.4.0.0.alpha0+ / LibreOffice Community
Build ID: 4a46a74a3de0ac7df5d3ce949dda5e809c1729ab
CPU threads: 8; OS: Linux 5.10; UI render: default; VCL: gtk3
Locale: es-ES (es_ES.UTF-8); UI: en-US
Calc: threaded
Comment 1 Xisco Faulí 2022-03-22 21:26:57 UTC Comment hidden (obsolete)
Comment 2 Luboš Luňák 2022-03-23 09:58:45 UTC
With current master (9633cad413960a4bd2963a3ace25ad52d5747929) there's no crash if I do Ctrl+A, Ctrl+C, Ctrl+V, confirm dialog, Ctrl+V, confirm dialog.

Can you provide more info (e.g. a backtrace)?
Comment 3 Xisco Faulí 2022-03-23 10:44:06 UTC
(In reply to Luboš Luňák from comment #2)
> With current master (9633cad413960a4bd2963a3ace25ad52d5747929) there's no
> crash if I do Ctrl+A, Ctrl+C, Ctrl+V, confirm dialog, Ctrl+V, confirm dialog.

yes, that's correct. Just tried with a clean profile and it's still reproducible. Let me get a backtrace
Comment 4 Xisco Faulí 2022-03-23 10:50:23 UTC
Created attachment 179047 [details]
gdb backtrace
Comment 5 Xisco Faulí 2022-04-29 14:31:55 UTC
Still crashing in

Version: 7.4.0.0.alpha0+ / LibreOffice Community
Build ID: f1dc51468137971295393ab438b57fe5f77c1420
CPU threads: 8; OS: Linux 5.10; UI render: default; VCL: gtk3
Locale: es-ES (es_ES.UTF-8); UI: en-US
Calc: threaded
Comment 6 Luboš Luňák 2022-05-05 06:31:14 UTC
I still cannot reproduce, not even running it in Valgrind shows anything. Does running it in Valgrind show anything for you (note that it takes a long time with a dbgutil build)?
Comment 7 Telesto 2022-05-05 06:52:04 UTC
I can't repro either
Version: 7.4.0.0.alpha0+ (x64) / LibreOffice Community
Build ID: 00e43c14558fa4d0369820fa3d2cd7b987e61377
CPU threads: 4; OS: Windows 6.3 Build 9600; UI render: Skia/Raster; VCL: win
Locale: en-US (nl_NL); UI: en-GB
Calc: CL Jumbo
Comment 8 Aron Budea 2022-05-08 00:03:46 UTC
I can't repro in a debug build, but can in a release build. Tried in Linux. I also tried getting a valgrind log with the release build, but strangely, didn't get a crash in that case.
Comment 9 Luboš Luňák 2022-05-12 18:29:48 UTC
I can also reproduce with a release build and not with a debug build. But the bisect information from comment #1 is incorrect, I can reproduce even with the commit right before the mentioned one.
Comment 10 Aron Budea 2022-05-13 02:15:27 UTC
Hm, I, too can reproduce with the preceding commit, but much more inconsistently, after 5-10 tries. Bibisecting with that in mind yielded the following commit, which seems relevant.

https://cgit.freedesktop.org/libreoffice/core/commit/?id=4c5f8ccf0a2320432b8fe91add1dcadf54d9fd58
author		Luboš Luňák <l.lunak@collabora.com>	2022-03-08 12:44:49 +0100
committer	Luboš Luňák <l.lunak@collabora.com>	2022-03-09 08:25:44 +0100

change default Calc number of columns to 16384 (tdf#50916)
Comment 11 Luboš Luňák 2022-05-13 04:34:53 UTC
That commit does not introduce any bug on its own, it at most exposes a bug elsewhere. It might help bisecting further and always temporarily setting the MAXCOLCOUNT value to 16384 before building, if somebody can do that.
Comment 12 Aron Budea 2022-05-13 04:57:35 UTC
Do you have a sensible "earliest" commit that might be worth checking?
Comment 13 Luboš Luňák 2022-05-13 05:01:27 UTC
Not really. I would first try e.g. libreoffice-7-3-branch-point in case it's something recent, then I'd try something old just in case it's been there forever and just hidden by the smaller column count.
Comment 14 Aron Budea 2022-05-17 02:03:22 UTC
I've arrived to the following commit with this round of bisecting.

https://cgit.freedesktop.org/libreoffice/core/commit/?id=703fb7739a5e604d90e147db6f67917b8d141150
author		Kohei Yoshida <kohei@libreoffice.org>	2022-01-31 22:45:21 -0500
committer	Kohei Yoshida <kohei@libreoffice.org>	2022-02-04 02:40:49 +0100

tdf#146795: Make sure to use valid position hints to avoid crash.
Comment 15 Kohei Yoshida 2023-03-03 04:27:57 UTC
It's probably caused by this: https://cgit.freedesktop.org/libreoffice/core/commit/?id=99cd1d8834bb708afc81c825ff2b7992b7acb37d

Prior to that commit, ColumnSpanSet::ColumnType was stored wrapped inside std::unique_ptr as a dynamically allocated memory, so when the std::vector reallocated its internal buffer and copied the stored values it would just copy the pointer values, and the ColumnType objects were still valid.  But since that commit, ColumnType is now stored wrapped inside std::optional which stores the object as part of the std::optional's memory footprint, not as a dynamically allocated memory.  So, when the std::vector store reallocates its buffer it now invokes ColumnType's copy constructor which copies both maSpans and miPos.

miPos is used as a position hint into flat_segment_tree (maSpans) which stores a pointer to a node.  And  after the copy construction, that pointer may point to a node object that may have been deleted, which would certainly cause an invalid memory access and the crash would follow...

I have my local fix, and now I cannot reproduce the crash after repeating the paste 10+ times.

No idea why your bisecting ended up pointing to my commit, but that one is unrelated as far as I can tell.  It may have just made the underlying issue easier to surface.
Comment 16 Commit Notification 2023-03-03 16:11:20 UTC
Kohei Yoshida committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/c4187189060a104cf36d8a8c9b2958b8c28cde0b

tdf#148143: Reset the position hint when flat_segment_tree gets copied

It will be available in 7.6.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 17 Commit Notification 2023-03-06 13:38:50 UTC
Kohei Yoshida committed a patch related to this issue.
It has been pushed to "libreoffice-7-5":

https://git.libreoffice.org/core/commit/b032101f1e253e83dbed58632882853021d11647

tdf#148143: Reset the position hint when flat_segment_tree gets copied

It will be available in 7.5.2.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 18 Kohei Yoshida 2023-03-09 00:40:00 UTC
I think it's reasonable to mark this as fixed.