valgrind: invalid read on deleted SwPageFrm

VALGRIND=memcheck make CppunitTest_sw_ooxmlimport

==2829== Invalid read of size 2
==2829==    at 0x10B9A2CC: SwPageFrm::GetPhyPageNum() const (pagefrm.hxx:191)
==2829==    by 0x1107A768: SwLayAction::InternalAction() (layact.cxx:573)
==2829==    by 0x1107A0B6: SwLayAction::Action() (layact.cxx:448)
==2829==    by 0x114E025E: SwViewShell::CalcLayout() (viewsh.cxx:915)
==2829==    by 0xFC52E5B: SwModelTestBase::calcLayout() (swmodeltestbase.hxx:214)
==2829==    by 0xFC54167: SwModelTestBase::load(char const*, char const*) (swmodeltestbase.hxx:425)
==2829==    by 0xFC52A74: SwModelTestBase::executeImportTest(char const*) (swmodeltestbase.hxx:131)
==2829==    by 0xFC6B046: testN830205::Import() (ooxmlimport.cxx:1248)
==2829==    by 0xFC99A6B: CppUnit::TestCaller<testN830205>::runTest() (TestCaller.h:166)
==2829==    by 0x4CAE1D3: CppUnit::TestCaseMethodFunctor::operator()() const (TestCase.cpp:32)
==2829==    by 0xCF009E2: (anonymous namespace)::Prot::protect(CppUnit::Functor const&, CppUnit::ProtectorContext const&) (unobootstrapprotector.cxx:88)
==2829==    by 0x4CA630E: CppUnit::ProtectorChain::ProtectFunctor::operator()() const (ProtectorChain.cpp:20)
==2829==    by 0xBB0E535: (anonymous namespace)::Prot::protect(CppUnit::Functor const&, CppUnit::ProtectorContext const&) (unoexceptionprotector.cxx:64)
==2829==    by 0x4CA630E: CppUnit::ProtectorChain::ProtectFunctor::operator()() const (ProtectorChain.cpp:20)
==2829==    by 0x4C97C83: CppUnit::DefaultProtector::protect(CppUnit::Functor const&, CppUnit::ProtectorContext const&) (DefaultProtector.cpp:15)
==2829==    by 0x4CA630E: CppUnit::ProtectorChain::ProtectFunctor::operator()() const (ProtectorChain.cpp:20)
==2829==    by 0x4CA61A3: CppUnit::ProtectorChain::protect(CppUnit::Functor const&, CppUnit::ProtectorContext const&) (ProtectorChain.cpp:77)
==2829==    by 0x4CBD3B9: CppUnit::TestResult::protect(CppUnit::Functor const&, CppUnit::Test*, std::string const&) (TestResult.cpp:181)
==2829==    by 0x4CADCA3: CppUnit::TestCase::run(CppUnit::TestResult*) (TestCase.cpp:92)
==2829==    by 0x4CAEA3F: CppUnit::TestComposite::doRunChildTests(CppUnit::TestResult*) (TestComposite.cpp:64)
==2829==    by 0x4CAE8C9: CppUnit::TestComposite::run(CppUnit::TestResult*) (TestComposite.cpp:23)
==2829==    by 0x4CAEA3F: CppUnit::TestComposite::doRunChildTests(CppUnit::TestResult*) (TestComposite.cpp:64)
==2829==    by 0x4CAE8C9: CppUnit::TestComposite::run(CppUnit::TestResult*) (TestComposite.cpp:23)
==2829==    by 0x4CC45A5: CppUnit::TestRunner::WrappingSuite::run(CppUnit::TestResult*) (TestRunner.cpp:47)
==2829==    by 0x4CBD0C3: CppUnit::TestResult::runTest(CppUnit::Test*) (TestResult.cpp:148)
==2829==    by 0x4CC4803: CppUnit::TestRunner::run(CppUnit::TestResult&, std::string const&) (TestRunner.cpp:96)
==2829==    by 0x403F3E: (anonymous namespace)::ProtectedFixtureFunctor::run() const (cppunittester.cxx:150)
==2829==    by 0x4045C6: sal_main() (cppunittester.cxx:242)
==2829==    by 0x40420E: main (cppunittester.cxx:166)
==2829==  Address 0x2b4bbd48 is 312 bytes inside a block of size 320 free'd
==2829==    at 0x4A074C4: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==2829==    by 0x4F386E7: rtl_freeMemory_SYSTEM(void*) (alloc_global.cxx:276)
==2829==    by 0x4F3899E: rtl_freeMemory (alloc_global.cxx:346)
==2829==    by 0x4F37494: rtl_cache_free (alloc_cache.cxx:1231)
==2829==    by 0x13D7EF6D: FixedMemPool::Free(void*) (mempool.cxx:48)
==2829==    by 0x1106890F: SwPageFrm::operator delete(void*, unsigned long) (in /home/caolan/LibreOffice/core/instdir/program/libswlo.so)
==2829==    by 0x110A113A: SwPageFrm::~SwPageFrm() (pagechg.cxx:301)
==2829==    by 0x110A3713: SwFrm::CheckPageDescs(SwPageFrm*, unsigned char) (pagechg.cxx:1122)
==2829==    by 0x1107A717: SwLayAction::InternalAction() (layact.cxx:566)
==2829==    by 0x1107A0B6: SwLayAction::Action() (layact.cxx:448)
==2829==    by 0x114E025E: SwViewShell::CalcLayout() (viewsh.cxx:915)
==2829==    by 0xFC52E5B: SwModelTestBase::calcLayout() (swmodeltestbase.hxx:214)
==2829==    by 0xFC54167: SwModelTestBase::load(char const*, char const*) (swmodeltestbase.hxx:425)
==2829==    by 0xFC52A74: SwModelTestBase::executeImportTest(char const*) (swmodeltestbase.hxx:131)
==2829==    by 0xFC6B046: testN830205::Import() (ooxmlimport.cxx:1248)
==2829==    by 0xFC99A6B: CppUnit::TestCaller<testN830205>::runTest() (TestCaller.h:166)
==2829==    by 0x4CAE1D3: CppUnit::TestCaseMethodFunctor::operator()() const (TestCase.cpp:32)
==2829==    by 0xCF009E2: (anonymous namespace)::Prot::protect(CppUnit::Functor const&, CppUnit::ProtectorContext const&) (unobootstrapprotector.cxx:88)
==2829==    by 0x4CA630E: CppUnit::ProtectorChain::ProtectFunctor::operator()() const (ProtectorChain.cpp:20)
==2829==    by 0xBB0E535: (anonymous namespace)::Prot::protect(CppUnit::Functor const&, CppUnit::ProtectorContext const&) (unoexceptionprotector.cxx:64)
==2829==    by 0x4CA630E: CppUnit::ProtectorChain::ProtectFunctor::operator()() const (ProtectorChain.cpp:20)
==2829==    by 0x4C97C83: CppUnit::DefaultProtector::protect(CppUnit::Functor const&, CppUnit::ProtectorContext const&) (DefaultProtector.cpp:15)
==2829==    by 0x4CA630E: CppUnit::ProtectorChain::ProtectFunctor::operator()() const (ProtectorChain.cpp:20)
==2829==    by 0x4CA61A3: CppUnit::ProtectorChain::protect(CppUnit::Functor const&, CppUnit::ProtectorContext const&) (ProtectorChain.cpp:77)
==2829==    by 0x4CBD3B9: CppUnit::TestResult::protect(CppUnit::Functor const&, CppUnit::Test*, std::string const&) (TestResult.cpp:181)
==2829==    by 0x4CADCA3: CppUnit::TestCase::run(CppUnit::TestResult*) (TestCase.cpp:92)
==2829==    by 0x4CAEA3F: CppUnit::TestComposite::doRunChildTests(CppUnit::TestResult*) (TestComposite.cpp:64)
==2829==    by 0x4CAE8C9: CppUnit::TestComposite::run(CppUnit::TestResult*) (TestComposite.cpp:23)
==2829==    by 0x4CAEA3F: CppUnit::TestComposite::doRunChildTests(CppUnit::TestResult*) (TestComposite.cpp:64)
==2829==    by 0x4CAE8C9: CppUnit::TestComposite::run(CppUnit::TestResult*) (TestComposite.cpp:23)
==2829==    by 0x4CC45A5: CppUnit::TestRunner::WrappingSuite::run(CppUnit::TestResult*) (TestRunner.cpp:47)
==2829==    by 0x4CBD0C3: CppUnit::TestResult::runTest(CppUnit::Test*) (TestResult.cpp:148)
==2829==    by 0x4CC4803: CppUnit::TestRunner::run(CppUnit::TestResult&, std::string const&) (TestRunner.cpp:96)
==2829==    by 0x403F3E: (anonymous namespace)::ProtectedFixtureFunctor::run() const (cppunittester.cxx:150)
==2829==    by 0x4045C6: sal_main() (cppunittester.cxx:242)
==2829==    by 0x40420E: main (cppunittester.cxx:166)

