From 9da574f4a0587d5aae1cab69b9a5a4753ce2486d Mon Sep 17 00:00:00 2001
From: Paul Fultz II <pfultz2@yahoo.com>
Date: Sun, 18 Dec 2022 15:07:43 -0600
Subject: [PATCH] Fix 11227: ValueFlow: Known value not set in condition for
 ternary operator  (#4653)

* Evaluate args before function call

* Fix tests

* Format

* Add test for 11227

* Format

* Fix known condition
---
 lib/astutils.cpp        |  3 ++-
 lib/checkother.cpp      |  4 ++--
 lib/forwardanalyzer.cpp | 28 +++++++++++++++++++++++++++-
 lib/valueflow.cpp       |  2 +-
 test/testcondition.cpp  | 11 +++++++++++
 5 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/lib/astutils.cpp b/lib/astutils.cpp
index 0ff701174..7264aecaf 100644
--- a/lib/astutils.cpp
+++ b/lib/astutils.cpp
@@ -2422,7 +2422,8 @@ bool isVariableChanged(const Token *tok, int indirect, const Settings *settings,
         tok2 = tok2->astParent();
     }
 
-    while (Token::simpleMatch(tok2->astParent(), "?") || (Token::simpleMatch(tok2->astParent(), ":") && Token::simpleMatch(tok2->astParent()->astParent(), "?")))
+    while ((Token::simpleMatch(tok2, ":") && Token::simpleMatch(tok2->astParent(), "?")) ||
+           (Token::simpleMatch(tok2->astParent(), ":") && Token::simpleMatch(tok2->astParent()->astParent(), "?")))
         tok2 = tok2->astParent();
 
     if (tok2->astParent() && tok2->astParent()->tokType() == Token::eIncDecOp)
diff --git a/lib/checkother.cpp b/lib/checkother.cpp
index 0400cf671..727843c53 100644
--- a/lib/checkother.cpp
+++ b/lib/checkother.cpp
@@ -1643,8 +1643,8 @@ void CheckOther::constVariableError(const Variable *var, const Function *functio
     ErrorPath errorPath;
     std::string id = "const" + vartype;
     std::string message = "$symbol:" + varname + "\n" + vartype + " '$symbol' can be declared as " + ptrRefArray;
-    errorPath.emplace_back(var ? var->nameToken() : nullptr, message);
-    if (var && var->isArgument() && function && function->functionPointerUsage) {
+    errorPath.emplace_back(var->nameToken(), message);
+    if (var->isArgument() && function && function->functionPointerUsage) {
         errorPath.emplace_front(function->functionPointerUsage, "You might need to cast the function pointer here");
         id += "Callback";
         message += ". However it seems that '" + function->name() + "' is a callback function, if '$symbol' is declared with const you might also need to cast function pointer(s).";
diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp
index 15baa44b1..68b82450b 100644
--- a/lib/forwardanalyzer.cpp
+++ b/lib/forwardanalyzer.cpp
@@ -805,6 +805,18 @@ struct ForwardTraversal {
                 if (updateRecursive(tok->next()->astOperand2()) == Progress::Break)
                     return Break();
                 return Break();
+            } else if (Token* callTok = callExpr(tok)) {
+                // Since the call could be an unknown macro, traverse the tokens as a range instead of recursively
+                if (!Token::simpleMatch(callTok, "( )") &&
+                    updateRange(callTok->next(), callTok->link(), depth - 1) == Progress::Break)
+                    return Break();
+                if (updateTok(callTok) == Progress::Break)
+                    return Break();
+                if (start != callTok && updateRecursive(callTok->astOperand1()) == Progress::Break)
+                    return Break();
+                tok = callTok->link();
+                if (!tok)
+                    return Break();
             } else {
                 if (updateTok(tok, &next) == Progress::Break)
                     return Break();
@@ -812,7 +824,7 @@ struct ForwardTraversal {
                     if (precedes(next, end))
                         tok = next->previous();
                     else
-                        return Break();
+                        return Progress::Continue;
                 }
             }
             // Prevent infinite recursion
@@ -852,6 +864,20 @@ struct ForwardTraversal {
         return nullptr;
     }
 
+    static Token* callExpr(Token* tok)
+    {
+        while (tok->astParent() && astIsLHS(tok)) {
+            if (!Token::Match(tok, "%name%|::|<|."))
+                break;
+            if (Token::simpleMatch(tok, "<") && !tok->link())
+                break;
+            tok = tok->astParent();
+        }
+        if (isFunctionCall(tok))
+            return tok;
+        return nullptr;
+    }
+
     static Token* skipTo(Token* tok, const Token* dest, const Token* end = nullptr) {
         if (end && dest->index() > end->index())
             return nullptr;
diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp
index cab427931..3a70d2df4 100644
--- a/lib/valueflow.cpp
+++ b/lib/valueflow.cpp
@@ -8358,7 +8358,7 @@ static void valueFlowContainerSize(TokenList* tokenlist,
                     ValueFlow::Value value(tok->tokAt(2)->astOperand2()->values().front());
                     value.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE;
                     value.setKnown();
-                    valueFlowForward(tok->next(), containerTok, value, tokenlist);
+                    valueFlowForward(tok->linkAt(2), containerTok, value, tokenlist);
                 }
             }
         }
diff --git a/test/testcondition.cpp b/test/testcondition.cpp
index 2a5640abf..62e3731bc 100644
--- a/test/testcondition.cpp
+++ b/test/testcondition.cpp
@@ -4457,6 +4457,17 @@ private:
               "        return;\n"
               "}\n");
         ASSERT_EQUALS("", errout.str());
+
+        // #11227
+        check("struct S {\n"
+              "	int get();\n"
+              "};\n"
+              "void f(const S* s) {\n"
+              "    if (!s)\n"
+              "        return;\n"
+              "    g(s ? s->get() : 0);\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:7]: (style) Condition 's' is always true\n", errout.str());
     }
 
     void alwaysTrueSymbolic()