Analysis: fix bugs found with Coverity 98/17198/1
Calixte DENIZET [Fri, 11 Sep 2015 17:30:03 +0000 (19:30 +0200)]
Change-Id: I66eb04dc5bef4a1f12e5e9e19f76b4ef2a7fc1af

scilab/modules/ast/includes/analysis/ConstantVisitor.hxx
scilab/modules/ast/includes/analysis/gvn/OpValue.hxx
scilab/modules/ast/includes/analysis/gvn/TestGVNVisitor.hxx
scilab/modules/ast/src/cpp/analysis/ArgnAnalyzer.cpp
scilab/modules/ast/src/cpp/analysis/ConstantVisitor.cpp
scilab/modules/ast/src/cpp/analysis/ConstraintManager.cpp
scilab/modules/ast/src/cpp/analysis/FunctionBlock.cpp
scilab/modules/ast/src/cpp/analysis/MacroDef.cpp
scilab/modules/ast/src/cpp/analysis/OperGVNValues.cpp
scilab/modules/ast/src/cpp/analysis/SizeAnalyzer.cpp
scilab/modules/functions/sci_gateway/cpp/sci_testAnalysis.cpp

index 05472fe..8b6c20f 100644 (file)
@@ -42,12 +42,12 @@ class EXTERN_AST ConstantVisitor : public ast::Visitor, public Chrono
 
 public:
 
-    ConstantVisitor(AnalysisVisitor & _parent) : parent(&_parent), lhs(1), evalSymbols(true)
+    ConstantVisitor(AnalysisVisitor & _parent) : _result(false), parent(&_parent), lhs(1), evalSymbols(true)
     {
         start_chrono();
     }
 
-    ConstantVisitor(const bool _evalSymbols = true) : parent(nullptr), evalSymbols(_evalSymbols)
+    ConstantVisitor(const bool _evalSymbols = true) : _result(false), parent(nullptr), evalSymbols(_evalSymbols)
     {
         start_chrono();
     }
index a153479..e7d69a2 100644 (file)
@@ -41,7 +41,7 @@ struct OpValue
      * \param _kind the operation kind
      * \param _lnum the value of the operand
      */
-    OpValue(Kind _kind, uint64_t _lnum) : kind(_kind), lnum(_lnum) { }
+    OpValue(Kind _kind, uint64_t _lnum) : kind(_kind), lnum(_lnum), rnum(0) { }
 
     /**
      * \brief constructor for binary operation
index 880b3e6..7f2d711 100644 (file)
@@ -35,7 +35,7 @@ class TestGVNVisitor : public ast::Visitor /*, public Chrono */
 
 public:
 
-    TestGVNVisitor()
+    TestGVNVisitor() : _result(nullptr)
     {
         //start_chrono();
     }
@@ -124,7 +124,7 @@ private:
         const GVN::Value & LV = getResult();
         e.getRight().accept(*this);
         const GVN::Value & RV = getResult();
-       
+
         switch (e.getOper())
         {
             case ast::OpExp::plus:
@@ -317,12 +317,12 @@ private:
     }
 
     void visit(ast::IntSelectExp & e)
-       {
-       }
-    
+    {
+    }
+
     void visit(ast::StringSelectExp & e)
-       {
-       }
+    {
+    }
 };
 
 } // namespace analysis
index 54de1d5..a739564 100644 (file)
@@ -40,10 +40,10 @@ bool ArgnAnalyzer::analyze(AnalysisVisitor & visitor, const unsigned int lhs, as
     }
 
 
