From fd8a7b9537684ec31c5edf3c66799af4df5d9968 Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Fri, 24 Mar 2023 07:31:26 -0500 Subject: [PATCH] ValueFlow: Evaluate if statement for function returns (#4908) --- lib/cppcheck.cpp | 1 + lib/programmemory.cpp | 160 +++++++++-- lib/programmemory.h | 3 + lib/symboldatabase.cpp | 2 + lib/valueflow.cpp | 99 ++++--- test/testsimplifytypedef.cpp | 4 +- test/testuninitvar.cpp | 540 +++++++++++++++++++++++++++++++++++ test/testvalueflow.cpp | 79 +++++ tools/run_more_tests.sh | 39 ++- 9 files changed, 839 insertions(+), 88 deletions(-) diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 893f96fe9..ad7b01a74 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -943,6 +943,7 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string checkUnusedFunctions.parseTokens(tokenizer, filename.c_str(), &mSettings); // handling of "simple" rules has been removed. + // cppcheck-suppress knownConditionTrueFalse if (mSimplify && hasRule("simple")) throw InternalError(nullptr, "Handling of \"simple\" rules has been removed in Cppcheck. Use --addon instead."); diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index 6876c1d87..1cddaee0f 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -233,6 +233,20 @@ static bool frontIs(const std::vector& v, bool i) return !i; } +static bool isTrue(const ValueFlow::Value& v) +{ + if (v.isImpossible()) + return v.intvalue == 0; + return v.intvalue != 0; +} + +static bool isFalse(const ValueFlow::Value& v) +{ + if (v.isImpossible()) + return false; + return v.intvalue == 0; +} + // If the scope is a non-range for loop static bool isBasicForLoop(const Token* tok) { @@ -543,6 +557,7 @@ static bool isIntegralValue(const ValueFlow::Value& value) static ValueFlow::Value evaluate(const std::string& op, const ValueFlow::Value& lhs, const ValueFlow::Value& rhs) { ValueFlow::Value result; + combineValueProperties(lhs, rhs, result); if (lhs.isImpossible() && rhs.isImpossible()) return ValueFlow::Value::unknown(); if (lhs.isImpossible() || rhs.isImpossible()) { @@ -593,11 +608,16 @@ static ValueFlow::Value evaluate(const std::string& op, const ValueFlow::Value& result.intvalue = calculate(op, lhs.intvalue, rhs.intvalue, &error); if (error) return ValueFlow::Value::unknown(); - if (result.isImpossible()) { - if ((result.intvalue == 0 && op == "!=") || (result.intvalue != 0 && op == "==")) { - result.setPossible(); - result.intvalue = !result.intvalue; + if (result.isImpossible() && op == "!=") { + if (isTrue(result)) { + result.intvalue = 1; + } else if (isFalse(result)) { + result.intvalue = 0; + } else { + return ValueFlow::Value::unknown(); } + result.setPossible(); + result.bound = ValueFlow::Value::Bound::Point; } return result; } @@ -1152,7 +1172,10 @@ static ValueFlow::Value executeImpl(const Token* expr, ProgramMemory& pm, const } else if (expr->isNumber()) { if (MathLib::isFloat(expr->str())) return unknown; - return ValueFlow::Value{MathLib::toLongNumber(expr->str())}; + MathLib::bigint i = MathLib::toLongNumber(expr->str()); + if (i < 0 && astIsUnsigned(expr)) + return unknown; + return ValueFlow::Value{i}; } else if (expr->isBoolean()) { return ValueFlow::Value{ expr->str() == "true" }; } else if (Token::Match(expr->tokAt(-2), ". %name% (") && astIsContainer(expr->tokAt(-2)->astOperand1())) { @@ -1197,16 +1220,20 @@ static ValueFlow::Value executeImpl(const Token* expr, ProgramMemory& pm, const ValueFlow::Value lhs = execute(expr->astOperand1(), pm); if (!lhs.isIntValue()) return unknown; - if (lhs.intvalue == 0) + if (isFalse(lhs)) return lhs; - return execute(expr->astOperand2(), pm); + if (isTrue(lhs)) + return execute(expr->astOperand2(), pm); + return unknown; } else if (expr->str() == "||" && expr->astOperand1() && expr->astOperand2()) { ValueFlow::Value lhs = execute(expr->astOperand1(), pm); - if (!lhs.isIntValue()) + if (!lhs.isIntValue() || lhs.isImpossible()) return unknown; - if (lhs.intvalue != 0) + if (isTrue(lhs)) return lhs; - return execute(expr->astOperand2(), pm); + if (isFalse(lhs)) + return execute(expr->astOperand2(), pm); + return unknown; } else if (expr->str() == "," && expr->astOperand1() && expr->astOperand2()) { execute(expr->astOperand1(), pm); return execute(expr->astOperand2(), pm); @@ -1251,31 +1278,42 @@ static ValueFlow::Value executeImpl(const Token* expr, ProgramMemory& pm, const } else if (Token::Match(expr, "%cop%") && expr->astOperand1() && expr->astOperand2()) { ValueFlow::Value lhs = execute(expr->astOperand1(), pm); ValueFlow::Value rhs = execute(expr->astOperand2(), pm); + ValueFlow::Value r = unknown; if (!lhs.isUninitValue() && !rhs.isUninitValue()) - return evaluate(expr->str(), lhs, rhs); - if (expr->isComparisonOp()) { + r = evaluate(expr->str(), lhs, rhs); + if (expr->isComparisonOp() && (r.isUninitValue() || r.isImpossible())) { if (rhs.isIntValue()) { std::vector result = infer(ValueFlow::makeIntegralInferModel(), expr->str(), expr->astOperand1()->values(), {rhs}); - if (result.empty() || !result.front().isKnown()) - return unknown; - return result.front(); - } else if (lhs.isIntValue()) { + if (!result.empty() && result.front().isKnown()) + return result.front(); + } + if (lhs.isIntValue()) { std::vector result = infer(ValueFlow::makeIntegralInferModel(), expr->str(), {lhs}, expr->astOperand2()->values()); - if (result.empty() || !result.front().isKnown()) - return unknown; - return result.front(); + if (!result.empty() && result.front().isKnown()) + return result.front(); } + return unknown; } + return r; } // Unary ops else if (Token::Match(expr, "!|+|-") && expr->astOperand1() && !expr->astOperand2()) { ValueFlow::Value lhs = execute(expr->astOperand1(), pm); if (!lhs.isIntValue()) return unknown; - if (expr->str() == "!") - lhs.intvalue = !lhs.intvalue; + if (expr->str() == "!") { + if (isTrue(lhs)) { + lhs.intvalue = 0; + } else if (isFalse(lhs)) { + lhs.intvalue = 1; + } else { + return unknown; + } + lhs.setPossible(); + lhs.bound = ValueFlow::Value::Bound::Point; + } if (expr->str() == "-") lhs.intvalue = -lhs.intvalue; return lhs; @@ -1284,10 +1322,12 @@ static ValueFlow::Value executeImpl(const Token* expr, ProgramMemory& pm, const if (!cond.isIntValue()) return unknown; const Token* child = expr->astOperand2(); - if (cond.intvalue == 0) + if (isFalse(cond)) return execute(child->astOperand2(), pm); - else + else if (isTrue(cond)) return execute(child->astOperand1(), pm); + else + return unknown; } else if (expr->str() == "(" && expr->isCast()) { if (Token::simpleMatch(expr->previous(), ">") && expr->previous()->link()) return execute(expr->astOperand2(), pm); @@ -1349,6 +1389,26 @@ static ValueFlow::Value executeImpl(const Token* expr, ProgramMemory& pm, const return unknown; } +static const ValueFlow::Value* getImpossibleValue(const Token* tok) +{ + if (!tok) + return nullptr; + std::vector values; + for (const ValueFlow::Value& v : tok->values()) { + if (!v.isImpossible()) + continue; + if (v.isContainerSizeValue() || v.isIntValue()) { + values.push_back(std::addressof(v)); + } + } + auto it = std::max_element(values.begin(), values.end(), [](const ValueFlow::Value* x, const ValueFlow::Value* y) { + return x->intvalue < y->intvalue; + }); + if (it == values.end()) + return nullptr; + return *it; +} + static ValueFlow::Value execute(const Token* expr, ProgramMemory& pm, const Settings* settings) { ValueFlow::Value v = executeImpl(expr, pm, settings); @@ -1358,9 +1418,63 @@ static ValueFlow::Value execute(const Token* expr, ProgramMemory& pm, const Sett return v; if (pm.hasValue(expr->exprId())) return pm.at(expr->exprId()); + if (const ValueFlow::Value* value = getImpossibleValue(expr)) + return *value; return v; } +std::vector execute(const Scope* scope, ProgramMemory& pm, const Settings* settings) +{ + static const std::vector unknown = {ValueFlow::Value::unknown()}; + if (!scope) + return unknown; + if (!scope->bodyStart) + return unknown; + for (const Token* tok = scope->bodyStart->next(); precedes(tok, scope->bodyEnd); tok = tok->next()) { + const Token* top = tok->astTop(); + if (!top) + return unknown; + + if (Token::simpleMatch(top, "return") && top->astOperand1()) { + return {execute(top->astOperand1(), pm, settings)}; + } else if (Token::Match(top, "%op%")) { + if (execute(top, pm, settings).isUninitValue()) + return unknown; + const Token* next = nextAfterAstRightmostLeaf(top); + if (!next) + return unknown; + tok = next; + } else if (Token::simpleMatch(top->previous(), "if (")) { + const Token* condTok = top->astOperand2(); + ValueFlow::Value v = execute(condTok, pm, settings); + if (!v.isIntValue()) + return unknown; + const Token* thenStart = top->link()->next(); + const Token* next = thenStart->link(); + const Token* elseStart = nullptr; + if (Token::simpleMatch(thenStart->link(), "} else {")) { + elseStart = thenStart->link()->tokAt(2); + next = elseStart->link(); + } + std::vector result; + if (isTrue(v)) { + result = execute(thenStart->scope(), pm, settings); + } else if (isFalse(v)) { + if (elseStart) + result = execute(elseStart->scope(), pm, settings); + } else { + return unknown; + } + if (!result.empty()) + return result; + tok = next; + } else { + return unknown; + } + } + return {}; +} + ValueFlow::Value evaluateLibraryFunction(const std::unordered_map& args, const std::string& returnValue, const Settings* settings) diff --git a/lib/programmemory.h b/lib/programmemory.h index 27f9afb1d..8cf5fd040 100644 --- a/lib/programmemory.h +++ b/lib/programmemory.h @@ -30,6 +30,7 @@ #include #include +class Scope; class Token; class Settings; @@ -142,6 +143,8 @@ struct ProgramMemoryState { ProgramMemory get(const Token* tok, const Token* ctx, const ProgramMemory::Map& vars) const; }; +std::vector execute(const Scope* scope, ProgramMemory& pm, const Settings* settings); + void execute(const Token* expr, ProgramMemory& programMemory, MathLib::bigint* result, diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 2c63d4200..0b5e9a308 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -3022,6 +3022,8 @@ std::vector Function::findReturns(const Function* f) const Scope* scope = f->functionScope; if (!scope) return result; + if (!scope->bodyStart) + return result; for (const Token* tok = scope->bodyStart->next(); tok && tok != scope->bodyEnd; tok = tok->next()) { if (tok->str() == "{" && tok->scope() && (tok->scope()->type == Scope::eLambda || tok->scope()->type == Scope::eClass)) { diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 1f54779bb..b08c379fa 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -7539,9 +7539,45 @@ static void valueFlowFunctionDefaultParameter(TokenList* tokenlist, SymbolDataba } } -static bool isKnown(const Token * tok) +static const ValueFlow::Value* getKnownValueFromToken(const Token* tok) { - return tok && tok->hasKnownIntValue(); + if (!tok) + return nullptr; + auto it = std::find_if(tok->values().begin(), tok->values().end(), [&](const ValueFlow::Value& v) { + return (v.isIntValue() || v.isContainerSizeValue() || v.isFloatValue()) && v.isKnown(); + }); + if (it == tok->values().end()) + return nullptr; + return std::addressof(*it); +} + +static const ValueFlow::Value* getKnownValueFromTokens(const std::vector& toks) +{ + if (toks.empty()) + return nullptr; + const ValueFlow::Value* result = getKnownValueFromToken(toks.front()); + if (!result) + return nullptr; + if (!std::all_of(std::next(toks.begin()), toks.end(), [&](const Token* tok) { + return std::any_of(tok->values().begin(), tok->values().end(), [&](const ValueFlow::Value& v) { + return v.equalValue(*result) && v.valueKind == result->valueKind; + }); + })) + return nullptr; + return result; +} + +static void setFunctionReturnValue(const Function* f, Token* tok, ValueFlow::Value v, const Settings* settings) +{ + if (f->hasVirtualSpecifier()) { + if (v.isImpossible()) + return; + v.setPossible(); + } else if (!v.isImpossible()) { + v.setKnown(); + } + v.errorPath.emplace_back(tok, "Calling function '" + f->name() + "' returns " + v.toString()); + setTokenValue(tok, std::move(v), settings); } static void valueFlowFunctionReturn(TokenList *tokenlist, ErrorLogger *errorLogger, const Settings* settings) @@ -7564,34 +7600,20 @@ static void valueFlowFunctionReturn(TokenList *tokenlist, ErrorLogger *errorLogg if (tok->hasKnownValue()) continue; - // Arguments.. - std::vector parvalues; - if (tok->astOperand2()) { - const Token *partok = tok->astOperand2(); - while (partok && partok->str() == "," && isKnown(partok->astOperand2())) - partok = partok->astOperand1(); - if (!isKnown(partok)) - continue; - parvalues.push_back(partok->values().front().intvalue); - partok = partok->astParent(); - while (partok && partok->str() == ",") { - parvalues.push_back(partok->astOperand2()->values().front().intvalue); - partok = partok->astParent(); - } - if (partok != tok) - continue; - } + std::vector returns = Function::findReturns(function); + if (returns.empty()) + continue; - // Get scope and args of function - const Scope * const functionScope = function->functionScope; - if (!functionScope || !Token::simpleMatch(functionScope->bodyStart, "{ return")) { - if (functionScope && settings->debugwarnings && Token::findsimplematch(functionScope->bodyStart, "return", functionScope->bodyEnd)) - bailout(tokenlist, errorLogger, tok, "function return; nontrivial function body"); + if (const ValueFlow::Value* v = getKnownValueFromTokens(returns)) { + setFunctionReturnValue(function, tok, *v, settings); continue; } + // Arguments.. + std::vector arguments = getArguments(tok); + ProgramMemory programMemory; - for (std::size_t i = 0; i < parvalues.size(); ++i) { + for (std::size_t i = 0; i < arguments.size(); ++i) { const Variable * const arg = function->getArgumentVar(i); if (!arg) { if (settings->debugwarnings) @@ -7599,25 +7621,18 @@ static void valueFlowFunctionReturn(TokenList *tokenlist, ErrorLogger *errorLogg programMemory.clear(); break; } - programMemory.setIntValue(arg->nameToken(), parvalues[i]); + const ValueFlow::Value* v = getKnownValueFromToken(arguments[i]); + if (!v) + continue; + programMemory.setValue(arg->nameToken(), *v); } - if (programMemory.empty() && !parvalues.empty()) + if (programMemory.empty() && !arguments.empty()) continue; - - // Determine return value of subfunction.. - MathLib::bigint result = 0; - bool error = false; - execute(functionScope->bodyStart->next()->astOperand1(), - programMemory, - &result, - &error); - if (!error) { - ValueFlow::Value v(result); - if (function->hasVirtualSpecifier()) - v.setPossible(); - else - v.setKnown(); - setTokenValue(tok, std::move(v), settings); + std::vector values = execute(function->functionScope, programMemory, settings); + for (const ValueFlow::Value& v : values) { + if (v.isUninitValue()) + continue; + setFunctionReturnValue(function, tok, v, settings); } } } diff --git a/test/testsimplifytypedef.cpp b/test/testsimplifytypedef.cpp index 4b930ec7c..b7959a9d6 100644 --- a/test/testsimplifytypedef.cpp +++ b/test/testsimplifytypedef.cpp @@ -1207,9 +1207,7 @@ private: "}"; ASSERT_EQUALS(expected, tok(code, false)); - ASSERT_EQUALS_WITHOUT_LINENUMBERS( - "[test.cpp:31]: (debug) valueflow.cpp:3109:valueFlowFunctionReturn bailout: function return; nontrivial function body\n", - errout.str()); + ASSERT_EQUALS_WITHOUT_LINENUMBERS("", errout.str()); } void simplifyTypedef36() { diff --git a/test/testuninitvar.cpp b/test/testuninitvar.cpp index 7813a0171..39924ef8e 100644 --- a/test/testuninitvar.cpp +++ b/test/testuninitvar.cpp @@ -66,6 +66,7 @@ private: TEST_CASE(uninitvar2_func); // function calls TEST_CASE(uninitvar2_value); // value flow TEST_CASE(valueFlowUninit2_value); + TEST_CASE(valueFlowUninit_uninitvar2); TEST_CASE(uninitStructMember); // struct members TEST_CASE(uninitvar2_while); TEST_CASE(uninitvar2_4494); // #4494 @@ -3636,6 +3637,545 @@ private: ASSERT_EQUALS("", errout.str()); } + void valueFlowUninit_uninitvar2() + { + // using uninit var + valueFlowUninit("void f() {\n" + " int x;\n" + " x++;\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Uninitialized variable: x\n", errout.str()); + + // extracttests.start: char str[10]; + valueFlowUninit("void f() {\n" + " int x;\n" + " str[x] = 0;\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Uninitialized variable: x\n", errout.str()); + + valueFlowUninit("void f() {\n" // #7736 + " int buf[12];\n" + " printf (\"%d\", buf[0] );\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Uninitialized variable: buf\n", errout.str()); + + valueFlowUninit("void f() {\n" + " int x;\n" + " int y = x & 3;\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Uninitialized variable: x\n", errout.str()); + + valueFlowUninit("void f() {\n" + " int x;\n" + " int y = 3 & x;\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Uninitialized variable: x\n", errout.str()); + + valueFlowUninit("void f() {\n" + " int x;\n" + " x = 3 + x;\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Uninitialized variable: x\n", errout.str()); + + valueFlowUninit("void f() {\n" + " int x;\n" + " x = x;\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Uninitialized variable: x\n", errout.str()); + + // extracttests.start: struct ABC {int a;}; + valueFlowUninit("void f() {\n" + " struct ABC *abc;\n" + " abc->a = 0;\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Uninitialized variable: abc\n", errout.str()); + + valueFlowUninit("int f() {\n" + " static int x;\n" + " return ++x;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("int f() {\n" + " extern int x;\n" + " return ++x;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("void f() {\n" // #3926 - weird cast. + " int x;\n" + " *(((char *)&x) + 0) = 0;\n" + "}", + "test.c"); + ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("void f() {\n" // #4737 - weird cast. + " int x;\n" + " do_something(&((char*)&x)[0], 1);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("void f() {\n" + " int x;\n" + " char *p = (char*)&x + 1;\n" + "}", + "test.cpp"); + ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("void f() {\n" + " int i;\n" + " i=f(), i!=2;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + // using uninit var in condition + valueFlowUninit("void f(void) {\n" + " int x;\n" + " if (x) { }\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Uninitialized variable: x\n", errout.str()); + + valueFlowUninit("void f() {\n" + " int x;\n" + " if (1 == (3 & x)) { }\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Uninitialized variable: x\n", errout.str()); + + // ?: + valueFlowUninit("int f(int *ptr) {\n" + " int a;\n" + " int *p = ptr ? ptr : &a;\n" + "}"); + TODO_ASSERT_EQUALS("", "[test.cpp:3]: (error) Uninitialized variable: &a\n", errout.str()); + + valueFlowUninit("int f(int a) {\n" + " int x;\n" + " if (a==3) { x=2; }\n" + " y = (a==3) ? x : a;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + // = ({ .. }) + valueFlowUninit("void f() {\n" + " int x = ({ 1 + 2; });\n" + " int y = 1 + (x ? y : y);\n" + "}"); + TODO_ASSERT_EQUALS("[test.cpp:3]: (error) Uninitialized variable: y\n", "", errout.str()); + + // >> => initialization / usage + { + const char code[] = "void f() {\n" + " int x;\n" + " if (i >> x) { }\n" + "}"; + valueFlowUninit(code, "test.cpp"); + ASSERT_EQUALS("", errout.str()); + + valueFlowUninit(code, "test.c"); + ASSERT_EQUALS("[test.c:3]: (error) Uninitialized variable: x\n", errout.str()); + } + + valueFlowUninit("void f() {\n" + " int i, i2;\n" + " strm >> i >> i2;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + // unconditional initialization + valueFlowUninit("int f() {\n" + " int ret;\n" + " if (a) { ret = 1; }\n" + " else { {} ret = 2; }\n" + " return ret;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("int f() {\n" + " int ret;\n" + " if (a) { ret = 1; }\n" + " else { s=foo(1,{2,3},4); ret = 2; }\n" + " return ret;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + // conditional initialization + valueFlowUninit("void f() {\n" + " int x;\n" + " if (y == 1) { x = 1; }\n" + " else { if (y == 2) { x = 1; } }\n" + " return x;\n" + "}"); + TODO_ASSERT_EQUALS("[test.cpp:5]: (error) Uninitialized variable: x\n", "", errout.str()); + + valueFlowUninit("void f() {\n" + " int x;\n" + " if (y == 1) { x = 1; }\n" + " else { if (y == 2) { x = 1; } }\n" + " if (y == 3) { }\n" // <- ignore condition + " return x;\n" + "}"); + TODO_ASSERT_EQUALS("[test.cpp:6]: (error) Uninitialized variable: x\n", "", errout.str()); + + // initialization in condition + valueFlowUninit("void f() {\n" + " int a;\n" + " if (init(&a)) { }\n" + " a++;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + // return, break, continue, goto + valueFlowUninit("void f() {\n" + " int x;\n" + " if (y == 1) { return; }\n" + " else { x = 1; }\n" + " return x;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("void f() {\n" + " int x;\n" + " if (y == 1) { return; }\n" + " return x;\n" + "}"); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (error) Uninitialized variable: x\n", errout.str()); + + valueFlowUninit("int f(int x) {\n" + " int ret;\n" + " if (!x) {\n" + " ret = -123;\n" + " goto out1;\n" + " }\n" + " return 0;\n" + "out1:\n" + "out2:\n" + " return ret;\n" + "}", + "test.c"); + ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("void f() {\n" + " int i;\n" + " if (x) {\n" + " i = 1;\n" + " } else {\n" + " goto out;\n" + " }\n" + " i++;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("int f() {\n" + " int i,x;\n" + " for (i=0;i<9;++i)\n" + " if (foo) break;\n" + " return x;\n" + "}"); + ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:5]: (error) Uninitialized variable: x\n", errout.str()); + + valueFlowUninit("int f() {\n" + " int x;\n" + " while (foo)\n" + " if (bar) break;\n" + " return x;\n" + "}"); + ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:5]: (error) Uninitialized variable: x\n", errout.str()); + + // try/catch : don't warn about exception variable + valueFlowUninit("void f() {\n" + " try {\n" + " } catch (CException* e) {\n" + " trace();\n" + " e->Delete();\n" + " }\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("void f() {\n" // #5347 + " try {\n" + " } catch (const char* e) {\n" + " A a = e;\n" + " }\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + // exit + valueFlowUninit("void f() {\n" + " int x;\n" + " if (y == 1) { exit(0); }\n" + " else { x = 1; }\n" + " return x;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + // strange code.. don't crash (#3415) + valueFlowUninit("void foo() {\n" + " int i;\n" + " ({ if (0); });\n" + " for_each(i) { }\n" + "}", + "test.c"); + + // if, if + valueFlowUninit("void f(int a) {\n" + " int i;\n" + " if (a) i = 0;\n" + " if (a) i++;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("void f() {\n" + " int a,b=0;\n" + " if (x) {\n" + " if (y) {\n" + " a = 0;\n" + " b = 1;\n" + " }\n" + " }\n" + " if (b) a++;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("void f() {\n" + " int a=0, b;\n" + " if (x) { }\n" + " else { if (y==2) { a=1; b=2; } }\n" + " if (a) { ++b; }\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("static void f(int x, int y) {\n" + " int a;\n" + " if (x == 0) { a = y; }\n" + " if (x == 0 && (a == 1)) { }\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("static void f() {\n" + " int a=0, b;\n" + " if (something) { a = dostuff(&b); }\n" + " if (!a || b) { }\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("static void f(int x, int y) {\n" + " int a;\n" + " if (x == 0 && (a == 1)) { }\n" + "}", + "test.cpp"); + ASSERT_EQUALS("[test.cpp:3]: (error) Uninitialized variable: a\n", errout.str()); + + valueFlowUninit("void f() {\n" + " int a;\n" + " if (x) { a = 0; }\n" + " if (x) { if (y) { a++; } }\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("void f() {\n" + " int a;\n" + " if (x) { a = 0; }\n" + " if (x) { if (y) { } else { a++; } }\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("struct AB { int a; int b; };\n" + "void f(void) {\n" + " struct AB ab;\n" + " if (x) ab = getAB();\n" + " else ab.a = 0;\n" + " if (ab.a == 1) b = ab.b;\n" + "}", + "test.c"); + ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("int f(void) {\n" + " int a;\n" + " int i;\n" + " if (x) { noreturn(); }\n" + " else { i = 0; }\n" + " if (i==1) { a = 0; }\n" + " else { a = 1; }\n" + " return a;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("int f(int a) {\n" // #4560 + " int x = 0, y;\n" + " if (a) x = 1;\n" + " else return 0;\n" + " if (x) y = 123;\n" // <- y is always initialized + " else {}\n" + " return y;\n" + "}"); + TODO_ASSERT_EQUALS("", "[test.cpp:5] -> [test.cpp:7]: (warning) Uninitialized variable: y\n", errout.str()); + + valueFlowUninit("int f(int a) {\n" // #6583 + " int x;\n" + " if (a < 2) exit(1);\n" + " else if (a == 2) x = 0;\n" + " else exit(2);\n" + " return x;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("int f(int a) {\n" // #4560 + " int x = 1, y;\n" + " if (a) x = 0;\n" + " else return 0;\n" + " if (x) {}\n" + " else y = 123;\n" // <- y is always initialized + " return y;\n" + "}"); + TODO_ASSERT_EQUALS("", "[test.cpp:5] -> [test.cpp:7]: (warning) Uninitialized variable: y\n", errout.str()); + + valueFlowUninit("void f(int x) {\n" // #3948 + " int value;\n" + " if (x !=-1)\n" + " value = getvalue();\n" + " if (x == -1 || value > 300) {}\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("enum t_err { ERR_NONE, ERR_BAD_ARGS };\n" // #9649 + "struct box_t { int value; };\n" + "int init_box(box_t *p, int v);\n" + "\n" + "void foo(int ret) {\n" + " box_t box2;\n" + " if (ret == ERR_NONE)\n" + " ret = init_box(&box2, 20);\n" + " if (ret == ERR_NONE)\n" + " z = x + box2.value;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("void f(int x) {\n" + " int value;\n" + " if (x == 32)\n" + " value = getvalue();\n" + " if (x == 1)\n" + " v = value;\n" + "}"); + TODO_ASSERT_EQUALS("[test.cpp:6]: (error) Uninitialized variable: value\n", "", errout.str()); + + valueFlowUninit("void f(int x) {\n" + " int value;\n" + " if (x == 32)\n" + " value = getvalue();\n" + " if (x == 32) {}\n" + " else v = value;\n" + "}"); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:6]: (warning) Uninitialized variable: value\n", errout.str()); + + valueFlowUninit("static int x;" // #4773 + "int f() {\n" + " int y;\n" + " if (x) g();\n" + " if (x) y = 123;\n" + " else y = 456;\n" + " return y;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("static int x;" // #4773 + "int f() {\n" + " int y;\n" + " if (!x) g();\n" + " if (x) y = 123;\n" + " else y = 456;\n" + " return y;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("void f(int a) {\n" + " int x;\n" + " if (a) x=123;\n" + " if (!a) {\n" + " if (!a) {}\n" + " else if (x) {}\n" + " }\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + // asm + valueFlowUninit("void f() {\n" + " int x;\n" + " asm();\n" + " x++;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + // sizeof / typeof / offsetof / etc + valueFlowUninit("void f() {\n" + " int i;\n" + " sizeof(i+1);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("void f() {\n" + " int i;\n" + " if (100 == sizeof(i+1));\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("void f() {\n" + " struct ABC *abc;\n" + " int i = ARRAY_SIZE(abc.a);" + "}"); + // FP ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("void f() {\n" + " int *abc;\n" + " typeof(*abc);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("void f() {\n" + " struct ABC *abc;\n" + " return do_something(typeof(*abc));\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("void f() {\n" + " A *a;\n" + " a = malloc(sizeof(*a));\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + // & + valueFlowUninit("void f() {\n" // #4426 - address of uninitialized variable + " int a,b;\n" + " if (&a == &b);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + valueFlowUninit("void f() {\n" // #4439 - cast address of uninitialized variable + " int a;\n" + " x((LPARAM)(RECT*)&a);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + valueFlowUninit( + "int main() {\n" + " int done;\n" + " dostuff(1, (AuPointer) &done);\n" // <- It is not conclusive if the "&" is a binary or unary operator. Bailout. + "}"); + TODO_ASSERT_EQUALS("", "[test.cpp:3]: (error) Uninitialized variable: done\n", errout.str()); + + valueFlowUninit("void f() {\n" // #4778 - cast address of uninitialized variable + " long a;\n" + " &a;\n" + "}"); + TODO_ASSERT_EQUALS("", "[test.cpp:3]: (error) Uninitialized variable: &a\n", errout.str()); + + valueFlowUninit("void f() {\n" // #4717 - ({}) + " int a = ({ long b = (long)(123); 2 + b; });\n" + "}", + "test.c"); + ASSERT_EQUALS("", errout.str()); + } + void uninitStructMember() { // struct members checkUninitVar("struct AB { int a; int b; };\n" "void f(void) {\n" diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 890c9674c..3e4fa7f88 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -4633,6 +4633,85 @@ private: " return x;\n" "}\n"; ASSERT_EQUALS(true, testValueOfX(code, 6U, 0)); + + code = "int* g(int& i, bool b) {\n" + " if(b)\n" + " return nullptr;\n" + " return &i;\n" + "} \n" + "int f(int i) {\n" + " int* x = g(i, true);\n" + " return x;\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfX(code, 8U, 0)); + + code = "int* g(int& i, bool b) {\n" + " if(b)\n" + " return nullptr;\n" + " return &i;\n" + "} \n" + "int f(int i) {\n" + " int* x = g(i, false);\n" + " return x;\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfXImpossible(code, 8U, 0)); + + code = "int* g(int& i, bool b) {\n" + " if(b)\n" + " return nullptr;\n" + " return &i;\n" + "} \n" + "int f(int i) {\n" + " int* x = g(i, i == 3);\n" + " return x;\n" + "}\n"; + ASSERT_EQUALS(false, testValueOfX(code, 8U, 0)); + + code = "struct A {\n" + " unsigned i;\n" + " bool f(unsigned x) const {\n" + " return ((i & x) != 0);\n" + " }\n" + "};\n" + "int g(A& a) {\n" + " int x = a.f(2);\n" + " return x;\n" + "}\n"; + ASSERT_EQUALS(false, testValueOfX(code, 9U, 0)); + ASSERT_EQUALS(false, testValueOfX(code, 9U, 1)); + ASSERT_EQUALS(false, testValueOfXImpossible(code, 9U, 0)); + ASSERT_EQUALS(false, testValueOfXImpossible(code, 9U, 1)); + + code = "struct A {\n" + " enum {\n" + " b = 0,\n" + " c = 1,\n" + " d = 2\n" + " };\n" + " bool isb() const {\n" + " return e == b;\n" + " }\n" + " unsigned int e;\n" + "};\n" + "int f(A g) {\n" + " int x = !g.isb();\n" + " return x;\n" + "}\n"; + ASSERT_EQUALS(false, testValueOfX(code, 14U, 0)); + ASSERT_EQUALS(false, testValueOfX(code, 14U, 1)); + + code = "bool h(char q);\n" + "bool g(char q) {\n" + " if (!h(q))\n" + " return false;\n" + " return true;\n" + "}\n" + "int f() {\n" + " int x = g(0);\n" + " return x;\n" + "}\n"; + ASSERT_EQUALS(false, testValueOfX(code, 9U, 0)); + ASSERT_EQUALS(false, testValueOfX(code, 9U, 1)); } void valueFlowFunctionDefaultParameter() { diff --git a/tools/run_more_tests.sh b/tools/run_more_tests.sh index 86c675f90..12633c41f 100755 --- a/tools/run_more_tests.sh +++ b/tools/run_more_tests.sh @@ -20,86 +20,85 @@ cd test1 $CPPCHECK -q --template=cppcheck1 . 2> 1.txt - -# (!x) => (x==0) +echo "(!x) => (x==0)" $SED_CMD -ri 's/([(&][ ]*)\!([a-z]+)([ ]*[&)])/\1\2==0\3/' *.cpp $CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt -# (x==0) => (0==x) +echo "(x==0) => (0==x)" $SED_CMD -ri 's/([(&][ ]*)([a-z]+)[ ]*==[ ]*0([ ]*[&)])/\10==\2\3/' *.cpp $CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt -# (0==x) => (!x) +echo "(0==x) => (!x)" $SED_CMD -ri 's/([(&][ ]*)0[ ]*==[ ]*([a-z]+)([ ]*[&)])/\1!\2\3/' *.cpp $CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt -# if (x) => (x!=0) +echo "if (x) => (x!=0)" $SED_CMD -ri 's/(if[ ]*\([ ]*[a-z]+)([ ]*[&)])/\1!=0\2/' *.cpp $CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt -# while (x) => (x!=0) +echo "while (x) => (x!=0)" $SED_CMD -ri 's/(while[ ]*\([ ]*[a-z]+)([ ]*[&)])/\1!=0\2/' *.cpp $CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt -# (x!=0) => (0!=x) +echo "(x!=0) => (0!=x)" $SED_CMD -ri 's/([(&][ ]*)([a-z]+)[ ]*!=[ ]*0([ ]*[&)])/\10!=\2\3/' *.cpp $CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt -# (0!=x) => (x) +echo "(0!=x) => (x)" $SED_CMD -ri 's/([(&][ ]*)0[ ]*!=[ ]*([a-z]+[ ]*[&)])/\1\2/' *.cpp $CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt -# (x < 0) => (0 > x) +echo "(x < 0) => (0 > x)" $SED_CMD -ri 's/([(&][ ]*)([a-z]+)[ ]*<[ ]*(\-?[0-9]+)([ ]*[&)])/\1\3>\2\4/' *.cpp $CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt -# (x <= 0) => (0 >= x) +echo "(x <= 0) => (0 >= x)" $SED_CMD -ri 's/([(&][ ]*)([a-z]+)[ ]*<=[ ]*(\-?[0-9]+)([ ]*[&)])/\1\3>=\2\4/' *.cpp $CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt -# (x > 0) => (0 < x) +echo "(x > 0) => (0 < x)" $SED_CMD -ri 's/([(&][ ]*)([a-z]+)[ ]*<=[ ]*(\-?[0-9]+)([ ]*[&)])/\1\3>=\2\4/' *.cpp $CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt -# (x >= 0) => (0 <= x) +echo "(x >= 0) => (0 <= x)" $SED_CMD -ri 's/([(&][ ]*)([a-z]+)[ ]*<=[ ]*(\-?[0-9]+)([ ]*[&)])/\1\3>=\2\4/' *.cpp $CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt -# (x == 123) => (123 == x) +echo "(x == 123) => (123 == x)" $SED_CMD -ri 's/([(&][ ]*)([a-z]+)[ ]*==[ ]*(\-?[0-9]+)([ ]*[&)])/\1\3==\2\4/' *.cpp $CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt -# (x != 123) => (123 != x) +echo "(x != 123) => (123 != x)" $SED_CMD -ri 's/([(&][ ]*)([a-z]+)[ ]*\!=[ ]*(\-?[0-9]+)([ ]*[&)])/\1\3!=\2\4/' *.cpp $CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt -# (0 < x) => (x > 0) +echo "(0 < x) => (x > 0)" $SED_CMD -ri 's/([(&][ ]*)(\-?[0-9]+)[ ]*<[ ]*([a-z]+)([ ]*[&)])/\1\3>\2\4/' *.cpp $CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt -# (0 <= x) => (x >= 0) +echo "(0 <= x) => (x >= 0)" $SED_CMD -ri 's/([(&][ ]*)(\-?[0-9]+)[ ]*<=[ ]*([a-z]+)([ ]*[&)])/\1\3>=\2\4/' *.cpp $CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt -# (0 > x) => (x < 0) +echo "(0 > x) => (x < 0)" $SED_CMD -ri 's/([(&][ ]*)(\-?[0-9]+)[ ]*<=[ ]*([a-z]+)([ ]*[&)])/\1\3>=\2\4/' *.cpp $CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt -# (0 >= x) => (x <= 0) +echo "(0 >= x) => (x <= 0)" $SED_CMD -ri 's/([(&][ ]*)(\-?[0-9]+)[ ]*<=[ ]*([a-z]+)([ ]*[&)])/\1\3>=\2\4/' *.cpp $CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt -# (123 == x) => (x == 123) +echo "(123 == x) => (x == 123)" $SED_CMD -ri 's/([(&][ ]*)(\-?[0-9]+)[ ]*==[ ]*([a-z]+)([ ]*[&)])/\1\3==\2\4/' *.cpp $CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt -# (123 != x) => (x <= 123) +echo "(123 != x) => (x <= 123)" $SED_CMD -ri 's/([(&][ ]*)(\-?[0-9]+)[ ]*\!=[ ]*([a-z]+)([ ]*[&)])/\1\3!=\2\4/' *.cpp $CPPCHECK -q --template=cppcheck1 . 2> 2.txt && diff 1.txt 2.txt