[Libreoffice-commits] core.git: sc/inc sc/source
Dennis Francis
dennis.francis at collabora.co.uk
Thu May 24 05:27:23 UTC 2018
sc/inc/formulacell.hxx | 1
sc/inc/recursionhelper.hxx | 8 ++++++
sc/source/core/data/formulacell.cxx | 23 ++++++++++++++++++-
sc/source/core/tool/recursionhelper.cxx | 37 ++++++++++++++++++++++++++++++++
4 files changed, 67 insertions(+), 2 deletions(-)
New commits:
commit 25cc0ab3b5d154fffbef27fad4adcf90f36ae92e
Author: Dennis Francis <dennis.francis at collabora.co.uk>
Date: Tue May 15 16:44:15 2018 +0530
Calc threading : Check for "self" references...
...on indirect dependencies too.
Here a self reference to any formula-group
means if there are any references in a formula
(of the formula-group itself or any of its dependencies)
that points to any element inside the formula-group.
If there are any self-references, then that formula-group
can't be computed in parallel.
For example, with this patch we can detect the following case:-
Suppose the formula-group that we want to check is:
"=(F2+G2-10)*10.0" spanning A2:A100. Let the formula-group
starting at F2 be "=A1*0.1-10". The indirect dependency
formula-group starting at F2, references back the elements of
our original formula-group at A2. This makes the F.G at
A2 unsafe for parallel computation.
Concretly, this patch fixes a recalc crash on tdf#63638/1
Change-Id: I7b999a34571b191d2f70da6a3831f78b24a6b0a7
Reviewed-on: https://gerrit.libreoffice.org/54433
Reviewed-by: Michael Meeks <michael.meeks at collabora.com>
Tested-by: Jenkins <ci at libreoffice.org>
diff --git a/sc/inc/formulacell.hxx b/sc/inc/formulacell.hxx
index 7656e7a3efec..cee9fec13fc2 100644
--- a/sc/inc/formulacell.hxx
+++ b/sc/inc/formulacell.hxx
@@ -66,6 +66,7 @@ public:
SvNumFormatType mnFormatType;
bool mbInvariant:1;
bool mbSubTotal:1;
+ bool mbSeenInPath:1; // For detecting cycle of formula groups
sal_uInt8 meCalcState;
diff --git a/sc/inc/recursionhelper.hxx b/sc/inc/recursionhelper.hxx
index 560b66ab357e..b8ca1d087509 100644
--- a/sc/inc/recursionhelper.hxx
+++ b/sc/inc/recursionhelper.hxx
@@ -23,9 +23,11 @@
#include "formularesult.hxx"
#include <list>
+#include <vector>
#include <stack>
class ScFormulaCell;
+struct ScFormulaCellGroup;
struct ScFormulaRecursionEntry
{
@@ -44,10 +46,12 @@ typedef ::std::list< ScFormulaRecursionEntry > ScFormulaRecursionList;
class ScRecursionHelper
{
typedef ::std::stack< ScFormulaCell* > ScRecursionInIterationStack;
+ typedef ::std::vector< ScFormulaCellGroup* > ScFGList;
ScFormulaRecursionList aRecursionFormulas;
ScFormulaRecursionList::iterator aInsertPos;
ScFormulaRecursionList::iterator aLastIterationStart;
ScRecursionInIterationStack aRecursionInIterationStack;
+ ScFGList aFGList;
sal_uInt16 nRecursionCount;
sal_uInt16 nIteration;
bool bInRecursionReturn;
@@ -94,6 +98,10 @@ public:
ScRecursionInIterationStack& GetRecursionInIterationStack() { return aRecursionInIterationStack; }
void Clear();
+
+ /** Detects whether the formula-group is part of a simple cycle of formula-groups. */
+ bool PushFormulaGroup(ScFormulaCellGroup* pGrp);
+ void PopFormulaGroup();
};
#endif // INCLUDED_SC_INC_RECURSIONHELPER_HXX
diff --git a/sc/source/core/data/formulacell.cxx b/sc/source/core/data/formulacell.cxx
index 9731f8a6f297..daaa80e5b921 100644
--- a/sc/source/core/data/formulacell.cxx
+++ b/sc/source/core/data/formulacell.cxx
@@ -528,6 +528,7 @@ ScFormulaCellGroup::ScFormulaCellGroup() :
mnFormatType(SvNumFormatType::NUMBER),
mbInvariant(false),
mbSubTotal(false),
+ mbSeenInPath(false),
meCalcState(sc::GroupCalcEnabled)
{
}
@@ -1527,6 +1528,14 @@ void ScFormulaCell::Interpret()
else
{
pDocument->IncInterpretLevel();
+
+ bool bCheckForFGCycle = mxGroup && !pDocument->mbThreadedGroupCalcInProgress &&
+ mxGroup->meCalcState == sc::GroupCalcEnabled &&
+ ScCalcConfig::isThreadingEnabled();
+ bool bPopFormulaGroup = false;
+ if (bCheckForFGCycle)
+ bPopFormulaGroup = rRecursionHelper.PushFormulaGroup(mxGroup.get());
+
#if DEBUG_CALCULATION
aDC.enterGroup();
bool bGroupInterpreted = InterpretFormulaGroup();
@@ -1537,6 +1546,10 @@ void ScFormulaCell::Interpret()
if (!InterpretFormulaGroup())
InterpretTail( pDocument->GetNonThreadedContext(), SCITP_NORMAL);
#endif
+
+ if (bPopFormulaGroup)
+ rRecursionHelper.PopFormulaGroup();
+
pDocument->DecInterpretLevel();
}
@@ -4129,14 +4142,14 @@ struct ScDependantsCalculator
SCROW nEndRow = mrPos.Row() + mnLen - 1;
- if (nRelRow < 0)
+ if (nRelRow <= 0)
{
SCROW nTest = nEndRow;
nTest += nRelRow;
if (nTest >= mrPos.Row())
return true;
}
- else if (nRelRow > 0)
+ else
{
SCROW nTest = mrPos.Row(); // top row.
nTest += nRelRow;
@@ -4406,6 +4419,12 @@ bool ScFormulaCell::InterpretFormulaGroup()
return false;
}
+ if (mxGroup->meCalcState == sc::GroupCalcDisabled)
+ {
+ aScope.addMessage("found circular formula-group dependencies");
+ return false;
+ }
+
static bool bHyperThreadingActive = tools::cpuid::hasHyperThreading();
// Then do the threaded calculation
diff --git a/sc/source/core/tool/recursionhelper.cxx b/sc/source/core/tool/recursionhelper.cxx
index 25591f41e823..de75e8557605 100644
--- a/sc/source/core/tool/recursionhelper.cxx
+++ b/sc/source/core/tool/recursionhelper.cxx
@@ -8,6 +8,7 @@
*/
#include <recursionhelper.hxx>
+#include <formulacell.hxx>
void ScRecursionHelper::Init()
{
@@ -15,6 +16,7 @@ void ScRecursionHelper::Init()
bInRecursionReturn = bDoingRecursion = bInIterationReturn = false;
aInsertPos = GetIterationEnd();
ResetIteration();
+ aFGList.clear();
}
void ScRecursionHelper::ResetIteration()
@@ -94,4 +96,39 @@ void ScRecursionHelper::Clear()
Init();
}
+bool ScRecursionHelper::PushFormulaGroup(ScFormulaCellGroup* pGrp)
+{
+ assert(pGrp);
+ if (pGrp->mbSeenInPath)
+ {
+ // Found a simple cycle of formula-groups.
+ // Disable group calc for all elements of this cycle.
+ sal_Int32 nIdx = aFGList.size();
+ assert(nIdx > 0);
+ do
+ {
+ --nIdx;
+ assert(nIdx >= 0);
+ auto& eCalcState = aFGList[nIdx]->meCalcState;
+ if (eCalcState == sc::GroupCalcEnabled)
+ eCalcState = sc::GroupCalcDisabled;
+ } while (aFGList[nIdx] != pGrp);
+
+ return false;
+ }
+
+ pGrp->mbSeenInPath = true;
+ aFGList.push_back(pGrp);
+ return true;
+}
+
+void ScRecursionHelper::PopFormulaGroup()
+{
+ if (aFGList.empty())
+ return;
+ ScFormulaCellGroup* pGrp = aFGList.back();
+ pGrp->mbSeenInPath = false;
+ aFGList.pop_back();
+}
+
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
More information about the Libreoffice-commits
mailing list