Update to use stopOnCondition

Author: Your Name

lib/forwardanalyzer.cpp

@@ -782,7 +782,7 @@ namespace {
                             } else if (thenBranch.check) {
                                 return Break();
                             } else {
-                                if (analyzer->isConditional() && stopUpdates())
+                                if (stopOnCondition(condTok) && stopUpdates())
                                     return Break(Analyzer::Terminate::Conditional);
                                 analyzer->assume(condTok, false);
                             }

lib/vf_analyzers.cpp

@@ -1198,18 +1198,44 @@ struct SingleValueFlowAnalyzer : ValueFlowAnalyzer {
 
     bool stopOnCondition(const Token* condTok) const override
     {
-        if (value.isNonValue())
-            return false;
         if (value.isImpossible())
             return false;
-        if (isConditional() && !value.isKnown())
+        // Lifetime values must keep flowing through conditions to detect dangling dereferences on every path.
+        if (value.isLifetimeValue())
+            return false;
+        // A value carrying the explicit 'conditional' flag (e.g. an uninitialized value, or a value lowered
+        // to possible after a branch that modifies the variable) can depend on conditions that don't mention
+        // the variable itself, so stop at any subsequent condition to stay conservative.
+        if (value.conditional && !value.isKnown())
             return true;
-        if (value.isSymbolicValue())
+        if (value.isNonValue())
             return false;
+        if (value.isSymbolicValue())
+            return isConditional() && !value.isKnown();
+        // The value may still be conditional via the originating 'condition' token (e.g. a possible null
+        // pointer after 'if (p && ...)'). Such a value may keep flowing past a later condition, but only
+        // when that condition actually refers to the tracked value: then cppcheck can reason about how the
+        // condition constrains it. If the value is not mentioned, a correlation that cppcheck cannot follow
+        // during forward analysis (e.g. 'bool ok = (p != nullptr); if (!ok) return;') could make a later
+        // dereference safe, so stop conservatively to avoid false positives.
+        if (value.condition && !value.isKnown() && !conditionReferencesValue(condTok))
+            return true;
         ConditionState cs = analyzeCondition(condTok);
         return cs.isUnknownDependent();
     }
 
+    // Does the condition mention the tracked value, either directly or through a symbolic alias?
+    bool conditionReferencesValue(const Token* condTok) const
+    {
+        return findAstNode(condTok, [&](const Token* tok) {
+            if (match(tok))
+                return true;
+            return std::any_of(tok->values().cbegin(), tok->values().cend(), [&](const ValueFlow::Value& v) {
+                return v.isSymbolicValue() && !v.isImpossible() && v.tokvalue && match(v.tokvalue);
+            });
+        }) != nullptr;
+    }
+
     bool updateScope(const Token* endBlock, bool /*modified*/) const override {
         const Scope* scope = endBlock->scope();
         if (!scope)

test/testnullpointer.cpp

@@ -143,6 +143,8 @@ class TestNullPointer : public TestFixture {
         TEST_CASE(nullpointer103);
         TEST_CASE(nullpointer104); // #13881
         TEST_CASE(nullpointer105); // #13861
+        TEST_CASE(nullpointer106); // #13682
+        TEST_CASE(nullpointer107); // #13682 (no false positive past unrelated conditions)
         TEST_CASE(nullpointer_addressOf); // address of
         TEST_CASE(nullpointerSwitch); // #2626
         TEST_CASE(nullpointer_cast); // #4692
@@ -2966,6 +2968,79 @@ class TestNullPointer : public TestFixture {
         ASSERT_EQUALS("", errout_str());
     }
 
+    void nullpointer106() // #13682
+    {
+        // An unrelated condition between the null check and the dereference must not stop the analysis
+        check("struct S {\n"
+              "    bool b;\n"
+              "    bool f() const;\n"
+              "};\n"
+              "void f(const S* p, const S* o) {\n"
+              "    const S* p1 = p;\n"
+              "    if (p1 && p1->f())\n"
+              "        return;\n"
+              "    if (p == o)\n"
+              "        return;\n"
+              "    if (p1->b) {}\n"
+              "}\n");
+        ASSERT_EQUALS(
+            "[test.cpp:7:9] -> [test.cpp:11:9]: (warning) Either the condition 'p1' is redundant or there is possible null pointer dereference: p1. [nullPointerRedundantCheck]\n",
+            errout_str());
+    }
+
+    void nullpointer107() // #13682 - don't flow past a condition that doesn't reference the pointer
+    {
+        // The pointer's null-ness is cached in a boolean; the guard 'if (!ok)' makes the deref safe but
+        // cppcheck cannot follow that correlation during forward analysis -> must not warn (no false positive)
+        check("struct S { void g(); bool f() const; };\n"
+              "void f(S* p) {\n"
+              "    bool ok = (p != nullptr);\n"
+              "    if (p && p->f())\n"
+              "        return;\n"
+              "    if (!ok)\n"
+              "        return;\n"
+              "    p->g();\n"
+              "}\n");
+        ASSERT_EQUALS("", errout_str());
+
+        // A guard on an unrelated boolean (that the caller may use to imply validity) is not something
+        // cppcheck can follow -> stay conservative and do not warn
+        check("struct S { void g(); bool f() const; };\n"
+              "void f(S* p, bool valid) {\n"
+              "    S* p1 = p;\n"
+              "    if (p1 && p1->f())\n"
+              "        return;\n"
+              "    if (!valid)\n"
+              "        return;\n"
+              "    p1->g();\n"
+              "}\n");
+        ASSERT_EQUALS("", errout_str());
+
+        // A guard on a different pointer must not be assumed to constrain this one
+        check("struct S { void g(); bool f() const; };\n"
+              "void f(S* p, S* q) {\n"
+              "    S* p1 = p;\n"
+              "    if (p1 && p1->f())\n"
+              "        return;\n"
+              "    if (!q)\n"
+              "        return;\n"
+              "    p1->g();\n"
+              "}\n");
+        ASSERT_EQUALS("", errout_str());
+
+        // A direct null guard on the pointer (or its alias) is still handled and must not warn
+        check("struct S { void g(); bool f() const; };\n"
+              "void f(S* p) {\n"
+              "    S* p1 = p;\n"
+              "    if (p1 && p1->f())\n"
+              "        return;\n"
+              "    if (!p)\n"
+              "        return;\n"
+              "    p1->g();\n"
+              "}\n");
+        ASSERT_EQUALS("", errout_str());
+    }
+
     void nullpointer_addressOf() { // address of
         check("void f() {\n"
               "  struct X *x = 0;\n"