Cleanup ExecutionPath from CheckBufferOverrun

lib/checkbufferoverrun.cpp

@@ -63,18 +63,33 @@ void CheckBufferOverrun::arrayIndexOutOfBoundsError(const Token *tok, const Arra
     reportError(tok, Severity::error, "arrayIndexOutOfBounds", oss.str());
 }
 
-void CheckBufferOverrun::arrayIndexOutOfBoundsError(const Token *tok, const ArrayInfo &arrayInfo, const ValueFlow::Value &index)
+void CheckBufferOverrun::arrayIndexOutOfBoundsError(const Token *tok, const ArrayInfo &arrayInfo, const std::vector<ValueFlow::Value> &index)
 {
     std::ostringstream errmsg;
 
-    errmsg << "Array '" << arrayInfo.varname() << "[" << arrayInfo.num(0)
+    errmsg << "Array '" << arrayInfo.varname();
-           << "]' accessed at index " << index.intvalue << ", which is out of bounds.";
+    for (unsigned int i = 0; i < arrayInfo.num().size(); ++i)
+        errmsg << "[" << arrayInfo.num(i) << "]";
+    if (index.size() == 1)
+        errmsg << "' accessed at index " << index[0].intvalue << ", which is out of bounds.";
+    else {
+        errmsg << "' index " << arrayInfo.varname();
+        for (unsigned int i = 0; i < index.size(); ++i)
+            errmsg << "[" << index[i].intvalue << "]";
+        errmsg << " out of bounds.";
+    }
 
-    if (index.condition) {
+    const Token *condition = 0;
-        errmsg << " Otherwise condition '" << index.condition->expressionString() << "' is redundant.";
+    for (unsigned int i = 0; i < index.size(); ++i) {
+        if (condition == NULL)
+            condition = index[i].condition;
+    }
+
+    if (condition != NULL) {
+        errmsg << " Otherwise condition '" << condition->expressionString() << "' is redundant.";
         std::list<const Token *> callstack;
         callstack.push_back(tok);
-        callstack.push_back(index.condition);
+        callstack.push_back(condition);
         reportError(callstack, Severity::warning, "arrayIndexOutOfBoundsCond", errmsg.str());
     } else {
         reportError(tok, Severity::error, "arrayIndexOutOfBounds", errmsg.str());
@@ -1156,81 +1171,85 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo
             continue;
         }
 
-        else if (arrayInfo.num().size() == 1U && Token::Match(tok, "%varid% [", arrayInfo.declarationId()) && tok->next()->astOperand2() && !tok->next()->astOperand2()->values.empty()) {
+        else if (Token::Match(tok, "%varid% [", arrayInfo.declarationId())) {
-            MathLib::bigint count = arrayInfo.num()[0];
+            // Look for errors first
-            const Token *parent = tok->next()->astParent();
+            for (int warn = 0; warn == 0 || warn == 1; ++warn) {
-            while (parent && parent->str() == ".")
+                std::vector<ValueFlow::Value> indexes;
-                parent = tok->astParent();
+                unsigned int valuevarid = 0;
-            if (parent && parent->str() == "&")
+                for (const Token *tok2 = tok->next(); Token::Match(tok2, "["); tok2 = tok2->link()->next()) {
-                ++count;
+                    if (!tok2->astOperand2()) {
-
+                        indexes.clear();
-            const ValueFlow::Value *errvalue = tok->next()->astOperand2()->getMaxValue(false);
+                        break;
-            const ValueFlow::Value *warnvalue = tok->next()->astOperand2()->getMaxValue(true);
+                    }
-            if (errvalue && errvalue->intvalue >= count)
+                    const ValueFlow::Value *value = tok2->astOperand2()->getMaxValue(warn == 1);
-                arrayIndexOutOfBoundsError(tok, arrayInfo, *errvalue);
+                    if (!value) {
-            else if (_settings->isEnabled("warning") && warnvalue && warnvalue->intvalue >= count)
+                        indexes.clear();
-                arrayIndexOutOfBoundsError(tok, arrayInfo, *warnvalue);
+                        break;
-        }
+                    }
-
+                    if (valuevarid == 0U)
-        else if (Token::Match(tok, "%varid% [ %num% ]", arrayInfo.declarationId())) {
+                        valuevarid = value->varId;
-            std::vector<MathLib::bigint> indexes;
+                    if (value->varId > 0 && valuevarid != value->varId) {
-            for (const Token *tok2 = tok->next(); Token::Match(tok2, "[ %num% ]"); tok2 = tok2->tokAt(3)) {
+                        indexes.clear();
-                const MathLib::bigint index = MathLib::toLongNumber(tok2->strAt(1));
+                        break;
-                if (index < 0) {
+                    }
-                    indexes.clear();
+                    if (value->intvalue < 0) {
-                    break;
+                        indexes.clear();
+                        break;
+                    }
+                    indexes.push_back(*value);
                 }
-                indexes.push_back(index);
+                if (indexes.size() == arrayInfo.num().size()) {
-            }
+                    // Check if the indexes point outside the whole array..
-            if (indexes.size() == arrayInfo.num().size()) {
+                    // char a[10][10];
-                // Check if the indexes point outside the whole array..
+                    // a[0][20]  <-- ok.
-                // char a[10][10];
+                    // a[9][20]  <-- error.
-                // a[0][20]  <-- ok.
-                // a[9][20]  <-- error.
 
-                // total number of elements of array..
+                    // total number of elements of array..
-                MathLib::bigint totalElements = 1;
+                    MathLib::bigint totalElements = 1;
 
-                // total index..
+                    // total index..
-                MathLib::bigint totalIndex = 0;
+                    MathLib::bigint totalIndex = 0;
 
-                // calculate the totalElements and totalIndex..
+                    // calculate the totalElements and totalIndex..
-                for (unsigned int i = 0; i < indexes.size(); ++i) {
-                    std::size_t ri = indexes.size() - 1 - i;
-                    totalIndex += indexes[ri] * totalElements;
-                    totalElements *= arrayInfo.num(ri);
-                }
-
-                // totalElements == 0 => Unknown size
-                if (totalElements == 0)
-                    continue;
-
-                const Token *tok2 = tok->previous();
-                while (tok2 && Token::Match(tok2->previous(), "%var% ."))
-                    tok2 = tok2->tokAt(-2);
-
-                // just taking the address?
-                const bool addr(tok2 && (tok2->str() == "&" ||
-                                         Token::simpleMatch(tok2->previous(), "& (")));
-
-                // taking address of 1 past end?
-                if (addr && totalIndex == totalElements)
-                    continue;
-
-                // Is totalIndex in bounds?
-                if (totalIndex >= totalElements) {
-                    arrayIndexOutOfBoundsError(tok, arrayInfo, indexes);
-                }
-                // Is any array index out of bounds?
-                else {
-                    // check each index for overflow
                     for (unsigned int i = 0; i < indexes.size(); ++i) {
-                        if (indexes[i] >= arrayInfo.num(i)) {
+                        std::size_t ri = indexes.size() - 1 - i;
-                            // The access is still within the memory range for the array
+                        totalIndex += indexes[ri].intvalue * totalElements;
-                            // so it may be intentional.
+                        totalElements *= arrayInfo.num(ri);
-                            if (_settings->inconclusive) {
+                    }
-                                arrayIndexOutOfBoundsError(tok, arrayInfo, indexes);
+
-                                break; // only warn about the first one
+                    // totalElements == 0 => Unknown size
+                    if (totalElements == 0)
+                        continue;
+
+                    const Token *tok2 = tok->previous();
+                    while (tok2 && Token::Match(tok2->previous(), "%var% ."))
+                        tok2 = tok2->tokAt(-2);
+
+                    // just taking the address?
+                    const bool addr(tok2 && (tok2->str() == "&" ||
+                                             Token::simpleMatch(tok2->previous(), "& (")));
+
+                    // taking address of 1 past end?
+                    if (addr && totalIndex == totalElements)
+                        continue;
+
+                    // Is totalIndex in bounds?
+                    if (totalIndex >= totalElements) {
+                        arrayIndexOutOfBoundsError(tok, arrayInfo, indexes);
+                        break;
+                    }
+
+                    // Is any array index out of bounds?
+                    else {
+                        // check each index for overflow
+                        for (unsigned int i = 0; i < indexes.size(); ++i) {
+                            if (indexes[i].intvalue >= arrayInfo.num(i)) {
+                                // The access is still within the memory range for the array
+                                // so it may be intentional.
+                                if (_settings->inconclusive) {
+                                    arrayIndexOutOfBoundsError(tok, arrayInfo, indexes);
+                                    break; // only warn about the first one
+                                }
                             }
                         }
                     }
@@ -2003,10 +2022,6 @@ void CheckBufferOverrun::negativeIndex()
 }
 
 
-
-
-#include "executionpath.h"
-
 /// @addtogroup Checks
 /// @{
 
@@ -2054,158 +2069,6 @@ CheckBufferOverrun::ArrayInfo CheckBufferOverrun::ArrayInfo::limit(MathLib::bigi
 }
 
 
-/**
- * @brief %Check for buffer overruns (using ExecutionPath)
- */
-
-class ExecutionPathBufferOverrun : public ExecutionPath {
-public:
-    /** Startup constructor */
-    ExecutionPathBufferOverrun(Check *c, const std::map<unsigned int, CheckBufferOverrun::ArrayInfo> &arrayinfo)
-        : ExecutionPath(c, 0), arrayInfo(arrayinfo), value(0) {
-    }
-
-private:
-    /** @brief Copy this check. Called from the ExecutionPath baseclass. */
-    ExecutionPath *copy() {
-        return new ExecutionPathBufferOverrun(*this);
-    }
-
-    /** @brief is other execution path equal? */
-    bool is_equal(const ExecutionPath *e) const {
-        const ExecutionPathBufferOverrun *c = static_cast<const ExecutionPathBufferOverrun *>(e);
-        return (value == c->value);
-    }
-
-    /** @brief Buffer information */
-    const std::map<unsigned int, CheckBufferOverrun::ArrayInfo> &arrayInfo;
-
-    /** no implementation => compiler error if used by accident */
-    void operator=(const ExecutionPathBufferOverrun &);
-
-    /** internal constructor for creating extra checks */
-    ExecutionPathBufferOverrun(Check *c, const std::map<unsigned int, CheckBufferOverrun::ArrayInfo> &arrayinfo, unsigned int varid_)
-        : ExecutionPath(c, varid_),
-          arrayInfo(arrayinfo),
-          value(0) { // Pretend that variables are initialized to 0. This checking is not about uninitialized variables.
-    }
-
-    /** @brief Variable value. */
-    MathLib::bigint value;
-
-    /**
-     * @brief Assign value to a variable
-     * @param checks the execution paths
-     * @param varid the variable id
-     * @param value the assigned value
-     */
-    static void assign_value(std::list<ExecutionPath *> &checks, unsigned int varid, const std::string &value) {
-        if (varid == 0)
-            return;
-
-        std::list<ExecutionPath *>::const_iterator it;
-        for (it = checks.begin(); it != checks.end(); ++it) {
-            ExecutionPathBufferOverrun *c = dynamic_cast<ExecutionPathBufferOverrun *>(*it);
-            if (c && c->varId == varid)
-                c->value = MathLib::toLongNumber(value);
-        }
-    }
-
-    /**
-     * @brief Found array usage, analyse the array usage
-     * @param tok token where usage occurs (only used when reporting the error)
-     * @param checks The execution paths
-     * @param varid1 variable id for the array
-     * @param varid2 variable id for the index
-     */
-    static void array_index(const Token *tok, std::list<ExecutionPath *> &checks, unsigned int varid1, unsigned int varid2) {
-        if (tok == NULL || checks.empty() || varid1 == 0 || varid2 == 0)
-            return;
-
-        // Locate array info corresponding to varid1
-        const ExecutionPathBufferOverrun * c = dynamic_cast<ExecutionPathBufferOverrun *>(checks.front());
-        if (c == NULL)
-            return;
-        std::map<unsigned int, CheckBufferOverrun::ArrayInfo>::const_iterator it1;
-        it1 = c->arrayInfo.find(varid1);
-        if (it1 == c->arrayInfo.end())
-            return;
-        const CheckBufferOverrun::ArrayInfo& ai = it1->second;
-
-        // Check if varid2 variable has a value that is out of bounds
-        std::list<ExecutionPath *>::const_iterator it;
-        for (it = checks.begin(); it != checks.end(); ++it) {
-            c = dynamic_cast<ExecutionPathBufferOverrun *>(*it);
-            if (c && c->varId == varid2 && c->value >= ai.num(0)) {
-                // variable value is out of bounds, report error
-                CheckBufferOverrun * const checkBufferOverrun = dynamic_cast<CheckBufferOverrun *>(c->owner);
-                if (checkBufferOverrun) {
-                    std::vector<MathLib::bigint> index;
-                    index.push_back(c->value);
-                    checkBufferOverrun->arrayIndexOutOfBoundsError(tok, ai, index);
-                    break;
-                }
-            }
-        }
-    }
-
-    const Token *parse(const Token &tok, std::list<ExecutionPath *> &checks) const {
-        if (Token::Match(tok.previous(), "[;{}]")) {
-            // Declaring variable..
-            if (Token::Match(&tok, "%type% %var% ;") /*&& (tok.isStandardType() || isC)*/) {
-                checks.push_back(new ExecutionPathBufferOverrun(owner, arrayInfo, tok.next()->varId()));
-                return tok.tokAt(2);
-            }
-
-            // Assign variable..
-            if (Token::Match(&tok, "%var% = %num% ;")) {
-                assign_value(checks, tok.varId(), tok.strAt(2));
-                return tok.tokAt(3);
-            }
-        }
-
-        // Assign variable (unknown value = 0)..
-        if (Token::Match(&tok, "%var% =")) {
-            assign_value(checks, tok.varId(), "0");
-            return &tok;
-        }
-
-        // Assign variable (unknown value = 0)..
-        if (Token::Match(tok.tokAt(-2), "(|, & %var% ,|)")) {
-            assign_value(checks, tok.varId(), "0");
-            return &tok;
-        }
-
-        // Array index..
-        if (Token::Match(&tok, "%var% [ %var% ]")) {
-            array_index(&tok, checks, tok.varId(), tok.tokAt(2)->varId());
-            return tok.tokAt(3);
-        }
-
-        return &tok;
-    }
-};
-
-/// @}
-
-
-void CheckBufferOverrun::executionPaths()
-{
-    // Parse all variables and extract array info..
-    std::map<unsigned int, ArrayInfo> arrayInfo;
-    for (unsigned int i = 1; i <= _tokenizer->varIdCount(); i++) {
-        const Variable * const var = _tokenizer->getSymbolDatabase()->getVariableFromVarId(i);
-        if (var && var->isArray() && var->dimension(0) > 0)
-            arrayInfo[i] = ArrayInfo(var, _tokenizer);
-    }
-
-    // Perform checking - check how the arrayInfo arrays are used
-    ExecutionPathBufferOverrun c(this, arrayInfo);
-    checkExecutionPaths(_tokenizer->getSymbolDatabase(), &c);
-}
-
-
-
 
 void CheckBufferOverrun::arrayIndexThenCheck()
 {

lib/checkbufferoverrun.h

@@ -64,9 +64,6 @@ public:
         checkBufferOverrun.bufferOverrun();
         checkBufferOverrun.negativeIndex();
         checkBufferOverrun.arrayIndexThenCheck();
-
-        /** ExecutionPath checking.. */
-        checkBufferOverrun.executionPaths();
         checkBufferOverrun.writeOutsideBufferSize();
     }
 
@@ -211,7 +208,7 @@ public:
     void checkFunctionCall(const Token *tok, const ArrayInfo &arrayInfo, std::list<const Token *> callstack);
 
     void arrayIndexOutOfBoundsError(const Token *tok, const ArrayInfo &arrayInfo, const std::vector<MathLib::bigint> &index);
-    void arrayIndexOutOfBoundsError(const Token *tok, const ArrayInfo &arrayInfo, const ValueFlow::Value &index);
+    void arrayIndexOutOfBoundsError(const Token *tok, const ArrayInfo &arrayInfo, const std::vector<ValueFlow::Value> &index);
     void arrayIndexInForLoop(const Token *tok, const ArrayInfo &arrayInfo);
 
 private:

test/testbufferoverrun.cpp

@@ -3675,7 +3675,7 @@ private:
 
         // Check for buffer overruns..
         CheckBufferOverrun checkBufferOverrun(&tokenizer, &settings, this);
-        checkBufferOverrun.executionPaths();
+        checkBufferOverrun.bufferOverrun();
     }
 
 
@@ -3698,7 +3698,7 @@ private:
                 "        i = 1000;\n"
                 "    buf[i][0] = 0;\n"
                 "}");
-        ASSERT_EQUALS("[test.cpp:7]: (error) Array 'buf[10][5]' accessed at index 1000, which is out of bounds.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:7]: (error) Array 'buf[10][5]' index buf[1000][0] out of bounds.\n", errout.str());
     }
 
     void executionPaths2() {