Fix FP when calling a function in a condition (#3412)

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

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;
 

lib/valueflow.cpp

@@ -114,6 +114,7 @@
 #include <string>
 #include <tuple>
 #include <type_traits>
+#include <unordered_set>
 #include <vector>
 
 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<nonneg int, const Token*> getSymbols(const Token* tok) const
+    {
+        std::unordered_map<nonneg int, const Token*> 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<const Token*> 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<nonneg int, const Token*> 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)

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;

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"