-    enum Kind 
+    enum Kind
     {
-        LHS, RHS, LHSRHS
-    } kind;
+        LHS, RHS, LHSRHS, DUNNO
+    } kind = DUNNO;
     const ast::exps_t args = e.getArgs();
     switch (args.size())
     {
@@ -109,6 +109,7 @@ bool ArgnAnalyzer::analyze(AnalysisVisitor & visitor, const unsigned int lhs, as
             res.getConstant() = visitor.getGVN().getValue(val);
             e.getDecorator().setCall(L"argn");
             visitor.setResult(res);
+            break;
         }
         case LHSRHS:
         {
@@ -124,7 +125,10 @@ bool ArgnAnalyzer::analyze(AnalysisVisitor & visitor, const unsigned int lhs, as
             mlhs.back().getConstant() = visitor.getGVN().getValue(frhs);
 
             e.getDecorator().setCall(L"argn");
+            break;
         }
+        default:
+            return false;
     }
 
     return true;
index e8b4564..9644fa1 100644 (file)
@@ -204,14 +204,14 @@ void ConstantVisitor::visit(ast::CallExp & e)
                         else
                         {
                             double val;
-                            ast::exps_t exps;
-                            exps.reserve(2);
+                            ast::exps_t * exps = new ast::exps_t();
+                            exps->reserve(2);
                             std::vector<Result> & res = parent->getLHSContainer();
                             res.front().getConstant().getDblValue(val);
-                            exps.push_back(new ast::DoubleExp(e.getLocation(), val));
+                            exps->push_back(new ast::DoubleExp(e.getLocation(), val));
                             res.back().getConstant().getDblValue(val);
-                            exps.push_back(new ast::DoubleExp(e.getLocation(), val));
-                            e.replace(new ast::ArrayListExp(e.getLocation(), exps));
+                            exps->push_back(new ast::DoubleExp(e.getLocation(), val));
+                            e.replace(new ast::ArrayListExp(e.getLocation(), *exps));
                             isConstant = true;
                         }
                     }
index 58cb869..3fedae7 100644 (file)
@@ -33,11 +33,11 @@ std::vector<std::shared_ptr<InferenceConstraint>> ConstraintManager::init()
     v.emplace_back(new GreaterConstraint());
     // Is a value strict greater than an other ?
     v.emplace_back(new StrictGreaterConstraint());
-   // Valid index
+    // Valid index
     v.emplace_back(new ValidIndexConstraint());
     // Valid range
     v.emplace_back(new ValidRangeConstraint());
+
     return v;
 }
 
@@ -83,23 +83,23 @@ bool ConstraintManager::check(const MPolyConstraintSet & set, const std::vector<
 
 bool ConstraintManager::check(Kind kind, const std::vector<GVN::Value *> & values)
 {
-    const InferenceConstraint & ic = *generalConstraints[kind];
-    InferenceConstraint::Result res = ic.check(function->getGVN(), values);
-    //std::wcerr << "DEBUG2=" << res << std::endl;
-
-    switch (res)
+    if (function)
     {
-        case InferenceConstraint::Result::RESULT_TRUE:
-        {
-            mpConstraints.add(ic.getMPConstraints(values));
-            ic.applyConstraints(values);
-            return true;
-        }
-        case InferenceConstraint::Result::RESULT_FALSE:
-            return false;
-        case InferenceConstraint::Result::RESULT_DUNNO:
+        const InferenceConstraint & ic = *generalConstraints[kind];
+        InferenceConstraint::Result res = ic.check(function->getGVN(), values);
+        //std::wcerr << "DEBUG2=" << res << std::endl;
+
+        switch (res)
         {
-            if (function)
+            case InferenceConstraint::Result::RESULT_TRUE:
+            {
+                mpConstraints.add(ic.getMPConstraints(values));
+                ic.applyConstraints(values);
+                return true;
+            }
+            case InferenceConstraint::Result::RESULT_FALSE:
+                return false;
+            case InferenceConstraint::Result::RESULT_DUNNO:
             {
                 MPolyConstraintSet set = ic.getMPConstraints(values);
                 const bool ret = check(set, function->getInValues());
@@ -110,43 +110,44 @@ bool ConstraintManager::check(Kind kind, const std::vector<GVN::Value *> & value
                 }
                 return ret;
             }
-            return false;
         }
     }
+
+    return false;
 }
 
-    bool ConstraintManager::checkGlobalConstant(const symbol::Symbol & sym)
+bool ConstraintManager::checkGlobalConstant(const symbol::Symbol & sym)
+{
+    if (constantConstraints.find(sym) == constantConstraints.end())
+    {
+        // TODO: fix that !!!
+        const bool ret = true; //symbol::Context::getInstance()->isOriginalSymbol(sym);
+        if (ret)
+        {
+            ConstraintManager * cm = this;
+            while (cm)
+            {
+                cm->constantConstraints.emplace(sym);
+                cm = cm->parent;
+            }
+        }
+        return ret;
+    }
+    else
     {
-       if (constantConstraints.find(sym) == constantConstraints.end())
-       {
-           // TODO: fix that !!!
-           const bool ret = true; //symbol::Context::getInstance()->isOriginalSymbol(sym);
-           if (ret)
-           {
-               ConstraintManager * cm = this;
-               while (cm)
-               {
-                   cm->constantConstraints.emplace(sym);
-                   cm = cm->parent;
-               }
-           }
-           return ret;
-       }
-       else
-       {
-           return true;
-       }
+        return true;
     }
+}
 
-    bool ConstraintManager::checkGlobalConstants(const std::set<symbol::Symbol> & gc)
-       {
-           for (const auto sym : gc)
-           {
-               if (!symbol::Context::getInstance()->isOriginalSymbol(sym))
-               {
-                   return false;
-               }
-           }
-           return true;
-       }
+bool ConstraintManager::checkGlobalConstants(const std::set<symbol::Symbol> & gc)
+{
+    for (const auto sym : gc)
+    {
+        if (!symbol::Context::getInstance()->isOriginalSymbol(sym))
+        {
+            return false;
+        }
+    }
+    return true;
+}
 }
