ODFstudio/odfstudio
2
0
Fork 0
Code Issues Packages Activity

sc: make LAMBDA and callable evaluation more robust

Browse Source 
Follow-up hardening for the recently added first-class callables
(LAMBDA, LET and macros).

Bound the nesting depth of callable evaluation. A lambda body runs in a
fresh nested interpreter that does not pass through the per-cell
recursion counter, so a lambda that ends up calling itself could keep
nesting. Cap the depth at the same limit used for ordinary formula cell
evaluation and return an error once it is reached.

When cloning a lambda body, skip an empty or inverted token span and
copy only the tokens that are actually present, so no uninitialized slot
is ever handed to the new token array.

Cast a callable to its concrete type directly instead of doing a runtime
type check on every call, since the opcode already identifies the type.
Widen the parameter and argument counts from short.

Rewrite the copied macro comments in plain English and remove the
commented-out matrix callable code and its declaration.

Add tests for a terminating self-referential lambda and for a lambda
that would otherwise recurse without bound.

Signed-off-by: Tomaž Vajngerl <tomaz.vajngerl@collabora.co.uk>
Change-Id: Ib520c48b5fa726f883ba4595d61786b45eeedbf5
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/7114
Tested-by: Jenkins CPCI <releng@collaboraoffice.com>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
(cherry picked from commit 5b8c9b85c20257449907c8c69c0145ccdb017b03)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/208345
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
Tested-by: Jenkins
This commit is contained in:
Tomaž Vajngerl
2026-08-19 08:39:59 +02:00
committed by Xisco Fauli
parent bf23464799
commit cd0b10a6fb
7 changed files with 93 additions and 67 deletions
Download Patch File Download Diff File Expand all files Collapse all files
sc/inc/callable.hxx
+2 -2
View File
@@ -91,8 +91,8 @@ public:
ScFormulaCell* GetFormulaCell() const { return mpCell; }
ScInterpreterContext& GetContext() const { return *mpContext; }
const ScTokenArray& GetLambdaBody() const { return maLambdaBody; }
short GetNumParams() const { return maReplacementPositions.size(); }
const std::forward_list<short>& GetReplacementPositions(short nParam) const
sal_Int32 GetNumParams() const { return maReplacementPositions.size(); }
const std::forward_list<short>& GetReplacementPositions(sal_Int32 nParam) const
{
return maReplacementPositions[nParam];
}
sc/inc/document.hxx
+12
View File
@@ -527,6 +527,7 @@ private:
sal_uInt64 nXMLImportedFormulaCount; // progress count during XML import
sal_uInt16 nInterpretLevel; // >0 if in interpreter
sal_uInt16 nMacroInterpretLevel; // >0 if macro in interpreter
sal_uInt16 nCallableInterpretLevel = 0; // depth of nested callable (LAMBDA) evaluation
sal_uInt16 nInterpreterTableOpLevel; // >0 if in interpreter TableOp
ScDocumentThreadSpecific maNonThreaded;
@@ -2684,6 +2685,17 @@ public:
if ( nMacroInterpretLevel )
nMacroInterpretLevel--;
}
sal_uInt16 GetCallableInterpretLevel() const { return nCallableInterpretLevel; }
void IncCallableInterpretLevel()
{
if (nCallableInterpretLevel < USHRT_MAX)
nCallableInterpretLevel++;
}
void DecCallableInterpretLevel()
{
if (nCallableInterpretLevel)
nCallableInterpretLevel--;
}
bool IsInInterpreterTableOp() const { return nInterpreterTableOpLevel != 0; }
void IncInterpreterTableOpLevel()
{
sc/qa/unit/ucalc_formula2.cxx
+30
View File
@@ -274,6 +274,36 @@ CPPUNIT_TEST_FIXTURE(TestFormula2, testFuncLAMBDA)
m_pDoc->DeleteTab(0);
}
CPPUNIT_TEST_FIXTURE(TestFormula2, testLambdaBoundedRecursion)
{
// A lambda stored in a cell can call itself through a reference to that
// cell. A recursion that terminates returns its computed value.
sc::AutoCalcSwitch aACSwitch(*m_pDoc, true);
m_pDoc->InsertTab(0, u"Sheet1"_ustr);
// A1 adds n down to zero by recursing on itself: 5 + 4 + 3 + 2 + 1 + 0.
m_pDoc->SetString(ScAddress(0, 0, 0), u"=LAMBDA(n; IF(n <= 0; 0; n + A1(n - 1)))"_ustr);
m_pDoc->SetString(ScAddress(1, 0, 0), u"=A1(5)"_ustr);
CPPUNIT_ASSERT_EQUAL(15.0, m_pDoc->GetValue(ScAddress(1, 0, 0)));
m_pDoc->DeleteTab(0);
}
CPPUNIT_TEST_FIXTURE(TestFormula2, testLambdaRunawayRecursionIsCapped)
{
// A lambda that always calls itself would recurse without bound. The
// interpreter caps the nesting depth and returns error 514 once the limit
// is reached.
sc::AutoCalcSwitch aACSwitch(*m_pDoc, true);
m_pDoc->InsertTab(0, u"Sheet1"_ustr);
m_pDoc->SetString(ScAddress(0, 0, 0), u"=LAMBDA(n; 1 + A1(n))"_ustr);
m_pDoc->SetString(ScAddress(1, 0, 0), u"=A1(5)"_ustr);
CPPUNIT_ASSERT_EQUAL(u"Err:514"_ustr, m_pDoc->GetString(ScAddress(1, 0, 0)));
m_pDoc->DeleteTab(0);
}
CPPUNIT_TEST_FIXTURE(TestFormula2, testConcatWithOmittedOptionalParameter)
{
// Concatenating an omitted optional lambda parameter yields just the
sc/source/core/inc/interpre.hxx
-1
View File
@@ -536,7 +536,6 @@ private:
ScMatrixRef GetMatrix( short & rParam, size_t & rInRefList );
sc::RangeMatrix GetRangeMatrix();
formula::FormulaCallableRef GetCallable();
//formula::FormulaCallableRef GetCallableFromMatrix(const ScMatrixRef& pMat);
void ScTableOp(); // repeated operations
sc/source/core/tool/callable.cxx
+17 -13
View File
@@ -62,14 +62,11 @@ ScMacroFunction::ScMacroFunction(const ScInterpreter& rInterpreter, const OUStri
return;
}
// no security queue beforehand (just CheckMacroWarn), moved to CallBasic
// The macro security prompt is not raised here. CallBasic checks it when
// the macro actually runs.
// If the Dok was loaded during a Basic-Call,
// is the Sbx-object created(?)
// pDocSh->GetSbxObject();
// search function with the name,
// then assemble SfxObjectShell::CallBasic from aBasicStr, aMacroStr
// Find the Basic method by name, then keep the library and macro name
// strings that CallBasic needs to invoke it.
StarBASIC* pRoot;
@@ -126,17 +123,24 @@ static void lcl_CloneSubarray(const ScTokenArray& rTokensIn, short nStartPos, sh
ScTokenArray& rTokensOut)
{
const short nOffset = nStartPos + 1;
// An empty or inverted span has no body to clone, so return an empty output
// rather than computing a negative length.
if (nEndPos <= nOffset)
return;
const short nLen = nEndPos - nOffset;
formula::FormulaToken** pResult = new formula::FormulaToken*[nLen];
formula::FormulaToken** p = pResult;
// Start every slot empty so a run that clones fewer tokens than nLen never
// hands an uninitialized pointer to the new array.
std::vector<formula::FormulaToken*> aResult(nLen, nullptr);
short nCount = 0;
formula::FormulaTokenArrayPlainIterator aIter(rTokensIn);
aIter.Jump(nOffset);
for (const formula::FormulaToken* pToken = aIter.NextRPN();
aIter.GetIndex() <= nEndPos && pToken; pToken = aIter.NextRPN())
aIter.GetIndex() <= nEndPos && pToken && nCount < nLen; pToken = aIter.NextRPN())
{
formula::FormulaToken* pNewToken = pToken->Clone();
pNewToken->IncRef();
*(p++) = pNewToken;
aResult[nCount++] = pNewToken;
auto pJumpToken = dynamic_cast<formula::FormulaJumpToken*>(pNewToken);
if (pJumpToken)
{
@@ -147,8 +151,8 @@ static void lcl_CloneSubarray(const ScTokenArray& rTokensIn, short nStartPos, sh
pJump[nJump] -= nOffset;
}
}
rTokensOut.CreateNewRPNArrayFromData(pResult, nLen);
delete[] pResult;
// Hand over only the tokens actually cloned.
rTokensOut.CreateNewRPNArrayFromData(aResult.data(), nCount);
}
/// find all the locations where the specified variable would have to be substituted
sc/source/core/tool/interpr1.cxx
+29 -18
View File
@@ -11021,18 +11021,31 @@ void ScInterpreter::ScCall( FormulaCallableRef pCallable, const std::vector<Form
OpCode eOpCode = pCallable->GetOpCode();
if (eOpCode == ocLambda)
{
auto pLambda = dynamic_cast<const ScFormulaFunction*>(pCallable.get());
if ( !pLambda )
// How deeply lambda bodies may nest before a call is refused. Each level
// runs in a fresh nested interpreter, so a lambda that ends up calling
// itself needs a bound. It matches the recursion limit used for ordinary
// formula cell evaluation.
const sal_Int32 MAX_CALLABLE_INTERPRET_LEVEL = 400;
// Only ScFormulaFunction reports ocLambda, so cast directly instead of
// paying for a type check on every call.
assert(dynamic_cast<const ScFormulaFunction*>(pCallable.get()));
auto pLambda = static_cast<const ScFormulaFunction*>(pCallable.get());
// A lambda body runs in a fresh nested interpreter and does not pass
// through the per-cell recursion counter, so bound the nesting depth
// here. A formula that exceeds the limit yields an error.
ScDocument& rLambdaDoc = pLambda->GetDocument();
if (rLambdaDoc.GetCallableInterpretLevel() >= MAX_CALLABLE_INTERPRET_LEVEL)
{
SAL_WARN("sc", "Callable is ocLambda, but is not a ScFormulaFunction");
SetError(FormulaError::IllegalArgument);
SetError(FormulaError::StackOverflow);
PushError(nGlobalError);
return;
}
short nParamCount = pLambda->GetNumParams();
short nRequiredParamCount = pLambda->GetNumRequiredParams();
short nArgCount = aArguments.size();
sal_Int32 nParamCount = pLambda->GetNumParams();
sal_Int32 nRequiredParamCount = pLambda->GetNumRequiredParams();
sal_Int32 nArgCount = aArguments.size();
if (nArgCount > nParamCount || nArgCount < nRequiredParamCount)
{
// A call with the wrong number of arguments is a value error,
@@ -11045,7 +11058,7 @@ void ScInterpreter::ScCall( FormulaCallableRef pCallable, const std::vector<Form
// clone tokens of lambda-body for replacing string name tokens with arguments
ScTokenArray aNewTokens = pLambda->GetLambdaBody().CloneValue();
for (short nParam = 0; nParam < nParamCount; ++nParam)
for (sal_Int32 nParam = 0; nParam < nParamCount; ++nParam)
{
const std::forward_list<short>& rPositions = pLambda->GetReplacementPositions(nParam);
@@ -11071,14 +11084,16 @@ void ScInterpreter::ScCall( FormulaCallableRef pCallable, const std::vector<Form
}
// calculate the final result, in the stored context
ScInterpreter aInt(pLambda->GetFormulaCell(), pLambda->GetDocument(), pLambda->GetContext(),
ScInterpreter aInt(pLambda->GetFormulaCell(), rLambdaDoc, pLambda->GetContext(),
pLambda->GetAddress(), aNewTokens);
aInt.aCode.Lambda(true);
sfx2::LinkManager aNewLinkMgr(pLambda->GetDocument().GetDocumentShell());
sfx2::LinkManager aNewLinkMgr(rLambdaDoc.GetDocumentShell());
aInt.SetLinkManager(&aNewLinkMgr);
rLambdaDoc.IncCallableInterpretLevel();
formula::StackVar aResultType = aInt.Interpret();
rLambdaDoc.DecCallableInterpretLevel();
formula::FormulaConstTokenRef xLambdaResult( aInt.GetResultToken() );
if (aResultType == formula::svMatrixCell)
@@ -11098,14 +11113,10 @@ void ScInterpreter::ScCall( FormulaCallableRef pCallable, const std::vector<Form
#if !HAVE_FEATURE_SCRIPTING
PushNoValue(); // without DocShell no CallBasic
#else
auto pMacro = dynamic_cast<const ScMacroFunction*>(pCallable.get());
if ( !pMacro )
{
SAL_WARN("sc", "Callable is ocMacro, but is not a ScMacroFunction");
SetError(FormulaError::IllegalArgument);
PushError(nGlobalError);
return;
}
// Only ScMacroFunction reports ocMacro, so cast directly instead of
// paying for a type check on every call.
assert(dynamic_cast<const ScMacroFunction*>(pCallable.get()));
auto pMacro = static_cast<const ScMacroFunction*>(pCallable.get());
if (!pMacro->IsValid())
{
sc/source/core/tool/interpr4.cxx
+3 -33
View File
@@ -2720,17 +2720,9 @@ formula::FormulaCallableRef ScInterpreter::GetCallable()
return nullptr;
}
}
/* case svExternalDoubleRef:
{
ScMatrixRef pMat;
PopExternalDoubleRef(pMat);
return GetCallableFromMatrix(pMat); // TODO
}
case svMatrix:
{
ScMatrixRef pMat = PopMatrix();
return GetCallableFromMatrix(pMat); // TODO
}*/
// TODO: a matrix or external double reference cannot yet produce a
// callable, so such an operand falls through to the illegal-argument
// case below.
default:
{
PopError();
@@ -2740,28 +2732,6 @@ formula::FormulaCallableRef ScInterpreter::GetCallable()
}
}
/*formula::FormulaCallableRef ScInterpreter::GetCallableFromMatrix(const ScMatrixRef& pMat) // TODO
{
if ( !pMat )
; // nothing
else if ( !pJumpMatrix )
{
return pMat->GetCallable( mrContext, 0, 0);
}
else
{
SCSIZE nCols, nRows, nC, nR;
pMat->GetDimensions( nCols, nRows);
pJumpMatrix->GetPos( nC, nR);
// Use vector replication for single row/column arrays.
if ( (nC < nCols || nCols == 1) && (nR < nRows || nRows == 1) )
return pMat->GetCallable( mrContext, nC, nR);
SetError( FormulaError::NoValue);
}
return nullptr;
}*/
void ScInterpreter::ScDBGet()
{
bool bMissingField = false;