ValueFlow: extracted valueFlowConditionExpressions() into separate …

…file
danmar · Sep 27, 2024 · 4f47109 · 4f47109
1 parent c348128
commit 4f47109
Show file tree

Hide file tree

Showing 16 changed files with 328 additions and 234 deletions.
diff --git a/Makefile b/Makefile
@@ -259,6 +259,7 @@ LIBOBJ =      $(libcppdir)/valueflow.o \
               $(libcppdir)/vf_bailout.o \
               $(libcppdir)/vf_bitand.o \
               $(libcppdir)/vf_common.o \
+              $(libcppdir)/vf_conditionexpressions.o \
               $(libcppdir)/vf_debug.o \
               $(libcppdir)/vf_enumvalue.o \
               $(libcppdir)/vf_functionreturn.o \
@@ -487,7 +488,7 @@ validateRules:
 
 ###### Build
 
-$(libcppdir)/valueflow.o: lib/valueflow.cpp lib/addoninfo.h lib/analyzer.h lib/astutils.h lib/check.h lib/checkuninitvar.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/findtoken.h lib/forwardanalyzer.h lib/infer.h lib/library.h lib/mathlib.h lib/platform.h lib/programmemory.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h lib/valueptr.h lib/vf_analyze.h lib/vf_analyzers.h lib/vf_array.h lib/vf_arraybool.h lib/vf_arrayelement.h lib/vf_bailout.h lib/vf_bitand.h lib/vf_common.h lib/vf_debug.h lib/vf_enumvalue.h lib/vf_functionreturn.h lib/vf_globalconstvar.h lib/vf_globalstaticvar.h lib/vf_impossiblevalues.h lib/vf_infercondition.h lib/vf_iteratorinfer.h lib/vf_iterators.h lib/vf_number.h lib/vf_pointeralias.h lib/vf_reverse.h lib/vf_rightshift.h lib/vf_sameexpressions.h lib/vf_settokenvalue.h lib/vf_string.h lib/vf_switchvariable.h lib/vf_symbolicinfer.h lib/vf_unknownfunctionreturn.h lib/vfvalue.h
+$(libcppdir)/valueflow.o: lib/valueflow.cpp lib/addoninfo.h lib/analyzer.h lib/astutils.h lib/check.h lib/checkuninitvar.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/findtoken.h lib/forwardanalyzer.h lib/infer.h lib/library.h lib/mathlib.h lib/platform.h lib/programmemory.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h lib/valueptr.h lib/vf_analyze.h lib/vf_analyzers.h lib/vf_array.h lib/vf_arraybool.h lib/vf_arrayelement.h lib/vf_bailout.h lib/vf_bitand.h lib/vf_common.h lib/vf_conditionexpressions.h lib/vf_debug.h lib/vf_enumvalue.h lib/vf_functionreturn.h lib/vf_globalconstvar.h lib/vf_globalstaticvar.h lib/vf_impossiblevalues.h lib/vf_infercondition.h lib/vf_iteratorinfer.h lib/vf_iterators.h lib/vf_number.h lib/vf_pointeralias.h lib/vf_reverse.h lib/vf_rightshift.h lib/vf_sameexpressions.h lib/vf_settokenvalue.h lib/vf_string.h lib/vf_switchvariable.h lib/vf_symbolicinfer.h lib/vf_unknownfunctionreturn.h lib/vfvalue.h
 	$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/valueflow.cpp
 
 $(libcppdir)/tokenize.o: lib/tokenize.cpp externals/simplecpp/simplecpp.h lib/addoninfo.h lib/astutils.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/preprocessor.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/summaries.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h lib/vfvalue.h
@@ -694,6 +695,9 @@ $(libcppdir)/vf_bitand.o: lib/vf_bitand.cpp lib/config.h lib/errortypes.h lib/ma
 $(libcppdir)/vf_common.o: lib/vf_common.cpp lib/addoninfo.h lib/astutils.h lib/config.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenlist.h lib/utils.h lib/valueflow.h lib/vf_common.h lib/vf_settokenvalue.h lib/vfvalue.h
 	$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/vf_common.cpp
 
+$(libcppdir)/vf_conditionexpressions.o: lib/vf_conditionexpressions.cpp lib/addoninfo.h lib/analyzer.h lib/astutils.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/forwardanalyzer.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenlist.h lib/utils.h lib/valueptr.h lib/vf_analyzers.h lib/vf_bailout.h lib/vf_common.h lib/vf_conditionexpressions.h lib/vfvalue.h
+	$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/vf_conditionexpressions.cpp
+
 $(libcppdir)/vf_debug.o: lib/vf_debug.cpp lib/addoninfo.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/sourcelocation.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenlist.h lib/utils.h lib/vf_common.h lib/vf_debug.h lib/vfvalue.h
 	$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/vf_debug.cpp
 

