[Libreoffice-commits] core.git: Branch 'distro/cib/libreoffice-6-1' - 2 commits - sw/source
Michael Stahl (via logerrit)
logerrit at kemper.freedesktop.org
Mon Dec 23 10:28:31 UTC 2019
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 d83f066a8f70e260b388275fe596b37df0ad1eeb
Author: Michael Stahl <Michael.Stahl at cib.de>
AuthorDate: Fri Dec 20 17:28:55 2019 +0100
Commit: Michael Stahl <Michael.Stahl at cib.de>
CommitDate: Mon Dec 23 11:26:49 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 <michael.stahl at cib.de>
(cherry picked from commit 68a5afaaabd0c75bba3439cfdff90fb75d1cdd3f)
diff --git a/sw/source/core/inc/txtfrm.hxx b/sw/source/core/inc/txtfrm.hxx
index 98a18f7ed8ad..ce5ec49232ed 100644
--- a/sw/source/core/inc/txtfrm.hxx
+++ b/sw/source/core/inc/txtfrm.hxx
@@ -964,6 +964,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 62b281ae6974..377dd17c5103 100644
--- a/sw/source/core/text/txtfrm.cxx
+++ b/sw/source/core/text/txtfrm.cxx
@@ -2759,9 +2759,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() )
@@ -2770,7 +2770,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 a9158cf3a52decd09fa0fb2b808764a7ae5adc71
Author: Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Thu Jun 27 13:33:27 2019 +0200
Commit: Michael Stahl <Michael.Stahl at cib.de>
CommitDate: Mon Dec 23 11:26:49 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 <michael.stahl at cib.de>
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();
{
More information about the Libreoffice-commits
mailing list