This bug was filed from the crash reporting server and is br-947d5fbe-199f-476e-a3b4-d87a9cb0ae34. ========================================= Impress consistently crashes upon trying to put cursor in a table that exists in a recently duplicated slide. Steps to quickly reproduce: 1. Open Impress application. 2. Cancel template dialog. [irrelevant] 3. Create new slide (via right-click) to get an area that allows table creation. 4. Select Table from the menu at area centerpoint. 5. Set 1x1 cell counts, confirm. [unknown if size is relevant] 6. Duplicate slide with table (I do it via right-click menu). 7. Select table in new slide (by single-click), then double-click inside it. Result: LO crashes. Remarks: * Double-clicking within tables in original slide or any existing slides works, only the duplicated slide is affected. * Crash occurs trying to edit table contents even after saving (Ctrl-S), BUT if file is closed and reopened after saving, bug does not reproduce. * All versions from 6.1.0 to 6.3.1 are affected. * Two different computers (built ca. 2008 and 2012) are affected, albeit running almost identical OS setup. * Safe mode is affected as well.
Confirmed on windows 7 x64 with Version: 6.3.1.2 (x64) Build ID: b79626edf0065ac373bd1df5c28bd630b4424273 CPU threads: 3; OS: Windows 6.1; UI render: default; VCL: win https://crashreport.libreoffice.org/stats/crash_details/c2de299b-804e-4bef-8bf1-9efed8b6075c
Created attachment 154096 [details] bt Windows (windbg) On Win10 with master sources updated yesterday, I could reproduce this.
regression from author Armin Le Grand <Armin.Le.Grand@cib.de (CIB)> 2018-03-01 15:54:32 +0100 committer Armin Le Grand <Armin.Le.Grand@cib.de> 2018-04-07 00:28:30 +0200 commit dfefe448c41921f2f1e54d3f69b8b9e89031d055 (patch) tree 1aace31054b5740e2faffcbc5de66a791be27f7d parent eba4d5b2b76cefde90cb3d6638c736f435023a45 (diff) SOSAW080: Added first bunch of basic changes to helpers Bisected with: bibisect-linux64-6.1
Created attachment 154237 [details] bt with debug symbols On pc Debian x86-64 with master sources updated today, I could reproduce this.
Created attachment 154238 [details] Valgrind trace
Just for the record, it only happens with a 1x1 table, quite a corner case @Thorsten, I thought you might be interested in this crash...
Not to be rude, but since I'm using single-cell tables within Impress slides all the time (as a container for other textual information than statement lists -- usually code listings), it means that I'm stuck with LO 6.0.7 unless this bug is fixed or I re-learn Impress (tables in Impress 6.0 are slow and stupid anyway, but I do not know of any other Impress layout element that would be usable in those cases). So if anyone can point out approximately what needs fixing (not just where in the code it breaks), I might try patching it myself.
(In reply to Stepas Toliautas from comment #7) > ... but I do not know of any other Impress layout > element that would be usable in those cases). What advantage has a single-cell table compared with a text box shape or with a custom-shape rectangle with text or with a legacy rectangle with text?
(In reply to Stepas Toliautas from comment #7) > ... > So if anyone can point out > approximately what needs fixing (not just where in the code it breaks), I > might try patching it myself. Great if you may propose a fix! Here's a link where to start: https://wiki.documentfoundation.org/Development/GetInvolved The first thing is to retrieve code and to be able to build it. Second thing is to submit license statement.
(In reply to Regina Henschel from comment #8) I responded to this comment privately, to keep the discussion off of bug-report.
There's a band-aid here: https://gerrit.libreoffice.org/#/c/85148/
(while I'm waiting for an endless build to end...) Cell also has GetObjectItemSet that does exactly the same thing as GetItemSet (except for not having second const qualifier), but is null-pointer-guarded and emits warning specific to the Cell. As far as I can see, that message is first of two things that differ between SdrText and Cell implementations of these two functions. The second thing being that Cell gets its "ObjectItemSet" through TextProperties, while SdrText accesses its parent, AttributeProperties. Actually, Cell.mpProperties member is of type CellProperties (child of TextProperties) that inside the Clone function has warning "does not work yet!" and calls CellProperties __with a nullptr as its last argument (which is supposed to be 'this' of the cell-in-construction)__, but still overrides upstream Clone with it. As the crash occurs on newly-duplicated slide (that one reasonably expects to involve some object cloning), I'm tempted to set a break in there and see what happens.
(In reply to Stepas Toliautas from comment #12) > (while I'm waiting for an endless build to end...) Yes it's an Office suite, not a 1k lines program. If you enable debug build, it's even longer.
It's OK, please don't get offended. Build took 4 hours, no problems except for some warnings. Single-file recompiles are actually OK, within 10 seconds. Please write me an email if you know where remarks to build process developers can be submitted, I have a couple. Now I'm looking at double- and triple-disposes of at least 5 cells in a file that "has" only 2 tables with 1 cell each. Thorsten's change works because it avoids using code from cell.cxx; simply checking for nullptr in Cell::GetItemSet from that file works too. I'll look into cell.cxx for a bit.
Created attachment 156596 [details] Test case run log with custom debug output
Created attachment 156597 [details] Stack traces of each cell construction/ disposal during test case
OK, here's what I know so far: * After table creation dialog, one table and one cell is created. * On page duplication (SdDrawDocument::DuplicatePage), - in SdrObject::CloneHelper<sdr::table::SdrTableObj>, via 'MakeNewObject', - TableModel with single row and single cell is created (2 cells now). But then, - SdrObject::CloneHelper<sdr::table::SdrTableObj> tries to fill newly created object with data from '*this' using 'operator='. - TableModel::dispose and Cell::dispose are called along the way, making just-created cell unusable. - TableModel::dispose also activates TableRow::dispose, which calls Cell::dispose AGAIN. (This double-dispose is checked against as part of earlier bugfix, tdf#118199.) - SdrObject::CloneHelper<sdr::table::SdrTableObj>'s 'operator=' then - creates another TableModel with single row and single cell. Since Cell 2 is only 'disposed of', but not deleted, there are now 3 pointers/ references to dynamically-allocated cells. - However, reference to the Cell 2 is kept for future use instead of Cell 3, which later causes the crash that was initially reported here. - Either checking for mpProperties != nullptr or not using Cell::GetItemSet disables/ avoids the crash, but reference assignment is not corrected. * Additionally, on table focus (single click) drag copy of the table is temporarily created and then deleted, with the same cell-duplication and double-dispose effects as described above. This process is repeated on each focus-unfocus cycle. * At the end of the program (I choose not to save file), all 3 initial Cell references are accounted for and deleted, but with multiple dispose calls along the way. As I feel really not qualified to make decisions on copy/move semantics, object disposal and memory management conventions of LO, I would be grateful if someone more knowledgeable would interject here and try to sort out which parts of this design are really indisputably wrong and which ones should be worked around. Again, either nullptr check or Thorsten's proposal seem to be enough to stop Impress from crashing. But the ghost-duplicates will remain.
Thorsten Behrens committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/4ddd76a7324c8b035e60a12ac06cf08339f6e366 tdf#127481 don't override SdrText::GetItemSet() for cells It will be available in 6.5.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.
Thorsten Behrens committed a patch related to this issue. It has been pushed to "libreoffice-6-4": https://git.libreoffice.org/core/commit/d99c2dad7f8479bbbef2a1c1883fadf580200d6a tdf#127481 don't override SdrText::GetItemSet() for cells It will be available in 6.4.0.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.
Verified in Version: 6.5.0.0.alpha0+ Build ID: 3e33a11d8a553a99bd5f23940a65c301924198fb CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk3; Locale: ca-ES (ca_ES.UTF-8); UI-Language: en-US Calc: threaded @Thosten, thanks for fixing this issue!
Thorsten Behrens committed a patch related to this issue. It has been pushed to "libreoffice-6-3": https://git.libreoffice.org/core/commit/65eed7dec08f7ae35906a329e8011a40e194399e tdf#127481 don't override SdrText::GetItemSet() for cells It will be available in 6.3.5. 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.
Stepas Toliautas committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/f6a4b8ca7f79126569db14b030b46733309dbda6 tdf#127481: reset active cell reference during table copy It will be available in 6.5.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.
Stepas Toliautas committed a patch related to this issue. It has been pushed to "libreoffice-6-4": https://git.libreoffice.org/core/commit/3079c028592bc5123e9fd96c5852dc89a87595bd tdf#127481: reset active cell reference during table copy It will be available in 6.4.0.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.
@Xisco: This one is fixed I guess?
(In reply to Telesto from comment #24) > @Xisco: This one is fixed I guess? Yes, let's close it as VERIFIED FIXED
Xisco Fauli committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/3acf7f20bdf8a54544b459235a84b5c26539a561 tdf#127481: Add unittest It will be available in 7.0.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.