--performance-valueflow-max-if-count: review comments

Author: danmar

Diff for: cli/cmdlineparser.cpp

@@ -1220,8 +1220,7 @@ void CmdLineParser::printHelp()
     "                         the number of possible control flow paths through that\n"
     "                         function explodes and that can lead to very long analysis\n"
     "                         time. Valueflow is limited in functions that have more\n"
-    "                         than <limit> conditional scopes. The default limit is\n"
-    "                         100, which few functions will reach.\n"
+    "                         than <limit> conditional scopes.\n"
     "    --platform=<type>, --platform=<file>\n"
     "                         Specifies platform specific types and sizes. The\n"
     "                         available builtin platforms are:\n"

Diff for: lib/valueflow.cpp

@@ -8501,10 +8501,17 @@ static void valueFlowContainerSetTokValue(TokenList* tokenlist, const Settings*
     }
 }
 
+static const Scope* getFunctionScope(const Scope* scope) {
+    while (scope && scope->type != Scope::ScopeType::eFunction)
+        scope = scope->nestedIn;
+    return scope;
+}
+
 static void valueFlowContainerSize(TokenList* tokenlist,
                                    SymbolDatabase* symboldatabase,
                                    ErrorLogger* /*errorLogger*/,
-                                   const Settings* settings)
+                                   const Settings* settings,
+                                   const std::set<const Scope*>& skippedFunctions)
 {
     // declaration
     for (const Variable *var : symboldatabase->variableList()) {
@@ -8516,6 +8523,8 @@ static void valueFlowContainerSize(TokenList* tokenlist,
             continue;
         if (!astIsContainer(var->nameToken()))
             continue;
+        if (skippedFunctions.count(getFunctionScope(var->scope())))
+            continue;
 
         bool known = true;
         int size = 0;
@@ -9077,7 +9086,7 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase,
     const std::uint64_t stopTime = getValueFlowStopTime(settings);
 
     std::set<const Scope*> skippedFunctions;
-    if (settings->performanceValueFlowMaxIfCount < 1000) {
+    if (settings->performanceValueFlowMaxIfCount > 0) {
         for (const Scope* functionScope: symboldatabase->functionScopes) {
             int countIfScopes = 0;
             std::vector<const Scope*> scopes{functionScope};
@@ -9093,15 +9102,17 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase,
             if (countIfScopes > settings->performanceValueFlowMaxIfCount) {
                 skippedFunctions.emplace(functionScope);
 
-                const std::string& functionName = functionScope->className;
-                const std::list<ErrorMessage::FileLocation> callstack(1, ErrorMessage::FileLocation(functionScope->bodyStart, tokenlist));
-                const ErrorMessage errmsg(callstack, tokenlist->getSourceFilePath(), Severity::information,
-                                          "ValueFlow analysis is limited in " + functionName + " because if-count in function " +
-                                          std::to_string(countIfScopes) + " exceeds limit " +
-                                          std::to_string(settings->performanceValueFlowMaxIfCount) + ". The limit can be adjusted with "
-                                          "--performance-valueflow-max-if-count. Increasing the if-count limit will likely increase the "
-                                          "analysis time.", "performanceValueflowMaxIfCountExceeded", Certainty::normal);
-                errorLogger->reportErr(errmsg);
+                if (settings->severity.isEnabled(Severity::information)) {
+                    const std::string& functionName = functionScope->className;
+                    const std::list<ErrorMessage::FileLocation> callstack(1, ErrorMessage::FileLocation(functionScope->bodyStart, tokenlist));
+                    const ErrorMessage errmsg(callstack, tokenlist->getSourceFilePath(), Severity::information,
+                                              "ValueFlow analysis is limited in " + functionName + " because if-count in function " +
+                                              std::to_string(countIfScopes) + " exceeds limit " +
+                                              std::to_string(settings->performanceValueFlowMaxIfCount) + ". The limit can be adjusted with "
+                                              "--performance-valueflow-max-if-count. Increasing the if-count limit will likely increase the "
+                                              "analysis time.", "performanceValueflowMaxIfCountExceeded", Certainty::normal);
+                    errorLogger->reportErr(errmsg);
+                }
             }
         }
     }
@@ -9159,8 +9170,8 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase,
                 valueFlowCondition(IteratorConditionHandler{}, tokenlist, symboldatabase, errorLogger, settings, skippedFunctions);
             if (std::time(nullptr) < stopTime)
                 valueFlowIteratorInfer(tokenlist, settings);
-            if (std::time(nullptr) < stopTime && skippedFunctions.empty())
-                valueFlowContainerSize(tokenlist, symboldatabase, errorLogger, settings);
+            if (std::time(nullptr) < stopTime)
+                valueFlowContainerSize(tokenlist, symboldatabase, errorLogger, settings, skippedFunctions);
             if (std::time(nullptr) < stopTime)
                 valueFlowCondition(ContainerConditionHandler{}, tokenlist, symboldatabase, errorLogger, settings, skippedFunctions);
         }

Diff for: man/manual.md

@@ -957,6 +957,34 @@ Example usage:
     ./cppcheck gui/test.cpp --xml 2> err.xml
     htmlreport/cppcheck-htmlreport --file=err.xml --report-dir=test1 --source-dir=.
 
+# Performance - Limit analysis
+
+## Limit preprocessor configurations
+
+For performance reasons it might be a good idea to limit preprocessor configurations to check.
+
+## Limit ValueFlow: max if count
+
+The command line option `--performance-valueflow-max-if-count` adjusts the max count for number of if in a function.
+
+When that limit is exceeded there is a limitation of data flow in that function. It is not drastic:
+ * Analysis of other functions are not affected.
+ * It's only for some specific data flow analysis, we have data flow analysis that is always executed.
+ * All checks are always executed. There can still be plenty of warnings in the limited function.
+
+There is data flow analysis that slows down exponentially when number of if increase. And the limit is intended to avoid that
+analysis time explodes.
+
+## GUI options
+
+In the GUI there are various options to limit analysis.
+
+In the GUI:
+ * Open the project dialog.
+ * In the "Analysis" tab there are several options.
+
+If you want to use these limitations on the command line also you can import the GUI project file with --project.
+
 # Cppcheck Premium
 
 ## Bug hunting

Diff for: releasenotes.txt

@@ -12,4 +12,4 @@ release notes for cppcheck-2.11
     - `constVariable`
     - `constVariableReference`
     - `constVariablePointer`
-
+- Limit valueflow analysis in function if the "if" count is high. By default max limit is 100. Limit can be tweaked with --performance-valueflow-max-if-count

Diff for: test/testvalueflow.cpp

@@ -164,6 +164,8 @@ class TestValueFlow : public TestFixture {
         TEST_CASE(valueFlowImpossibleMinMax);
         TEST_CASE(valueFlowImpossibleUnknownConstant);
         TEST_CASE(valueFlowContainerEqual);
+
+        TEST_CASE(performanceIfCount);
     }
 
     static bool isNotTokValue(const ValueFlow::Value &val) {
@@ -7942,6 +7944,33 @@ class TestValueFlow : public TestFixture {
         ASSERT_EQUALS(false, testValueOfX(code, 5U, 1));
         ASSERT_EQUALS(false, testValueOfX(code, 5U, 0));
     }
+
+    void performanceIfCount() {
+        Settings s(settings);
+        s.performanceValueFlowMaxIfCount = 1;
+
+        const char *code;
+
+        code = "int f() {\n"
+               "  if (x>0){}\n"
+               "  if (y>0){}\n"
+               "  int a = 14;\n"
+               "  return a+1;\n"
+               "}\n";
+        ASSERT_EQUALS(0U, tokenValues(code, "+", &s).size());
+        ASSERT_EQUALS(1U, tokenValues(code, "+").size());
+
+        // Do not skip all functions
+        code = "void g(int i) {\n"
+               "  if (i == 1) {}\n"
+               "  if (i == 2) {}\n"
+               "}\n"
+               "int f() {\n"
+               "  std::vector<int> v;\n"
+               "  return v.front();\n"
+               "}\n";
+        ASSERT_EQUALS(1U, tokenValues(code, "v .", &s).size());
+    }
 };
 
 REGISTER_TEST(TestValueFlow)