Fix for 6597: false negative: uninitialized variable usage not detected (ValueFlow , multi variables) (#3535)

Makefile

@@ -546,7 +546,7 @@ $(libcppdir)/platform.o: lib/platform.cpp externals/tinyxml2/tinyxml2.h lib/conf
 $(libcppdir)/preprocessor.o: lib/preprocessor.cpp externals/simplecpp/simplecpp.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/preprocessor.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h
 	$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(libcppdir)/preprocessor.o $(libcppdir)/preprocessor.cpp
 
-$(libcppdir)/programmemory.o: lib/programmemory.cpp lib/astutils.h lib/calculate.h lib/config.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/programmemory.h lib/settings.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/utils.h lib/valueflow.h
+$(libcppdir)/programmemory.o: lib/programmemory.cpp lib/astutils.h lib/calculate.h lib/config.h lib/errortypes.h lib/importproject.h lib/infer.h lib/library.h lib/mathlib.h lib/platform.h lib/programmemory.h lib/settings.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/utils.h lib/valueflow.h lib/valueptr.h
 	$(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(libcppdir)/programmemory.o $(libcppdir)/programmemory.cpp
 
 $(libcppdir)/reverseanalyzer.o: lib/reverseanalyzer.cpp lib/analyzer.h lib/astutils.h lib/config.h lib/errortypes.h lib/forwardanalyzer.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/reverseanalyzer.h lib/settings.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/utils.h lib/valueflow.h lib/valueptr.h

lib/astutils.cpp

@@ -1080,6 +1080,8 @@ static bool isUsedAsBool_internal(const Token * const tok, bool checkingParent)
         }
     } else if (isForLoopCondition(tok))
         return true;
+    else if (Token::simpleMatch(parent, "?") && astIsLHS(tok))
+        return true;
 
     return isUsedAsBool_internal(parent, true);
 }

lib/programmemory.cpp

@@ -3,11 +3,13 @@
 #include "astutils.h"
 #include "calculate.h"
 #include "errortypes.h"
+#include "infer.h"
 #include "mathlib.h"
 #include "settings.h"
 #include "symboldatabase.h"
 #include "token.h"
 #include "valueflow.h"
+#include "valueptr.h"
 #include <algorithm>
 #include <cassert>
 #include <cmath>
@@ -215,11 +217,9 @@ void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, const Toke
                 programMemoryParseCondition(pm, tok->astOperand1(), endTok, settings, then);
         }
     } else if (tok->exprId() > 0) {
-        if (then && !astIsPointer(tok) && !astIsBool(tok))
-            return;
         if (endTok && isExpressionChanged(tok, tok->next(), endTok, settings, true))
             return;
-        pm.setIntValue(tok->exprId(), then);
+        pm.setIntValue(tok->exprId(), 0, then);
         const Token* containerTok = settings->library.getContainerFromYield(tok, Library::Container::Yield::EMPTY);
         if (containerTok)
             pm.setContainerSizeValue(containerTok->exprId(), 0, then);
@@ -637,14 +637,18 @@ static ValueFlow::Value execute(const Token* expr, ProgramMemory& pm)
         if (!lhs.isUninitValue() && !rhs.isUninitValue())
             return evaluate(expr->str(), lhs, rhs);
         if (expr->isComparisonOp()) {
-            if (rhs.isIntValue() && !rhs.isImpossible()) {
+            if (rhs.isIntValue()) {
-                ValueFlow::Value v = inferCondition(expr->str(), expr->astOperand1(), rhs.intvalue);
+                std::vector<ValueFlow::Value> result =
-                if (v.isKnown())
+                    infer(makeIntegralInferModel(), expr->str(), expr->astOperand1()->values(), {rhs});
-                    return v;
+                if (result.empty())
-            } else if (lhs.isIntValue() && !lhs.isImpossible()) {
+                    return unknown;
-                ValueFlow::Value v = inferCondition(expr->str(), lhs.intvalue, expr->astOperand2());
+                return result.front();
-                if (v.isKnown())
+            } else if (lhs.isIntValue()) {
-                    return v;
+                std::vector<ValueFlow::Value> result =
+                    infer(makeIntegralInferModel(), expr->str(), {lhs}, expr->astOperand2()->values());
+                if (result.empty())
+                    return unknown;
+                return result.front();
             }
         }
     }
@@ -674,7 +678,12 @@ static ValueFlow::Value execute(const Token* expr, ProgramMemory& pm)
             return execute(expr->astOperand1(), pm);
     }
     if (expr->exprId() > 0 && pm.hasValue(expr->exprId())) {
-        return pm.values.at(expr->exprId());
+        ValueFlow::Value result = pm.values.at(expr->exprId());
+        if (result.isImpossible() && result.isIntValue() && result.intvalue == 0 && isUsedAsBool(expr)) {
+            result.intvalue = !result.intvalue;
+            result.setKnown();
+        }
+        return result;
     }
 
     return unknown;

