From 0c2f501beb6dc06a124e92642f70f24f54a1aeab 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 342062 -> 342090 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 0b8b373a9de..d0bf069c963 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 9aaeeb96259..7a9fff3e930 100644 --- a/sc/source/core/inc/interpre.hxx +++ b/sc/source/core/inc/interpre.hxx @@ -235,6 +235,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(); @@ -1089,6 +1090,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 a7c0012a7ad01d5526685b212a991cea6bedd7f9..8a2e913a507e9d98b91f7c97413cdb5764e57efe 100644 GIT binary patch delta 61 zcmZ4YLFCj2k%kt=7N#xCDv$WWGfOgpOA?c_oikFCv$v}}VxGm0CfL63CG+-uFIfuw E01plscK`qY delta 46 zcmX^0L1f(rk%kt=7N#xCDvzcs++gIN{)vrMWP9i%=5ls0zy0M)=It+EvLyNe0MQ2% Awg3PC diff --git a/sc/source/core/tool/interpr3.cxx b/sc/source/core/tool/interpr3.cxx index 994d821f6d8..cd4e3efc98e 100644 --- a/sc/source/core/tool/interpr3.cxx +++ b/sc/source/core/tool/interpr3.cxx @@ -3478,7 +3478,7 @@ void ScInterpreter::ScPercentile( bool bInclusive ) GetNumberSequenceArray( 1, aArray, false ); if ( aArray.empty() || nGlobalError != FormulaError::NONE ) { - SetError( FormulaError::NoValue ); + PushNoValue(); return; } if ( bInclusive ) @@ -3501,7 +3501,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 347377ed0a9..80a0e3027bc 100644 --- a/sc/source/core/tool/interpr4.cxx +++ b/sc/source/core/tool/interpr4.cxx @@ -4023,7 +4023,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, pDok) + << " eOp: " << static_cast(eOp) + << " params: " << static_cast(pCur->GetParamCount()) + << " nStackBase: " << nStackBase << " sp: " << sp); + nStackBase = sp; + assert(!"underflow"); + } } switch( eOp ) -- 2.30.2