diff --git a/lib/analyzer.h b/lib/analyzer.h index d80a45af3..224a1c877 100644 --- a/lib/analyzer.h +++ b/lib/analyzer.h @@ -169,6 +169,8 @@ struct Analyzer { virtual void forkScope(const Token* /*endBlock*/) {} /// If the value is conditional virtual bool isConditional() const = 0; + /// If analysis should stop on the condition + virtual bool stopOnCondition(const Token* condTok) const = 0; /// The condition that will be assumed during analysis virtual void assume(const Token* tok, bool state, unsigned int flags = 0) = 0; /// Return analyzer for expression at token diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index f1bdcb804..dd23afbba 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -151,9 +151,8 @@ struct ForwardTraversal { bool checkThen, checkElse; std::tie(checkThen, checkElse) = evalCond(condTok); if (!checkThen && !checkElse) { - // Stop if the value is conditional - if (!traverseUnknown && analyzer->isConditional() && stopUpdates()) { - return Break(Analyzer::Terminate::Conditional); + if (!traverseUnknown && analyzer->stopOnCondition(condTok) && stopUpdates()) { + return Progress::Continue; } checkThen = true; checkElse = true; @@ -593,6 +592,8 @@ struct ForwardTraversal { Branch elseBranch{endBlock->tokAt(2) ? endBlock->linkAt(2) : nullptr}; // Check if condition is true or false std::tie(thenBranch.check, elseBranch.check) = evalCond(condTok); + if (!thenBranch.check && !elseBranch.check && analyzer->stopOnCondition(condTok) && stopUpdates()) + return Break(Analyzer::Terminate::Conditional); bool hasElse = Token::simpleMatch(endBlock, "} else {"); bool bail = false; diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 1db761089..860875963 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -114,6 +114,7 @@ #include #include #include +#include #include static void bailoutInternal(const std::string& type, TokenList *tokenlist, ErrorLogger *errorLogger, const Token *tok, const std::string &what, const std::string &file, int line, std::string function) @@ -1996,6 +1997,103 @@ struct ValueFlowAnalyzer : Analyzer { return tokenlist->getSettings(); } + struct ConditionState { + bool dependent = true; + bool unknown = true; + + bool isUnknownDependent() const { + return unknown && dependent; + } + }; + + std::unordered_map getSymbols(const Token* tok) const + { + std::unordered_map result; + if (!tok) + return result; + for (const ValueFlow::Value& v : tok->values()) { + if (!v.isSymbolicValue()) + continue; + if (v.isImpossible()) + continue; + if (!v.tokvalue) + continue; + if (v.tokvalue->exprId() == 0) + continue; + if (match(v.tokvalue)) + continue; + result[v.tokvalue->exprId()] = v.tokvalue; + } + return result; + } + + ConditionState analyzeCondition(const Token* tok, int depth = 20) const + { + ConditionState result; + if (!tok) + return result; + if (depth < 0) + return result; + depth--; + if (analyze(tok, Direction::Forward).isRead()) { + result.dependent = true; + result.unknown = false; + return result; + } else if (tok->hasKnownIntValue() || tok->isLiteral()) { + result.dependent = false; + result.unknown = false; + return result; + } else if (Token::Match(tok, "%cop%")) { + if (isLikelyStream(isCPP(), tok->astOperand1())) { + result.dependent = false; + return result; + } + ConditionState lhs = analyzeCondition(tok->astOperand1(), depth - 1); + if (lhs.isUnknownDependent()) + return lhs; + ConditionState rhs = analyzeCondition(tok->astOperand2(), depth - 1); + if (rhs.isUnknownDependent()) + return rhs; + if (Token::Match(tok, "%comp%")) + result.dependent = lhs.dependent && rhs.dependent; + else + result.dependent = lhs.dependent || rhs.dependent; + result.unknown = lhs.unknown || rhs.unknown; + return result; + } else if (Token::Match(tok->previous(), "%name% (")) { + std::vector args = getArguments(tok->previous()); + if (Token::Match(tok->tokAt(-2), ". %name% (")) { + args.push_back(tok->tokAt(-2)->astOperand1()); + } + result.dependent = std::any_of(args.begin(), args.end(), [&](const Token* arg) { + ConditionState cs = analyzeCondition(arg, depth - 1); + return cs.dependent; + }); + if (result.dependent) { + // Check if we can evaluate the function + if (!evaluate(Evaluate::Integral, tok).empty()) + result.unknown = false; + } + return result; + } else { + std::unordered_map symbols = getSymbols(tok); + result.dependent = false; + for (auto&& p : symbols) { + const Token* arg = p.second; + ConditionState cs = analyzeCondition(arg, depth - 1); + result.dependent = cs.dependent; + if (result.dependent) + break; + } + if (result.dependent) { + // Check if we can evaluate the token + if (!evaluate(Evaluate::Integral, tok).empty()) + result.unknown = false; + } + return result; + } + } + virtual Action isModified(const Token* tok) const { Action read = Action::Read; bool inconclusive = false; @@ -2411,6 +2509,18 @@ struct SingleValueFlowAnalyzer : ValueFlowAnalyzer { return false; } + virtual bool stopOnCondition(const Token* condTok) const OVERRIDE + { + if (value.isNonValue()) + return false; + if (value.isImpossible()) + return false; + if (isConditional() && !value.isKnown() && !value.isImpossible()) + return true; + ConditionState cs = analyzeCondition(condTok); + return cs.isUnknownDependent(); + } + virtual bool updateScope(const Token* endBlock, bool) const OVERRIDE { const Scope* scope = endBlock->scope(); if (!scope) @@ -5807,6 +5917,10 @@ struct MultiValueFlowAnalyzer : ValueFlowAnalyzer { return false; } + virtual bool stopOnCondition(const Token*) const OVERRIDE { + return isConditional(); + } + virtual bool updateScope(const Token* endBlock, bool) const OVERRIDE { const Scope* scope = endBlock->scope(); if (!scope) diff --git a/test/cfg/gtk.c b/test/cfg/gtk.c index d75f2495a..5ec5ab043 100644 --- a/test/cfg/gtk.c +++ b/test/cfg/gtk.c @@ -400,8 +400,9 @@ void g_once_init_enter_leave_test() gsize * init_val3 = NULL; // cppcheck-suppress nullPointer if (g_once_init_enter(init_val3)) { + gsize* init_val31 = NULL; // cppcheck-suppress nullPointer - g_once_init_leave(init_val3, 1); + g_once_init_leave(init_val31, 1); } gsize * init_val4; diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index 93076b3de..79c24580e 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -116,6 +116,7 @@ private: TEST_CASE(nullpointer74); TEST_CASE(nullpointer75); TEST_CASE(nullpointer76); // #10408 + TEST_CASE(nullpointer77); TEST_CASE(nullpointer_addressOf); // address of TEST_CASE(nullpointerSwitch); // #2626 TEST_CASE(nullpointer_cast); // #4692 @@ -2377,6 +2378,24 @@ private: ASSERT_EQUALS("", errout.str()); } + void nullpointer77() + { + check("bool h(int*);\n" + "void f(int* i) {\n" + " int* i = nullptr;\n" + " if (h(i) && *i == 1) {}\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("bool h(int*);\n" + "void f(int* i) {\n" + " int* i = nullptr;\n" + " if (h(i))\n" + " if (*i == 1) {}\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + void nullpointer_addressOf() { // address of check("void f() {\n" " struct X *x = 0;\n"