[Libreoffice-commits] core.git: sw/source

[Stephan Bergmann]

 sw/source/core/layout/flylay.cxx |   40 +++++++++++++++++++++------------------
 1 file changed, 22 insertions(+), 18 deletions(-)

New commits:
commit 0eb52534de536fbe33585c91f4f173653b184e24
Author: Stephan Bergmann <sbergman at redhat.com>
Date:   Tue Jun 9 11:17:13 2015 +0200

    Unlock SwCacheObj before potentially deleting it from SwCache
    
    Running e.g. CppunitTest_sw_ooxmlexport under UBSan revealed problems with
    SwBorderAttrs instances with dangling pOwner pointers, which caused bad memory
    access during exit when ~SwCache calling ~SwBorderAttrs tried to use pOwner.
    
    The problem started with a4dee94afed9ade6ac50237c8d99a6e49d3bebc1 "tdf#91260:
    allow textboxes extending beyond the page bottom" (and hadn't changed with
    follow-up 2f779fc046c9afec04b4a4500b213e77aee51ae1 "tdf#91260: cleanup -
    textboxes extending beyond the page"):  The call to
    
      pFrameFormat->SetFormatAttr(aSize);
    
    in SwAnchoredObjectPosition::_ImplAdjustVertRelPos ultimately calls
    SwCache::DeleteObj on the SwBorderAttrs instance that is locked by the
    
      SwBorderAttrAccess aAccess( SwFrm::GetCache(), this );
    
    down the call stack in SwFlyFreeFrm::MakeAll.  That means that
    SwCache::DeleteObj will return early without doing anything (apart from
    triggering the OSL_ENSURE "SwCache::Delete: object is locked."), leading to this
    leftover SwBorderAttrs instance causing trouble during ~SwCache.
    
    The scope of aAccess in SwFlyFreeFrm::MakeAll had always extended well past the
    uses of rAttrs (= *aAccess.Get()), covering also the
    
      if ( !mbValidPos )
    
    block (that contains the call to MakeObjPos leading to the call of
    SwAnchoredObjectPosition::_ImplAdjustVertRelPos), ever since
    84a3db80b4fd66c6854b3135b5f69b61fd828e62 "initial import."
    
    With cb19042f4395c97d123a27c6960d5e30d666c010 "New feature: vertical alignment
    for text frames: Layout part," an additional use of rAttrs (in
    MakeContentPos( rAttrs )) had been added after the block calling MakeObjPos.
    
    The hope is that (1) it is OK to release aAccess earlier, after any (original)
    uses of rAttrs, but before the call to MakeObjPos; and (2) it is OK to just set
    up a second aAccess/rAttrs for the later added use of rAttrs in the call to
    MakeContentPos.  (That is, to punch a hole into the aAccess scope, so that
    ultimately SwCache::DeleteObj succeeds on a now-unlocked SwBorderAttrs.)
    
    Change-Id: I7cb9919b1c9d7c87464ac3a0fe1edfed5b46e122

diff --git a/sw/source/core/layout/flylay.cxx b/sw/source/core/layout/flylay.cxx
index a5b27b6..9b351c0 100644
--- a/sw/source/core/layout/flylay.cxx
+++ b/sw/source/core/layout/flylay.cxx
@@ -174,29 +174,33 @@ void SwFlyFreeFrm::MakeAll()
                 Format( &rAttrs );
                 bFormatHeightOnly = false;
             }
+        }
 
-            if ( !mbValidPos )
-            {
-                const Point aOldPos( (Frm().*fnRect->fnGetPos)() );
+        if ( !mbValidPos )
+        {
+            const Point aOldPos( (Frm().*fnRect->fnGetPos)() );
+            // #i26791# - use new method <MakeObjPos()>
+            // #i34753# - no positioning, if requested.
+            if ( IsNoMakePos() )
+                mbValidPos = true;
+            else
                 // #i26791# - use new method <MakeObjPos()>
-                // #i34753# - no positioning, if requested.
-                if ( IsNoMakePos() )
+                MakeObjPos();
+            if( aOldPos == (Frm().*fnRect->fnGetPos)() )
+            {
+                if( !mbValidPos && GetAnchorFrm()->IsInSct() &&
+                    !GetAnchorFrm()->FindSctFrm()->IsValid() )
                     mbValidPos = true;
-                else
-                    // #i26791# - use new method <MakeObjPos()>
-                    MakeObjPos();
-                if( aOldPos == (Frm().*fnRect->fnGetPos)() )
-                {
-                    if( !mbValidPos && GetAnchorFrm()->IsInSct() &&
-                        !GetAnchorFrm()->FindSctFrm()->IsValid() )
-                        mbValidPos = true;
-                }
-                else
-                    mbValidSize = false;
             }
+            else
+                mbValidSize = false;
+        }
 
-            if ( !m_bValidContentPos )
-                MakeContentPos( rAttrs );
+        if ( !m_bValidContentPos )
+        {
+            SwBorderAttrAccess aAccess( SwFrm::GetCache(), this );
+            const SwBorderAttrs &rAttrs = *aAccess.Get();
+            MakeContentPos( rAttrs );
         }
 
         if ( mbValidPos && mbValidSize )