[PATCH] Obtain actual 0-parameter count for OR(), AND() and 1-parameter functions

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 <erack@redhat.com>
Tested-by: Jenkins
(cherry picked from commit e7ce9bddadb2db222eaa5f594ef1de2e36d57e5c)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/147129
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
(cherry picked from commit d6599a2af131994487d2d9223a4fd32a8c3ddc49)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/147132
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
Tested-by: Caolán McNamara <caolanm@redhat.com>
Gbp-Pq: Name sc-stack-parameter-count.diff

diff --git a/formula/source/core/api/token.cxx b/formula/source/core/api/token.cxx
index 0b8b373a9de3443bdf7d099cb7510b5c8dd18a41..d0bf069c9631b4836f724b5674eccbfa412d096e 100644 (file)
--- 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 9aaeeb96259c9b68ba928b43f1bc94bd6fdbebce..7a9fff3e9306a98590fa9e2679d2231620f318b6 100644 (file)
--- 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 (file)
Binary files a/sc/source/core/tool/interpr1.cxx and b/sc/source/core/tool/interpr1.cxx differ

diff --git a/sc/source/core/tool/interpr3.cxx b/sc/source/core/tool/interpr3.cxx
index 994d821f6d80677e2d12119f5c65bbf95b15eb94..cd4e3efc98e5d32ef3bb11142d0b0bcf2d0e2ecc 100644 (file)
--- 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 347377ed0a9089f043f8b98b09f48b8b64fb5458..80a0e3027bc322a68ace6d5663a74bc45470d4ce 100644 (file)
--- 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<int>(eOp)
+                            << "  params: " << static_cast<int>(pCur->GetParamCount())
+                            << "  nStackBase: " << nStackBase << "  sp: " << sp);
+                    nStackBase = sp;
+                    assert(!"underflow");
+                }
             }
 
             switch( eOp )