Update to handle the vars in execute

Author: Your Name

lib/analyzer.h

@@ -158,6 +158,10 @@ struct Analyzer {
             ContainerEmpty = (1 << 2),
             // Do not record the program state at the branch boundaries. Used when assuming a
             // condition before the branch is traversed, where those states would be premature.
+            // When this is not set the branch has already been traversed and control is continuing
+            // past it, so the assumed state is anchored at the block's end (see the analyzer's
+            // assume()): this keeps assumptions on variables modified inside the block from being
+            // discarded as "modified" once control leaves it.
             NoState = (1 << 3),
         };
     };

lib/programmemory.cpp

@@ -262,26 +262,26 @@ ProgramMemory::Map::iterator ProgramMemory::find(nonneg int exprid)
     return mValues->find(ExprIdToken::create(exprid));
 }
 
-static ValueFlow::Value execute(const Token* expr, ProgramMemory& pm, const Settings& settings);
+static ValueFlow::Value execute(const Token* expr, ProgramMemory& pm, const Settings& settings, const ProgramMemory::Map& vars = {});
 
-static bool evaluateCondition(MathLib::bigint r, const Token* condition, ProgramMemory& pm, const Settings& settings)
+static bool evaluateCondition(MathLib::bigint r, const Token* condition, ProgramMemory& pm, const Settings& settings, const ProgramMemory::Map& vars = {})
 {
     if (!condition)
         return false;
     MathLib::bigint result = 0;
     bool error = false;
-    execute(condition, pm, &result, &error, settings);
+    execute(condition, pm, &result, &error, settings, vars);
     return !error && result == r;
 }
 
-bool conditionIsFalse(const Token* condition, ProgramMemory pm, const Settings& settings)
+bool conditionIsFalse(const Token* condition, ProgramMemory pm, const Settings& settings, const ProgramMemory::Map& vars)
 {
-    return evaluateCondition(0, condition, pm, settings);
+    return evaluateCondition(0, condition, pm, settings, vars);
 }
 
-bool conditionIsTrue(const Token* condition, ProgramMemory pm, const Settings& settings)
+bool conditionIsTrue(const Token* condition, ProgramMemory pm, const Settings& settings, const ProgramMemory::Map& vars)
 {
-    return evaluateCondition(1, condition, pm, settings);
+    return evaluateCondition(1, condition, pm, settings, vars);
 }
 
 static bool frontIs(const std::vector<MathLib::bigint>& v, bool i)
