[Libreoffice-commits] core.git: 4 commits - sw/source
Michael Stahl
mstahl at redhat.com
Thu Jun 22 08:15:21 UTC 2017
sw/source/core/layout/calcmove.cxx | 20 +++++++++++++++++
sw/source/core/layout/flowfrm.cxx | 14 ++++++++++--
sw/source/core/layout/ftnfrm.cxx | 3 +-
sw/source/core/layout/objectformattertxtfrm.cxx | 27 +++++++++++++++++++++++-
sw/source/core/text/xmldump.cxx | 10 ++++++++
5 files changed, 70 insertions(+), 4 deletions(-)
New commits:
commit 9ec1ccb23af0de56f141d906f2eb60bab40aefb8
Author: Michael Stahl <mstahl at redhat.com>
Date: Wed Jun 21 16:44:45 2017 +0200
sw: dump more connections of those obnoxious footnotes in layout.xml
Change-Id: I62caffcacb710aa079ddc9c81fb49f702cdc84af
diff --git a/sw/source/core/text/xmldump.cxx b/sw/source/core/text/xmldump.cxx
index 9358e922de50..7a28ceed4862 100644
--- a/sw/source/core/text/xmldump.cxx
+++ b/sw/source/core/text/xmldump.cxx
@@ -9,6 +9,7 @@
#include "frame.hxx"
#include "frmfmt.hxx"
+#include "ftnfrm.hxx"
#include "sectfrm.hxx"
#include "tabfrm.hxx"
#include "txtfrm.hxx"
@@ -386,6 +387,15 @@ void SwFrame::dumpAsXmlAttributes( xmlTextWriterPtr writer ) const
xmlTextWriterWriteFormatAttribute( writer, BAD_CAST( "upper" ), "%" SAL_PRIuUINT32, GetUpper()->GetFrameId() );
if ( GetLower( ) )
xmlTextWriterWriteFormatAttribute( writer, BAD_CAST( "lower" ), "%" SAL_PRIuUINT32, GetLower()->GetFrameId() );
+ if (IsFootnoteFrame())
+ {
+ SwFootnoteFrame const*const pFF(static_cast<SwFootnoteFrame const*>(this));
+ xmlTextWriterWriteFormatAttribute( writer, BAD_CAST("ref"), "%" SAL_PRIuUINT32, pFF->GetRef()->GetFrameId() );
+ if (pFF->GetMaster())
+ xmlTextWriterWriteFormatAttribute( writer, BAD_CAST("master"), "%" SAL_PRIuUINT32, pFF->GetMaster()->GetFrameId() );
+ if (pFF->GetFollow())
+ xmlTextWriterWriteFormatAttribute( writer, BAD_CAST("follow"), "%" SAL_PRIuUINT32, pFF->GetFollow()->GetFrameId() );
+ }
if ( IsTextFrame( ) )
{
const SwTextFrame *pTextFrame = static_cast<const SwTextFrame *>(this);
commit c83a443eb106e426901d0ba8a809eedcd24c42c5
Author: Michael Stahl <mstahl at redhat.com>
Date: Wed Jun 21 15:26:01 2017 +0200
tdf#101821 sw: fix another layout footnote use-after-free
In SwContentFrame::MakeAll() the pPre (previous frame) is deleted in a
call to MoveFwd(). This SwTextFrame is inside of a footnote, and
the SwFlowFrame::CutTree() for whatever reason wants to format all
of the SwTextFrames inside the same SwFootnoteFrame, which causes
the pPre to be joined with another one.
Let's try to avoid that by checking that it's still in the layout after
the call to MoveFwd(); on the one hand it's not obvious that this frame
is important enough that it should be kept alive with ForbidDelete(),
on the other hand i have no idea if simply removing the ValidateSz()
call would introduce new loops or whatever.
Invalid read of size 4
at 0x414E8D14: SwFrame::IsSctFrame() const (frame.hxx:1018)
by 0x41A85A29: SwContentFrame::MakeAll(OutputDevice*) (calcmove.cxx:1713)
by 0x41A7F499: SwFrame::OptPrepareMake() (calcmove.cxx:368)
by 0x41ADF0CF: SwFrame::OptCalc() const (frame.hxx:892)
by 0x41ADCC07: SwLayAction::FormatContent_(SwContentFrame const*, SwPageFrame const*) (layact.cxx:1789)
by 0x41ADC195: SwLayAction::FormatContent(SwPageFrame const*) (layact.cxx:1620)
by 0x41AD88DE: SwLayAction::InternalAction(OutputDevice*) (layact.cxx:760)
by 0x41AD7080: SwLayAction::Action(OutputDevice*) (layact.cxx:351)
by 0x41ADE32E: SwLayIdle::SwLayIdle(SwRootFrame*, SwViewShellImp*) (layact.cxx:2133)
by 0x41FFC97E: SwViewShell::LayoutIdle() (viewsh.cxx:711)
Address 0x530ccf80 is 160 bytes inside a block of size 264 free'd
at 0x4C2ED4A: free (vg_replace_malloc.c:530)
by 0x4E5BCD0: rtl_freeMemory_SYSTEM(void*) (alloc_global.cxx:271)
by 0x4E5C00A: rtl_freeMemory (alloc_global.cxx:341)
by 0x4E5AAA0: rtl_cache_free (alloc_cache.cxx:1231)
by 0xEFC9A6F: FixedMemPool::Free(void*) (mempool.cxx:49)
by 0x41CA7DFA: SwTextFrame::operator delete(void*, unsigned long) (txtfrm.hxx:377)
by 0x41C9F7B6: SwTextFrame::~SwTextFrame() (txtfrm.cxx:415)
by 0x41B5B74C: SwFrame::DestroyFrame(SwFrame*) (ssfrm.cxx:391)
by 0x41C1589A: SwTextFrame::JoinFrame() (frmform.cxx:656)
by 0x41C153B1: SwTextFrame::AdjustFollow_(SwTextFormatter&, int, int, unsigned char) (frmform.cxx:555)
by 0x41C172E1: SwTextFrame::FormatAdjust(SwTextFormatter&, WidowsAndOrphans&, int, bool) (frmform.cxx:1108)
by 0x41C18D5E: SwTextFrame::Format_(SwTextFormatter&, SwTextFormatInfo&, bool) (frmform.cxx:1550)
by 0x41C19340: SwTextFrame::Format_(OutputDevice*, SwParaPortion*) (frmform.cxx:1660)
by 0x41C19CD8: SwTextFrame::Format(OutputDevice*, SwBorderAttrs const*) (frmform.cxx:1807)
by 0x41A847A1: SwContentFrame::MakeAll(OutputDevice*) (calcmove.cxx:1393)
by 0x41A7F211: SwFrame::PrepareMake(OutputDevice*) (calcmove.cxx:346)
by 0x41B75758: SwFrame::Calc(OutputDevice*) const (trvlfrm.cxx:1761)
by 0x41A973E7: SwFlowFrame::CutTree(SwFrame*) (flowfrm.cxx:424)
by 0x41A979AE: SwFlowFrame::MoveSubTree(SwLayoutFrame*, SwFrame*) (flowfrm.cxx:592)
by 0x41ACFB69: SwContentFrame::MoveFootnoteCntFwd(bool, SwFootnoteBossFrame*) (ftnfrm.cxx:2756)
by 0x41A9B78E: SwFlowFrame::MoveFwd(bool, bool, bool) (flowfrm.cxx:1813)
by 0x41A85864: SwContentFrame::MakeAll(OutputDevice*) (calcmove.cxx:1681)
by 0x41A7F499: SwFrame::OptPrepareMake() (calcmove.cxx:368)
by 0x41ADF0CF: SwFrame::OptCalc() const (frame.hxx:892)
by 0x41ADCC07: SwLayAction::FormatContent_(SwContentFrame const*, SwPageFrame const*) (layact.cxx:1789)
by 0x41ADC195: SwLayAction::FormatContent(SwPageFrame const*) (layact.cxx:1620)
by 0x41AD88DE: SwLayAction::InternalAction(OutputDevice*) (layact.cxx:760)
by 0x41AD7080: SwLayAction::Action(OutputDevice*) (layact.cxx:351)
by 0x41ADE32E: SwLayIdle::SwLayIdle(SwRootFrame*, SwViewShellImp*) (layact.cxx:2133)
by 0x41FFC97E: SwViewShell::LayoutIdle() (viewsh.cxx:711)
Change-Id: I35b27a32608d4dcf98e0933cce76ce5ddb1c09d9
diff --git a/sw/source/core/layout/calcmove.cxx b/sw/source/core/layout/calcmove.cxx
index 56886e4a95c7..2fdbb6b4f62f 100644
--- a/sw/source/core/layout/calcmove.cxx
+++ b/sw/source/core/layout/calcmove.cxx
@@ -1681,6 +1681,26 @@ void SwContentFrame::MakeAll(vcl::RenderContext* /*pRenderContext*/)
if ( !bMovedFwd && !MoveFwd( bMakePage, false ) )
bMakePage = false;
aRectFnSet.Refresh(this);
+ if (!bMovedFwd && bFootnote && GetIndPrev() != pPre)
+ { // SwFlowFrame::CutTree() could have formatted and deleted pPre
+ auto const pPrevFootnoteFrame(static_cast<SwFootnoteFrame const*>(GetUpper())->GetMaster());
+ bool bReset = true;
+ if (pPrevFootnoteFrame)
+ { // use GetIndNext() in case there are sections
+ for (auto p = pPrevFootnoteFrame->Lower(); p; p = p->GetIndNext())
+ {
+ if (p == pPre)
+ {
+ bReset = false;
+ break;
+ }
+ }
+ }
+ if (bReset)
+ {
+ pPre = nullptr;
+ }
+ }
// If MoveFwd moves the paragraph to the next page, a following
// paragraph, which contains footnotes can cause the old upper
commit 9dcb767c5bdccdf6606240afd6aa2c6bd3dcc9f4
Author: Michael Stahl <mstahl at redhat.com>
Date: Wed Jun 21 12:26:19 2017 +0200
tdf#101821 sw: fix layout footnote use-after-free
After inserting a header in the bugdoc, during SwFrame::Calc of a
SwTextFrame that is in a footnote, it decides move forward to the next
page. This deletes the SwFootnoteFrame and SwFootnoteContFrame that
lcl_FormatContentOfLayoutFrame() are iterating over.
For want of a more elegant solution, use a big hammer to prevent the
problem and try to clean up so that no empty SwFootnoteFrame and
SwFootnoteContFrame remain (as that is known to crash in other places,
see commit c9fb347642729017ad0c613fe26310befd021db8)
Invalid read of size 8
at 0x414E1F96: SwFrame::GetNext() (frame.hxx:485)
by 0x41AFDD07: lcl_FormatContentOfLayoutFrame(SwLayoutFrame*, SwFrame*) (objectformattertxtfrm.cxx:646)
by 0x41AFDCC0: lcl_FormatContentOfLayoutFrame(SwLayoutFrame*, SwFrame*) (objectformattertxtfrm.cxx:642)
by 0x41AFDCC0: lcl_FormatContentOfLayoutFrame(SwLayoutFrame*, SwFrame*) (objectformattertxtfrm.cxx:642)
by 0x41AFDEA7: SwObjectFormatterTextFrame::FormatAnchorFrameAndItsPrevs(SwTextFrame&) (objectformattertxtfrm.cxx:696)
by 0x41AAA680: SwFlyAtContentFrame::MakeAll(OutputDevice*) (flycnt.cxx:415)
by 0x41A7F211: SwFrame::PrepareMake(OutputDevice*) (calcmove.cxx:346)
by 0x41B75758: SwFrame::Calc(OutputDevice*) const (trvlfrm.cxx:1761)
by 0x41AA8927: SwFlyFrame::Calc(OutputDevice*) const (fly.cxx:2559)
by 0x41ADB36B: SwLayAction::FormatLayoutFly(SwFlyFrame*) (layact.cxx:1414)
by 0x41AF9658: SwObjectFormatter::FormatObj_(SwAnchoredObject&) (objectformatter.cxx:321)
by 0x41AFCB6E: SwObjectFormatterTextFrame::DoFormatObj(SwAnchoredObject&, bool) (objectformattertxtfrm.cxx:126)
by 0x41AF9A6A: SwObjectFormatter::FormatObjsAtFrame_(SwTextFrame*) (objectformatter.cxx:443)
by 0x41AFD275: SwObjectFormatterTextFrame::DoFormatObjs() (objectformattertxtfrm.cxx:328)
by 0x41AF924A: SwObjectFormatter::FormatObjsAtFrame(SwFrame&, SwPageFrame const&, SwLayAction*) (objectformatter.cxx:191)
by 0x41ADC213: SwLayAction::FormatContent(SwPageFrame const*) (layact.cxx:1633)
by 0x41AD88DE: SwLayAction::InternalAction(OutputDevice*) (layact.cxx:760)
by 0x41AD7080: SwLayAction::Action(OutputDevice*) (layact.cxx:351)
by 0x41ADE32E: SwLayIdle::SwLayIdle(SwRootFrame*, SwViewShellImp*) (layact.cxx:2133)
by 0x41FFC97E: SwViewShell::LayoutIdle() (viewsh.cxx:711)
Address 0x505541a8 is 72 bytes inside a block of size 272 free'd
at 0x4C2F21A: operator delete(void*) (vg_replace_malloc.c:576)
by 0x41AD371A: SwFootnoteFrame::~SwFootnoteFrame() (ftnfrm.hxx:52)
by 0x41B5B74C: SwFrame::DestroyFrame(SwFrame*) (ssfrm.cxx:391)
by 0x41A97294: SwFlowFrame::CutTree(SwFrame*) (flowfrm.cxx:406)
by 0x41A979AE: SwFlowFrame::MoveSubTree(SwLayoutFrame*, SwFrame*) (flowfrm.cxx:592)
by 0x41ACFB69: SwContentFrame::MoveFootnoteCntFwd(bool, SwFootnoteBossFrame*) (ftnfrm.cxx:2756)
by 0x41A9B78E: SwFlowFrame::MoveFwd(bool, bool, bool) (flowfrm.cxx:1813)
by 0x41A85864: SwContentFrame::MakeAll(OutputDevice*) (calcmove.cxx:1681)
by 0x41A7F211: SwFrame::PrepareMake(OutputDevice*) (calcmove.cxx:346)
by 0x41B75758: SwFrame::Calc(OutputDevice*) const (trvlfrm.cxx:1761)
by 0x41AFDCFB: lcl_FormatContentOfLayoutFrame(SwLayoutFrame*, SwFrame*) (objectformattertxtfrm.cxx:644)
by 0x41AFDCC0: lcl_FormatContentOfLayoutFrame(SwLayoutFrame*, SwFrame*) (objectformattertxtfrm.cxx:642)
by 0x41AFDCC0: lcl_FormatContentOfLayoutFrame(SwLayoutFrame*, SwFrame*) (objectformattertxtfrm.cxx:642)
by 0x41AFDCC0: lcl_FormatContentOfLayoutFrame(SwLayoutFrame*, SwFrame*) (objectformattertxtfrm.cxx:642)
by 0x41AFDEA7: SwObjectFormatterTextFrame::FormatAnchorFrameAndItsPrevs(SwTextFrame&) (objectformattertxtfrm.cxx:696)
by 0x41AAA680: SwFlyAtContentFrame::MakeAll(OutputDevice*) (flycnt.cxx:415)
by 0x41A7F211: SwFrame::PrepareMake(OutputDevice*) (calcmove.cxx:346)
by 0x41B75758: SwFrame::Calc(OutputDevice*) const (trvlfrm.cxx:1761)
by 0x41AA8927: SwFlyFrame::Calc(OutputDevice*) const (fly.cxx:2559)
by 0x41ADB36B: SwLayAction::FormatLayoutFly(SwFlyFrame*) (layact.cxx:1414)
by 0x41AF9658: SwObjectFormatter::FormatObj_(SwAnchoredObject&) (objectformatter.cxx:321)
by 0x41AFCB6E: SwObjectFormatterTextFrame::DoFormatObj(SwAnchoredObject&, bool) (objectformattertxtfrm.cxx:126)
by 0x41AF9A6A: SwObjectFormatter::FormatObjsAtFrame_(SwTextFrame*) (objectformatter.cxx:443)
by 0x41AFD275: SwObjectFormatterTextFrame::DoFormatObjs() (objectformattertxtfrm.cxx:328)
by 0x41AF924A: SwObjectFormatter::FormatObjsAtFrame(SwFrame&, SwPageFrame const&, SwLayAction*) (objectformatter.cxx:191)
by 0x41ADC213: SwLayAction::FormatContent(SwPageFrame const*) (layact.cxx:1633)
by 0x41AD88DE: SwLayAction::InternalAction(OutputDevice*) (layact.cxx:760)
by 0x41AD7080: SwLayAction::Action(OutputDevice*) (layact.cxx:351)
by 0x41ADE32E: SwLayIdle::SwLayIdle(SwRootFrame*, SwViewShellImp*) (layact.cxx:2133)
by 0x41FFC97E: SwViewShell::LayoutIdle() (viewsh.cxx:711)
Change-Id: I656cc2303eeccd5eef68ad3b8e81bb0fd47b94fb
diff --git a/sw/source/core/layout/flowfrm.cxx b/sw/source/core/layout/flowfrm.cxx
index bfb1b8be3b8b..0d80955af0bd 100644
--- a/sw/source/core/layout/flowfrm.cxx
+++ b/sw/source/core/layout/flowfrm.cxx
@@ -402,8 +402,18 @@ SwLayoutFrame *SwFlowFrame::CutTree( SwFrame *pStart )
if ( !pLay->Lower() && !pLay->IsColLocked() &&
!static_cast<SwFootnoteFrame*>(pLay)->IsBackMoveLocked() )
{
- pLay->Cut();
- SwFrame::DestroyFrame(pLay);
+ // tdf#101821 don't delete it while iterating over it
+ if (!pLay->IsDeleteForbidden())
+ {
+ pLay->Cut();
+ SwFrame::DestroyFrame(pLay);
+ }
+ // else: assume there is code on the stack to clean up empty
+ // footnote frames
+ // (don't go into the else branch below, it produces a disconnected
+ // footnote with null upper that can be returned by
+ // SwFootnoteBossFrame::FindFootnote() causing null pointer deref
+ // in SwTextFrame::ConnectFootnote()
}
else
{
diff --git a/sw/source/core/layout/objectformattertxtfrm.cxx b/sw/source/core/layout/objectformattertxtfrm.cxx
index 032f0cf9a304..54cb686637e6 100644
--- a/sw/source/core/layout/objectformattertxtfrm.cxx
+++ b/sw/source/core/layout/objectformattertxtfrm.cxx
@@ -29,6 +29,7 @@
#include <fmtwrapinfluenceonobjpos.hxx>
#include <fmtfollowtextflow.hxx>
#include <layact.hxx>
+#include <ftnfrm.hxx>
using namespace ::com::sun::star;
@@ -638,12 +639,36 @@ static void lcl_FormatContentOfLayoutFrame( SwLayoutFrame* pLayFrame,
break;
}
if ( pLowerFrame->IsLayoutFrame() )
+ {
+ SwFrameDeleteGuard aCrudeHack(pLowerFrame); // ??? any issue setting this for non-footnote frames?
lcl_FormatContentOfLayoutFrame( static_cast<SwLayoutFrame*>(pLowerFrame),
pLastLowerFrame );
+ }
else
pLowerFrame->Calc(pLowerFrame->getRootFrame()->GetCurrShell()->GetOut());
- pLowerFrame = pLowerFrame->GetNext();
+ // Calc on a SwTextFrame in a footnote can move it to the next page -
+ // deletion of the SwFootnoteFrame was disabled with SwFrameDeleteGuard
+ // but now we have to clean up empty footnote frames to prevent crashes.
+ // Note: check it at this level, not lower: both container and footnote
+ // can be deleted at the same time!
+ SwFrame *const pNext = pLowerFrame->GetNext();
+ if (pLowerFrame->IsFootnoteContFrame())
+ {
+ for (SwFrame * pFootnote = pLowerFrame->GetLower(); pFootnote; )
+ {
+ assert(pFootnote->IsFootnoteFrame());
+ SwFrame *const pNextNote = pFootnote->GetNext();
+ if (!pFootnote->GetLower() && !pFootnote->IsColLocked() &&
+ !static_cast<SwFootnoteFrame*>(pFootnote)->IsBackMoveLocked())
+ {
+ pFootnote->Cut();
+ SwFrame::DestroyFrame(pFootnote);
+ }
+ pFootnote = pNextNote;
+ }
+ }
+ pLowerFrame = pNext;
}
}
commit 4c0b3520b66477334a7971dbed7ffcdcd265e749
Author: Michael Stahl <mstahl at redhat.com>
Date: Mon Jun 19 11:40:12 2017 +0200
tdf#101821 sw: layout: don't move endnotes into footnotes' container
The bugdoc has a single 1-column section from start to end, no
footnotes but lots of endnotes, and the section has the settings
"Footnotes - collect at end of text" unchecked and "Endnotes - collect
at end of section" checked.
This means that the SwFootnoteContFrame for footnotes would be put
directly below the SwPageFrame (so that multiple sections on a single
page can share it), but the SwFootnoteContFrame for the endnotes is
put below the SwColumnFrame (which is created despite only 1 column)
below the SwSectionFrame.
Hence content in endnotes has the mbInfSct flag set, and the crash
happens because the endnotes are moved from below the SwSectionFrame to
a new SwFootnoteContFrame that is directly below a SwPageFrame, without
clearing the mbInfSct flag.
Fix the wrong call in SwFootnoteBossFrame::MoveFootnotes_() to
FindFootnoteBossFrame() that resulted in the wrong (unsuitable for
endnotes) SwFootnoteContFrame to be used as the target for the move.
Change-Id: I64f6b86441e5ac1f16433f005e97c274a1c69dfa
diff --git a/sw/source/core/layout/ftnfrm.cxx b/sw/source/core/layout/ftnfrm.cxx
index 8084d28e5a8b..e14b50289733 100644
--- a/sw/source/core/layout/ftnfrm.cxx
+++ b/sw/source/core/layout/ftnfrm.cxx
@@ -1904,7 +1904,8 @@ void SwFootnoteBossFrame::MoveFootnotes_( SwFootnoteFrames &rFootnoteArr, bool b
SwFootnoteFrame* pLastInsertedFootnote = nullptr;
for (SwFootnoteFrame* pFootnote : rFootnoteArr)
{
- SwFootnoteBossFrame* pRefBoss = pFootnote->GetRef()->FindFootnoteBossFrame( true );
+ SwFootnoteBossFrame* pRefBoss(pFootnote->GetRef()->FindFootnoteBossFrame(
+ !pFootnote->GetAttr()->GetFootnote().IsEndNote()));
if( pRefBoss != this )
{
const sal_uInt16 nRefNum = pRefBoss->FindPageFrame()->GetPhyPageNum();
More information about the Libreoffice-commits
mailing list