Fix issue 7804: ValueFlow: possible value in second if body (#2543)

lib/forwardanalyzer.cpp

@@ -267,7 +267,7 @@ struct ForwardTraversal {
                 } else if (condTok->values().front().intvalue == !inElse) {
                     return Progress::Break;
                 }
-                analyzer->assume(condTok, !inElse);
+                analyzer->assume(condTok, !inElse, tok);
                 if (Token::simpleMatch(tok, "} else {"))
                     tok = tok->linkAt(2);
             } else if (Token::Match(tok, "if|while|for (") && Token::simpleMatch(tok->next()->link(), ") {")) {
@@ -366,7 +366,7 @@ struct ForwardTraversal {
                 if (checkElse)
                     return Progress::Break;
                 if (!checkThen)
-                    analyzer->assume(condTok, true);
+                    analyzer->assume(condTok, true, tok);
             } else if (Token::simpleMatch(tok, "switch (")) {
                 if (updateRecursive(tok->next()->astOperand2()) == Progress::Break)
                     return Progress::Break;

lib/forwardanalyzer.h

@@ -108,8 +108,8 @@ struct ForwardAnalyzer {
     virtual bool updateScope(const Token* endBlock, bool modified) const = 0;
     /// If the value is conditional
     virtual bool isConditional() const = 0;
-    /// The condition that will be assumed during analysis
+    /// The condtion that wil be assumes during analysis
-    virtual void assume(const Token* tok, bool state) = 0;
+    virtual void assume(const Token* tok, bool state, const Token* at = nullptr) = 0;
     virtual ~ForwardAnalyzer() {}
 };
 

lib/programmemory.cpp

@@ -12,7 +12,7 @@ void ProgramMemory::setValue(nonneg int varid, const ValueFlow::Value &value)
 
 bool ProgramMemory::getIntValue(nonneg int varid, MathLib::bigint* result) const
 {
-    const std::map<int, ValueFlow::Value>::const_iterator it = values.find(varid);
+    const ProgramMemory::Map::const_iterator it = values.find(varid);
     const bool found = it != values.end() && it->second.isIntValue();
     if (found)
         *result = it->second.intvalue;
@@ -26,7 +26,7 @@ void ProgramMemory::setIntValue(nonneg int varid, MathLib::bigint value)
 
 bool ProgramMemory::getTokValue(nonneg int varid, const Token** result) const
 {
-    const std::map<int, ValueFlow::Value>::const_iterator it = values.find(varid);
+    const ProgramMemory::Map::const_iterator it = values.find(varid);
     const bool found = it != values.end() && it->second.isTokValue();
     if (found)
         *result = it->second.tokvalue;
@@ -70,7 +70,6 @@ void ProgramMemory::insert(const ProgramMemory &pm)
         values.insert(p);
 }
 
-
 bool conditionIsFalse(const Token *condition, const ProgramMemory &programMemory)
 {
     if (!condition)
@@ -117,7 +116,7 @@ static void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, con
             return;
         if (!truevalue.isIntValue())
             return;
-        if (isVariableChanged(tok->next(), endTok, vartok->varId(), false, settings, true))
+        if (endTok && isVariableChanged(tok->next(), endTok, vartok->varId(), false, settings, true))
             return;
         pm.setIntValue(vartok->varId(),  then ? truevalue.intvalue : falsevalue.intvalue);
     } else if (Token::Match(tok, "%var%")) {
@@ -125,7 +124,7 @@ static void programMemoryParseCondition(ProgramMemory& pm, const Token* tok, con
             return;
         if (then && !astIsPointer(tok) && !astIsBool(tok))
             return;
-        if (isVariableChanged(tok->next(), endTok, tok->varId(), false, settings, true))
+        if (endTok && isVariableChanged(tok->next(), endTok, tok->varId(), false, settings, true))
             return;
         pm.setIntValue(tok->varId(), then);
     } else if (Token::simpleMatch(tok, "!")) {
@@ -172,7 +171,7 @@ static void fillProgramMemoryFromConditions(ProgramMemory& pm, const Token* tok,
     fillProgramMemoryFromConditions(pm, tok->scope(), tok, settings);
 }
 
-static void fillProgramMemoryFromAssignments(ProgramMemory& pm, const Token* tok, const ProgramMemory& state, std::unordered_map<nonneg int, ValueFlow::Value> vars)
+static void fillProgramMemoryFromAssignments(ProgramMemory& pm, const Token* tok, const ProgramMemory& state, ProgramMemory::Map vars)
 {
     int indentlevel = 0;
     for (const Token *tok2 = tok; tok2; tok2 = tok2->previous()) {
@@ -234,7 +233,7 @@ static void removeModifiedVars(ProgramMemory& pm, const Token* tok, const Token*
 
 static ProgramMemory getInitialProgramState(const Token* tok,
         const Token* origin,
-        const std::unordered_map<nonneg int, ValueFlow::Value>& vars = std::unordered_map<nonneg int, ValueFlow::Value> {})
+        const ProgramMemory::Map& vars = ProgramMemory::Map {})
 {
     ProgramMemory pm;
     if (origin) {
@@ -246,7 +245,67 @@ static ProgramMemory getInitialProgramState(const Token* tok,
     return pm;
 }
 
-ProgramMemory getProgramMemory(const Token *tok, const std::unordered_map<nonneg int, ValueFlow::Value>& vars)
+void ProgramMemoryState::insert(const ProgramMemory &pm, const Token* origin)
+{
+    if(origin)
+        for(auto&& p:pm.values)
+            origins.insert(std::make_pair(p.first, origin));
+    state.insert(pm);
+}
+
+void ProgramMemoryState::replace(const ProgramMemory &pm, const Token* origin)
+{
+    if(origin)
+        for(auto&& p:pm.values)
+            origins[p.first] = origin;
+    state.replace(pm);
+}
+
+void ProgramMemoryState::addState(const Token* tok, const ProgramMemory::Map& vars)
+{
+    ProgramMemory pm;
+    fillProgramMemoryFromConditions(pm, tok, nullptr);
+    ProgramMemory local;
+    for (const auto& p:vars) {
+        nonneg int varid = p.first;
+        const ValueFlow::Value &value = p.second;
+        pm.setValue(varid, value);
+        if (value.varId)
+            pm.setIntValue(value.varId, value.varvalue);
+    }
+    local = pm;
+    fillProgramMemoryFromAssignments(pm, tok, local, vars);
+    replace(pm, tok);
+}
+
+void ProgramMemoryState::assume(const Token* tok, bool b)
+{
+    ProgramMemory pm = state;
+    programMemoryParseCondition(pm, tok, nullptr, nullptr, b);
+    insert(pm, tok);
+}
+
+void ProgramMemoryState::removeModifiedVars(const Token* tok)
+{
+    for (auto i = state.values.begin(), last = state.values.end(); i != last;) {
+        if (isVariableChanged(origins[i->first], tok, i->first, false, nullptr, true)) {
+            origins.erase(i->first);
+            i = state.values.erase(i);
+        } else {
+            ++i;
+        }
+    }
+}
+
+ProgramMemory ProgramMemoryState::get(const Token *tok, const ProgramMemory::Map& vars) const
+{
+    ProgramMemoryState local = *this;
+    local.addState(tok, vars);
+    local.removeModifiedVars(tok);
+    return local.state;
+}
+
+ProgramMemory getProgramMemory(const Token *tok, const ProgramMemory::Map& vars)
 {
     ProgramMemory programMemory;
     for (const auto& p:vars) {
@@ -262,8 +321,8 @@ ProgramMemory getProgramMemory(const Token *tok, const std::unordered_map<nonneg
         programMemory.setValue(varid, value);
         if (value.varId)
             programMemory.setIntValue(value.varId, value.varvalue);
-        state = programMemory;
     }
+    state = programMemory;
     fillProgramMemoryFromAssignments(programMemory, tok, state, vars);
     return programMemory;
 }

lib/programmemory.h

@@ -9,7 +9,8 @@
 #include <unordered_map>
 
 struct ProgramMemory {
-    std::map<int, ValueFlow::Value> values;
+    using Map = std::unordered_map<nonneg int, ValueFlow::Value>;
+    Map values;
 
     void setValue(nonneg int varid, const ValueFlow::Value &value);
 
@@ -32,6 +33,23 @@ struct ProgramMemory {
     void insert(const ProgramMemory &pm);
 };
 
+struct ProgramMemoryState {
+    ProgramMemory state;
+    std::map<nonneg int, const Token*> origins;
+
+    void insert(const ProgramMemory &pm, const Token* origin = nullptr);
+    void replace(const ProgramMemory &pm, const Token* origin = nullptr);
+
+    void addState(const Token* tok, const ProgramMemory::Map& vars);
+
+    void assume(const Token* tok, bool b);
+
+    void removeModifiedVars(const Token* tok);
+
+    ProgramMemory get(const Token *tok, const ProgramMemory::Map& vars) const;
+
+};
+
 void execute(const Token *expr,
              ProgramMemory * const programMemory,
              MathLib::bigint *result,
@@ -56,7 +74,7 @@ bool conditionIsTrue(const Token *condition, const ProgramMemory &programMemory)
  */
 ProgramMemory getProgramMemory(const Token *tok, nonneg int varid, const ValueFlow::Value &value);
 
-ProgramMemory getProgramMemory(const Token *tok, const std::unordered_map<nonneg int, ValueFlow::Value>& vars);
+ProgramMemory getProgramMemory(const Token *tok, const ProgramMemory::Map& vars);
 
 #endif
 

lib/templatesimplifier.cpp

@@ -3025,9 +3025,10 @@ bool TemplateSimplifier::simplifyTemplateInstantiations(
                 startToken = startToken->tokAt(-2);
         }
 
-        if (Token::Match(startToken->previous(), ";|{|}|=|const") &&
+        // TODO: re-enable when specialized check is removed
-            (!specialized && !instantiateMatch(tok2, typeParametersInDeclaration.size(), isfunc ? "(" : isVar ? ";|%op%|(" : "*|&|::| %name%")))
+        // if (Token::Match(startToken->previous(), ";|{|}|=|const") &&
-            return false;
+        //     (!specialized && !instantiateMatch(tok2, typeParametersInDeclaration.size(), isfunc ? "(" : isVar ? ";|%op%|(" : "*|&|::| %name%")))
+        //     return false;
 
         // already simplified
         if (!Token::Match(tok2, "%name% <"))

lib/valueflow.cpp

@@ -2164,13 +2164,14 @@ struct SelectMapKeys {
 
 struct ValueFlowForwardAnalyzer : ForwardAnalyzer {
     const TokenList* tokenlist;
+    ProgramMemoryState pms;
 
     ValueFlowForwardAnalyzer()
-        : tokenlist(nullptr)
+        : tokenlist(nullptr), pms()
     {}
 
     ValueFlowForwardAnalyzer(const TokenList* t)
-        : tokenlist(t)
+        : tokenlist(t), pms()
     {}
 
     virtual int getIndirect() const = 0;
@@ -2271,11 +2272,10 @@ struct ValueFlowForwardAnalyzer : ForwardAnalyzer {
         if (tok->hasKnownIntValue())
             return {static_cast<int>(tok->values().front().intvalue)};
         std::vector<int> result;
-        ProgramState ps = getProgramState();
+        ProgramMemory pm = pms.get(tok, getProgramState());
-        ProgramMemory programMemory = getProgramMemory(tok, ps);
+        if (conditionIsTrue(tok, pm))
-        if (conditionIsTrue(tok, programMemory))
             result.push_back(1);
-        if (conditionIsFalse(tok, programMemory))
+        if (conditionIsFalse(tok, pm))
             result.push_back(0);
         return result;
     }
@@ -2338,9 +2338,21 @@ struct SingleValueFlowForwardAnalyzer : ValueFlowForwardAnalyzer {
         return true;
     }
 
-    virtual void assume(const Token*, bool) OVERRIDE {
+    virtual void assume(const Token* tok, bool state, const Token* at) OVERRIDE {
-        // TODO: Use this to improve Evaluate
+        // Update program state
-        value.conditional = true;
+        pms.removeModifiedVars(tok);
+        pms.addState(tok, getProgramState());
+        pms.assume(tok, state);
+
+        const bool isAssert = Token::Match(at, "assert|ASSERT");
+        const bool isEndScope = Token::simpleMatch(at, "}");
+
+        if (!isAssert && !isEndScope) {
+            std::string s = state ? "true" : "false";
+            value.errorPath.emplace_back(tok, "Assuming condition is " + s);
+        }
+        if (!isAssert)
+            value.conditional = true;
     }
 
     virtual bool isConditional() const OVERRIDE {
@@ -4488,7 +4500,7 @@ static void valueFlowForLoop(TokenList *tokenlist, SymbolDatabase* symboldatabas
         } else {
             ProgramMemory mem1, mem2, memAfter;
             if (valueFlowForLoop2(tok, &mem1, &mem2, &memAfter)) {
-                std::map<int, ValueFlow::Value>::const_iterator it;
+                ProgramMemory::Map::const_iterator it;
                 for (it = mem1.values.begin(); it != mem1.values.end(); ++it) {
                     if (!it->second.isIntValue())
                         continue;

test/testnullpointer.cpp

@@ -1667,7 +1667,7 @@ private:
               "    *p +=2;\n"
               "    if(n < 120) *q+=12;\n"
               "}\n");
-        TODO_ASSERT_EQUALS("[test.cpp:5]: (warning) Possible null pointer dereference: q\n", "", errout.str());
+        ASSERT_EQUALS("[test.cpp:5]: (warning) Possible null pointer dereference: q\n", errout.str());
 
         check("void f(int *p, int n) {\n"
               "    int *q = 0;\n"