crashtesting: clone a matrix cell's result when it's not refcounted

A matrix formula cell holds the "top-left" result of its matrix in the
reference-counted xUpperLeft of ScMatrixFormulaCellToken.

CloneUpperLeftIfNecessary cloned that only for svDouble; other types
were kept by shared reference.

But threaded group calculation switches every token in the group's shared pCode
to RefCntPolicy::None while worker threads interpret it, on the basis that
interpreting only reads pCode, since:

commit 791a2489ee
Date:   Sun Mar 24 21:31:38 2024 +0000

    Related: tdf#160056 elide ref counting for initial tokens

A matrix formula whose result is a string constant from its own pCode
broke that: the pCode token was taken as the upper left during the None
window, so its IncRef was a no-op.

At teardown the matrix released it and freed a token the pCode array
still pointed at.

Where the uncounted reference is taken:

    #1 boost::intrusive_ptr<formula::FormulaToken const>::operator=
         boost/smart_ptr/intrusive_ptr.hpp:121
    #2 ScMatrixFormulaCellToken::Assign         sc/source/core/tool/token.cxx
    #3 ScFormulaResult::SetToken                sc/source/core/tool/formularesult.cxx:206
    #4 ScFormulaResult::Assign                  sc/source/core/tool/formularesult.cxx:171
    #5 ScFormulaCell::InterpretTail             sc/source/core/data/formulacell.cxx:2476
    #6 ScColumn::CalculateInThread              sc/source/core/data/column2.cxx:3266
    #7 ScTable::CalculateInColumnInThread       sc/source/core/data/table1.cxx:2715
    #8 ScDocument::CalculateInColumnInThread    sc/source/core/data/documen8.cxx:420
    #9 ...Executor::doWork()                    sc/source/core/data/formulacell.cxx:5437
    #10 comphelper::ThreadTask::exec

Clone the upper left when it is svDouble (as before) or not reference counted
(RefCntPolicy::None). The clone is a counted token the cell owns, so no
reference into pCode is taken during the threaded window. The non-threaded
path is unchanged.

Also assert in HandleStuffAfterParallelCalculation that a matrix result's
upper left does not alias the cell's own pCode.

Found with an ASan+UBSan build reloading crashtest document
forum-mso-en4-676909.

The original motivating tdf#160056 benchmark document never visits this
clone branch, so that case remains unchanged.

Signed-off-by: Caolán McNamara <caolan.mcnamara@collabora.com>
Change-Id: Ibd9e93aa9e59fabb99941a92a16aef247f7ae160
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/5638
Tested-by: Jenkins CPCI <releng@collaboraoffice.com>
Reviewed-by: Noel Grandin <noel.grandin@collabora.com>
(cherry picked from commit 2a70b7a75ce8768e077ea42175e59465d3adba31)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/208769
Tested-by: Jenkins
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>

sc/inc/token.hxx

@@ -366,7 +366,7 @@ public:
 private:
 
     /** xUpperLeft is modifiable through SetUpperLeftDouble(), so clone it
-        whenever an svDouble token is assigned to. */
+        whenever an svDouble token is assigned to or it is not reference counted. */
     void CloneUpperLeftIfNecessary();
 };
 

sc/source/core/data/formulacell.cxx

@@ -2680,6 +2680,28 @@ void ScFormulaCell::HandleStuffAfterParallelCalculation(ScInterpreter* pInterpre
 {
     aResult.HandleStuffAfterParallelCalculation();
 
+#if defined DBG_UTIL
+    // Threaded group calculation switches the group's pCode tokens to
+    // RefCntPolicy::None for the duration, on the basis that interpreting only
+    // reads pCode. If a matrix result's upper left ends up pointing back into
+    // pCode, a new reference into pCode was taken during that window; because
+    // the policy was None it went uncounted, and the shared token is freed too
+    // early at teardown. Flag that here.
+    if (pCode)
+    {
+        if (const ScMatrixFormulaCellToken* pMat = aResult.GetMatrixFormulaCellToken())
+        {
+            if (const formula::FormulaToken* pUpperLeft = pMat->GetUpperLeftToken().get())
+            {
+                formula::FormulaToken** pArray = pCode->GetArray();
+                for (sal_uInt16 i = 0, n = pCode->GetLen(); i < n; ++i)
+                    assert(pArray[i] != pUpperLeft
+                        && "matrix result upper left aliases a pCode token after threaded calculation");
+            }
+        }
+    }
+#endif
+
     if( !pCode->GetCodeLen() )
         return;
 

sc/source/core/tool/token.cxx

@@ -939,7 +939,13 @@ bool ScMatrixFormulaCellToken::operator==( const FormulaToken& r ) const
 
 void ScMatrixFormulaCellToken::CloneUpperLeftIfNecessary()
 {
-    if (xUpperLeft && xUpperLeft->GetType() == svDouble)
+    // Clone an svDouble because SetUpperLeftDouble changes it in place. Clone an
+    // uncounted token because storing it here would take no counted reference
+    // and leave the upper left dangling; this happens during threaded group
+    // calculation, when the group's pCode tokens are switched to
+    // RefCntPolicy::None. The clone is a normally reference-counted copy.
+    if (xUpperLeft && (xUpperLeft->GetType() == svDouble
+            || xUpperLeft->GetRefCntPolicy() == formula::RefCntPolicy::None))
         xUpperLeft = xUpperLeft->Clone();
 }