ValueFlow: Set values in else branch even when the first branch modifies the value (danmar#1309)

pfultz2 · danmar · commit f09369255169 · 2018-07-25T22:59:54.000+02:00
* Set values in else branch even when the first branch modifies the value

* Move tests

* Add check for goto

* Remvoe todo

* Also check scope is noreturn

* Use isEscapeScope when variables are changed
diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp
@@ -1543,6 +1543,14 @@ static bool evalAssignment(ValueFlow::Value &lhsValue, const std::string &assign
     return true;
 }
 
+static bool isEscapeScope(const Token* tok, TokenList * tokenlist)
+{
+    if(!Token::simpleMatch(tok, "{"))
+        return false;
+    return Token::findmatch(tok, "return|continue|break|throw|goto", tok->link()) ||
+        (tokenlist && tokenlist->getSettings()->library.isScopeNoReturn(tok->link(), nullptr));
+}
+
 static bool valueFlowForward(Token * const               startToken,
                              const Token * const         endToken,
                              const Variable * const      var,
@@ -1848,8 +1856,8 @@ static bool valueFlowForward(Token * const               startToken,
 
             // noreturn scopes..
             if ((number_of_if > 0 || Token::findmatch(tok2, "%varid%", start, varid)) &&
-                (Token::findmatch(start, "return|continue|break|throw", end) ||
-                 (Token::simpleMatch(end,"} else {") && Token::findmatch(end, "return|continue|break|throw", end->linkAt(2))))) {
+                (isEscapeScope(start, tokenlist) ||
+                 (Token::simpleMatch(end,"} else {") && isEscapeScope(end->tokAt(2), tokenlist)))) {
                 if (settings->debugwarnings)
                     bailout(tokenlist, errorLogger, tok2, "variable " + var->name() + ". noreturn conditional scope.");
                 return false;
@@ -1859,8 +1867,7 @@ static bool valueFlowForward(Token * const               startToken,
                 if ((!read || number_of_if == 0) &&
                     Token::simpleMatch(tok2, "if (") &&
                     !(Token::simpleMatch(end, "} else {") &&
-                      (Token::findmatch(end, "%varid%", end->linkAt(2), varid) ||
-                       Token::findmatch(end, "return|continue|break|throw", end->linkAt(2))))) {
+                       isEscapeScope(end->tokAt(2), tokenlist))) {
                     ++number_of_if;
                     tok2 = end;
                 } else {
diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp
@@ -1583,6 +1583,23 @@ class TestValueFlow : public TestFixture {
                "}";
         ASSERT_EQUALS(false, testValueOfX(code, 9U, 0));
 
+        code = "void f(int i) {\n"
+                "    bool x = false;\n"
+                "    if (i == 0) { x = true; }\n"
+                "    else if (x && i == 1) {}\n"
+                "}\n";
+        ASSERT_EQUALS(true, testValueOfX(code, 4U, 0));
+
+        code = "void f(int i) {\n"
+                "    bool x = false;\n"
+                "    while(i > 0) {\n"
+                "        i++;\n"
+                "        if (i == 0) { x = true; }\n"
+                "        else if (x && i == 1) {}\n"
+                "    }\n"
+                "}\n";
+        ASSERT_EQUALS(false, testValueOfX(code, 6U, 0));
+
         // multivariables
         code = "void f(int a) {\n"
                "    int x = a;\n"
@@ -3137,6 +3154,53 @@ class TestValueFlow : public TestFixture {
                "}";
         values = tokenValues(code, "x ; }");
         ASSERT_EQUALS(true, values.empty());
+
+        code = "void b(bool d, bool e) {\n"
+               "  int c;\n"
+               "  if (d)\n"
+               "    c = 0;\n"
+               "  if (e)\n"
+               "    goto;\n"
+               "  c++;\n"
+               "}\n";
+        values = tokenValues(code, "c ++ ; }");
+        ASSERT_EQUALS(true, values.empty());
+
+        code = "void b(bool d, bool e) {\n"
+               "  int c;\n"
+               "  if (d)\n"
+               "    c = 0;\n"
+               "  if (e)\n"
+               "    return;\n"
+               "  c++;\n"
+               "}\n";
+        values = tokenValues(code, "c ++ ; }");
+        ASSERT_EQUALS(true, values.empty());
+
+        code = "void b(bool d, bool e) {\n"
+               "  int c;\n"
+               "  if (d)\n"
+               "    c = 0;\n"
+               "  if (e)\n"
+               "    exit();\n"
+               "  c++;\n"
+               "}\n";
+        values = tokenValues(code, "c ++ ; }");
+        ASSERT_EQUALS(true, values.empty());
+
+        code = "void b(bool d, bool e) {\n"
+               "  int c;\n"
+               "  if (d)\n"
+               "    c = 0;\n"
+               " else if (!d)\n"
+               "   c = 0;\n"
+               "  c++;\n"
+               "}\n";
+        values = tokenValues(code, "c ++ ; }");
+        ASSERT_EQUALS(true, values.size() == 2);
+        ASSERT_EQUALS(true, values.front().isUninitValue() || values.back().isUninitValue());
+        ASSERT_EQUALS(true, values.front().isPossible() || values.back().isPossible());
+        ASSERT_EQUALS(true, values.front().intvalue == 0 || values.back().intvalue == 0);
     }
 };
 