diff --git a/lib/cppcheck.vcxproj b/lib/cppcheck.vcxproj
@@ -94,6 +94,7 @@
     <ClCompile Include="vf_bailout.cpp" />
     <ClCompile Include="vf_bitand.cpp" />
     <ClCompile Include="vf_common.cpp" />
+    <ClCompile Include="vf_conditionexpressions.cpp" />
     <ClCompile Include="vf_debug.cpp" />
     <ClCompile Include="vf_enumvalue.cpp" />
     <ClCompile Include="vf_functionreturn.cpp" />
@@ -202,6 +203,7 @@
     <ClInclude Include="vf_bailout.h" />
     <ClInclude Include="vf_bitand.h" />
     <ClInclude Include="vf_common.h" />
+    <ClInclude Include="vf_conditionexpressions.h" />
     <ClInclude Include="vf_debug.h" />
     <ClInclude Include="vf_enumvalue.h" />
     <ClInclude Include="vf_functionreturn.h" />

diff --git a/lib/lib.pri b/lib/lib.pri
@@ -87,6 +87,7 @@ HEADERS += $${PWD}/addoninfo.h \
            $${PWD}/vf_bailout.h \
            $${PWD}/vf_bitand.h \
            $${PWD}/vf_common.h \
+           $${PWD}/vf_conditionexpressions.h \
            $${PWD}/vf_debug.h \
            $${PWD}/vf_enumvalue.h \
            $${PWD}/vf_functionreturn.h \
@@ -178,6 +179,7 @@ SOURCES += $${PWD}/valueflow.cpp \
            $${PWD}/vf_bailout.cpp \
            $${PWD}/vf_bitand.cpp \
            $${PWD}/vf_common.cpp \
+           $${PWD}/vf_conditionexpressions.cpp \
            $${PWD}/vf_debug.cpp \
            $${PWD}/vf_enumvalue.cpp \
            $${PWD}/vf_functionreturn.cpp \

diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp
@@ -335,20 +335,6 @@ const Token *ValueFlow::parseCompareInt(const Token *tok, ValueFlow::Value &true
     });
 }
 
-static bool isEscapeScope(const Token* tok, const Settings& settings, bool unknown = false)
-{
-    if (!Token::simpleMatch(tok, "{"))
-        return false;
-    // TODO this search for termTok in all subscopes. It should check the end of the scope.
-    const Token * termTok = Token::findmatch(tok, "return|continue|break|throw|goto", tok->link());
-    if (termTok && termTok->scope() == tok->scope())
-        return true;
-    std::string unknownFunction;
-    if (settings.library.isScopeNoReturn(tok->link(), &unknownFunction))
-        return unknownFunction.empty() || unknown;
-    return false;
-}
-
 void ValueFlow::combineValueProperties(const ValueFlow::Value &value1, const ValueFlow::Value &value2, ValueFlow::Value &result)
 {
     if (value1.isKnown() && value2.isKnown())
@@ -2479,167 +2465,6 @@ static void valueFlowAfterMove(const TokenList& tokenlist, const SymbolDatabase&
     }
 }
 