lib/valueflow.cpp

@@ -5465,6 +5465,10 @@ struct IntegralInferModel : InferModel {
     }
 };
 
+ValuePtr<InferModel> makeIntegralInferModel() {
+    return IntegralInferModel{};
+}
+
 ValueFlow::Value inferCondition(const std::string& op, const Token* varTok, MathLib::bigint val)
 {
     if (!varTok)

lib/valueflow.h

@@ -35,6 +35,7 @@
 #include <vector>
 
 class ErrorLogger;
+struct InferModel;
 class Settings;
 class SymbolDatabase;
 class Token;
@@ -42,6 +43,9 @@ class TokenList;
 class ValueType;
 class Variable;
 
+template<class T>
+class ValuePtr;
+
 namespace ValueFlow {
     struct increment {
         template<class T>
@@ -484,6 +488,8 @@ const Token *parseCompareInt(const Token *tok, ValueFlow::Value &true_value, Val
 ValueFlow::Value inferCondition(std::string op, MathLib::bigint val, const Token* varTok);
 ValueFlow::Value inferCondition(const std::string& op, const Token* varTok, MathLib::bigint val);
 
+ValuePtr<InferModel> makeIntegralInferModel();
+
 std::vector<LifetimeToken> getLifetimeTokens(const Token* tok,
                                              bool escape = false,
                                              ValueFlow::Value::ErrorPath errorPath = ValueFlow::Value::ErrorPath{});

test/testuninitvar.cpp

@@ -63,6 +63,7 @@ private:
         TEST_CASE(uninitvar5);          // #3861
         TEST_CASE(uninitvar2_func);     // function calls
         TEST_CASE(uninitvar2_value);    // value flow
+        TEST_CASE(valueFlowUninit2_value);
         TEST_CASE(uninitStructMember);  // struct members
         TEST_CASE(uninitvar2_while);
         TEST_CASE(uninitvar2_4494);      // #4494
@@ -3308,13 +3309,6 @@ private:
                        "}");
         ASSERT_EQUALS("", errout.str());
 
-        checkUninitVar("void f(int x) {\n"
-                       "    int i;\n"
-                       "    if (!x) i = 0;\n"
-                       "    if (!x || i>0) {}\n" // <- error
-                       "}");
-        TODO_ASSERT_EQUALS("[test.cpp:4]: (error) Uninitialized variable: i\n", "", errout.str());
-
         checkUninitVar("void f(int x) {\n"
                        "    int i;\n"
                        "    if (x) i = 0;\n"
@@ -3396,6 +3390,127 @@ private:
         TODO_ASSERT_EQUALS("error", "", errout.str());
     }
 
+    void valueFlowUninit2_value()
+    {
+        valueFlowUninit("void f() {\n"
+                        "    int i;\n"
+                        "    if (x) {\n"
+                        "        int y = -ENOMEM;\n" // assume constant ENOMEM is nonzero since it's negated
+                        "        if (y != 0) return;\n"
+                        "        i++;\n"
+                        "    }\n"
+                        "}",
+                        "test.cpp");
+        ASSERT_EQUALS("", errout.str());
+
+        valueFlowUninit("void f() {\n"
+                        "    int i, y;\n"
+                        "    if (x) {\n"
+                        "        y = -ENOMEM;\n" // assume constant ENOMEM is nonzero since it's negated
+                        "        if (y != 0) return;\n"
+                        "        i++;\n"
+                        "    }\n"
+                        "}",
+                        "test.cpp");
+        ASSERT_EQUALS("", errout.str());
+
+        valueFlowUninit("void f() {\n"
+                        "    int i, y;\n"
+                        "    if (x) y = -ENOMEM;\n" // assume constant ENOMEM is nonzero since it's negated
+                        "    else y = get_value(i);\n"
+                        "    if (y != 0) return;\n" // <- condition is always true if i is uninitialized
+                        "    i++;\n"
+                        "}");
+        ASSERT_EQUALS("", errout.str());
+
+        valueFlowUninit("void f(int x) {\n"
+                        "    int i;\n"
+                        "    if (!x) i = 0;\n"
+                        "    if (!x || i>0) {}\n" // <- error
+                        "}");
+        ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (error) Uninitialized variable: i\n", errout.str());
+
+        valueFlowUninit("void f(int x) {\n"
+                        "    int i;\n"
+                        "    if (x) i = 0;\n"
+                        "    if (!x || i>0) {}\n" // <- no error
+                        "}");
+        ASSERT_EQUALS("", errout.str());
+
+        valueFlowUninit("void f(int x) {\n"
+                        "    int i;\n"
+                        "    if (!x) { }\n"
+                        "    else i = 0;\n"
+                        "    if (x || i>0) {}\n"
+                        "}");
+        ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:5]: (error) Uninitialized variable: i\n", errout.str());
+
+        valueFlowUninit("void f(int x) {\n"
+                        "    int i;\n"
+                        "    if (x) { }\n"
+                        "    else i = 0;\n"
+                        "    if (x || i>0) {}\n" // <- no error
+                        "}");
+        ASSERT_EQUALS("", errout.str());
+
+        valueFlowUninit("int f(int x) {\n"
+                        "    int y;\n"
+                        "    if (x) y = do_something();\n"
+                        "    if (!x) return 0;\n"
+                        "    return y;\n"
+                        "}");
+        ASSERT_EQUALS("", errout.str());
+
+        // extracttests.start: int y;
+        valueFlowUninit("int f(int x) {\n" // FP with ?:
+                        "    int a;\n"
+                        "    if (x)\n"
+                        "        a = y;\n"
+                        "    return x ? 2*a : 0;\n"
+                        "}");
+        ASSERT_EQUALS("", errout.str());
+
+        valueFlowUninit("int f(int x) {\n"
+                        "    int a;\n"
+                        "    if (x)\n"
+                        "        a = y;\n"
+                        "    return y ? 2*a : 3*a;\n"
+                        "}");
+        ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:5]: (error) Uninitialized variable: a\n", errout.str());
+
+        valueFlowUninit("void f() {\n" // Don't crash
+                        "    int a;\n"
+                        "    dostuff(\"ab\" cd \"ef\", x?a:z);\n" // <- No AST is created for ?:
+                        "}");
+
+        // Unknown => bail out..
+        valueFlowUninit("void f(int x) {\n"
+                        "    int i;\n"
+                        "    if (a(x)) i = 0;\n"
+                        "    if (b(x)) return;\n"
+                        "    i++;\n" // <- no error if b(x) is always true when a(x) is false
+                        "}");
+        ASSERT_EQUALS("", errout.str());
+
+        valueFlowUninit("void f(int x) {\n"
+                        "    int i;\n"
+                        "    if (x) i = 0;\n"
+                        "    while (condition) {\n"
+                        "        if (x) i++;\n" // <- no error
+                        "    }\n"
+                        "}");
+        ASSERT_EQUALS("", errout.str());
+
+        valueFlowUninit("void f(int x) {\n"
+                        "    int i;\n"
+                        "    if (x) i = 0;\n"
+                        "    while (condition) {\n"
+                        "        i++;\n"
+                        "    }\n"
+                        "}");
+        TODO_ASSERT_EQUALS("error", "", errout.str());
+    }
+
     void uninitStructMember() { // struct members
         checkUninitVar("struct AB { int a; int b; };\n"
                        "void f(void) {\n"
@@ -4954,6 +5069,18 @@ private:
                         "    return out;\n"
                         "}\n");
         ASSERT_EQUALS("", errout.str());
+
+        // #6597
+        valueFlowUninit("int f(int b) {\n"
+                        "    int a;\n"
+                        "    if (!b)\n"
+                        "        a = 1;\n"
+                        "    if (b)\n"
+                        "        return a;\n"
+                        "    else\n"
+                        "        return -1;\n"
+                        "}\n");
+        ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:6]: (error) Uninitialized variable: a\n", errout.str());
     }
 
     void valueFlowUninitBreak() { // Do not show duplicate warnings about the same uninitialized value

tools/extracttests.py

@@ -355,10 +355,7 @@ if filename is not None:
         if not os.path.exists(codedir):
             os.mkdir(codedir)
 
-        testfile = filename
+        testfile = os.path.splitext(os.path.basename(filename))[0]
-        if testfile.find('/'):
-            testfile = testfile[testfile.rfind('/'):]
-        testfile = testfile[:testfile.find('.')]
 
         for node in e.nodes:
             if onlyTP and node['expected'] == '':

tools/generate_and_run_more_tests.sh

@@ -3,17 +3,19 @@
 # cd ~/cppcheck
 # tools/generate_and_run_more_tests.sh
 
+DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+
 set -e
 
 echo testleakautovar
-tools/run_more_tests.sh test/testleakautovar.cpp
+$DIR/run_more_tests.sh $DIR/../test/testleakautovar.cpp
 
 echo testmemleak
-tools/run_more_tests.sh test/testmemleak.cpp
+$DIR/run_more_tests.sh $DIR/../test/testmemleak.cpp
 
 echo testnullpointer
-tools/run_more_tests.sh test/testnullpointer.cpp
+$DIR/run_more_tests.sh $DIR/../test/testnullpointer.cpp
 
 echo testuninitvar
-tools/run_more_tests.sh test/testuninitvar.cpp
+$DIR/run_more_tests.sh $DIR/../test/testuninitvar.cpp
 

tools/run_more_tests.sh

@@ -1,96 +1,104 @@
 #!/bin/bash
 # Script Used by generate_and_run_more_tests.sh
 
+DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+
 set -e
 
-python tools/extracttests.py --code=test1 $1
+CPPCHECK=$(pwd)/cppcheck
+
+if test -f ./bin/cppcheck; then
+    CPPCHECK=$(pwd)/bin/cppcheck
+fi
+
+python $DIR/extracttests.py --code=$(pwd)/test1 $1
 
 cd test1
 
-../cppcheck -q --template=cppcheck1 . 2> 1.txt
+$CPPCHECK -q --template=cppcheck1 . 2> 1.txt
 
 
 # (!x) => (x==0)
 sed  -ri 's/([(&][ ]*)\!([a-z]+)([ ]*[&)])/\1\2==0\3/' *.cpp
-../cppcheck -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
+$CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
 
 # (x==0) => (0==x)
 sed  -ri 's/([(&][ ]*)([a-z]+)[ ]*==[ ]*0([ ]*[&)])/\10==\2\3/' *.cpp
-../cppcheck -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
+$CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
 
 # (0==x) => (!x)
 sed  -ri 's/([(&][ ]*)0[ ]*==[ ]*([a-z]+)([ ]*[&)])/\1!\2\3/' *.cpp
-../cppcheck -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
+$CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
 
 
 
 
 # if (x) => (x!=0)
 sed  -ri 's/(if[ ]*\([ ]*[a-z]+)([ ]*[&)])/\1!=0\2/' *.cpp
-../cppcheck -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
+$CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
 
 # while (x) => (x!=0)
 sed  -ri 's/(while[ ]*\([ ]*[a-z]+)([ ]*[&)])/\1!=0\2/' *.cpp
-../cppcheck -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
+$CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
 
 # (x!=0) => (0!=x)
 sed  -ri 's/([(&][ ]*)([a-z]+)[ ]*!=[ ]*0([ ]*[&)])/\10!=\2\3/' *.cpp
-../cppcheck -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
+$CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
 
 # (0!=x) => (x)
 sed  -ri 's/([(&][ ]*)0[ ]*!=[ ]*([a-z]+[ ]*[&)])/\1\2/' *.cpp
-../cppcheck -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
+$CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
 
 
 # (x < 0) => (0 > x)
 sed  -ri 's/([(&][ ]*)([a-z]+)[ ]*<[ ]*(\-?[0-9]+)([ ]*[&)])/\1\3>\2\4/' *.cpp
-../cppcheck -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
+$CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
 
 # (x <= 0) => (0 >= x)
 sed  -ri 's/([(&][ ]*)([a-z]+)[ ]*<=[ ]*(\-?[0-9]+)([ ]*[&)])/\1\3>=\2\4/' *.cpp
-../cppcheck -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
+$CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
 
 # (x > 0) => (0 < x)
 sed  -ri 's/([(&][ ]*)([a-z]+)[ ]*<=[ ]*(\-?[0-9]+)([ ]*[&)])/\1\3>=\2\4/' *.cpp
-../cppcheck -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
+$CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
 
 # (x >= 0) => (0 <= x)
 sed  -ri 's/([(&][ ]*)([a-z]+)[ ]*<=[ ]*(\-?[0-9]+)([ ]*[&)])/\1\3>=\2\4/' *.cpp
-../cppcheck -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
+$CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
 
 # (x == 123) => (123 == x)
 sed  -ri 's/([(&][ ]*)([a-z]+)[ ]*==[ ]*(\-?[0-9]+)([ ]*[&)])/\1\3==\2\4/' *.cpp
-../cppcheck -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
+$CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
 
 # (x != 123) => (123 != x)
 sed  -ri 's/([(&][ ]*)([a-z]+)[ ]*\!=[ ]*(\-?[0-9]+)([ ]*[&)])/\1\3!=\2\4/' *.cpp
-../cppcheck -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
+$CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
 
 
 
 # (0 < x) => (x > 0)
 sed  -ri 's/([(&][ ]*)(\-?[0-9]+)[ ]*<[ ]*([a-z]+)([ ]*[&)])/\1\3>\2\4/' *.cpp
-../cppcheck -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
+$CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
 
 # (0 <= x) => (x >= 0)
 sed  -ri 's/([(&][ ]*)(\-?[0-9]+)[ ]*<=[ ]*([a-z]+)([ ]*[&)])/\1\3>=\2\4/' *.cpp
-../cppcheck -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
+$CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
 
 # (0 > x) => (x < 0)
 sed  -ri 's/([(&][ ]*)(\-?[0-9]+)[ ]*<=[ ]*([a-z]+)([ ]*[&)])/\1\3>=\2\4/' *.cpp
-../cppcheck -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
+$CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
 
 # (0 >= x) => (x <= 0)
 sed  -ri 's/([(&][ ]*)(\-?[0-9]+)[ ]*<=[ ]*([a-z]+)([ ]*[&)])/\1\3>=\2\4/' *.cpp
-../cppcheck -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
+$CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
 
 # (123 == x) => (x == 123)
 sed  -ri 's/([(&][ ]*)(\-?[0-9]+)[ ]*==[ ]*([a-z]+)([ ]*[&)])/\1\3==\2\4/' *.cpp
-../cppcheck -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
+$CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
 
 # (123 != x) => (x <= 123)
 sed  -ri 's/([(&][ ]*)(\-?[0-9]+)[ ]*\!=[ ]*([a-z]+)([ ]*[&)])/\1\3!=\2\4/' *.cpp
-../cppcheck -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
+$CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt
 
 
 cd ..