Description: In writer I want to eliminate certain repeat items in an alphabetical index. After deleting a few of the items the system crashes and all the previous work is lost. The response is a Windows 10 panel which asks one to either allow the program to continue or close the file. If allowed to continue, it will continue for hours without closing. Choosing to close the program loses all previous effort. Perhaps related. Some of the index occur in other indices, eg the Table of Contents. The search function will find these items, they will be reported as alphabetical index items, but the edit reference feature will not allow entrance to them. The index was created by finding all instances of an item and commanding then to be inserted. As a consequence the index contains many more instances than necessray. Steps to Reproduce: Pleas tell me how to update the file to you 1.In the program go to pg 74 2.Scroll down to "compound variable" A prompt will indicate is is in the alphabeitcal index. 3.edit entry 4. Delete the entry. Writer may crash at this point. 5. If not find the next "compound variable entry listed in the alphabeetial index. Delete that one. The file may crashg. 6. Continue step 5. The progrem will crash after a few iterations. Actual Results: The file crashes under Windows after one or more iterations. Expected Results: The selected items should have been removed from the alphaBetical Index without craching. Reproducible: Sometimes User Profile Reset: No Additional Info: [Information automatically included from LibreOffice] Locale: en-US Module: TextDocument [Information guessed from browser] OS: Windows (All) OS is 64bit: no
Created attachment 191509 [details] books length file on a mathematical subject. Indices follow sections.
Created attachment 191713 [details] smaller sample file with coloured fields Thank you for the report! Reproduced various issues. A) In 4.0.0.3, all the way to: Version: 7.6.4.1 (X86_64) / LibreOffice Community Build ID: e19e193f88cd6c0525a17fb7a176ed8e6a3e2aa1 CPU threads: 8; OS: Linux 5.15; UI render: default; VCL: gtk3 Locale: en-AU (en_AU.UTF-8); UI: en-US Calc: threaded With steps: 1. Open this attachment 2. Double-click on the yellow "functions": "Edit Index Entry" dialog opens (before LO 7.5, you need to place the cursor in front of the index, then use Edit > Reference > Index entry) 3. Click Delete twice 4. Close dialog Result: crash (or freeze) -> https://crashreport.libreoffice.org/stats/crash_details/1cb92bc4-a72c-45ea-99ae-eb3488292090 In console: terminate called after throwing an instance of 'std::out_of_range' what(): basic_string_view::substr: __pos (which is 50) > __size (which is 35) Not reproduced in OOo 3.3 -> regression. However, this is can't be reproduced in 24.2 as the dialog now closes on the first delete, since Armin's ab7c81f55621d7b0d1468c63305163016dd78837 (build commit [1b7cb4eeeef9b131220865ad098d3c8e1bc53cdb] in linux-64-24.2 bibisect repo). ------ B) Since Armin's ab7c81f55621d7b0d1468c63305163016dd78837, and still in a recent trunk build: Version: 24.8.0.0.alpha0+ (X86_64) / LibreOffice Community Build ID: e71934471442a8bbba7e661d3ebe5f708627c5d6 CPU threads: 8; OS: Linux 5.15; UI render: default; VCL: gtk3 Locale: en-AU (en_AU.UTF-8); UI: en-US Calc: threaded With steps: 1. Open this attachment 2. Double-click on the red "functions": "Edit Index Entry" dialog opens 3. Click Delete (or close the dialog) Alternatively: 1. Open attachment 191509 [details] 2. Go to page 78 3. Double-click on "functions" in the first sentence: "Edit Index Entry" dialog opens 4. Close the dialog Result: freeze
Armin, can you please have a look at the dialog not being able to move to the next entry automatically (A), and the regression freeze in (B)? Then we could have a look at older crash (which might be bibisectable).
Taking a look...
Tried (A) and indeed Dlg closes on 1st 'Delete' and no crash. This smells like SfxPoolItem lifetime, so it is viable that my last changes might already have eliminated this. The mentioned ab7c81f55621d7b0d1468c63305163016dd78837 is only one of these. Thus: *IS* there a problem with (A) still or not? I do not know that functionalities, for me it looks good (?)
I get a crash in SwScanner::SwScanner due to an illegal index access to the string m_aPreDashReplacementText which contains "\tf(r(x1,a)) a generalized function;" -> length (36) AFAICC The loop there for (sal_Int32 i = m_nStartPos; i < m_nEndPos; ++i) with m_nStartPos==34 and m_nEndPos==42. Thus the access sal_Unicode cChar = aBuf[i]; with I==36 FAILS. What is clear. Question is why m_nStartPos/nEndPos values like these are handed in and if that was different before. It clearly is an error. I could just ensure no accesses after string length, but that might not solve the problem, I just do not know that functionality. I also get (VERY often) the warning info:svl.items:567770:567770:svl/source/items/poolitem.cxx:633: ITEM: PtrCompare != ContentCompare (!) as SAL_INFO from areSfxPoolItemPtrsEqual. It means that two SfxPoolItems get compared whereby the *content* compare is equal, but the pointers differ. This is a hint that evtl. content comapre and ItemRe-use should be used at some place. That warning is from the mentioned change and may be relevant.
The SAL_INFO comes from areSfxPoolItemPtrsEqual in SwDoc::DeleteTOXMark. Here the content compare of two SwTOXMark is equal, but SwTOXMark::operator== just compares the *type* and ignores all the content, so this is a false positive. Is that SwTOXMark::operator== sufficient...? SwTOXMark *is* derived from SfxPoolItem, so part of the ItemPool/PoolItem concept, this seems vague at least...? Also it seems not to happen every time, hunting where that string and the wrong indices might come from...
Not every time, but if it comes from SwDoc::CountWords last line. pTNd contains the text, and the indices come from const sal_Int32 nSttCnt = pStt->GetContentIndex(); const sal_Int32 nEndCnt = pEnd->GetContentIndex(); and seem to be wrong already, at least do not fit the text. Added Michael Stahl to cc, maybe he knows why this can be wrong (?) I can do a 'fix' by making sure that the mentioned loop in comment 6 does avoid invalid indices, but would that be correct?
I replaced for (sal_Int32 i = m_nStartPos; i < m_nEndPos; ++i) with for (sal_Int32 i = std::min(aBuf.getLength(), m_nStartPos); i < std::min(aBuf.getLength(), m_nEndPos); ++i) and that solves the direct problem. But I get another problem on the same string: Also from SwDoc::CountWords, same string, but the pTNd->CountWords call in line 859, pTNd->CountWords( rStat, 0, nEndCnt ); thus crashes in SwBreakIt::getGraphemeCount due to nEnd==42 being out of bounds for that string. Thus somehow my Item changes have led to more possibly wrong indices in texts...?
@mst: In SwTextNode::CountWords *somehow* after mentioned change nExpandEnd can be bigger that aExpandText.getLength(), so called methods SwScanner aScanner(...) and getGraphemeCount(...) that use aExpandText and nExpandEnd trigger out of bound accesses and can lead to crash. nExpandBegin may also be wrong. I have no idea how my item changes (ab7c81f55621d7b0d1468c63305163016dd78837) may lead to this, would it be OK to change const sal_Int32 nExpandBegin = aConversionMap.ConvertToViewPosition( nStt ); const sal_Int32 nExpandEnd = aConversionMap.ConvertToViewPosition( nEnd ); to const sal_Int32 nExpandBegin = std::min(aExpandText.getLength(), aConversionMap.ConvertToViewPosition( nStt )); const sal_Int32 nExpandEnd = std::min(aExpandText.getLength(), aConversionMap.ConvertToViewPosition( nEnd )); what would make the calls safe, but without knowing why this happens. I have probably not enough SW knowledge to identify the real problem (?)
(In reply to Armin Le Grand from comment #7) > The SAL_INFO comes from areSfxPoolItemPtrsEqual in SwDoc::DeleteTOXMark. > Here the content compare of two SwTOXMark is equal, but > SwTOXMark::operator== just compares the *type* and ignores all the content, > so this is a false positive. Is that SwTOXMark::operator== sufficient...? > SwTOXMark *is* derived from SfxPoolItem, so part of the ItemPool/PoolItem > concept, this seems vague at least...? possibly the SwTOXMark was not POOLABLE and thus the operator== is unused in practice?
Discussed with mst (thanks a lot) and indeed SwTOXMark as Item was never pooled, so op== was not really ever used. I discussed to implement it (there is quite some content, currently only type is used), but we came to the point that it's only safe to say instances are equal when same instance -> fallback to ptr compare. Also corrected places where it is known that it's SwTOXMark* from SfxPoolItem::areSame (which would use op==) to areSfxPoolItemPtrsEqual (which is just ptr compare). Also SwTOXMark::operator== uses ptr compare now. All in all that would be enough, but is more expressive/clear that way. Also there might be compares of item* where it is not clear that SwTOXMark is used. Making some more checks...
various crashes happen because SwIndexMarkPane::InitControls() calls GotoTOXMark and then checks if it went to a different mark and if so calls GotoTOXMark again to move back. problem is that now all TOXMarks compare equal so it doesn't move back and the cursor point remains on a node that's different from the node that contains the SwTOXMark and then SwCursorShell::SelectTextAttr() creates a very invalid cursor position.
Michael Stahl committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/36f232e27616f267577851c38a24c2a5c3caad90 tdf#158783 sw: add some asserts for calling SelectTextAttr() ... It will be available in 24.8.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.
Fix at https://gerrit.libreoffice.org/c/core/+/161963
Still have a problem when doing (B) from comment 2, but using right arrow -> at some time (turnaround to 1st candidate?) I stll get a hang/crash, so not yet done.
Michael Stahl committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/3fefff7ce29a234694343105c50c7bc3259a9cb8 tdf#158783 sw: SwCursorShell::GotoTOXMark() must actually move cursor It will be available in 24.8.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.
Armin Le Grand (allotropia) committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/d22a86089edfcadbef5231525a2947b954f4784e tdf#158783 Correct compares of SwTOXMark Items It will be available in 24.8.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.
Should be good from what I see. @Stéphane: Thanks for that well written report!
(In reply to Armin Le Grand from comment #19) > Should be good from what I see. > @Stéphane: Thanks for that well written report! Thank you Armin and Michael for looking into it! Fix verified for all comment 2 steps in: Version: 24.8.0.0.alpha0+ (X86_64) / LibreOffice Community Build ID: 8b393bba91111bd4f8988457f3a78b0306462bf2 CPU threads: 8; OS: Linux 5.15; UI render: default; VCL: gtk3 Locale: en-AU (en_AU.UTF-8); UI: en-US Calc: threaded No freeze, no crash, and dialog moves to the next item automatically.
Michael Stahl committed a patch related to this issue. It has been pushed to "libreoffice-24-2": https://git.libreoffice.org/core/commit/908abf0b0e36fd1c6e8d3df48e9c35ce911c9bda tdf#158783 sw: SwCursorShell::GotoTOXMark() must actually move cursor It will be available in 24.2.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.
Armin Le Grand (allotropia) committed a patch related to this issue. It has been pushed to "libreoffice-24-2": https://git.libreoffice.org/core/commit/a48ee46a085abfa04779ece38c08dddb5bf017ea tdf#158783 Correct compares of SwTOXMark Items It will be available in 24.2.3. 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.
Armin Le Grand (allotropia) committed a patch related to this issue. It has been pushed to "libreoffice-24-2-2": https://git.libreoffice.org/core/commit/c5b878a7c55f7e6f8d3a66fa9728c8fca5d0e13b tdf#158783 Correct compares of SwTOXMark Items It will be available in 24.2.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.