-static const Token* findIncompleteVar(const Token* start, const Token* end)
-{
-    for (const Token* tok = start; tok != end; tok = tok->next()) {
-        if (tok->isIncompleteVar())
-            return tok;
-    }
-    return nullptr;
-}
-
-static ValueFlow::Value makeConditionValue(long long val,
-                                           const Token* condTok,
-                                           bool assume,
-                                           bool impossible,
-                                           const Settings& settings,
-                                           SourceLocation loc = SourceLocation::current())
-{
-    ValueFlow::Value v(val);
-    v.setKnown();
-    if (impossible) {
-        v.intvalue = !v.intvalue;
-        v.setImpossible();
-    }
-    v.condition = condTok;
-    if (assume)
-        v.errorPath.emplace_back(condTok, "Assuming condition '" + condTok->expressionString() + "' is true");
-    else
-        v.errorPath.emplace_back(condTok, "Assuming condition '" + condTok->expressionString() + "' is false");
-    if (settings.debugnormal)
-        setSourceLocation(v, loc, condTok);
-    return v;
-}
-
-static std::vector<const Token*> getConditions(const Token* tok, const char* op)
-{
-    std::vector<const Token*> conds = {tok};
-    if (tok->str() == op) {
-        std::vector<const Token*> args = astFlatten(tok, op);
-        std::copy_if(args.cbegin(), args.cend(), std::back_inserter(conds), [&](const Token* tok2) {
-            if (tok2->exprId() == 0)
-                return false;
-            if (tok2->hasKnownIntValue())
-                return false;
-            if (Token::Match(tok2, "%var%|.") && !astIsBool(tok2))
-                return false;
-            return true;
-        });
-    }
-    return conds;
-}
-
-static bool isBreakOrContinueScope(const Token* endToken)
-{
-    if (!Token::simpleMatch(endToken, "}"))
-        return false;
-    return Token::Match(endToken->tokAt(-2), "break|continue ;");
-}
-
-static const Scope* getLoopScope(const Token* tok)
-{
-    if (!tok)
-        return nullptr;
-    const Scope* scope = tok->scope();
-    while (scope && scope->type != Scope::eWhile && scope->type != Scope::eFor && scope->type != Scope::eDo)
-        scope = scope->nestedIn;
-    return scope;
-}
-
-//
-static void valueFlowConditionExpressions(const TokenList &tokenlist, const SymbolDatabase& symboldatabase, ErrorLogger &errorLogger, const Settings &settings)
-{
-    if (!settings.daca && !settings.vfOptions.doConditionExpressionAnalysis)
-    {
-        if (settings.debugwarnings) {
-            ErrorMessage::FileLocation loc(tokenlist.getSourceFilePath(), 0, 0);
-            const ErrorMessage errmsg({std::move(loc)}, tokenlist.getSourceFilePath(), Severity::debug, "Analysis of condition expressions is disabled. Use --check-level=exhaustive to enable it.", "normalCheckLevelConditionExpressions", Certainty::normal);
-            errorLogger.reportErr(errmsg);
-        }
-        return;
-    }
-
-    for (const Scope * scope : symboldatabase.functionScopes) {
-        if (const Token* incompleteTok = findIncompleteVar(scope->bodyStart, scope->bodyEnd)) {
-            if (settings.debugwarnings)
-                bailoutIncompleteVar(tokenlist, errorLogger, incompleteTok, "Skipping function due to incomplete variable " + incompleteTok->str());
-            continue;
-        }
-
-        if (settings.daca && !settings.vfOptions.doConditionExpressionAnalysis)
-            continue;
-
-        for (auto* tok = const_cast<Token*>(scope->bodyStart); tok != scope->bodyEnd; tok = tok->next()) {
-            if (!Token::simpleMatch(tok, "if ("))
-                continue;
-            Token* parenTok = tok->next();
-            if (!Token::simpleMatch(parenTok->link(), ") {"))
-                continue;
-            Token * blockTok = parenTok->link()->tokAt(1);
-            const Token* condTok = parenTok->astOperand2();
-            if (condTok->exprId() == 0)
-                continue;
-            if (condTok->hasKnownIntValue())
-                continue;
-            if (!isConstExpression(condTok, settings.library))
-                continue;
-            const bool isOp = condTok->isComparisonOp() || condTok->tokType() == Token::eLogicalOp;
-            const bool is1 = isOp || astIsBool(condTok);
-
-            Token* startTok = blockTok;
-            // Inner condition
-            {
-                for (const Token* condTok2 : getConditions(condTok, "&&")) {
-                    if (is1) {
-                        const bool isBool = astIsBool(condTok2) || Token::Match(condTok2, "%comp%|%oror%|&&");
-                        auto a1 = makeSameExpressionAnalyzer(condTok2, makeConditionValue(1, condTok2, /*assume*/ true, !isBool, settings), settings); // don't set '1' for non-boolean expressions
-                        valueFlowGenericForward(startTok, startTok->link(), a1, tokenlist, errorLogger, settings);
-                    }
-
-                    auto a2 = makeOppositeExpressionAnalyzer(true, condTok2, makeConditionValue(0, condTok2, true, false, settings), settings);
-                    valueFlowGenericForward(startTok, startTok->link(), a2, tokenlist, errorLogger, settings);
-                }
-            }
-
-            std::vector<const Token*> conds = getConditions(condTok, "||");
-
-            // Check else block
-            if (Token::simpleMatch(startTok->link(), "} else {")) {
-                startTok = startTok->link()->tokAt(2);
-                for (const Token* condTok2:conds) {
-                    auto a1 = makeSameExpressionAnalyzer(condTok2, makeConditionValue(0, condTok2, false, false, settings), settings);
-                    valueFlowGenericForward(startTok, startTok->link(), a1, tokenlist, errorLogger, settings);
-
-                    if (is1) {
-                        auto a2 = makeOppositeExpressionAnalyzer(true, condTok2, makeConditionValue(isOp, condTok2, false, false, settings), settings);
-                        valueFlowGenericForward(startTok, startTok->link(), a2, tokenlist, errorLogger, settings);
-                    }
-                }
-            }
-
-            // Check if the block terminates early
-            if (isEscapeScope(blockTok, settings)) {
-                const Scope* scope2 = scope;
-                // If escaping a loop then only use the loop scope
-                if (isBreakOrContinueScope(blockTok->link())) {
-                    scope2 = getLoopScope(blockTok->link());
-                    if (!scope2)
-                        continue;
-                }
-                for (const Token* condTok2:conds) {
-                    auto a1 = makeSameExpressionAnalyzer(condTok2, makeConditionValue(0, condTok2, false, false, settings), settings);
-                    valueFlowGenericForward(startTok->link()->next(), scope2->bodyEnd, a1, tokenlist, errorLogger, settings);
-
-                    if (is1) {
-                        auto a2 = makeOppositeExpressionAnalyzer(true, condTok2, makeConditionValue(1, condTok2, false, false, settings), settings);
-                        valueFlowGenericForward(startTok->link()->next(), scope2->bodyEnd, a2, tokenlist, errorLogger, settings);
-                    }
-                }
-            }
-        }
-    }
-}
-
 static bool isTruncated(const ValueType* src, const ValueType* dst, const Settings& settings)
 {
     if (src->pointer > 0 || dst->pointer > 0)
@@ -3981,8 +3806,8 @@ struct ConditionHandler {
                 const bool isKnown = std::any_of(values.cbegin(), values.cend(), [&](const ValueFlow::Value& v) {
                     return v.isKnown() || v.isImpossible();
                 });
-                if (isKnown && isBreakOrContinueScope(after)) {
-                    const Scope* loopScope = getLoopScope(cond.vartok);
+                if (isKnown && ValueFlow::isBreakOrContinueScope(after)) {
+                    const Scope* loopScope = ValueFlow::getLoopScope(cond.vartok);
                     if (loopScope) {
                         Analyzer::Result r = forward(after, loopScope->bodyEnd, cond.vartok, values, tokenlist, errorLogger, settings);
                         if (r.terminate != Analyzer::Terminate::None)
@@ -6007,7 +5832,7 @@ void ValueFlow::setValues(TokenList& tokenlist,
         VFA(valueFlowSymbolic(tokenlist, symboldatabase, errorLogger, settings)),
         VFA(analyzeBitAnd(tokenlist, settings)),
         VFA(analyzeSameExpressions(tokenlist, settings)),
-        VFA(valueFlowConditionExpressions(tokenlist, symboldatabase, errorLogger, settings)),
+        VFA(analyzeConditionExpressions(tokenlist, symboldatabase, errorLogger, settings)),
     });
 
     runner.run({

diff --git a/lib/vf_analyze.h b/lib/vf_analyze.h
@@ -23,6 +23,7 @@
 #include "vf_arraybool.h" // IWYU pragma: export
 #include "vf_arrayelement.h" // IWYU pragma: export
 #include "vf_bitand.h" // IWYU pragma: export
+#include "vf_conditionexpressions.h" // IWYU pragma: export
 #include "vf_debug.h" // IWYU pragma: export
 #include "vf_enumvalue.h" // IWYU pragma: export
 #include "vf_functionreturn.h" // IWYU pragma: export

diff --git a/lib/vf_common.cpp b/lib/vf_common.cpp
@@ -420,4 +420,21 @@ namespace ValueFlow
         }
         return 0;
     }
+
+    bool isBreakOrContinueScope(const Token* endToken)
+    {
+        if (!Token::simpleMatch(endToken, "}"))
+            return false;
+        return Token::Match(endToken->tokAt(-2), "break|continue ;");
+    }
+
+    const Scope* getLoopScope(const Token* tok)
+    {
+        if (!tok)
+            return nullptr;
+        const Scope* scope = tok->scope();
+        while (scope && scope->type != Scope::eWhile && scope->type != Scope::eFor && scope->type != Scope::eDo)
+            scope = scope->nestedIn;
+        return scope;
+    }
 }
diff --git a/lib/vf_common.h b/lib/vf_common.h
@@ -55,6 +55,10 @@ namespace ValueFlow
     std::list<Value> getIteratorValues(std::list<Value> values, const Value::ValueKind* kind = nullptr);
 
     MathLib::bigint valueFlowGetStrLength(const Token* tok);
+
+    bool isBreakOrContinueScope(const Token* endToken);
+
+    const Scope* getLoopScope(const Token* tok);
 }
 
 #endif // vfCommonH