From 08041e3a0b33ccb4b695df5fa14ac1c384c243c4 Mon Sep 17 00:00:00 2001
From: Paul Fultz II <pfultz2@yahoo.com>
Date: Mon, 2 Aug 2021 23:31:52 -0500
Subject: [PATCH] Refactor: Assert an expression has an id and fix assertion
 failures (#3377)

* Fix 10390: FP: knownConditionTrueFalse

* Assert expression has an ID

* Fix assertion errors

* Format
---
 lib/reverseanalyzer.cpp |  3 ++-
 lib/valueflow.cpp       | 39 ++++++++++++++++++++++++++++-----------
 2 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/lib/reverseanalyzer.cpp b/lib/reverseanalyzer.cpp
index d9989c1e1..4481df841 100644
--- a/lib/reverseanalyzer.cpp
+++ b/lib/reverseanalyzer.cpp
@@ -162,7 +162,7 @@ struct ReverseTraversal {
                     Analyzer::Action lhsAction =
                         analyzer->analyze(assignTok->astOperand1(), Analyzer::Direction::Reverse);
                     // Assignment from
-                    if (rhsAction.isRead() && !lhsAction.isInvalid()) {
+                    if (rhsAction.isRead() && !lhsAction.isInvalid() && assignTok->astOperand1()->exprId() > 0) {
                         const std::string info = "Assignment from '" + assignTok->expressionString() + "'";
                         ValuePtr<Analyzer> a = analyzer->reanalyze(assignTok->astOperand1(), info);
                         if (a) {
@@ -173,6 +173,7 @@ struct ReverseTraversal {
                         }
                         // Assignment to
                     } else if (lhsAction.matches() && !assignTok->astOperand2()->hasKnownIntValue() &&
+                               assignTok->astOperand2()->exprId() > 0 &&
                                isConstExpression(assignTok->astOperand2(), settings->library, true, true)) {
                         const std::string info = "Assignment to '" + assignTok->expressionString() + "'";
                         ValuePtr<Analyzer> a = analyzer->reanalyze(assignTok->astOperand2(), info);
diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp
index ae7d7ac07..20521adab 100644
--- a/lib/valueflow.cpp
+++ b/lib/valueflow.cpp
@@ -2412,6 +2412,7 @@ struct ExpressionAnalyzer : SingleValueFlowAnalyzer {
     ExpressionAnalyzer(const Token* e, const ValueFlow::Value& val, const TokenList* t)
         : SingleValueFlowAnalyzer(val, t), expr(e), local(true), unknown(false), dependOnThis(false) {
 
+        assert(e && e->exprId() != 0 && "Not a valid expression");
         dependOnThis = exprDependsOnThis(expr);
         setupExprVarIds(expr);
         if (val.isSymbolicValue())
@@ -2521,6 +2522,10 @@ static const Token* parseBinaryIntOp(const Token* expr, MathLib::bigint& known)
         return nullptr;
     if (!expr->astOperand1() || !expr->astOperand2())
         return nullptr;
+    if (expr->astOperand1()->exprId() == 0 && !expr->astOperand1()->hasKnownIntValue())
+        return nullptr;
+    if (expr->astOperand2()->exprId() == 0 && !expr->astOperand2()->hasKnownIntValue())
+        return nullptr;
     const Token* knownTok = nullptr;
     const Token* varTok = nullptr;
     if (expr->astOperand1()->hasKnownIntValue() && !expr->astOperand2()->hasKnownIntValue()) {
@@ -3932,6 +3937,23 @@ static ValueFlow::Value makeConditionValue(long long val, const Token* condTok,
         v.errorPath.emplace_back(condTok, "Assuming condition '" + condTok->expressionString() + "' is false");
     return v;
 }
+
+static std::vector<const Token*> getConditions(const Token* tok, const char* op)
+{
+    std::vector<const Token*> conds = {tok};
+    if (tok->str() == op) {
+        std::vector<const Token*> args = astFlatten(tok, op);
+        std::copy_if(args.begin(), args.end(), std::back_inserter(conds), [&](const Token* tok2) {
+            if (tok2->exprId() == 0)
+                return false;
+            if (tok2->hasKnownIntValue())
+                return false;
+            return true;
+        });
+    }
+    return conds;
+}
+
 //
 static void valueFlowConditionExpressions(TokenList *tokenlist, SymbolDatabase* symboldatabase, ErrorLogger *errorLogger, const Settings *settings)
 {
@@ -3952,6 +3974,8 @@ static void valueFlowConditionExpressions(TokenList *tokenlist, SymbolDatabase*
                 continue;
             Token * blockTok = parenTok->link()->tokAt(1);
             const Token* condTok = parenTok->astOperand2();
+            if (condTok->exprId() == 0)
+                continue;
             if (condTok->hasKnownIntValue())
                 continue;
             if (!isConstExpression(condTok, settings->library, true, tokenlist->isCPP()))
@@ -3961,12 +3985,7 @@ static void valueFlowConditionExpressions(TokenList *tokenlist, SymbolDatabase*
             Token* startTok = blockTok;
             // Inner condition
             {
-                std::vector<const Token*> conds = {condTok};
-                if (Token::simpleMatch(condTok, "&&")) {
-                    std::vector<const Token*> args = astFlatten(condTok, "&&");
-                    conds.insert(conds.end(), args.begin(), args.end());
-                }
-                for (const Token* condTok2:conds) {
+                for (const Token* condTok2 : getConditions(condTok, "&&")) {
                     if (is1) {
                         ExpressionAnalyzer a1(condTok2, makeConditionValue(1, condTok2, true), tokenlist);
                         valueFlowGenericForward(startTok, startTok->link(), a1, settings);
@@ -3977,11 +3996,7 @@ static void valueFlowConditionExpressions(TokenList *tokenlist, SymbolDatabase*
                 }
             }
 
-            std::vector<const Token*> conds = {condTok};
-            if (Token::simpleMatch(condTok, "||")) {
-                std::vector<const Token*> args = astFlatten(condTok, "||");
-                conds.insert(conds.end(), args.begin(), args.end());
-            }
+            std::vector<const Token*> conds = getConditions(condTok, "||");
 
             // Check else block
             if (Token::simpleMatch(startTok->link(), "} else {")) {
@@ -4459,6 +4474,8 @@ struct ConditionHandler {
                 for (const Condition& cond : parse(tok, settings)) {
                     if (!cond.vartok)
                         continue;
+                    if (cond.vartok->exprId() == 0)
+                        continue;
                     if (cond.vartok->hasKnownIntValue())
                         continue;
                     if (cond.true_values.empty() || cond.false_values.empty())