Bug 149198 - Crash on DOCX save
Summary: Crash on DOCX save
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
7.2.1.2 release
Hardware: All Windows (All)
: medium normal
Assignee: Stephan Bergmann
URL:
Whiteboard: target:7.4.0 target:7.3.5 target:7.3.4
Keywords:
Depends on:
Blocks: DOCX Crash
  Show dependency treegraph
 
Reported: 2022-05-20 09:22 UTC by Vasily Melenchuk (CIB)
Modified: 2022-06-01 17:01 UTC (History)
3 users (show)

See Also:
Crash report or crash signature: ["DocxAttributeOutput::WriteCollectedRunProperties()"]


Attachments
Sample DOCX file (2.95 KB, application/vnd.openxmlformats-officedocument.wordprocessingml.document)
2022-05-20 09:23 UTC, Vasily Melenchuk (CIB)
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Vasily Melenchuk (CIB) 2022-05-20 09:22:35 UTC
Description:
Document crashed during save

Steps to Reproduce:
1. Open attached doc
2. Save it back (or with a new filename)


Actual Results:
Crash:
 	

Expected Results:
-


Reproducible: Always


User Profile Reset: No



Additional Info:
Backtrace:

mswordlo.dll!std::_Narrow_char_traits<char,int>::length(const char * const _First) Line 401
mswordlo.dll!std::basic_string_view<char,std::char_traits<char>>::basic_string_view<char,std::char_traits<char>>(const char * const _Ntcts) Line 1257
mswordlo.dll!DocxAttributeOutput::WriteCollectedRunProperties() Line 3134
mswordlo.dll!DocxAttributeOutput::EndRunProperties(const SwRedlineData * pRedlineData) Line 3175

It seems this is reproducible only in Windows.
Version 7.0.2.2 and below are not affected
Version 7.2.1.2 and above (including master) are crashing.

It seems that story begins since (need to bbsect to prove that):

 commit af16aa625682b649e8843237652b9246d519cbae
 Author: Stephan Bergmann <sbergman@redhat.com>
 Date:   Thu May 13 11:57:17 2021 +0200

    Improve loplugin:stringview



In debugger i see that in DocxAttributeOutput::WriteCollectedRunProperties() comparison std::string_view("auto") != pVal when pVal=nullptr does crash in windows stdlib.

I suppose that other similar comparisons can be affected.
Comment 1 Vasily Melenchuk (CIB) 2022-05-20 09:23:03 UTC
Created attachment 180248 [details]
Sample DOCX file
Comment 2 Mike Kaganski 2022-05-20 09:48:42 UTC
FTR: STR51-CPP [1] explains that [char.traits.require] requires that std::char_traits::length() must not be used with nullptr; and indeed, construction of a string view from a char pointer is done using char_traits::length.

[1] https://wiki.sei.cmu.edu/confluence/display/cplusplus/STR51-CPP.+Do+not+attempt+to+create+a+std%3A%3Astring+from+a+null+pointer
Comment 3 Gabor Kelemen (allotropia) 2022-05-20 09:52:42 UTC
Confirming crash upon docx save in fresh master nightly on Windows:

Version: 7.4.0.0.alpha1+ (x64) / LibreOffice Community
Build ID: 09822cf77cdbe32b03553cd05154100b5f2591d0
CPU threads: 14; OS: Windows 10.0 Build 19044; UI render: Skia/Raster; VCL: win
Locale: en-US (hu_HU); UI: en-US
Calc: threaded

But not in Linux bibisect-7.4 repo, this saved it as docx fine:

Version: 7.4.0.0.alpha1+ / LibreOffice Community
Build ID: 75f7e057039aaa49558e22d18cad651d11589da9
CPU threads: 8; OS: Linux 5.4; UI render: default; VCL: gtk3
Locale: hu-HU (hu_HU.UTF-8); UI: en-US
Calc: threaded

Adding CC to: Stephan Bergmann
Comment 4 Stephan Bergmann 2022-05-23 08:13:15 UTC
(In reply to Vasily Melenchuk (CIB) from comment #0)
> I suppose that other similar comparisons can be affected.

The hope was that any such problematic code would have been found and fixed with <https://git.libreoffice.org/core/+/4f0c70fb5554325e0cc2129741175bf07de22029%5E!/> "Avoid calling OString ctor with null pointer" before code was changed from using OString to using std::string_view.
Comment 5 Commit Notification 2022-05-23 10:01:40 UTC
Stephan Bergmann committed a patch related to this issue.
It has been pushed to "master":

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

tdf#149198 Fix use of nullptr

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.
Comment 6 Commit Notification 2022-05-23 13:21:30 UTC
Stephan Bergmann committed a patch related to this issue.
It has been pushed to "master":

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

tdf#149198 Fix previous fix

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.
Comment 7 Commit Notification 2022-05-23 19:25:23 UTC
Stephan Bergmann committed a patch related to this issue.
It has been pushed to "libreoffice-7-3":

https://git.libreoffice.org/core/commit/6f6d61ea5d7700fb18bfb49c6e1d16a55944e581

tdf#149198 Fix use of nullptr

It will be available in 7.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.
Comment 8 Commit Notification 2022-05-24 07:45:20 UTC
Xisco Fauli committed a patch related to this issue.
It has been pushed to "master":

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

tdf#149198: sw_ooxmlexport16: 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.
Comment 9 Commit Notification 2022-06-01 17:01:52 UTC
Stephan Bergmann committed a patch related to this issue.
It has been pushed to "libreoffice-7-3-4":

https://git.libreoffice.org/core/commit/33e1d55aba6b6f91c056de7ed6c9aff2832782bf

tdf#149198 Fix use of nullptr

It will be available in 7.3.4.

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.