Created attachment 109495 [details]
bt with debug symbols
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)
No crash from master built yesterday.
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:
Author: Jan Holesovsky <firstname.lastname@example.org>
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
Created attachment 109578 [details]
Valgrind memcheck log showing the (an?) issue
Adding a CC to email@example.com - could you shed any light on this one? Thanks
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
Matthew J. Francis committed a patch related to this issue.
It has been pushed to "master":
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:
Affected users are encouraged to test the fix and report feedback.
*** Bug 86412 has been marked as a duplicate of this bug. ***
*** Bug 86316 has been marked as a duplicate of this bug. ***
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 )
562 aMskLine.AddByteOffset( (rSrcBuffer.mnHeight - 1) * nMskLinestep );
563 nMskLinestep = -nMskLinestep;
566 // source and destination don't match: upside down
567 if( (rSrcBuffer.mnFormat ^ rDstBuffer.mnFormat) & BMP_FORMAT_TOP_DOWN )
- aDstLine.AddByteOffset( (rSrcBuffer.mnHeight - 1) * nDstLinestep );
+ aDstLine.AddByteOffset( (rDstBuffer.mnHeight - 1) * nDstLinestep );
570 nDstLinestep = -nDstLinestep;
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!
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.
Thank you Matthew for the detailed explanation! :-)