From 3a54a49b83d70e43a44066a12fb5e06f2307a35e Mon Sep 17 00:00:00 2001 From: Eike Rathke Date: Thu, 16 Feb 2023 20:20:31 +0100 Subject: [PATCH] [PATCH] Obtain actual 0-parameter count for OR(), AND() and 1-parameter functions MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit OR and AND for legacy infix notation are classified as binary operators but in fact are functions with parameter count. In case no argument is supplied, GetByte() returns 0 and for that case the implicit binary operator 2 parameters were wrongly assumed. Similar for functions expecting 1 parameter, without argument 1 was assumed. For "real" unary and binary operators the compiler already checks parameters. Omit OR and AND and 1-parameter functions from this implicit assumption and return the actual 0 count. Change-Id: Ie05398c112a98021ac2875cf7b6de994aee9d882 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/147173 Reviewed-by: Eike Rathke Tested-by: Jenkins (cherry picked from commit e7ce9bddadb2db222eaa5f594ef1de2e36d57e5c) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/147129 Reviewed-by: Caolán McNamara (cherry picked from commit d6599a2af131994487d2d9223a4fd32a8c3ddc49) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/147132 Reviewed-by: Xisco Fauli Tested-by: Caolán McNamara Gbp-Pq: Name sc-stack-parameter-count.diff --- formula/source/core/api/token.cxx | 13 +++++-------- sc/source/core/inc/interpre.hxx | 12 ++++++++++++ sc/source/core/tool/interpr1.cxx | Bin 340592 -> 340620 bytes sc/source/core/tool/interpr3.cxx | 4 ++-- sc/source/core/tool/interpr4.cxx | 10 +++++++++- 5 files changed, 28 insertions(+), 11 deletions(-) diff --git a/formula/source/core/api/token.cxx b/formula/source/core/api/token.cxx index 9a22d9cd220..f194a4f7492 100644 --- a/formula/source/core/api/token.cxx +++ b/formula/source/core/api/token.cxx @@ -93,17 +93,14 @@ sal_uInt8 FormulaToken::GetParamCount() const return 0; // parameters and specials // ocIf... jump commands not for FAP, have cByte then //2do: bool parameter whether FAP or not? - else if ( GetByte() ) + else if (GetByte()) return GetByte(); // all functions, also ocExternal and ocMacro - else if (SC_OPCODE_START_BIN_OP <= eOp && eOp < SC_OPCODE_STOP_BIN_OP) - return 2; // binary - else if ((SC_OPCODE_START_UN_OP <= eOp && eOp < SC_OPCODE_STOP_UN_OP) - || eOp == ocPercentSign) - return 1; // unary + else if (SC_OPCODE_START_BIN_OP <= eOp && eOp < SC_OPCODE_STOP_BIN_OP && eOp != ocAnd && eOp != ocOr) + return 2; // binary operators, compiler checked; OR and AND legacy but are functions + else if ((SC_OPCODE_START_UN_OP <= eOp && eOp < SC_OPCODE_STOP_UN_OP) || eOp == ocPercentSign) + return 1; // unary operators, compiler checked else if (SC_OPCODE_START_NO_PAR <= eOp && eOp < SC_OPCODE_STOP_NO_PAR) return 0; // no parameter - else if (SC_OPCODE_START_1_PAR <= eOp && eOp < SC_OPCODE_STOP_1_PAR) - return 1; // one parameter else if (FormulaCompiler::IsOpCodeJumpCommand( eOp )) return 1; // only the condition counts as parameter else diff --git a/sc/source/core/inc/interpre.hxx b/sc/source/core/inc/interpre.hxx index 30e79d9272b..c1fc5f458dc 100644 --- a/sc/source/core/inc/interpre.hxx +++ b/sc/source/core/inc/interpre.hxx @@ -221,6 +221,7 @@ private: inline bool MustHaveParamCount( short nAct, short nMust ); inline bool MustHaveParamCount( short nAct, short nMust, short nMax ); inline bool MustHaveParamCountMin( short nAct, short nMin ); + inline bool MustHaveParamCountMinWithStackCheck( short nAct, short nMin ); void PushParameterExpected(); void PushIllegalParameter(); void PushIllegalArgument(); @@ -1074,6 +1075,17 @@ inline bool ScInterpreter::MustHaveParamCountMin( short nAct, short nMin ) return false; } +inline bool ScInterpreter::MustHaveParamCountMinWithStackCheck( short nAct, short nMin ) +{ + assert(sp >= nAct); + if (sp < nAct) + { + PushParameterExpected(); + return false; + } + return MustHaveParamCountMin( nAct, nMin); +} + inline bool ScInterpreter::CheckStringPositionArgument( double & fVal ) { if (!std::isfinite( fVal)) diff --git a/sc/source/core/tool/interpr1.cxx b/sc/source/core/tool/interpr1.cxx index e09620605d353178aabd8a3472cad244b0be3531..eb77bf07e51468ec2d2c2d06d39eda55be4570de 100644 GIT binary patch delta 84 zcmezHM5O1bNJ9%_3)2>6>8E_*nI##)C5g$|&Kar6+1sU`GIOv?qY7#$~M7Pj`_H_Y3E->}HK0RV40AWi@P delta 41 ucmeBqD)QlpNJ9%_3)2>6>8I1}ZZPt1w|L6@lMT#l_j|*<-R}*Hof`mqfDt_a diff --git a/sc/source/core/tool/interpr3.cxx b/sc/source/core/tool/interpr3.cxx index c0ac25b257e..77c767f8828 100644 --- a/sc/source/core/tool/interpr3.cxx +++ b/sc/source/core/tool/interpr3.cxx @@ -3465,7 +3465,7 @@ void ScInterpreter::ScPercentile( bool bInclusive ) GetNumberSequenceArray( 1, aArray, false ); if ( aArray.empty() || nGlobalError != FormulaError::NONE ) { - SetError( FormulaError::NoValue ); + PushNoValue(); return; } if ( bInclusive ) @@ -3488,7 +3488,7 @@ void ScInterpreter::ScQuartile( bool bInclusive ) GetNumberSequenceArray( 1, aArray, false ); if ( aArray.empty() || nGlobalError != FormulaError::NONE ) { - SetError( FormulaError::NoValue ); + PushNoValue(); return; } if ( bInclusive ) diff --git a/sc/source/core/tool/interpr4.cxx b/sc/source/core/tool/interpr4.cxx index 0895c75b5cc..120aea5d3e9 100644 --- a/sc/source/core/tool/interpr4.cxx +++ b/sc/source/core/tool/interpr4.cxx @@ -4034,7 +4034,15 @@ StackVar ScInterpreter::Interpret() else if (sp >= pCur->GetParamCount()) nStackBase = sp - pCur->GetParamCount(); else - nStackBase = sp; // underflow?!? + { + SAL_WARN("sc.core", "Stack anomaly at " << aPos.Format( + ScRefFlags::VALID | ScRefFlags::FORCE_DOC | ScRefFlags::TAB_3D, &mrDoc) + << " eOp: " << static_cast(eOp) + << " params: " << static_cast(pCur->GetParamCount()) + << " nStackBase: " << nStackBase << " sp: " << sp); + nStackBase = sp; + assert(!"underflow"); + } } switch( eOp ) -- 2.30.2