From 48c91abba685d28129a8d89ce5c6d0e375c1e2b1 Mon Sep 17 00:00:00 2001
From: Paul Fultz II <pfultz2@yahoo.com>
Date: Sun, 13 Aug 2023 15:31:38 -0500
Subject: [PATCH] Fix issue 11867: Assert failure in valueFlowContainerSize()
 (#5317)

---
 lib/astutils.cpp       | 12 ++++++++++++
 lib/astutils.h         |  1 +
 lib/valueflow.cpp      | 35 +++++++++++++++++++++++------------
 test/testcondition.cpp | 15 +++++++++++++++
 4 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/lib/astutils.cpp b/lib/astutils.cpp
index db5058b07..e69b78659 100644
--- a/lib/astutils.cpp
+++ b/lib/astutils.cpp
@@ -259,6 +259,18 @@ bool astIsContainerOwned(const Token* tok) {
     return astIsContainer(tok) && !astIsContainerView(tok);
 }
 
+bool astIsContainerString(const Token* tok)
+{
+    if (!tok)
+        return false;
+    if (!tok->valueType())
+        return false;
+    const Library::Container* container = tok->valueType()->container;
+    if (!container)
+        return false;
+    return container->stdStringLike;
+}
+
 static const Token* getContainerFunction(const Token* tok)
 {
     if (!tok || !tok->valueType() || !tok->valueType()->container)
diff --git a/lib/astutils.h b/lib/astutils.h
index f486418ef..4ffd0aa79 100644
--- a/lib/astutils.h
+++ b/lib/astutils.h
@@ -149,6 +149,7 @@ bool astIsContainer(const Token *tok);
 
 bool astIsContainerView(const Token* tok);
 bool astIsContainerOwned(const Token* tok);
+bool astIsContainerString(const Token* tok);
 
 Library::Container::Action astContainerAction(const Token* tok, const Token** ftok = nullptr);
 Library::Container::Yield astContainerYield(const Token* tok, const Token** ftok = nullptr);
diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp
index 85a64f022..c18e2ce45 100644
--- a/lib/valueflow.cpp
+++ b/lib/valueflow.cpp
@@ -696,7 +696,11 @@ static void setTokenValue(Token* tok,
             if (value.isImpossible()) {
                 if (value.intvalue == 0)
                     v.setKnown();
-                else
+                else if ((value.bound == ValueFlow::Value::Bound::Upper && value.intvalue > 0) ||
+                         (value.bound == ValueFlow::Value::Bound::Lower && value.intvalue < 0)) {
+                    v.intvalue = 0;
+                    v.setKnown();
+                } else
                     v.setPossible();
             } else {
                 v.intvalue = !v.intvalue;
@@ -8660,6 +8664,9 @@ static void valueFlowContainerSetTokValue(TokenList& tokenlist, const Settings*
     ValueFlow::Value value;
     value.valueType = ValueFlow::Value::ValueType::TOK;
     value.tokvalue = initList;
+    if (astIsContainerString(tok) && Token::simpleMatch(initList, "{") && Token::Match(initList->astOperand2(), "%str%")) {
+        value.tokvalue = initList->astOperand2();
+    }
     value.setKnown();
     Token* start = initList->link() ? initList->link() : initList->next();
     if (tok->variable() && tok->variable()->isConst()) {
@@ -8675,6 +8682,19 @@ static const Scope* getFunctionScope(const Scope* scope) {
     return scope;
 }
 
+static MathLib::bigint valueFlowGetStrLength(const Token* tok)
+{
+    if (tok->tokType() == Token::eString)
+        return Token::getStrLength(tok);
+    if (astIsGenericChar(tok) || tok->tokType() == Token::eChar)
+        return 1;
+    if (const ValueFlow::Value* v2 = tok->getKnownValue(ValueFlow::Value::ValueType::CONTAINER_SIZE))
+        return v2->intvalue;
+    if (const ValueFlow::Value* v1 = tok->getKnownValue(ValueFlow::Value::ValueType::TOK))
+        return valueFlowGetStrLength(v1->tokvalue);
+    return 0;
+}
+
 static void valueFlowContainerSize(TokenList& tokenlist,
                                    const SymbolDatabase& symboldatabase,
                                    ErrorLogger* /*errorLogger*/,
@@ -8819,21 +8839,12 @@ static void valueFlowContainerSize(TokenList& tokenlist,
             } else if (Token::simpleMatch(tok, "+=") && astIsContainer(tok->astOperand1())) {
                 const Token* containerTok = tok->astOperand1();
                 const Token* valueTok = tok->astOperand2();
-                MathLib::bigint size = 0;
-                if (valueTok->tokType() == Token::eString)
-                    size = Token::getStrLength(valueTok);
-                else if (astIsGenericChar(tok) || valueTok->tokType() == Token::eChar)
-                    size = 1;
-                else if (const ValueFlow::Value* v1 = valueTok->getKnownValue(ValueFlow::Value::ValueType::TOK))
-                    size = Token::getStrLength(v1->tokvalue);
-                else if (const ValueFlow::Value* v2 =
-                             valueTok->getKnownValue(ValueFlow::Value::ValueType::CONTAINER_SIZE))
-                    size = v2->intvalue;
+                MathLib::bigint size = valueFlowGetStrLength(valueTok);
                 if (size == 0)
                     continue;
                 ValueFlow::Value value(size - 1);
                 value.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE;
-                value.bound = ValueFlow::Value::Bound::Lower;
+                value.bound = ValueFlow::Value::Bound::Upper;
                 value.setImpossible();
                 Token* next = nextAfterAstRightmostLeaf(tok);
                 if (!next)
diff --git a/test/testcondition.cpp b/test/testcondition.cpp
index c79b50a84..5cca85c05 100644
--- a/test/testcondition.cpp
+++ b/test/testcondition.cpp
@@ -5028,6 +5028,21 @@ private:
               "    return -1;\n"
               "}\n");
         ASSERT_EQUALS("[test.cpp:5]: (style) Condition 's.empty()' is always false\n", errout.str());
+
+        check("void f(std::string& p) {\n"
+              "    const std::string d{ \"abc\" };\n"
+              "    p += d;\n"
+              "    if(p.empty()) {}\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:4]: (style) Condition 'p.empty()' is always false\n", errout.str());
+
+        check("bool f(int i, FILE* fp) {\n"
+              "  std::string s = \"abc\";\n"
+              "  s += std::to_string(i);\n"
+              "  s += \"\\n\";\n"
+              "  return fwrite(s.c_str(), 1, s.length(), fp) == s.length();\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
     }
 
     void alwaysTrueLoop()