Revert "Revert "bnc#382137 SwFrm::CheckPageDescs: notify clients about deleted SwPageFrm""

This reverts commit 97035c95c27b34313eadd09692804045f67f440a seeing as the modified
SwFrm::CheckPageDescs that updates the passed in previous page if it will be
deleted is better than crashing and adds it to the other callsite that deletes the
page.

Change-Id: I653bab423ffa704aef45c2a7b2ac0699b72e9d3a

3 files changed, 13 insertions, 3 deletions

diff --git a/sw/source/core/inc/frame.hxx b/sw/source/core/inc/frame.hxx
index 36a9622e0401..721deee3ab72 100644
--- a/sw/source/core/inc/frame.hxx
+++ b/sw/source/core/inc/frame.hxx
@@ -598,7 +598,7 @@ public:
     inline void SetFixSize( sal_Bool bNew ) { mbFixSize = bNew; }
 
     // check all pages (starting from the given) and correct them if needed
-    static void CheckPageDescs( SwPageFrm *pStart, sal_Bool bNotifyFields = sal_True );
+    static void CheckPageDescs( SwPageFrm *pStart, sal_Bool bNotifyFields = sal_True, SwPageFrm** ppPrev = 0);
 
     // might return 0, with and without const
     SwFrm               *GetNext()  { return mpNext; }
