Fixed #6272 (Improve check: multifile checking in checkbufferoverrun)

cli/cppcheckexecutor.cpp

@@ -786,6 +786,7 @@ int CppCheckExecutor::check_internal(CppCheck& cppcheck, int /*argc*/, const cha
         }
 
         cppcheck.checkFunctionUsage();
+        cppcheck.analyseWholeProgram();
     } else if (!ThreadExecutor::isEnabled()) {
         std::cout << "No thread support yet implemented for this platform." << std::endl;
     } else {

lib/check.h

@@ -58,26 +58,6 @@ public:
         return _instances;
     }
 
-    /**
-     * analyse code - must be thread safe
-     * @param tokens The tokens to analyse
-     * @param result container where results are stored
-     */
-    virtual void analyse(const Token *tokens, std::set<std::string> &result) const {
-        // suppress compiler warnings
-        (void)tokens;
-        (void)result;
-    }
-
-    /**
-     * Save analysis data - the caller ensures thread safety
-     * @param data The data where the results are saved
-     */
-    virtual void saveAnalysisData(const std::set<std::string> &data) const {
-        // suppress compiler warnings
-        (void)data;
-    }
-
     /** run checks, the token list is not simplified */
     virtual void runChecks(const Tokenizer *, const Settings *, ErrorLogger *) {
     }
@@ -107,6 +87,23 @@ public:
         return _settings && _settings->inconclusive;
     }
 
+    /** Base class used for whole-program analysis */
+    class FileInfo {
+    public:
+        FileInfo() {}
+        virtual ~FileInfo() {}
+    };
+
+    virtual FileInfo * getFileInfo(const Tokenizer *tokenizer) const {
+        (void)tokenizer;
+        return nullptr;
+    }
+
+    virtual void analyseWholeProgram(const std::list<FileInfo*> &fileInfo, ErrorLogger &errorLogger) {
+        (void)fileInfo;
+        (void)errorLogger;
+    }
+
 protected:
     const Tokenizer * const _tokenizer;
     const Settings * const _settings;

lib/checkbufferoverrun.cpp

@@ -1804,3 +1804,94 @@ void CheckBufferOverrun::writeOutsideBufferSizeError(const Token *tok, const std
                 "The number of bytes to write (" + MathLib::toString(writeLength) + " bytes) are bigger than the source buffer (" +MathLib::toString(stringLength)+ " bytes)."
                 " Please check the second and the third parameter of the function '"+strFunctionName+"'.");
 }
