Steps to Reproduce: 1. Open attached test ods file. 2. Filter "11" in column b. 3. Type "a" in B2. 4. Ctrl+C in B2. 5. Click in B3, then Ctrl+Shift+Down. 6. Click "OK" at the override confirmation dialog. -> Crash. Version: 7.4.0.0.alpha0+ / LibreOffice Community Build ID: d73602dc51aa8829fc88e5e67e2b0c4da6b8f715 CPU threads: 8; OS: Linux 5.15; UI render: default; VCL: gtk3 Locale: zh-CN (zh_CN.UTF-8); UI: zh-CN Build Platform: Fedora34@X64, Branch:master, bibisect-linux-64-7.4-CN Calc: threaded Fedora 34, Gnome 40.5, Wayland.
Created attachment 177574 [details] test1.ods
(In reply to Kevin Suo from comment #0) Steps to Reproduce: 1. Open attached test ods file. 2. Filter "11" in column b. 3. Type "a" in B2. 4. Ctrl+C in B2. 5. Click in B3, then Ctrl+Shift+Down. 6. Ctrl+V. Click "OK" at the override confirmation dialog. -> Crash.
$ git log 0464b86787da269be7b16a6f1f124d774f78fa97..eb07a0e76fe240a184348d96a6cebf7c0a229ac0 --pretty=reference eb07a0e76fe2 (Upgrade mdds and liborcus to 2.0.0 and 0.17.0, respectively., 2021-11-01) 9b9f4a4487e9 (Update git submodules, 2021-11-03) ff22a3ff99da (Add back `== 2`, 2021-11-03) 913c9aa8f5bb (Update git submodules, 2021-11-03) 29b39dfb667d (Update git submodules, 2021-11-03) 317100ad1d22 (Update git submodules, 2021-11-03) 0c67012a1886 (Update git submodules, 2021-11-03) c587ac9e9449 (Update git submodules, 2021-11-03) 05a57cee0123 (Update git submodules, 2021-11-03) 5340ba24dda5 (Update git submodules, 2021-11-03) cbe437ea1e5a (Update git submodules, 2021-11-03) 2aef034db210 (Update git submodules, 2021-11-03) 0481ed9e1381 (Update git submodules, 2021-11-03) f6435fed1192 (Update git submodules, 2021-11-03) 6d93317ab62f (Update git submodules, 2021-11-03) 4ed4f18d62c1 (Update git submodules, 2021-11-03) 7fed1b6be186 (Update git submodules, 2021-11-03) adea4657391d (Update git submodules, 2021-11-03) 02d8b43827ad (Update git submodules, 2021-11-03) b64efd7b291c (Revert "loplugin:finalclasses", 2021-11-03) Adding Kohei Yoshida to cc. Could you please take a look? Thanks.
Created attachment 177575 [details] gdb_backtrace.log
Reproduced in Version: 7.4.0.0.alpha0+ / LibreOffice Community Build ID: bd5492275d31f59b1d269205018d1487af52426f CPU threads: 8; OS: Linux 5.10; UI render: default; VCL: gtk3 Locale: es-ES (es_ES.UTF-8); UI: en-US Calc: threaded
I do confirm the issue was introduced by: author Kohei Yoshida <kohei@libreoffice.org> 2021-11-01 14:01:22 -0400 committer Kohei Yoshida <kohei@libreoffice.org> 2021-11-03 21:17:18 +0100 commit eb07a0e76fe240a184348d96a6cebf7c0a229ac0 (patch) tree 23ab960b7a163696e4a7c1d4c4c20c1340fa14b3 parent 9b9f4a4487e9ada1885d45a8b1ba0234a4a9fc26 (diff) Upgrade mdds and liborcus to 2.0.0 and 0.17.0, respectively. Bisected with: bibisect-linux64-7.3 @Kohei, Could you please take a look ?
If someone can isolate this to a reproducible case with mdds alone, that would be helpful.
Crash report: https://crashreport.libreoffice.org/stats/signature/mdds::mtv::soa::detail::iterator_updater%3Cmdds::mtv::soa::multi_type_vector%3Cmdds::mtv::custom_block_func3%3Cmdds::mtv::default_element_block%3C52,svl::SharedString%3E,mdds::mtv::noncopyable_managed_element_block%3C53,EditTextObject%3E,mdds::mtv::noncopyable_managed_
(In reply to Kohei Yoshida from comment #7) > If someone can isolate this to a reproducible case with mdds alone, that > would be helpful. Hi Kohei, Could you please elaborate? How can I know a case is reproducible in mdds only ?
So, mdds is an independent library that provides C++ API. Calc uses it to manage cell data I/O. Any access patterns Calc does to mdds can be reduced to a sequence of C++ calls to mdds::multi_type_vector. In this case, I'd like such reduced test case code to be provided to make it easier for me to look into this in mdds. Otherwise, that would be the first thing I'd have to do: to reduce Calc's access pattern that triggers the crash to a series of C++ calls to mdds::multi_type_vector only so that I can use that as a future test case for mdds. I need to do this for efficiency reasons; debugging Calc takes 10x more resources than debugging just mdds alone. If that's not reasonable ask, then fine. But I can't guarantee a timely investigation.
Kohei, is it possible to do bisect, on libreoffice level, into the mdds commits? I see the changebwas from 1.7.0 to 2.0.0 and there may not be many commits between that.
(In reply to Kevin Suo from comment #11) > Kohei, is it possible to do bisect, on libreoffice level, into the mdds > commits? I see the changebwas from 1.7.0 to 2.0.0 and there may not be many > commits between that. No.
(In reply to Kohei Yoshida from comment #7) > If someone can isolate this to a reproducible case with mdds alone, that > would be helpful. Kohei, could you add code pointers where mdds is called during the described steps? (or what mdds functions are called) I'd assume no one else reading this bug report has experience with that.
(In reply to Aron Budea from comment #13) > (In reply to Kohei Yoshida from comment #7) > > If someone can isolate this to a reproducible case with mdds alone, that > > would be helpful. > Kohei, could you add code pointers where mdds is called during the described > steps? I don't have that info yet, but I can tell you how one may start investigating it. Before you begin, it would be helpful to look through this page: https://mdds.readthedocs.io/en/latest/multi_type_vector.html to get an overview of what mdds::multi_type_vector does. You don't have to read the entire page, but just going through the Quickstart section should give you a good idea of how to use mdds::multi_type_vector standalone. The page also have API reference toward the bottom. People here casually refers to mdds::multi_type_vector as just "mdds", but technically that's not correct. mdds contains multiple data structures that Calc uses, and multi_type_vector is just one of them. mdds::flat_segment_tree is another one Calc uses, but I won't go into that here. ScColumn::maCells data member is where all cell values are stored, so tracing its use would be the starting point. Here https://opengrok.libreoffice.org/s?refs=maCells&project=core shows all call sites of ScColumn::maCells which is quite extensive, understandably so since lots and lots of Calc code does access it since it's the core of Calc cell data I/O. Now, the entry point for pasting data into Calc can be tricky since there are several entry points depending on where the data being pasted is coming from. But I believe it's ScViewFunc:PasteFromClip() https://opengrok.libreoffice.org/xref/core/sc/source/ui/view/viewfun3.cxx?r=9436f7e2#871 for copy-n-pasting within the same Calc document. The bMarkIsFiltered flag on line 1024 is set, since the value is being pasted onto a filtered area i.e. autofilter is applied in the destination area. Eventually the code here will find its way into ScDocument::CopyToDocument, which moves to ScTable (sheet), and then eventually to ScColumn (column storage), and you can inspect how that code accesses and updates the maCells there... That's the gist of it. The tricky part is that, every action creates an undo object which also has its own ScDocument store, so that could be another source of the problem too. I just don't know where the problem may be...
(In reply to Kohei Yoshida from comment #14) > Eventually the code here will find its way into ScDocument::CopyToDocument, Actually ScDocument::CopyFromClip().
Ok, so in this case, the code eventually ends up in ScColumn::CopyOneCellFromClip(), which gets called twice, and crashes on the 2nd time.
Actually I'm working on adding an optional tracer to mdds::multi_type_vector https://gitlab.com/mdds/mdds/-/issues/70 With this, getting the above call info will be a bit easier. I'm also investigating into the issue on the Calc side. But it's going very slow.
Is downgrading the LO mdds version an option, in case it will take too long for a fix, provided that 7.3 is going to be released as "stable" to the public?
The short story is that the issue is on the Calc side. The crash is caused by the uses of invalid position hints when modifying the cell store. Sorting that out on the Calc side will fix this issue. Having said that, the issue existed from at least 2014, but somehow it never manifested itself as a crash using the old storage layout of mdds::multi_type_vector (mtv). In 2.0, mtv switched to an alternative layout, and somehow that triggers the crash from invalid position hints more easily. No idea why that is. I have a proof-of-concept fix that fixes the crash, but unfortunately that involves quite a bit of non-trivial change in Calc's CopyFromClip* code which may potentially introduce other regressions if not done right. So I would still need to flesh out my fix there, which may take a while.
(In reply to Kevin Suo from comment #18) > Is downgrading the LO mdds version an option, in case it will take too long > for a fix, provided that 7.3 is going to be released as "stable" to the > public? From my POV that makes little sense. But I'll leave that up to TDF.
(In reply to Kevin Suo from comment #18) > Is downgrading the LO mdds version an option, in case it will take too long > for a fix, provided that 7.3 is going to be released as "stable" to the > public? When possible, it's always better to have a fix rather than a revert.
FYI my proposed fix is here: https://gerrit.libreoffice.org/c/core/+/129267 Two buildbots fail, but I'm having a hard time reproducing the test failures in my environment.
Created attachment 178003 [details] bt_with_patch.txt I can reproduce the fail in testTdf92963 with your proposed patch. Error: attempt to copy from a singular iterator. Fedora 34 x64. dbgutil build with "CC=clang" and "CXX=clang++" autogen.sh options enabled. Attached is the gdb "bt" and "bt full".
> I can reproduce the fail in testTdf92963 with your proposed patch. Manually, With the proposed patch, Calc crashes when do the following: 1. Open sc/qa/unit/uicalc/data/tdf92963.ods. 2. Select cell range A1:C4, copy. 3. Select cell range A1:C1, paste. --> Crash.
(In reply to Kevin Suo from comment #24) > > I can reproduce the fail in testTdf92963 with your proposed patch. > > Manually, With the proposed patch, Calc crashes when do the following: > 1. Open sc/qa/unit/uicalc/data/tdf92963.ods. > 2. Select cell range A1:C4, copy. > 3. Select cell range A1:C1, paste. > --> Crash. Yup, I can finally reproduce this locally. Thanks! I'm slowly starting to understand what's going on... It's still caused by invalid position hints on the Calc side, but I'm still trying to figure out why that happens.
The latest crash is very much in the same league as tdf#146795. Even the stack trace looks very similar.
The latest crash is very much in the same league as tdf#100852. Even the stack trace looks very similar.(In reply to Kohei Yoshida from comment #26) > The latest crash is very much in the same league as tdf#146795. Even the > stack trace looks very similar. Correction: The latest crash is very much in the same league as tdf#100852. Even the stack trace looks very similar. I curse Bugzilla's immutable comments.
The latest crash is very much in the same league as tdf#100852. Even the stack trace looks very similar. I curse Bugzilla's immutable comments.
Kohei Yoshida committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/703fb7739a5e604d90e147db6f67917b8d141150 tdf#146795: Make sure to use valid position hints to avoid crash. It will be available in 7.4.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.
backport for libreoffice-7-3: https://gerrit.libreoffice.org/c/core/+/129267
I can verify that it is now fixed on master. Thank you Kohei!
Xisco Fauli committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/9f0d27c973323f522a4ca2bce557e1c095d32f77 tdf#146795: sc_uicalc: Add unittest It will be available in 7.4.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.
(In reply to Kohei Yoshida from comment #30) > backport for libreoffice-7-3: https://gerrit.libreoffice.org/c/core/+/129267 *sigh* This is the right URL: https://gerrit.libreoffice.org/c/core/+/129478 Bugzilla is only for the smart people who never make mistakes...
Kohei Yoshida committed a patch related to this issue. It has been pushed to "libreoffice-7-3": https://git.libreoffice.org/core/commit/ea02ca06de604370061a59dd3084f7cfacccbe39 tdf#146795: Make sure to use valid position hints to avoid crash. It will be available in 7.3.1. 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.