sc/inc/formulacell.hxx | 3 +++
sc/source/core/data/conditio.cxx | 4 ++++
sc/source/core/data/formulacell.cxx | 32 ++++++++++++++++++++++++++++++--
3 files changed, 37 insertions(+), 2 deletions(-)
New commits:
commit abc68a66cc19f8f9259cad124f67bfe325d6cb8f
Author: Eike Rathke <er...@redhat.com>
AuthorDate: Tue Oct 5 20:04:19 2021 +0200
Commit: Michael Stahl <michael.st...@allotropia.de>
CommitDate: Wed Oct 6 12:04:10 2021 +0200
Fix crash if conditional format triggers recursion with iterations enabled
Could occur if a conditional format is evaluated (for example if
row height is to be obtained) while a formula cell it references
is still running and iterations are enabled so the conditional
format's temporary formula cell is added to the iteration
recursion list but iterations are not triggered if there are no
circular references. In that case the temporary formula cell's
pointer remained in the recursion list and it's dangling instance
was accessed in the next round. Mark such formula cell as
free-flying and remove from recursion list if it was added.
Observed at
https://ask.libreoffice.org/t/lo-calc-crashes-when-calling-a-macro/68800
with the original attached file that now got replaced with another
version that doesn't have iterations enabled so wouldn't trigger
the bug (and apparently even doesn't if enabling iterations).
Change-Id: I23a023356f920b8413874cab14acdc8b25580052
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/123115
Reviewed-by: Eike Rathke <er...@redhat.com>
Tested-by: Jenkins
(cherry picked from commit ce8a7278e1304f7aaa65bce34aeeda5e83b231f1)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122949
Reviewed-by: Michael Stahl <michael.st...@allotropia.de>
diff --git a/sc/inc/formulacell.hxx b/sc/inc/formulacell.hxx
index 002d5e7c2e29..0b5b51921e14 100644
--- a/sc/inc/formulacell.hxx
+++ b/sc/inc/formulacell.hxx
@@ -124,6 +124,7 @@ private:
bool mbPostponedDirty : 1; // if cell needs to be set dirty
later
bool mbIsExtRef : 1; // has references in
ScExternalRefManager; never cleared after set
bool mbSeenInPath : 1; // For detecting cycle involving
formula groups and singleton formulacells
+ bool mbFreeFlying : 1; // Cell is out of sheets
interpreted, like in conditional format
ScMatrixMode cMatrixFlag : 8;
sal_uInt16 nSeenInIteration : 16; // Iteration cycle in which the
cell was last encountered
SvNumFormatType nFormatType : 16;
@@ -209,6 +210,8 @@ public:
ScFormulaCell(const ScFormulaCell& rCell, ScDocument& rDoc, const
ScAddress& rPos, ScCloneFlags nCloneFlags = ScCloneFlags::Default);
+ void SetFreeFlying( bool b ) { mbFreeFlying = b; }
+
size_t GetHash() const;
void GetFormula( OUString& rFormula,
diff --git a/sc/source/core/data/conditio.cxx b/sc/source/core/data/conditio.cxx
index 11e74fa258db..35a6ff99c699 100644
--- a/sc/source/core/data/conditio.cxx
+++ b/sc/source/core/data/conditio.cxx
@@ -400,6 +400,7 @@ void ScConditionEntry::MakeCells( const ScAddress& rPos )
// pFCell1 will hold a flat-copied ScTokenArray sharing ref-counted
// code tokens with pFormula1
pFCell1.reset( new ScFormulaCell(*mpDoc, rPos, *pFormula1) );
+ pFCell1->SetFreeFlying(true);
pFCell1->StartListeningTo( *mpDoc );
}
@@ -408,6 +409,7 @@ void ScConditionEntry::MakeCells( const ScAddress& rPos )
// pFCell2 will hold a flat-copied ScTokenArray sharing ref-counted
// code tokens with pFormula2
pFCell2.reset( new ScFormulaCell(*mpDoc, rPos, *pFormula2) );
+ pFCell2->SetFreeFlying(true);
pFCell2->StartListeningTo( *mpDoc );
}
}
@@ -652,6 +654,7 @@ void ScConditionEntry::Interpret( const ScAddress& rPos )
{
pTemp1.reset(pFormula1 ? new ScFormulaCell(*mpDoc, rPos, *pFormula1) :
new ScFormulaCell(*mpDoc, rPos));
pEff1 = pTemp1.get();
+ pEff1->SetFreeFlying(true);
}
if ( pEff1 )
{
@@ -682,6 +685,7 @@ void ScConditionEntry::Interpret( const ScAddress& rPos )
{
pTemp2.reset(pFormula2 ? new ScFormulaCell(*mpDoc, rPos, *pFormula2) :
new ScFormulaCell(*mpDoc, rPos));
pEff2 = pTemp2.get();
+ pEff2->SetFreeFlying(true);
}
if ( pEff2 )
{
diff --git a/sc/source/core/data/formulacell.cxx
b/sc/source/core/data/formulacell.cxx
index 3eef76bf3550..0b8dfd9c6f9d 100644
--- a/sc/source/core/data/formulacell.cxx
+++ b/sc/source/core/data/formulacell.cxx
@@ -616,6 +616,7 @@ ScFormulaCell::ScFormulaCell( ScDocument& rDoc, const
ScAddress& rPos ) :
mbPostponedDirty(false),
mbIsExtRef(false),
mbSeenInPath(false),
+ mbFreeFlying(false),
cMatrixFlag(ScMatrixMode::NONE),
nSeenInIteration(0),
nFormatType(SvNumFormatType::NUMBER),
@@ -648,6 +649,7 @@ ScFormulaCell::ScFormulaCell( ScDocument& rDoc, const
ScAddress& rPos,
mbPostponedDirty(false),
mbIsExtRef(false),
mbSeenInPath(false),
+ mbFreeFlying(false),
cMatrixFlag ( cMatInd ),
nSeenInIteration(0),
nFormatType ( SvNumFormatType::NUMBER ),
@@ -683,6 +685,7 @@ ScFormulaCell::ScFormulaCell(
mbPostponedDirty(false),
mbIsExtRef(false),
mbSeenInPath(false),
+ mbFreeFlying(false),
cMatrixFlag ( cMatInd ),
nSeenInIteration(0),
nFormatType ( SvNumFormatType::NUMBER ),
@@ -735,6 +738,7 @@ ScFormulaCell::ScFormulaCell(
mbPostponedDirty(false),
mbIsExtRef(false),
mbSeenInPath(false),
+ mbFreeFlying(false),
cMatrixFlag ( cMatInd ),
nSeenInIteration(0),
nFormatType ( SvNumFormatType::NUMBER ),
@@ -784,6 +788,7 @@ ScFormulaCell::ScFormulaCell(
mbPostponedDirty(false),
mbIsExtRef(false),
mbSeenInPath(false),
+ mbFreeFlying(false),
cMatrixFlag ( cInd ),
nSeenInIteration(0),
nFormatType(xGroup->mnFormatType),
@@ -816,6 +821,7 @@ ScFormulaCell::ScFormulaCell(const ScFormulaCell& rCell,
ScDocument& rDoc, const
mbPostponedDirty(false),
mbIsExtRef(false),
mbSeenInPath(false),
+ mbFreeFlying(false),
cMatrixFlag ( rCell.cMatrixFlag ),
nSeenInIteration(0),
nFormatType( rCell.nFormatType ),
@@ -1645,9 +1651,12 @@ bool ScFormulaCell::Interpret(SCROW nStartOffset, SCROW
nEndOffset)
// recursion list in reverse order.
if (rRecursionHelper.IsInReturn())
{
- if (rRecursionHelper.GetRecursionCount() > 0 ||
- !rRecursionHelper.IsDoingRecursion())
+ bool bFreeFlyingInserted = false;
+ if (rRecursionHelper.GetRecursionCount() > 0 ||
!rRecursionHelper.IsDoingRecursion())
+ {
rRecursionHelper.Insert( this, bOldRunning, aResult);
+ bFreeFlyingInserted = mbFreeFlying;
+ }
bool bIterationFromRecursion = false;
bool bResumeIteration = false;
do
@@ -1867,6 +1876,25 @@ bool ScFormulaCell::Interpret(SCROW nStartOffset, SCROW
nEndOffset)
rRecursionHelper.Clear();
}
} while (bIterationFromRecursion || bResumeIteration);
+
+ if (bFreeFlyingInserted)
+ {
+ // Remove this from recursion list, it may get deleted.
+ // It additionally also should mean that the recursion/iteration
+ // ends here as it must had been triggered by this free-flying
+ // out-of-sheets cell
+ /* TODO: replace by a simple rRecursionHelper.EndIteration() call
+ * if the assertions hold. */
+ const bool bOnlyThis = (rRecursionHelper.GetList().size() == 1);
+ assert(bOnlyThis);
+ rRecursionHelper.GetList().remove_if([this](const
ScFormulaRecursionEntry& r){return r.pCell == this;});
+ if (bOnlyThis)
+ {
+ assert(rRecursionHelper.GetList().empty());
+ if (rRecursionHelper.GetList().empty())
+ rRecursionHelper.EndIteration();
+ }
+ }
}
#if DEBUG_CALCULATION