@@ -443,7 +443,7 @@ static void fillProgramMemoryFromAssignments(ProgramMemory& pm, const Token* tok
                 if (!pm.hasValue(vartok->exprId())) {
                     const Token* valuetok = tok2->astOperand2();
                     ProgramMemory local = state;
-                    pm.setValue(vartok, execute(valuetok, local, settings));
+                    pm.setValue(vartok, execute(valuetok, local, settings, vars));
                 }
             }
         } else if (Token::simpleMatch(tok2, ")") && tok2->link() &&
@@ -547,19 +547,21 @@ void ProgramMemoryState::addState(const Token* tok, const ProgramMemory::Map& va
     replace(std::move(local), tok);
 }
 
-void ProgramMemoryState::assume(const Token* tok, bool b, bool isEmpty)
+void ProgramMemoryState::assume(const Token* tok, bool b, bool isEmpty, const Token* origin)
 {
     ProgramMemory pm = state;
     if (isEmpty)
         pm.setContainerSizeValue(tok, 0, b);
     else
         programMemoryParseCondition(pm, tok, nullptr, settings, b);
-    const Token* origin = tok;
-    const Token* top = tok->astTop();
-    if (Token::Match(top->previous(), "for|while|if (") && !Token::simpleMatch(tok->astParent(), "?")) {
-        origin = top->link()->next();
-        if (!b && origin->link()) {
-            origin = origin->link();
+    if (!origin) {
+        origin = tok;
+        const Token* top = tok->astTop();
+        if (Token::Match(top->previous(), "for|while|if (") && !Token::simpleMatch(tok->astParent(), "?")) {
+            origin = top->link()->next();
+            if (!b && origin->link()) {
+                origin = origin->link();
+            }
         }
     }
     replace(std::move(pm), origin);
@@ -1316,6 +1318,10 @@ namespace {
     struct Executor {
         ProgramMemory* pm;
         const Settings& settings;
+        // The values that are being tracked by the forward/reverse analysis. These take precedence
+        // over what is stored in the program memory: a tracked value is the authoritative current
+        // value of its expression, so any cached value that depends on it must be re-evaluated.
+        const ProgramMemory::Map* vars = nullptr;
         int fdepth = 4;
         int depth = 10;
 
@@ -1324,6 +1330,25 @@ namespace {
             assert(pm != nullptr);
         }
 
+        // Is the tracked value for this expression available?
+        const ValueFlow::Value* getTrackedValue(const Token* expr) const {
+            if (!vars || expr->exprId() == 0)
+                return nullptr;
+            const auto it = vars->find(ExprIdToken::create(expr->exprId()));
+            return it == vars->end() ? nullptr : &it->second;
+        }
+
+        // Does the expression read a tracked value? If so, any value cached in the program memory
+        // for it may be stale (the tracked value may have changed since it was cached), so it must
+        // be re-evaluated rather than served from the cache.
+        bool dependsOnTrackedValue(const Token* expr) const {
+            if (!vars || vars->empty())
+                return false;
+            return findAstNode(expr, [&](const Token* tok) {
+                return getTrackedValue(tok) != nullptr;
+            }) != nullptr;
+        }
+
         static ValueFlow::Value unknown() {
             return ValueFlow::Value::unknown();
         }
@@ -1622,7 +1647,10 @@ namespace {
                 }
                 return execute(expr->astOperand1());
             }
-            if (expr->exprId() > 0 && pm->hasValue(expr->exprId())) {
+            // A tracked value is the authoritative current value of its expression.
+            if (const ValueFlow::Value* tracked = getTrackedValue(expr))
+                return *tracked;
+            if (expr->exprId() > 0 && pm->hasValue(expr->exprId()) && !dependsOnTrackedValue(expr)) {
                 ValueFlow::Value result = utils::as_const(*pm).at(expr->exprId());
                 if (result.isImpossible() && result.isIntValue() && result.intvalue == 0 && isUsedAsBool(expr, settings)) {
                     result.intvalue = !result.intvalue;
@@ -1815,9 +1843,10 @@ namespace {
     };
 }     // namespace
 
-static ValueFlow::Value execute(const Token* expr, ProgramMemory& pm, const Settings& settings)
+static ValueFlow::Value execute(const Token* expr, ProgramMemory& pm, const Settings& settings, const ProgramMemory::Map& vars)
 {
     Executor ex{&pm, settings};
+    ex.vars = &vars;
     return ex.execute(expr);
 }
 
@@ -1907,9 +1936,10 @@ void execute(const Token* expr,
              ProgramMemory& programMemory,
              MathLib::bigint* result,
              bool* error,
-             const Settings& settings)
+             const Settings& settings,
+             const ProgramMemory::Map& vars)
 {
-    ValueFlow::Value v = execute(expr, programMemory, settings);
+    ValueFlow::Value v = execute(expr, programMemory, settings, vars);
     if (!v.isIntValue() || v.isImpossible()) {
         if (error)
             *error = true;

lib/programmemory.h

@@ -171,7 +171,7 @@ struct ProgramMemoryState {
 
     void addState(const Token* tok, const ProgramMemory::Map& vars);
 
-    void assume(const Token* tok, bool b, bool isEmpty = false);
+    void assume(const Token* tok, bool b, bool isEmpty = false, const Token* origin = nullptr);
 
     void removeModifiedVars(const Token* tok);
 
@@ -184,21 +184,24 @@ void execute(const Token* expr,
              ProgramMemory& programMemory,
              MathLib::bigint* result,
              bool* error,
-             const Settings& settings);
+             const Settings& settings,
+             const ProgramMemory::Map& vars = {});
 
 /**
  * Is condition always false when variable has given value?
  * \param condition   top ast token in condition
  * \param pm   program memory
+ * \param vars  optional tracked values that take precedence over the program memory
  */
-bool conditionIsFalse(const Token* condition, ProgramMemory pm, const Settings& settings);
+bool conditionIsFalse(const Token* condition, ProgramMemory pm, const Settings& settings, const ProgramMemory::Map& vars = {});
 
 /**
  * Is condition always true when variable has given value?
  * \param condition   top ast token in condition
  * \param pm   program memory
+ * \param vars  optional tracked values that take precedence over the program memory
  */
-bool conditionIsTrue(const Token* condition, ProgramMemory pm, const Settings& settings);
+bool conditionIsTrue(const Token* condition, ProgramMemory pm, const Settings& settings, const ProgramMemory::Map& vars = {});
 
 /**
  * Get program memory by looking backwards from given token.

lib/vf_analyzers.cpp

@@ -679,15 +679,19 @@ struct ValueFlowAnalyzer : Analyzer {
             return {v->intvalue};
         std::vector<MathLib::bigint> result;
         ProgramMemory pm = getProgramMemoryFunc();
+        // The tracked values are authoritative: pass them so a value cached in the program memory
+        // that depends on a tracked value (e.g. 'h(p)' after 'p' was reassigned) is re-evaluated
+        // rather than served stale.
+        const ProgramState vars = getProgramState();
         if (Token::Match(tok, "&&|%oror%")) {
-            if (conditionIsTrue(tok, pm, getSettings()))
+            if (conditionIsTrue(tok, pm, getSettings(), vars))
                 result.push_back(1);
-            if (conditionIsFalse(tok, std::move(pm), getSettings()))
+            if (conditionIsFalse(tok, std::move(pm), getSettings(), vars))
                 result.push_back(0);
         } else {
             MathLib::bigint out = 0;
             bool error = false;
-            execute(tok, pm, &out, &error, getSettings());
+            execute(tok, pm, &out, &error, getSettings(), vars);
             if (!error)
                 result.push_back(out);
         }
@@ -724,22 +728,36 @@ struct ValueFlowAnalyzer : Analyzer {
     }
 
     void assume(const Token* tok, bool state, unsigned int flags) override {
-        // Update program state
-        pms.removeModifiedVars(tok);
-        pms.addState(tok, getProgramState());
-        pms.assume(tok, state, flags & Assume::ContainerEmpty);
-
         bool isCondBlock = false;
         const Token* parent = tok->astParent();
         if (parent) {
             isCondBlock = Token::Match(parent->previous(), "if|while (");
         }
 
-        if (isCondBlock && !(flags & Assume::NoState)) {
-            const Token* startBlock = parent->link()->next();
+        const Token* startBlock = nullptr;
+        const Token* endBlock = nullptr;
+        if (isCondBlock) {
+            startBlock = parent->link()->next();
             if (Token::simpleMatch(startBlock, ";") && Token::simpleMatch(parent->tokAt(-2), "} while ("))
                 startBlock = parent->linkAt(-2);
-            const Token* endBlock = startBlock->link();
+            endBlock = startBlock->link();
+        }
+
+        // NoState is set only for the pre-traversal assume; without it the 'then' block has already
+        // been traversed and control is leaving it, so anchor the assumed state at the end of the
+        // block rather than at the condition. Assumptions about variables modified inside the block
+        // (e.g. an 'if' that narrows a value computed in the block) then survive past it, instead of
+        // being discarded because the variable was "modified" since the condition was evaluated.
+        const bool scopeEnd = !(flags & Assume::NoState) && state && endBlock;
+        const Token* anchor = scopeEnd ? endBlock : tok;
+        const Token* origin = scopeEnd ? endBlock : nullptr;
+
+        // Update program state
+        pms.removeModifiedVars(anchor);
+        pms.addState(anchor, getProgramState());
+        pms.assume(tok, state, flags & Assume::ContainerEmpty, origin);
+
+        if (isCondBlock && !(flags & Assume::NoState) && !scopeEnd) {
             if (state) {
                 pms.removeModifiedVars(endBlock);
                 pms.addState(endBlock->previous(), getProgramState());

test/testnullpointer.cpp

@@ -2450,6 +2450,9 @@ class TestNullPointer : public TestFixture {
 
     void nullpointer77()
     {
+        // No warning: 'i' is passed to the unknown function 'h' in the same condition that guards the
+        // dereference. 'h' may validate the pointer (e.g. return false for null), so '*i' can be safe
+        // - this is the common "if (check(p) && p->...)" pattern, so we must not assume 'i' is null.
         check("bool h(int*);\n"
               "void f(int* i) {\n"
               "    int* i = nullptr;\n"
@@ -2465,6 +2468,8 @@ class TestNullPointer : public TestFixture {
               "}\n");
         ASSERT_EQUALS("", errout_str());
 
+        // Likewise here, even though 'i' is null when the first 'h(i)' was true: the second 'h(i)' is an
+        // independent call that may validate 'i', so '*i' is not necessarily a null dereference.
         check("bool h(int*);\n"
               "void f(int* x) {\n"
               "    int* i = x;\n"

test/testuninitvar.cpp

@@ -5614,6 +5614,45 @@ class TestUninitVar : public TestFixture {
                         "}");
         ASSERT_EQUALS("[test.cpp:18:9] -> [test.cpp:12:13] -> [test.cpp:8:15]: (warning) Uninitialized variable: s->flag [uninitvar]\n", errout_str());
 
+        // A value narrowed by a nested condition (dimensions < 1 here) must survive the enclosing
+        // 'if' so a later correlated condition can be resolved: dimensions == 1 is then false, the
+        // function does not return, and the uninitialized read is reached.
+        valueFlowUninit("unsigned int g();\n"
+                        "void f(bool a) {\n"
+                        "    unsigned int dimensions = 0;\n"
+                        "    bool mightBeLarger;\n"
+                        "    if (a) {\n"
+                        "        dimensions = g();\n"
+                        "        if (dimensions >= 1)\n"
+                        "            mightBeLarger = false;\n"
+                        "    } else {\n"
+                        "        mightBeLarger = false;\n"
+                        "    }\n"
+                        "    if (dimensions == 1)\n"
+                        "        return;\n"
+                        "    if (!mightBeLarger) {}\n"
+                        "}");
+        ASSERT_EQUALS("[test.cpp:5:9] -> [test.cpp:7:24] -> [test.cpp:14:10]: (warning) Uninitialized variable: mightBeLarger [uninitvar]\n", errout_str());
+
+        // Same shape, but the later condition (dimensions == 0) is implied by the narrowing, so the
+        // early return fires on the uninitialized path and there must be no warning.
+        valueFlowUninit("unsigned int g();\n"
+                        "void f(bool a) {\n"
+                        "    unsigned int dimensions = 0;\n"
+                        "    bool mightBeLarger;\n"
+                        "    if (a) {\n"
+                        "        dimensions = g();\n"
+                        "        if (dimensions >= 1)\n"
+                        "            mightBeLarger = false;\n"
+                        "    } else {\n"
+                        "        mightBeLarger = false;\n"
+                        "    }\n"
+                        "    if (dimensions == 0)\n"
+                        "        return;\n"
+                        "    if (!mightBeLarger) {}\n"
+                        "}");
+        ASSERT_EQUALS("", errout_str());
+
         // Ticket #2207 - False negative
         valueFlowUninit("void foo() {\n"
                         "    int a;\n"