Writer problem with a dangling SwModify pointer
Németh László
nemeth at numbertext.org
Mon Jun 1 10:26:08 PDT 2015
Hi Stephan,
_ImplAdjustVertRelPos( ) function was for only limiting a numerical
value (vertical position of text frames) without any modification of
its arguments, but I extended it with a direct modification of a frame
attribute using this SetFlyFrmAttr call. I am afraid, this was a bad
idea, when _ImplAdjustVertRelPos() is called during a similar
modification of an attribute set. Perhaps moving the
SetFlyFrmAttr/modification to the calling code part will solve this
problem, I will check it. Thanks for your mail!
Best regards,
Laszlo
2015-06-01 17:39 GMT+02:00 Stephan Bergmann <sbergman at redhat.com>:
> I'm chasing a problem that my UBSan build originally pointed me at, but
> where I fail to make good progress. Maybe some Writer expert has a clue.
>
> I have broken it down to two pieces of information:
>
> For one, valgrind reports a dangling SwModify pointer toward the end of
> CppunitTest_sw_ooxmlexport,
>
>> Invalid read of size 1
>> at 0x20FD6659: SwModify::SetInCache(bool) (in
>> /home/sbergman/lo/core/instdir/program/libswlo.so)
>> by 0x215FECD7: SwBorderAttrs::~SwBorderAttrs()
>> (/sw/source/core/layout/frmtool.cxx:1872)
>> by 0x215FED48: SwBorderAttrs::~SwBorderAttrs()
>> (/sw/source/core/layout/frmtool.cxx:1871)
>> by 0x20FFE78B: SwCache::~SwCache()
>> (/sw/source/core/bastyp/swcache.cxx:123)
>> by 0x21640B21: _FrmFinit() (/sw/source/core/layout/newfrm.cxx:369)
>> by 0x20FF86CF: _FinitCore() (/sw/source/core/bastyp/init.cxx:750)
>> by 0x21EDEBC8: SwDLL::~SwDLL() (/sw/source/uibase/app/swdll.cxx:158)
>> by 0x21EE045A: std::default_delete<SwDLL>::operator()(SwDLL*) const
>> (/usr/lib/gcc/x86_64-redhat-linux/4.9.2/../../../../include/c++/4.9.2/bits/unique_ptr.h:76)
>> by 0x21EE0598: std::unique_ptr<SwDLL, std::default_delete<SwDLL>
>> >::reset(SwDLL*)
>> (/usr/lib/gcc/x86_64-redhat-linux/4.9.2/../../../../include/c++/4.9.2/bits/unique_ptr.h:344)
>> by 0x21EDF5EB: comphelper::unique_disposing_ptr<SwDLL>::reset(SwDLL*)
>> (/include/comphelper/unique_disposing_ptr.hxx:41)
>> by 0x21EDF02D:
>> comphelper::unique_disposing_solar_mutex_reset_ptr<SwDLL>::reset(SwDLL*)
>> (/include/comphelper/unique_disposing_ptr.hxx:152)
>> by 0x21EDFFA1:
>> comphelper::unique_disposing_ptr<SwDLL>::TerminateListener::disposing(com::sun::star::lang::EventObject
>> const&) (/include/comphelper/unique_disposing_ptr.hxx:118)
>> by 0x21EE01BB: non-virtual thunk to
>> comphelper::unique_disposing_ptr<SwDLL>::TerminateListener::disposing(com::sun::star::lang::EventObject
>> const&) (/include/comphelper/unique_disposing_ptr.hxx:102)
>> by 0xD4A4561:
>> cppu::OInterfaceContainerHelper::disposeAndClear(com::sun::star::lang::EventObject
>> const&) (/cppuhelper/source/interfacecontainer.cxx:312)
>> by 0xD4A54C3:
>> cppu::OMultiTypeInterfaceContainerHelper::disposeAndClear(com::sun::star::lang::EventObject
>> const&) (/cppuhelper/source/interfacecontainer.cxx:485)
>> by 0x2BB8A540: framework::Desktop::disposing()
>> (/framework/source/services/desktop.cxx:1051)
>> by 0xD49EB2E: cppu::WeakComponentImplHelperBase::dispose()
>> (/cppuhelper/source/implbase.cxx:109)
>> by 0x2BB8E710:
>> cppu::WeakComponentImplHelper6<com::sun::star::lang::XServiceInfo,
>> com::sun::star::frame::XDesktop2, com::sun::star::frame::XTasksSupplier,
>> com::sun::star::frame::XDispatchResultListener,
>> com::sun::star::task::XInteractionHandler,
>> com::sun::star::frame::XUntitledNumbers>::dispose() (in
>> /home/sbergman/lo/core/instdir/program/libfwklo.so)
>> by 0x2BB8E988: non-virtual thunk to
>> cppu::WeakComponentImplHelper6<com::sun::star::lang::XServiceInfo,
>> com::sun::star::frame::XDesktop2, com::sun::star::frame::XTasksSupplier,
>> com::sun::star::frame::XDispatchResultListener,
>> com::sun::star::task::XInteractionHandler,
>> com::sun::star::frame::XUntitledNumbers>::dispose()
>> (/include/cppuhelper/compbase6.hxx:59)
>> by 0xD4F3BAD: cppuhelper::ServiceManager::disposing()
>> (/cppuhelper/source/servicemanager.cxx:925)
>> by 0xD49EB2E: cppu::WeakComponentImplHelperBase::dispose()
>> (/cppuhelper/source/implbase.cxx:109)
>> by 0xD484A70:
>> cppu::WeakComponentImplHelper8<com::sun::star::lang::XServiceInfo,
>> com::sun::star::lang::XMultiServiceFactory,
>> com::sun::star::lang::XMultiComponentFactory,
>> com::sun::star::container::XSet,
>> com::sun::star::container::XContentEnumerationAccess,
>> com::sun::star::beans::XPropertySet,
>> com::sun::star::beans::XPropertySetInfo,
>> com::sun::star::lang::XEventListener>::dispose() (in
>> /home/sbergman/lo/core/instdir/program/libuno_cppuhelpergcc3.so.3)
>> by 0xD484C78: non-virtual thunk to
>> cppu::WeakComponentImplHelper8<com::sun::star::lang::XServiceInfo,
>> com::sun::star::lang::XMultiServiceFactory,
>> com::sun::star::lang::XMultiComponentFactory,
>> com::sun::star::container::XSet,
>> com::sun::star::container::XContentEnumerationAccess,
>> com::sun::star::beans::XPropertySet,
>> com::sun::star::beans::XPropertySetInfo,
>> com::sun::star::lang::XEventListener>::dispose()
>> (/include/cppuhelper/compbase8.hxx:59)
>> by 0xD468F85:
>> cppu::try_dispose(com::sun::star::uno::Reference<com::sun::star::uno::XInterface>
>> const&) (/cppuhelper/source/component_context.cxx:276)
>> by 0xD469A2E: cppu::ComponentContext::disposing()
>> (/cppuhelper/source/component_context.cxx:724)
>> by 0xD49EB2E: cppu::WeakComponentImplHelperBase::dispose()
>> (/cppuhelper/source/implbase.cxx:109)
>> by 0xD46F880:
>> cppu::WeakComponentImplHelper<com::sun::star::uno::XComponentContext,
>> com::sun::star::container::XNameContainer>::dispose() (in
>> /home/sbergman/lo/core/instdir/program/libuno_cppuhelpergcc3.so.3)
>> by 0xD46FA68: non-virtual thunk to
>> cppu::WeakComponentImplHelper<com::sun::star::uno::XComponentContext,
>> com::sun::star::container::XNameContainer>::dispose()
>> (/include/cppuhelper/compbase.hxx:82)
>> by 0x116D49D9: (anonymous namespace)::Protector::deinitHook((anonymous
>> namespace)::Protector*, void*) (/test/source/vclbootstrapprotector.cxx:81)
>> by 0x116D46C7: (anonymous
>> namespace)::Protector::LinkStubdeinitHook(void*, void*)
>> (/test/source/vclbootstrapprotector.cxx:66)
>> by 0x122B931F: Link<void*, long>::Call(void*) const (in
>> /home/sbergman/lo/core/instdir/program/libvcllo.so)
>> by 0x12A49634: DeInitVCL() (/vcl/source/app/svmain.cxx:456)
>> by 0x116D46F1: (anonymous namespace)::Protector::~Protector()
>> (/test/source/vclbootstrapprotector.cxx:51)
>> by 0x116D4748: (anonymous namespace)::Protector::~Protector()
>> (/test/source/vclbootstrapprotector.cxx:50)
>> by 0x4EBE750: CppUnit::ProtectorChain::pop()
>> (/workdir/UnpackedTarball/cppunit/src/cppunit/ProtectorChain.cpp:47)
>> by 0x4ED9068: CppUnit::TestResult::popProtector()
>> (/workdir/UnpackedTarball/cppunit/src/cppunit/TestResult.cpp:195)
>> by 0x405BFF: (anonymous namespace)::ProtectedFixtureFunctor::run()
>> const (/sal/cppunittester/cppunittester.cxx:285)
>> by 0x404F0F: sal_main() (/sal/cppunittester/cppunittester.cxx:379)
>> by 0x404846: main (/sal/cppunittester/cppunittester.cxx:297)
>
>
> but unfortunately the memory is dead long enough already that the point
> where it got deleted has already been lost from valgrind's database (it only
> reports "Address 0x311d9b28 is 16 bytes after a block of size 24 free'd" in
> a delete in basebmp::detail::CompositeIteratorBase<...>, so clearly
> unrelated).
>
> For another, bisecting shows that
> <http://cgit.freedesktop.org/libreoffice/core/commit/?id=a4dee94afed9ade6ac50237c8d99a6e49d3bebc1>
> "tdf#91260: allow textboxes extending beyond the page bottom" is what
> started the dangling pointer dereference to happen during
> CppunitTest_sw_ooxmlexport. Commenting out the call to SetFlyFrmAttr
> towards the end of that change's
>
>> diff --git a/sw/source/core/objectpositioning/anchoredobjectposition.cxx
>> b/sw/source/core/objectpositioning/anchoredobjectposition.cxx
>> index c7b916a..54bf5e0 100644
>> --- a/sw/source/core/objectpositioning/anchoredobjectposition.cxx
>> +++ b/sw/source/core/objectpositioning/anchoredobjectposition.cxx
>> @@ -471,8 +473,28 @@ SwTwips
>> SwAnchoredObjectPosition::_ImplAdjustVertRelPos( const SwTwips nTopOfAnc
>> {
>> nAdjustedRelPosY = aPgAlignArea.Top() - nTopOfAnch;
>> }
>> - }
>>
>> + // tdf#91260 - allow textboxes extending beyond the page bottom
>> + if ( nAdjustedRelPosY < nProposedRelPosY )
>> + {
>> + const SwFrmFmt* pFmt = &(GetFrmFmt());
>> + if ( SwTextBoxHelper::isTextBox(&GetObject()) )
>> + {
>> + // shrink textboxes to extend beyond the page bottom
>> + SwFrmFmt* pFrmFmt = ::FindFrmFmt(&GetObject());
>> + SfxItemSet aTextBoxSet(pFrmFmt->GetDoc()->GetAttrPool(),
>> aFrmFmtSetRange);
>> + SwFmtFrmSize aSize(pFmt->GetFrmSize());
>> + SwTwips nShrinked = aSize.GetHeight() - (nProposedRelPosY
>> - nAdjustedRelPosY);
>> + aSize.SetHeight( nShrinked > 0 ? nShrinked : 0 );
>> + aTextBoxSet.Put(aSize);
>> + if (aTextBoxSet.Count())
>> + pFrmFmt->GetDoc()->SetFlyFrmAttr(*pFrmFmt,
>> aTextBoxSet);
>> + nAdjustedRelPosY = nProposedRelPosY;
>> + } else if ( SwTextBoxHelper::findTextBox(pFmt) )
>> + // when the shape has a textbox, use only the proposed
>> vertical position
>> + nAdjustedRelPosY = nProposedRelPosY;
>> + }
>> + }
>> return nAdjustedRelPosY;
>> }
>>
>
> would make the dangling pointer dereference go away again, but it is unclear
> to me how that call to SetFlyFrmAttr affects the later ~SwCache clean-up.
>
> Ideas, anyone?
> _______________________________________________
> LibreOffice mailing list
> LibreOffice at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/libreoffice
More information about the LibreOffice
mailing list