Skip forwarding values for unique expressions (#5103)

lib/analyzer.h

@@ -181,6 +181,9 @@ struct Analyzer {
     virtual void assume(const Token* tok, bool state, unsigned int flags = 0) = 0;
     /// Return analyzer for expression at token
     virtual ValuePtr<Analyzer> reanalyze(Token* tok, const std::string& msg = emptyString) const = 0;
+    virtual bool invalid() const {
+        return false;
+    }
     virtual ~Analyzer() {}
     Analyzer(const Analyzer&) = default;
 protected:

lib/forwardanalyzer.cpp

@@ -894,6 +894,8 @@ struct ForwardTraversal {
 
 Analyzer::Result valueFlowGenericForward(Token* start, const Token* end, const ValuePtr<Analyzer>& a, const Settings& settings)
 {
+    if (a->invalid())
+        return Analyzer::Result{Analyzer::Action::None, Analyzer::Terminate::Bail};
     ForwardTraversal ft{a, settings};
     ft.updateRange(start, end);
     return Analyzer::Result{ ft.actions, ft.terminate };
@@ -903,6 +905,8 @@ Analyzer::Result valueFlowGenericForward(Token* start, const ValuePtr<Analyzer>&
 {
     if (Settings::terminated())
         throw TerminateException();
+    if (a->invalid())
+        return Analyzer::Result{Analyzer::Action::None, Analyzer::Terminate::Bail};
     ForwardTraversal ft{a, settings};
     ft.updateRecursive(start);
     return Analyzer::Result{ ft.actions, ft.terminate };

lib/reverseanalyzer.cpp

@@ -390,6 +390,8 @@ struct ReverseTraversal {
 
 void valueFlowGenericReverse(Token* start, const Token* end, const ValuePtr<Analyzer>& a, const Settings& settings)
 {
+    if (a->invalid())
+        return;
     ReverseTraversal rt{a, settings};
     rt.traverse(start, end);
 }

lib/symboldatabase.cpp

@@ -1625,7 +1625,7 @@ void SymbolDatabase::createSymbolDatabaseExprIds()
                 exprs[tok->str()].push_back(tok);
                 tok->exprId(id++);
 
-                if (id == std::numeric_limits<nonneg int>::max()) {
+                if (id == std::numeric_limits<nonneg int>::max() / 4) {
                     throw InternalError(nullptr, "Ran out of expression ids.", InternalError::INTERNAL);
                 }
             } else if (isCPP() && Token::simpleMatch(tok, "this")) {
@@ -1653,6 +1653,27 @@ void SymbolDatabase::createSymbolDatabaseExprIds()
                 }
             }
         }
+        // Mark expressions that are unique
+        std::unordered_map<nonneg int, Token*> exprMap;
+        for (Token* tok = const_cast<Token*>(scope->bodyStart); tok != scope->bodyEnd; tok = tok->next()) {
+            if (tok->exprId() == 0)
+                continue;
+            auto p = exprMap.emplace(tok->exprId(), tok);
+            // Already exists so set it to null
+            if (!p.second) {
+                p.first->second = nullptr;
+            }
+        }
+        for (const auto& p : exprMap) {
+            if (!p.second)
+                continue;
+            if (p.second->variable()) {
+                const Variable* var = p.second->variable();
+                if (var->nameToken() != p.second)
+                    continue;
+            }
+            p.second->setUniqueExprId();
+        }
     }
 }
 

lib/token.h

@@ -901,6 +901,20 @@ public:
         mImpl->mExprId = id;
     }
 
+    void setUniqueExprId()
+    {
+        assert(mImpl->mExprId > 0);
+        mImpl->mExprId |= 1 << efIsUnique;
+    }
+
+    bool isUniqueExprId() const
+    {
+        if (mImpl->mExprId > 0) {
+            return (mImpl->mExprId & (1 << efIsUnique)) != 0;
+        }
+        return false;
+    }
+
     /**
      * For debugging purposes, prints token and all tokens
      * followed by it.
@@ -1327,6 +1341,11 @@ private:
         fIsFinalType            = (1ULL << 40), // Is this a type with final specifier
     };
 
+    enum : uint64_t {
+        efMaxSize = sizeof(nonneg int) * 8,
+        efIsUnique = efMaxSize - 2,
+    };
+
     Token::Type mTokType;
 
     uint64_t mFlags;

lib/valueflow.cpp

@@ -2438,10 +2438,6 @@ struct ValueFlowAnalyzer : Analyzer {
         return false;
     }
 
-    virtual bool invalid() const {
-        return false;
-    }
-
     bool isCPP() const {
         return tokenlist->isCPP();
     }
@@ -3134,11 +3130,20 @@ struct ExpressionAnalyzer : SingleValueFlowAnalyzer {
     bool local;
     bool unknown;
     bool dependOnThis;
+    bool uniqueExprId;
 
-    ExpressionAnalyzer() : SingleValueFlowAnalyzer(), expr(nullptr), local(true), unknown(false), dependOnThis(false) {}
+    ExpressionAnalyzer()
+        : SingleValueFlowAnalyzer(), expr(nullptr), local(true), unknown(false), dependOnThis(false), uniqueExprId(false)
+    {}
 
     ExpressionAnalyzer(const Token* e, ValueFlow::Value val, const TokenList* t, const Settings* s)
-        : SingleValueFlowAnalyzer(std::move(val), t, s), expr(e), local(true), unknown(false), dependOnThis(false) {
+        : SingleValueFlowAnalyzer(std::move(val), t, s),
+        expr(e),
+        local(true),
+        unknown(false),
+        dependOnThis(false),
+        uniqueExprId(false)
+    {
 
         assert(e && e->exprId() != 0 && "Not a valid expression");
         dependOnThis = exprDependsOnThis(expr);
@@ -3147,6 +3152,8 @@ struct ExpressionAnalyzer : SingleValueFlowAnalyzer {
             dependOnThis |= exprDependsOnThis(value.tokvalue);
             setupExprVarIds(value.tokvalue);
         }
+        uniqueExprId =
+            expr->isUniqueExprId() && (Token::Match(expr, "%cop%") || !isVariableChanged(expr, 0, s, t->isCPP()));
     }
 
     static bool nonLocal(const Variable* var, bool deref) {
@@ -3196,7 +3203,13 @@ struct ExpressionAnalyzer : SingleValueFlowAnalyzer {
         });
     }
 
+    virtual bool skipUniqueExprIds() const {
+        return true;
+    }
+
     bool invalid() const override {
+        if (skipUniqueExprIds() && uniqueExprId)
+            return true;
         return unknown;
     }
 
@@ -3231,6 +3244,10 @@ struct SameExpressionAnalyzer : ExpressionAnalyzer {
         : ExpressionAnalyzer(e, std::move(val), t, s)
     {}
 
+    bool skipUniqueExprIds() const override {
+        return false;
+    }
+
     bool match(const Token* tok) const override
     {
         return isSameExpression(isCPP(), true, expr, tok, getSettings()->library, true, true);
@@ -3246,6 +3263,10 @@ struct OppositeExpressionAnalyzer : ExpressionAnalyzer {
         : ExpressionAnalyzer(e, std::move(val), t, s), isNot(pIsNot)
     {}
 
+    bool skipUniqueExprIds() const override {
+        return false;
+    }
+
     bool match(const Token* tok) const override {
         return isOppositeCond(isNot, isCPP(), expr, tok, getSettings()->library, true, true);
     }

test/testsimplifytypedef.cpp

@@ -1602,7 +1602,9 @@ private:
                             "}";
 
         checkSimplifyTypedef(code);
-        ASSERT_EQUALS("", errout.str());
+        ASSERT_EQUALS_WITHOUT_LINENUMBERS(
+            "[test.cpp:3]: (debug) valueflow.cpp:6541:(valueFlow) bailout: valueFlowAfterCondition: bailing in conditional block\n",
+            errout.str());
     }
 
     void simplifyTypedef46() {

test/testvarid.cpp

@@ -3749,14 +3749,15 @@ private:
             "    return x + y + a.y + b.y;\n"
             "}\n");
 
-        const char expected[] = "1: struct A {\n"
+        const char expected[] =
-                                "2: int x ; int y ;\n"
+            "1: struct A {\n"
-                                "3: } ;\n"
+            "2: int x ; int y ;\n"
-                                "4: int f ( A a , A b ) {\n"
+            "3: } ;\n"
-                                "5: int x@5 ; x@5 =@9 a@3 .@10 x@6 +@11 b@4 .@12 x@7 ;\n"
+            "4: int f ( A a , A b ) {\n"
-                                "6: int y@8 ; y@8 =@13 b@4 .@12 x@7 +@11 a@3 .@10 x@6 ;\n"
+            "5: int x@5 ; x@5 =@9 a@3 .@10 x@6 +@11 b@4 .@12 x@7 ;\n"
-                                "7: return x@5 +@17 y@8 +@18 a@3 .@19 y@9 +@20 b@4 .@21 y@10 ;\n"
+            "6: int y@8 ; y@8 =@1073741837 b@4 .@12 x@7 +@11 a@3 .@10 x@6 ;\n"
-                                "8: }\n";
+            "7: return x@5 +@1073741841 y@8 +@1073741842 a@3 .@1073741843 y@9 +@1073741844 b@4 .@1073741845 y@10 ;\n"
+            "8: }\n";
 
         ASSERT_EQUALS(expected, actual);
     }