Writer problem with a dangling SwModify pointer
Stephan Bergmann
sbergman at redhat.com
Tue Jun 9 02:27:09 PDT 2015
On 06/01/2015 07:26 PM, Németh László wrote:
> _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!
I have now come up with a solution for this that at least makes
ASan+UBSan "make check" happy,
<http://cgit.freedesktop.org/libreoffice/core/commit/?id=0eb52534de536fbe33585c91f4f173653b184e24>
"Unlock SwCacheObj before potentially deleting it from SwCache."
László, Tamás, please verify that this does not negatively impact your
changes
<http://cgit.freedesktop.org/libreoffice/core/commit/?id=a4dee94afed9ade6ac50237c8d99a6e49d3bebc1>
"tdf#91260: allow textboxes extending beyond the page bottom" etc. and
<http://cgit.freedesktop.org/libreoffice/core/commit/?id=cb19042f4395c97d123a27c6960d5e30d666c010>
"New feature: vertical alignment for text frames: Layout part."
And I would not mind any Writer expert looking at that "The hope is..."
part, either.
> 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?
More information about the LibreOffice
mailing list