Download it now!
Bug 86298 - Crash when File>Labels>New Document
Summary: Crash when File>Labels>New Document
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
4.4.0.0.alpha0+ Master
Hardware: x86-64 (AMD64) Linux (All)
: medium major
Assignee: Matthew Francis
URL:
Whiteboard: target:4.4.0
Keywords:
: 86316 86412 (view as bug list)
Depends on:
Blocks:
 
Reported: 2014-11-14 22:51 UTC by Julien Nabet
Modified: 2014-11-20 20:23 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments
bt with debug symbols (10.76 KB, text/plain)
2014-11-14 22:51 UTC, Julien Nabet
Details
Valgrind memcheck log showing the (an?) issue (114.38 KB, text/x-log)
2014-11-17 02:27 UTC, Matthew Francis
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Julien Nabet 2014-11-14 22:51:45 UTC
Created attachment 109495 [details]
bt with debug symbols

Problem description: 
On pc Debian x86-64 with master sources updated today with enable-dbgutil, I tried to reproduce fdo#83853 but tried from Writer.
I had a crash when I did:
- Open Writer
- Select File
- Select New then Labels
- Click New Document Button

(brand new profile, since I did : make clean && make postprocess.clean && ./g pull -r && make)
Comment 1 Joel Madero 2014-11-14 22:58:07 UTC
No crash from master built yesterday.
Comment 2 Matthew Francis 2014-11-17 02:26:04 UTC
I confirm this - the crash is intermittent but it's there

I haven't got a backtrace which looks anything like the attached, so whether this is related or not I don't know, but some poking around with valgrind, bibisect and plain git bisect reveals that there is some kind of memory related nastiness which appears to have begun from the following commit:

commit 93b34d091756de6fc3e5424aa45e00397146cc51
Author: Jan Holesovsky <kendy@collabora.com>
Date:   Tue Nov 11 21:33:53 2014 +0100

    vcl: Use the fast path for rendering.


The diff from the above is tiny, so it's almost certainly not directly the above but some other code path that it enables


Setting -> NEW
Comment 3 Matthew Francis 2014-11-17 02:27:53 UTC
Created attachment 109578 [details]
Valgrind memcheck log showing the (an?) issue
Comment 4 Matthew Francis 2014-11-17 03:38:03 UTC
Adding a CC to kendy@collabora.com - could you shed any light on this one? Thanks
Comment 5 Matthew Francis 2014-11-19 03:49:21 UTC
Possible fix: https://gerrit.libreoffice.org/#/c/12961

The bug appears to revolve around attempting to blend two Bitmaps of differing size (source height > dest height) when the format of one is upside down from the other.

Why we would need to do this, and why specifically the above commit would suddenly make it start happening I'm not clear on
Comment 6 Commit Notification 2014-11-19 09:17:33 UTC
Matthew J. Francis committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=f857358d83e7c105271eb0e2c43f0b036f14f284

fdo#86298 Avoid crash blending upside down Bitmaps of differing size

It will be available in 4.4.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 7 Michael Stahl (CIB) 2014-11-19 14:24:07 UTC
*** Bug 86412 has been marked as a duplicate of this bug. ***
Comment 8 Michael Stahl (CIB) 2014-11-19 14:37:21 UTC
*** Bug 86316 has been marked as a duplicate of this bug. ***
Comment 9 Julien Nabet 2014-11-19 21:28:32 UTC
I must recognize I don't understand the logic, we've got this:

559     // source and mask don't match: upside down
560     if( (rSrcBuffer.mnFormat ^ rMskBuffer.mnFormat) & BMP_FORMAT_TOP_DOWN )
561     {
562         aMskLine.AddByteOffset( (rSrcBuffer.mnHeight - 1) * nMskLinestep );
563         nMskLinestep = -nMskLinestep;
564     }
565 
566     // source and destination don't match: upside down
567     if( (rSrcBuffer.mnFormat ^ rDstBuffer.mnFormat) & BMP_FORMAT_TOP_DOWN )
568     {
-         aDstLine.AddByteOffset( (rSrcBuffer.mnHeight - 1) * nDstLinestep );
+         aDstLine.AddByteOffset( (rDstBuffer.mnHeight - 1) * nDstLinestep );
570         nDstLinestep = -nDstLinestep;
571     }
(see http://opengrok.libreoffice.org/xref/core/vcl/source/gdi/bmpfast.cxx#559)

If the first if block is ok, I would have thought that second block, at initial state, would be ok.
But if the fix is ok, shouldn't the line 562 be:
aMskLine.AddByteOffset( (rMskBuffer.mnHeight - 1) * nMskLinestep );
?

Anyway the fix worked, I don't reproduce the crash, thank you Matthew!
Comment 10 Matthew Francis 2014-11-20 01:57:06 UTC
The second block isn't OK because the height of the dest bitmap can be less than that of the source bitmap - adding the size of the source bitmap to it, as happens through that path, causes memory off the end of the dest bitmap to be scribbled on.

From tracing back through the code, it appears that the mask bitmap is taken directly from the source bitmap, so their heights will be equal and the first block won't cause a crash (it makes sense that it shouldn't be an arbitrary size).
[see http://opengrok.libreoffice.org/xref/core/vcl/source/outdev/bitmap.cxx#494 which eventually leads to the problem method]

It may however be worth adding an assert to ensure/document that this is the case.
Comment 11 Julien Nabet 2014-11-20 20:23:35 UTC
Thank you Matthew for the detailed explanation! :-)