summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorCaolán McNamara <caolanm@redhat.com>2013-12-30 00:47:02 +0000
committerCaolán McNamara <caolanm@redhat.com>2013-12-30 10:38:47 +0000
commit582d3cb9346e0e1c1c3dc3188cd28976ac24e1bf (patch)
tree6502c3132cfd8e70336044c4c20fcdcd1fc4f2fc
parent6cc888babdc3e6414e55f57c0df65135f2ef4804 (diff)
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 (cherry picked from commit 24b89acffeafe4361a63121b8ec2463c97f863e9)
-rw-r--r--sw/source/core/inc/frame.hxx2
-rw-r--r--sw/source/core/layout/layact.cxx2
-rw-r--r--sw/source/core/layout/pagechg.cxx12
3 files changed, 13 insertions, 3 deletions
diff --git a/sw/source/core/inc/frame.hxx b/sw/source/core/inc/frame.hxx
index 8c1006174cf1..599c0219751b 100644
--- a/sw/source/core/inc/frame.hxx
+++ b/sw/source/core/inc/frame.hxx
@@ -588,7 +588,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 4026277bdf3d..b9b6b49d49ae 100644
--- a/sw/source/core/layout/layact.cxx
+++ b/sw/source/core/layout/layact.cxx
@@ -562,7 +562,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 9e1a49d52588..cd61a7c1f4d9 100644
--- a/sw/source/core/layout/pagechg.cxx
+++ b/sw/source/core/layout/pagechg.cxx
@@ -1045,7 +1045,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." );
@@ -1120,10 +1120,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.
@@ -1200,10 +1205,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;
}
}