sw/source/core/inc/txtfrm.hxx | 2 ++ sw/source/core/text/frmform.cxx | 9 ++++++++- sw/source/core/text/txtfrm.cxx | 8 ++++---- 3 files changed, 14 insertions(+), 5 deletions(-)
New commits: commit 81c4e92689a05828206acd6568ccb3080394d52a Author: Michael Stahl <[email protected]> AuthorDate: Fri Dec 20 17:28:55 2019 +0100 Commit: Michael Stahl <[email protected]> CommitDate: Mon Dec 23 10:10:15 2019 +0100 sw: fix widow loop with as-char flys in text formatting The document has a paragraph with 4 as-character anchored flys depicting Zirbenholz; due to their size and an additional fly that is anchored at the paragraph, there are 3 lines that do not fit onto a single page. This situation causes a loop that proceeds like this: text frame 80 is the follow of text frame 21. when formatting 80: the 1 line violates the widow rule (>=2) and PREP_WIDOWS is sent to 21, invalidating its FrameAreaSize 80 validates its FrameAreaSize when formatting 21: PREP_WIDOWS_ORPHANS sent to 21 CalcPreps() sees IsPrepWidows() and sets a huge height and calls SetWidow(true) SwTextFrame::WouldFit() sees IsWidow() true and resets it false SwTextFrame::WouldFit() sees IsWidow() false and a huge but insufficiently huge height 21 validates its FrameAreaSize CalcPreps() sees IsPrepAdjust() FindBreak() calls TruncLines() and because of as-char fly invalidates FrameAreaSize of 80 The loop is most easily reproduced by printing via the API; it's possible to get a loop when loading the document in the UI, but typically the UI remains responsive even though the layout never finishes. As it happens, before commit ee299664940139f6f9543592ece3b3c0210b59f4 "SalInstance::DoYield: Don't drop SolarMutex when accessing user event queue" the loop on printing via API was broken by releasing SolarMutex; the result, however, was incorrect, with the last line of Zirbenholz that should be on the second page missing in the PDF. This loop is presumably a regression from commit f2e3655255db4032738849cd4b77ce67a6e2c984 "Avoid -fsanitize=signed-integer-overflow", which changed a magic number in SwTextFrame::CalcPreps(), but didn't adapt the poorly documented corresponding magic numbers in SwTextFrame::WouldFit(); in LO 5.1.6.2 the CPU is idle after loading the document. Change-Id: Ib6563c21edb68945c14a61b51ba34f0ee3f2544a Reviewed-on: https://gerrit.libreoffice.org/85623 Tested-by: Jenkins Reviewed-by: Michael Stahl <[email protected]> (cherry picked from commit 68a5afaaabd0c75bba3439cfdff90fb75d1cdd3f) diff --git a/sw/source/core/inc/txtfrm.hxx b/sw/source/core/inc/txtfrm.hxx index 5656353d361a..9c31d19538f1 100644 --- a/sw/source/core/inc/txtfrm.hxx +++ b/sw/source/core/inc/txtfrm.hxx @@ -966,6 +966,8 @@ public: }; +const SwTwips WIDOW_MAGIC = (SAL_MAX_INT32 - 1)/2; + } // namespace sw #endif diff --git a/sw/source/core/text/frmform.cxx b/sw/source/core/text/frmform.cxx index 959daa4ea9a1..d5a402ebe4c6 100644 --- a/sw/source/core/text/frmform.cxx +++ b/sw/source/core/text/frmform.cxx @@ -849,7 +849,7 @@ bool SwTextFrame::CalcPreps() // the range of 'long', while the value (SAL_MAX_INT32 - 1)/2 (which matches the // old value on platforms where 'long' is 'sal_Int32') is empirically shown to // be large enough in practice even on platforms where 'long' is 'sal_Int64': - SwTwips nTmp = (SAL_MAX_INT32 - 1)/2 - (getFrameArea().Top()+10000); + SwTwips const nTmp = sw::WIDOW_MAGIC - (getFrameArea().Top()+10000); SwTwips nDiff = nTmp - getFrameArea().Height(); { diff --git a/sw/source/core/text/txtfrm.cxx b/sw/source/core/text/txtfrm.cxx index a2d8001c80bc..1d8fb43dcebd 100644 --- a/sw/source/core/text/txtfrm.cxx +++ b/sw/source/core/text/txtfrm.cxx @@ -2760,9 +2760,9 @@ bool SwTextFrame::WouldFit( SwTwips &rMaxHeight, bool &bSplit, bool bTst ) // Because the Orphan flag only exists for a short moment, we also check // whether the Framesize is set to very huge by CalcPreps, in order to // force a MoveFwd - if( IsWidow() || ( aRectFnSet.IsVert() ? - ( 0 == getFrameArea().Left() ) : - ( LONG_MAX - 20000 < getFrameArea().Bottom() ) ) ) + if (IsWidow() || (aRectFnSet.IsVert() + ? (0 == getFrameArea().Left()) + : (sw::WIDOW_MAGIC - 20000 < getFrameArea().Bottom()))) { SetWidow(false); if ( GetFollow() ) @@ -2771,7 +2771,7 @@ bool SwTextFrame::WouldFit( SwTwips &rMaxHeight, bool &bSplit, bool bTst ) // whether there's a Follow with a real height at all. // Else (e.g. for newly created SctFrames) we ignore the IsWidow() and // still check if we can find enough room - if( ( ( ! aRectFnSet.IsVert() && LONG_MAX - 20000 >= getFrameArea().Bottom() ) || + if (((!aRectFnSet.IsVert() && getFrameArea().Bottom() <= sw::WIDOW_MAGIC - 20000) || ( aRectFnSet.IsVert() && 0 < getFrameArea().Left() ) ) && ( GetFollow()->IsVertical() ? !GetFollow()->getFrameArea().Width() : commit 61bfaaa8c317d48b556dfeb405cbe84fc2081dd8 Author: Stephan Bergmann <[email protected]> AuthorDate: Thu Jun 27 13:33:27 2019 +0200 Commit: Michael Stahl <[email protected]> CommitDate: Mon Dec 23 10:10:05 2019 +0100 tdf#126127: Make nTmp smaller still, avoid -fsanitize=signed-integer-overflow ...after f2e3655255db4032738849cd4b77ce67a6e2c984 "Avoid -fsanitize=signed-integer-overflow" had already reduced it from using LONG_MAX to TWIPS_MAX/2 in the past. This time, avoid the computation of > const sal_uInt64 nCurrentDist = sal_Int64(aDiff.getX()) * sal_Int64(aDiff.getX()) + sal_Int64(aDiff.getY()) * sal_Int64(aDiff.getY()); // opt: no sqrt in GetFrameOfModify (sw/source/core/layout/frmtool.cxx) from overflowing (where aDiff.getY() derives from nTmp and can be close to it in magnitude, so computing its square would overflow on platforms where TWIPS_MAX is a large sal_Int64 value). (The "empirically shown to be large enough in practice" in the comment is a successful `make check` on Linux 64-bit with UBSan.) Change-Id: Ic7f058bd6853ff04ccb50a150509e98f850d12d2 Reviewed-on: https://gerrit.libreoffice.org/74801 Reviewed-by: Michael Stahl <[email protected]> Tested-by: Jenkins (cherry picked from commit 8723ac4e20eda87a82393f2f6c7d28ece8514238) diff --git a/sw/source/core/text/frmform.cxx b/sw/source/core/text/frmform.cxx index 176914b69040..959daa4ea9a1 100644 --- a/sw/source/core/text/frmform.cxx +++ b/sw/source/core/text/frmform.cxx @@ -842,7 +842,14 @@ bool SwTextFrame::CalcPreps() } else { - SwTwips nTmp = TWIPS_MAX/2 - (getFrameArea().Top()+10000); + // nTmp should be very large, but not so large as to cause overflow later (e.g., + // GetFrameOfModify in sw/source/core/layout/frmtool.cxx calculates nCurrentDist + // from, among others, the square of aDiff.getY(), which can be close to nTmp); + // the previously used value TWIPS_MAX/2 (i.e., (LONG_MAX - 1)/2) depended on + // the range of 'long', while the value (SAL_MAX_INT32 - 1)/2 (which matches the + // old value on platforms where 'long' is 'sal_Int32') is empirically shown to + // be large enough in practice even on platforms where 'long' is 'sal_Int64': + SwTwips nTmp = (SAL_MAX_INT32 - 1)/2 - (getFrameArea().Top()+10000); SwTwips nDiff = nTmp - getFrameArea().Height(); { _______________________________________________ Libreoffice-commits mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