diff --git a/sw/source/core/layout/layact.cxx b/sw/source/core/layout/layact.cxx
index 4325e382c0ec..033ed4a58b25 100644
--- a/sw/source/core/layout/layact.cxx
+++ b/sw/source/core/layout/layact.cxx
@@ -563,7 +563,7 @@ void SwLayAction::InternalAction()
             SwPageFrm *pTmp = pPage->GetPrev() ?
                                         (SwPageFrm*)pPage->GetPrev() : pPage;
             SetCheckPages( sal_True );
-            SwFrm::CheckPageDescs( pPage );
+            SwFrm::CheckPageDescs( pPage, sal_True, &pTmp );
             SetCheckPages( sal_False );
             nCheckPageNum = USHRT_MAX;
             pPage = pTmp;
diff --git a/sw/source/core/layout/pagechg.cxx b/sw/source/core/layout/pagechg.cxx
index 2cfb864f3390..0cbf9f39d93e 100644
--- a/sw/source/core/layout/pagechg.cxx
+++ b/sw/source/core/layout/pagechg.cxx
@@ -1044,7 +1044,7 @@ void SwPageFrm::PrepareRegisterChg()
 |*      einfache zu bereinigen.
 |*
 |*************************************************************************/
-void SwFrm::CheckPageDescs( SwPageFrm *pStart, sal_Bool bNotifyFields )
+void SwFrm::CheckPageDescs( SwPageFrm *pStart, sal_Bool bNotifyFields, SwPageFrm** ppPrev )
 {
     OSL_ENSURE( pStart, "Keine Startpage." );
 
@@ -1119,10 +1119,15 @@ void SwFrm::CheckPageDescs( SwPageFrm *pStart, sal_Bool bNotifyFields )
             {
                 SwPageFrm *pTmp = (SwPageFrm*)pPage->GetNext();
                 pPage->Cut();
+                bool bUpdatePrev = false;
+                if (ppPrev && *ppPrev == pPage)
+                    bUpdatePrev = true;
                 delete pPage;
                 if ( pStart == pPage )
                     pStart = pTmp;
                 pPage = pTmp;
+                if (bUpdatePrev)
+                    *ppPrev = pTmp;
                 continue;
             }
             else if ( pPage->IsEmptyPage() && !pFmtWish &&  //2.
@@ -1199,10 +1204,15 @@ void SwFrm::CheckPageDescs( SwPageFrm *pStart, sal_Bool bNotifyFields )
                 //Nachfolger, also ist die Leerseite ueberfluessig.
                 SwPageFrm *pTmp = (SwPageFrm*)pPage->GetNext();
                 pPage->Cut();
+                bool bUpdatePrev = false;
+                if (ppPrev && *ppPrev == pPage)
+                    bUpdatePrev = true;
                 delete pPage;
                 if ( pStart == pPage )
                     pStart = pTmp;
                 pPage = pTmp;
+                if (bUpdatePrev)
+                    *ppPrev = pTmp;
                 continue;
             }
         }