Re: Writer problem with a dangling SwModify pointer

Németh László <nemeth -AT- numbertext.org>
Tue, 9 Jun 2015 17:00:24 +0200

Hi Stephan,

I have checked Tamás and my changes, and both of them work well with
your fix. Many thanks for it and for the detailed commit message!

Best regards,
László


2015-06-09 11:27 GMT+02:00 Stephan Bergmann <sbergman@redhat.com>:

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@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?

Context

Writer problem with a dangling SwModify pointer · Stephan Bergmann
- Re: Writer problem with a dangling SwModify pointer · Németh László
  - Re: Writer problem with a dangling SwModify pointer · Stephan Bergmann
    - Re: Writer problem with a dangling SwModify pointer · Németh László

Privacy Policy | Impressum (Legal Info) | Copyright information: Unless otherwise specified, all text and images on this website are licensed under the Creative Commons Attribution-Share Alike 3.0 License. This does not include the source code of LibreOffice, which is licensed under the Mozilla Public License (MPLv2). "LibreOffice" and "The Document Foundation" are registered trademarks of their corresponding registered owners or are in actual use as trademarks in one or more countries. Their respective logos and icons are also subject to international copyright laws. Use thereof is explained in our trademark policy.