index db67c60..1b47f3e 100644 (file)
@@ -21,7 +21,7 @@
 namespace analysis
 {
 
-FunctionBlock::FunctionBlock(const unsigned int id, Block * parent, ast::Exp * exp) : Block(id, parent, exp), loopAnalyzer(exp), constraintManager(*this, dm->topFunction())
+FunctionBlock::FunctionBlock(const unsigned int id, Block * parent, ast::Exp * exp) : Block(id, parent, exp), loopAnalyzer(exp), constraintManager(*this, dm->topFunction()), lhs(0), rhs(0), maxVarId(0)
 {
     gvn = &fgvn;
     dm->pushFunction(this);
index dc67cae..9264641 100644 (file)
@@ -51,7 +51,7 @@ MacroDef * ExistingMacroDef::clone() const
     return new ExistingMacroDef(*this);
 }
 
-DeclaredMacroDef::DeclaredMacroDef(ast::FunctionDec * const _dec) : MacroDef(_dec->getReturns().getVars().size(), _dec->getArgs().getVars().size(), dec), dec(nullptr)
+DeclaredMacroDef::DeclaredMacroDef(ast::FunctionDec * const _dec) : MacroDef(_dec->getReturns().getVars().size(), _dec->getArgs().getVars().size(), _dec), dec(nullptr)
 {
     GlobalsCollector::collect(*this);
 }
index dfd3d94..700438b 100644 (file)
 
 namespace analysis
 {
-    bool AnalysisVisitor::operGVNValues(ast::OpExp & oe)
-    {
-        Result & resL = oe.getLeft().getDecorator().getResult();
-        Result & resR = oe.getRight().getDecorator().getResult();
-        const ConstantValue & valL = resL.getConstant();
-        const ConstantValue & valR = resR.getConstant();
-        GVN::Value * gvnL = nullptr;
-        GVN::Value * gvnR = nullptr;
-       bool LisInt = false;
-       bool RisInt = false;
+bool AnalysisVisitor::operGVNValues(ast::OpExp & oe)
+{
+    Result & resL = oe.getLeft().getDecorator().getResult();
+    Result & resR = oe.getRight().getDecorator().getResult();
+    const ConstantValue & valL = resL.getConstant();
+    const ConstantValue & valR = resR.getConstant();
+    GVN::Value * gvnL = nullptr;
+    GVN::Value * gvnR = nullptr;
+    bool LisInt = false;
+    bool RisInt = false;
 
-        if (valL.getKind() == ConstantValue::GVNVAL)
-        {
-            gvnL = valL.getGVNValue();
-        }
-        else
+    if (valL.getKind() == ConstantValue::GVNVAL)
+    {
+        gvnL = valL.getGVNValue();
+    }
+    else
+    {
+        double val;
+        if (valL.getDblValue(val) && tools::isAnInt(val))
         {
-            double val;
-            if (valL.getDblValue(val) && tools::isAnInt(val))
-            {
-                gvnL = getGVN().getValue(val);
-               LisInt = true;
-            }
+            gvnL = getGVN().getValue(val);
+            LisInt = true;
         }
+    }
 
-        if (valR.getKind() == ConstantValue::GVNVAL)
-        {
-            gvnR = valR.getGVNValue();
-        }
-        else
+    if (valR.getKind() == ConstantValue::GVNVAL)
+    {
+        gvnR = valR.getGVNValue();
+    }
+    else
+    {
+        double val;
+        if (valR.getDblValue(val) && tools::isAnInt(val))
         {
-            double val;
-            if (valR.getDblValue(val) && tools::isAnInt(val))
-            {
-                gvnR = getGVN().getValue(val);
-               RisInt = true;
-            }
+            gvnR = getGVN().getValue(val);
+            RisInt = true;
         }
+    }
 
-        if (gvnL && gvnR)
-        {
-            TIType typ(getGVN(), TIType::DOUBLE, 1, 1);
+    if (gvnL && gvnR)
+    {
+        TIType typ(getGVN(), TIType::DOUBLE, 1, 1);
 
-            switch (oe.getOper())
-            {
+        switch (oe.getOper())
+        {
             case ast::OpExp::plus:
             {
-               if (LisInt)
-               {
-                   resL.getConstant() = gvnL;
-               }
-               if (RisInt)
-               {
-                   resR.getConstant() = gvnR;
-               }
+                if (LisInt)
+                {
+                    resL.getConstant() = gvnL;
+                }
+                if (RisInt)
+                {
+                    resR.getConstant() = gvnR;
+                }
                 Result & res = oe.getDecorator().setResult(typ);
                 res.getConstant() = getGVN().getValue(OpValue::PLUS, *gvnL, *gvnR);
-               oe.getDecorator().safe = true;
+                oe.getDecorator().safe = true;
                 setResult(res);
                 return true;
             }
             case ast::OpExp::minus:
             {
-               if (LisInt)
-               {
-                   resL.getConstant() = gvnL;
-               }
-               if (RisInt)
-               {
-                   resR.getConstant() = gvnR;
-               }
+                if (LisInt)
+                {
+                    resL.getConstant() = gvnL;
+                }
+                if (RisInt)
+                {
+                    resR.getConstant() = gvnR;
+                }
                 Result & res = oe.getDecorator().setResult(typ);
                 res.getConstant() = getGVN().getValue(OpValue::MINUS, *gvnL, *gvnR);
-               oe.getDecorator().safe = true;
-               setResult(res);
+                oe.getDecorator().safe = true;
+                setResult(res);
                 return true;
                 break;
             }
             case ast::OpExp::times:
             {
                 if (LisInt)
-               {
-                   resL.getConstant() = gvnL;
-               }
-               if (RisInt)
-               {
-                   resR.getConstant() = gvnR;
-               }
+                {
+                    resL.getConstant() = gvnL;
+                }
+                if (RisInt)
+                {
+                    resR.getConstant() = gvnR;
+                }
                 Result & res = oe.getDecorator().setResult(typ);
                 res.getConstant() = getGVN().getValue(OpValue::TIMES, *gvnL, *gvnR);
                 oe.getDecorator().safe = true;
-               setResult(res);
+                setResult(res);
                 return true;
             }
             case ast::OpExp::dottimes:
             {
                 if (LisInt)
-               {
-                   resL.getConstant() = gvnL;
-               }
-               if (RisInt)
-               {
-                   resR.getConstant() = gvnR;
-               }
+                {
+                    resL.getConstant() = gvnL;
+                }
+                if (RisInt)
+                {
+                    resR.getConstant() = gvnR;
+                }
                 Result & res = oe.getDecorator().setResult(typ);
                 res.getConstant() = getGVN().getValue(OpValue::DOTTIMES, *gvnL, *gvnR);
                 oe.getDecorator().safe = true;
-               setResult(res);
+                setResult(res);
                 return true;
             }
             case ast::OpExp::power:
@@ -130,23 +130,23 @@ namespace analysis
                 Result & res = oe.getDecorator().setResult(typ);
                 res.getConstant() = getGVN().getValue(OpValue::POWER, *gvnL, *gvnR);
                 oe.getDecorator().safe = true;
-               setResult(res);
+                setResult(res);
                 return true;
             }
             case ast::OpExp::dotpower:
             {
                 if (LisInt)
-               {
-                   resL.getConstant() = gvnL;
-               }
-               if (RisInt)
-               {
-                   resR.getConstant() = gvnR;
-               }
+                {
+                    resL.getConstant() = gvnL;
+                }
+                if (RisInt)
+                {
+                    resR.getConstant() = gvnR;
+                }
                 Result & res = oe.getDecorator().setResult(typ);
                 res.getConstant() = getGVN().getValue(OpValue::DOTPOWER, *gvnL, *gvnR);
                 oe.getDecorator().safe = true;
-               setResult(res);
+                setResult(res);
                 return true;
             }
             case ast::OpExp::rdivide:
@@ -154,42 +154,43 @@ namespace analysis
                 if (gvnL->poly->isDivisibleBy(*gvnR->poly))
                 {
                     if (LisInt)
-                   {
-                       resL.getConstant() = gvnL;
-                   }
-                   if (RisInt)
-                   {
-                       resR.getConstant() = gvnR;
-                   }
-                   Result & res = oe.getDecorator().setResult(typ);
+                    {
+                        resL.getConstant() = gvnL;
+                    }
+                    if (RisInt)
+                    {
+                        resR.getConstant() = gvnR;
+                    }
+                    Result & res = oe.getDecorator().setResult(typ);
                     res.getConstant() = getGVN().getValue(OpValue::RDIV, *gvnL, *gvnR);
                     oe.getDecorator().safe = true;
-                   setResult(res);
+                    setResult(res);
                     return true;
                 }
+                break;
             }
             case ast::OpExp::dotrdivide:
             {
                 if (gvnL->poly->isDivisibleBy(*gvnR->poly))
                 {
                     if (LisInt)
-                   {
-                       resL.getConstant() = gvnL;
-                   }
-                   if (RisInt)
-                   {
-                       resR.getConstant() = gvnR;
-                   }
-                   Result & res = oe.getDecorator().setResult(typ);
+                    {
+                        resL.getConstant() = gvnL;
+                    }
+                    if (RisInt)
+                    {
+                        resR.getConstant() = gvnR;
+                    }
+                    Result & res = oe.getDecorator().setResult(typ);
                     res.getConstant() = getGVN().getValue(OpValue::DOTRDIV, *gvnL, *gvnR);
                     oe.getDecorator().safe = true;
-                   setResult(res);
+                    setResult(res);
                     return true;
                 }
             }
-            }
         }
-
-        return false;
     }
+
+    return false;
+}
 }
index a4cf353..2440cd1 100644 (file)
@@ -28,8 +28,8 @@ bool SizeAnalyzer::analyze(AnalysisVisitor & visitor, const unsigned int lhs, as
     const ast::exps_t args = e.getArgs();
     enum Kind
     {
-        ROWS, COLS, ROWSTIMESCOLS, ROWSCOLS, ONE, BOTH
-    } kind;
+        ROWS, COLS, ROWSTIMESCOLS, ROWSCOLS, ONE, BOTH, DUNNO
+    } kind = DUNNO;
     const std::size_t size = args.size();
     if (size == 0 || size >= 3)
     {
@@ -186,6 +186,8 @@ bool SizeAnalyzer::analyze(AnalysisVisitor & visitor, const unsigned int lhs, as
             visitor.setResult(_res);
             break;
         }
+        default:
+            return false;
     }
 
     return true;
index 8612366..54be596 100644 (file)
@@ -57,6 +57,7 @@ Function::ReturnValue sci_testAnalysis(types::typed_list &in, int _iRetCount, ty
     {
         if (!arg->isString() || arg->getAs<types::String>()->getSize() != 1)
         {
+            delete args;
             Scierror(999, _("%s: Wrong type for input argument #%d: A string expected.\n"), "testAnalysis", i);
             return Function::Error;
         }