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)
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: 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
Created attachment 109578 [details] Valgrind memcheck log showing the (an?) issue
Adding a CC to kendy@collabora.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": 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.
*** 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 ) 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!
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! :-)