+
+Check::FileInfo* CheckBufferOverrun::getFileInfo(const Tokenizer *tokenizer) const
+{
+    MyFileInfo *fileInfo = new MyFileInfo;
+
+    // Array usage..
+    const SymbolDatabase* const symbolDatabase = tokenizer->getSymbolDatabase();
+    const std::size_t functions = symbolDatabase->functionScopes.size();
+    for (std::size_t i = 0; i < functions; ++i) {
+        const Scope * const scope = symbolDatabase->functionScopes[i];
+        for (const Token *tok = scope->classStart; tok && tok != scope->classEnd; tok = tok->next()) {
+            if (Token::Match(tok, "%var% [")          &&
+                Token::Match(tok->linkAt(1), "] !![") &&
+                tok->variable()                       &&
+                tok->variable()->isExtern()           &&
+                tok->variable()->isGlobal()           &&
+                tok->next()->astOperand2()) {
+                const ValueFlow::Value *value = tok->next()->astOperand2()->getMaxValue(false);
+                if (value && value->intvalue > 0) {
+                    struct MyFileInfo::ArrayUsage arrayUsage;
+                    arrayUsage.index = value->intvalue;
+                    arrayUsage.fileName = tokenizer->list.file(tok);
+                    arrayUsage.linenr = tok->linenr();
+                    std::map<std::string, struct MyFileInfo::ArrayUsage>::iterator it = fileInfo->arrayUsage.find(tok->str());
+                    if (it == fileInfo->arrayUsage.end() || it->second.index < arrayUsage.index)
+                        fileInfo->arrayUsage[tok->str()] = arrayUsage;
+                }
+            }
+        }
+    }
+
+    // Arrays..
+    const std::list<Variable> &varlist = symbolDatabase->scopeList.front().varlist;
+    for (std::list<Variable>::const_iterator it = varlist.begin(); it != varlist.end(); ++it) {
+        const Variable &var = *it;
+        if (!var.isStatic() && var.isArray() && var.dimensions().size() == 1U)
+            fileInfo->arraySize[var.name()] = var.dimension(0U);
+    }
+
+    return fileInfo;
+}
+
+void CheckBufferOverrun::analyseWholeProgram(const std::list<Check::FileInfo*> &fileInfo, ErrorLogger &errorLogger)
+{
+    // Merge all fileInfo
+    MyFileInfo all;
+    for (std::list<Check::FileInfo*>::const_iterator it = fileInfo.begin(); it != fileInfo.end(); ++it) {
+        const MyFileInfo *fi = dynamic_cast<MyFileInfo*>(*it);
+        if (!fi)
+            continue;
+
+        // merge array usage
+        for (std::map<std::string, struct MyFileInfo::ArrayUsage>::const_iterator it2 = fi->arrayUsage.begin(); it2 != fi->arrayUsage.end(); ++it2) {
+            std::map<std::string, struct MyFileInfo::ArrayUsage>::const_iterator allit = all.arrayUsage.find(it2->first);
+            if (allit == all.arrayUsage.end() || it2->second.index > allit->second.index)
+                all.arrayUsage[it2->first] = it2->second;
+        }
+
+        // merge array info
+        for (std::map<std::string, MathLib::bigint>::const_iterator it2 = fi->arraySize.begin(); it2 != fi->arraySize.end(); ++it2) {
+            std::map<std::string, MathLib::bigint>::const_iterator allit = all.arraySize.find(it2->first);
+            if (allit == all.arraySize.end())
+                all.arraySize[it2->first] = it2->second;
+            else
+                all.arraySize[it2->first] = -1;
+        }
+    }
+
+    // Check buffer usage
+    for (std::map<std::string, struct MyFileInfo::ArrayUsage>::const_iterator it = all.arrayUsage.begin(); it != all.arrayUsage.end(); ++it) {
+        std::map<std::string, MathLib::bigint>::const_iterator sz = all.arraySize.find(it->first);
+        if (sz != all.arraySize.end() && sz->second > 0 && sz->second < it->second.index) {
+            ErrorLogger::ErrorMessage::FileLocation fileLoc;
+            fileLoc.setfile(it->second.fileName);
+            fileLoc.line = it->second.linenr;
+
+            std::list<ErrorLogger::ErrorMessage::FileLocation> locationList;
+            locationList.push_back(fileLoc);
+
+            std::ostringstream ostr;
+            ostr << "Array " << it->first << '[' << sz->second << "] accessed at index " << it->second.index << " which is out of bounds";
+
+            const ErrorLogger::ErrorMessage errmsg(locationList,
+                                                   Severity::error,
+                                                   ostr.str(),
+                                                   "arrayIndexOutOfBounds",
+                                                   false);
+            errorLogger.reportErr(errmsg);
+        }
+    }
+}

lib/checkbufferoverrun.h

@@ -203,6 +203,28 @@ public:
     void arrayIndexOutOfBoundsError(const Token *tok, const ArrayInfo &arrayInfo, const std::vector<MathLib::bigint> &index);
     void arrayIndexOutOfBoundsError(const Token *tok, const ArrayInfo &arrayInfo, const std::vector<ValueFlow::Value> &index);
 
+    /* data for multifile checking */
+    class MyFileInfo : public Check::FileInfo {
+    public:
+        struct ArrayUsage {
+            MathLib::bigint   index;
+            std::string       fileName;
+            unsigned int      linenr;
+        };
+
+        /* key:arrayName */
+        std::map<std::string, struct ArrayUsage> arrayUsage;
+
+        /* key:arrayName, data:arraySize */
+        std::map<std::string, MathLib::bigint>  arraySize;
+    };
+
+    /** @brief Parse current TU and extract file info */
+    Check::FileInfo *getFileInfo(const Tokenizer *tokenizer) const;
+
+    /** @brief Analyse all file infos for all TU */
+    virtual void analyseWholeProgram(const std::list<Check::FileInfo*> &fileInfo, ErrorLogger &errorLogger);
+
 private:
 
     static bool isArrayOfStruct(const Token* tok, int &position);

