cid#1468270 Wrapper object use after free

I think this is a better reflection of the original intent here before

commit 1441ab9c75a2f0ac664983db22b681a1b602f8a9
fix possible SIGSEGV

and

commit 8f54136caa786523fd224f6c98fc8e7c45cd805d
use std::unique_ptr for SalLayout

Change-Id: Ib4ab63334e644a8136b9f7da20916715850563ff
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/107171
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolanm@redhat.com>

3 files changed, 20 insertions, 6 deletions

diff --git a/vcl/inc/sallayout.hxx b/vcl/inc/sallayout.hxx
index bf93c0f64028..30fd5580b4c5 100644
--- a/vcl/inc/sallayout.hxx
+++ b/vcl/inc/sallayout.hxx
@@ -138,6 +138,8 @@ public:
     // used only by OutputDevice::ImplLayout, TODO: make friend
     explicit        MultiSalLayout( std::unique_ptr<SalLayout> pBaseLayout );
     void            AddFallback(std::unique_ptr<SalLayout> pFallbackLayout, ImplLayoutRuns const &);
+    // give up ownership of the initial pBaseLayout taken by the ctor
+    std::unique_ptr<SalLayout>  ReleaseBaseLayout();
     bool            LayoutText(ImplLayoutArgs&, const SalLayoutGlyphs*) override;
     void            AdjustLayout(ImplLayoutArgs&) override;
     void            InitFont() const override;
diff --git a/vcl/source/gdi/sallayout.cxx b/vcl/source/gdi/sallayout.cxx
index 7beea9a01091..eaa03d22e5f0 100644
--- a/vcl/source/gdi/sallayout.cxx
+++ b/vcl/source/gdi/sallayout.cxx
@@ -996,6 +996,11 @@ MultiSalLayout::MultiSalLayout( std::unique_ptr<SalLayout> pBaseLayout )
     mnUnitsPerPixel = mpLayouts[ 0 ]->GetUnitsPerPixel();
 }
 
+std::unique_ptr<SalLayout> MultiSalLayout::ReleaseBaseLayout()
+{
+    return std::move(mpLayouts[0]);
+}
+
 void MultiSalLayout::SetIncomplete(bool bIncomplete)
 {
     mbIncomplete = bIncomplete;
diff --git a/vcl/source/outdev/font.cxx b/vcl/source/outdev/font.cxx
index 6963fd77c3e1..2cd38a58ba11 100644
--- a/vcl/source/outdev/font.cxx
+++ b/vcl/source/outdev/font.cxx
@@ -1246,9 +1246,6 @@ std::unique_ptr<SalLayout> OutputDevice::ImplGlyphFallbackLayout( std::unique_pt
         return nullptr;
     }
 
-    // keep a pointer to the layout because we might move ownership of the unique_ptr
-    const SalLayout* pSalLayoutTmp = pSalLayout.get();
-
     // prepare multi level glyph fallback
     std::unique_ptr<MultiSalLayout> pMultiSalLayout;
     ImplLayoutRuns aLayoutRuns = rLayoutArgs.maRuns;
@@ -1306,11 +1303,21 @@ std::unique_ptr<SalLayout> OutputDevice::ImplGlyphFallbackLayout( std::unique_pt
             break;
     }
 
-    if( pMultiSalLayout && pMultiSalLayout->LayoutText( rLayoutArgs, nullptr ) )
-        pSalLayout = std::move(pMultiSalLayout);
+    if (pMultiSalLayout) // due to missing glyphs, multilevel layout fallback attempted
+    {
+        // if it works, use that Layout
+        if (pMultiSalLayout->LayoutText(rLayoutArgs, nullptr))
+            pSalLayout = std::move(pMultiSalLayout);
+        else
+        {
+            // if it doesn't, give up and restore ownership of the pSalLayout
+            // back to its original state
+            pSalLayout = pMultiSalLayout->ReleaseBaseLayout();
+        }
+    }
 
     // restore orig font settings
-    pSalLayoutTmp->InitFont();
+    pSalLayout->InitFont();
     rLayoutArgs.maRuns = aLayoutRuns;
 
     return pSalLayout;