diff options
author | Stephan Bergmann <sbergman@redhat.com> | 2015-06-09 11:17:13 +0200 |
---|---|---|
committer | Stephan Bergmann <sbergman@redhat.com> | 2015-06-09 11:18:21 +0200 |
commit | 0eb52534de536fbe33585c91f4f173653b184e24 (patch) | |
tree | 90596b8492272709bfe382416a4e1a915fb53884 /sw | |
parent | 2afe94dbfc85cbcde1399267379a466d527998a4 (diff) |
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
Diffstat (limited to 'sw')
-rw-r--r-- | sw/source/core/layout/flylay.cxx | 40 |
1 files changed, 22 insertions, 18 deletions
diff --git a/sw/source/core/layout/flylay.cxx b/sw/source/core/layout/flylay.cxx index a5b27b6f5725..9b351c0c0f96 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 ) |