lib/checkmemoryleak.cpp

@@ -2678,7 +2678,7 @@ void CheckMemoryLeakNoVar::check()
     std::set<std::string> uvarFunctions;
     {
         const CheckUninitVar c(_tokenizer, _settings, _errorLogger);
-        c.analyse(_tokenizer->tokens(), uvarFunctions);
+        c.analyseFunctions(_tokenizer, uvarFunctions);
     }
 
     const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();

lib/checkuninitvar.cpp

@@ -601,7 +601,7 @@ private:
             }
         }
 
-        if (Token::Match(&tok, "%var% (") && uvarFunctions.find(tok.str()) == uvarFunctions.end()) {
+        if (Token::Match(&tok, "%var% (")) {
             // sizeof/typeof doesn't dereference. A function name that is all uppercase
             // might be an unexpanded macro that uses sizeof/typeof
             if (Token::Match(&tok, "sizeof|typeof ("))
@@ -920,9 +920,6 @@ private:
 
 public:
 
-    /** Functions that don't handle uninitialized variables well */
-    static std::set<std::string> uvarFunctions;
-
     static void analyseFunctions(const Token * const tokens, std::set<std::string> &func) {
         for (const Token *tok = tokens; tok; tok = tok->next()) {
             if (tok->str() == "{") {
@@ -1013,34 +1010,33 @@ public:
     }
 };
 
-/** Functions that don't handle uninitialized variables well */
-std::set<std::string> UninitVar::uvarFunctions;
-
-
 /// @}
 
 
-void CheckUninitVar::analyse(const Token * tokens, std::set<std::string> &func) const
+Check::FileInfo *CheckUninitVar::getFileInfo(const Tokenizer *tokenizer) const
 {
-    UninitVar::analyseFunctions(tokens, func);
+    MyFileInfo * mfi = new MyFileInfo;
+    analyseFunctions(tokenizer, mfi->uvarFunctions);
+    // TODO: add suspicious function calls
+    return mfi;
 }
 
-void CheckUninitVar::saveAnalysisData(const std::set<std::string> &data) const
+void CheckUninitVar::analyseWholeProgram(const std::list<Check::FileInfo*> &fileInfo, ErrorLogger &errorLogger)
 {
-    UninitVar::uvarFunctions.insert(data.begin(), data.end());
+    (void)fileInfo;
+    (void)errorLogger;
+}
+
+void CheckUninitVar::analyseFunctions(const Tokenizer *tokenizer, std::set<std::string> &f) const
+{
+    UninitVar::analyseFunctions(tokenizer->tokens(), f);
 }
 
 void CheckUninitVar::executionPaths()
 {
     // check if variable is accessed uninitialized..
-    {
-        // no writing if multiple threads are used (TODO: thread safe analysis?)
-        if (_settings->_jobs == 1)
-            UninitVar::analyseFunctions(_tokenizer->tokens(), UninitVar::uvarFunctions);
-
     UninitVar c(this, _tokenizer->getSymbolDatabase(), &_settings->library, _tokenizer->isC());
     checkExecutionPaths(_tokenizer->getSymbolDatabase(), &c);
-    }
 }
 
 

lib/checkuninitvar.h

@@ -69,15 +69,23 @@ public:
     void deadPointer();
     void deadPointerError(const Token *pointer, const Token *alias);
 
-    /**
+    /* data for multifile checking */
-     * @brief Uninitialized variables: analyse functions to see how they work with uninitialized variables
+    class MyFileInfo : public Check::FileInfo {
-     * @param tokens [in] the token list
+    public:
-     * @param func [out] names of functions that don't handle uninitialized variables well. the function names are added to the set. No clearing is made.
+        /* functions that must have initialized data */
-     */
+        std::set<std::string>  uvarFunctions;
-    void analyse(const Token * tokens, std::set<std::string> &func) const;
 
-    /** Save analysis results */
+        /* functions calls with uninitialized data */
-    void saveAnalysisData(const std::set<std::string> &data) const;
+        std::set<std::string>  functionCalls;
+    };
+
+    /** @brief Parse current TU and extract file info */
+    Check::FileInfo *getFileInfo(const Tokenizer *tokenizer) const;
+
+    /** @brief Analyse all file infos for all TU */
+    virtual void analyseWholeProgram(const std::list<Check::FileInfo*> &fileInfo, ErrorLogger &errorLogger);
+
+    void analyseFunctions(const Tokenizer *tokenizer, std::set<std::string> &f) const;
 
     /** @brief new type of check: check execution paths */
     void executionPaths();

lib/cppcheck.cpp

@@ -48,6 +48,10 @@ CppCheck::CppCheck(ErrorLogger &errorLogger, bool useGlobalSuppressions)
 
 CppCheck::~CppCheck()
 {
+    while (!fileInfo.empty()) {
+        delete fileInfo.back();
+        fileInfo.pop_back();
+    }
     S_timerResults.ShowResults(_settings._showtime);
 }
 
@@ -309,19 +313,6 @@ void CppCheck::analyseFile(std::istream &fin, const std::string &filename)
     std::istringstream istr(code);
     tokenizer.tokenize(istr, filename.c_str());
     tokenizer.simplifyTokenList2();
-
-    // Analyse the tokens..
-    std::set<std::string> data;
-    for (std::list<Check *>::const_iterator it = Check::instances().begin(); it != Check::instances().end(); ++it) {
-        (*it)->analyse(tokenizer.tokens(), data);
-    }
-
-    // Save analysis results..
-    // TODO: This loop should be protected by a mutex or something like that
-    //       The saveAnalysisData must _not_ be called from many threads at the same time.
-    for (std::list<Check *>::const_iterator it = Check::instances().begin(); it != Check::instances().end(); ++it) {
-        (*it)->saveAnalysisData(data);
-    }
 }
 
 //---------------------------------------------------------------------------
@@ -411,6 +402,13 @@ bool CppCheck::checkFile(const std::string &code, const char FileName[], std::se
             (*it)->runSimplifiedChecks(&_tokenizer, &_settings, this);
         }
 
+        // Analyse the tokens..
+        for (std::list<Check *>::const_iterator it = Check::instances().begin(); it != Check::instances().end(); ++it) {
+            Check::FileInfo *fi = (*it)->getFileInfo(&_tokenizer);
+            if (fi != nullptr)
+                fileInfo.push_back(fi);
+        }
+
         if (_settings.terminated())
             return true;
 
@@ -690,3 +688,11 @@ void CppCheck::getErrorMessages()
     Tokenizer::getErrorMessages(this, &_settings);
     Preprocessor::getErrorMessages(this, &_settings);
 }
+
+void CppCheck::analyseWholeProgram()
+{
+    // Analyse the tokens..
+    for (std::list<Check *>::const_iterator it = Check::instances().begin(); it != Check::instances().end(); ++it)
+        (*it)->analyseWholeProgram(fileInfo, *this);
+}
+

lib/cppcheck.h

@@ -24,6 +24,7 @@
 #include "config.h"
 #include "settings.h"
 #include "errorlogger.h"
+#include "check.h"
 
 #include <string>
 #include <list>
@@ -134,6 +135,9 @@ public:
         _simplify = false;
     }
 
+    /** analyse whole program, run this after all TUs has been scanned. */
+    void analyseWholeProgram();
+
 private:
 
     /** @brief There has been a internal error => Report information message */
@@ -210,6 +214,9 @@ private:
 
     /** Simplify code? true by default */
     bool _simplify;
+
+    /** File info used for whole program analysis */
+    std::list<Check::FileInfo*> fileInfo;
 };
 
 /// @}

test/testuninitvar.cpp

@@ -1735,7 +1735,7 @@ private:
 
         std::set<std::string> f;
         const CheckUninitVar check((const Tokenizer *)0, (const Settings *)0, (ErrorLogger *)0);
-        check.analyse(tokenizer.tokens(), f);
+        check.analyseFunctions(&tokenizer, f);
 
         std::string ret;
         for (std::set<std::string>::const_iterator it = f.begin(); it != f.end(); ++it)