Fix #13326: FP shiftNegative for template argument (danmar#7045)

swasti16 · Dec 4, 2024 · da410bb · da410bb
1 parent f7d0c25
commit da410bb
Show file tree

Hide file tree

Showing 6 changed files with 119 additions and 0 deletions.
diff --git a/lib/astutils.cpp b/lib/astutils.cpp
@@ -3797,3 +3797,78 @@ bool isUnreachableOperand(const Token *tok)
 
     return false;
 }
+
+static bool unknownLeafValuesAreTemplateArgs(const Token *tok)
+{
+    if (!tok)
+        return true;
+
+    if (!tok->astOperand1() && !tok->astOperand2())
+        return tok->isTemplateArg() || tok->hasKnownIntValue();
+
+    return unknownLeafValuesAreTemplateArgs(tok->astOperand1())
+           && unknownLeafValuesAreTemplateArgs(tok->astOperand2());
+}
+
+static const Token *skipUnreachableIfBranch(const Token *tok)
+{
+    const Token *condTok = tok->linkAt(-1);
+    if (!condTok)
+        return tok;
+
+    if (!Token::simpleMatch(condTok->tokAt(-1), "if") && !Token::simpleMatch(condTok->tokAt(-2), "if constexpr"))
+        return tok;
+
+    condTok = condTok->astOperand2();
+    if (!condTok)
+        return tok;
+
+    if ((condTok->hasKnownIntValue() && condTok->getKnownIntValue() == 0)
+        || (unknownLeafValuesAreTemplateArgs(condTok) && condTok->getValue(0))) {
+        tok = tok->link();
+    }
+
+    return tok;
+}
+
+static const Token *skipUnreachableElseBranch(const Token *tok)
+{
+    if (!Token::simpleMatch(tok->tokAt(-2), "} else {"))
+        return tok;
+
+    const Token *condTok = tok->linkAt(-2);
+    if (!condTok)
+        return tok;
+
+    condTok = condTok->linkAt(-1);
+    if (!condTok)
+        return tok;
+
+    if (!Token::simpleMatch(condTok->tokAt(-1), "if (") && !Token::simpleMatch(condTok->tokAt(-2), "if constexpr ("))
+        return tok;
+
+    condTok = condTok->astOperand2();
+
+    if ((condTok->hasKnownIntValue() && condTok->getKnownIntValue() != 0)
+        || (unknownLeafValuesAreTemplateArgs(condTok) && condTok->getValueNE(0))) {
+        tok = tok->link();
+    }
+
+    return tok;
+}
+
+const Token *skipUnreachableBranch(const Token *tok)
+{
+    if (!Token::simpleMatch(tok, "{"))
+        return tok;
+
+    if (tok->scope()->type == Scope::ScopeType::eIf) {
+        return skipUnreachableIfBranch(tok);
+    }
+
+    if (tok->scope()->type == Scope::ScopeType::eElse) {
+        return skipUnreachableElseBranch(tok);
+    }
+
+    return tok;
+}
diff --git a/lib/astutils.h b/lib/astutils.h
@@ -453,4 +453,6 @@ bool isExhaustiveSwitch(const Token *startbrace);
 
 bool isUnreachableOperand(const Token *tok);
 
+const Token *skipUnreachableBranch(const Token *tok);
+
 #endif // astutilsH
diff --git a/lib/checkother.cpp b/lib/checkother.cpp
@@ -3114,6 +3114,8 @@ void CheckOther::checkNegativeBitwiseShift()
     logChecker("CheckOther::checkNegativeBitwiseShift");
 
     for (const Token* tok = mTokenizer->tokens(); tok; tok = tok->next()) {
+        tok = skipUnreachableBranch(tok);
+
         if (!tok->astOperand1() || !tok->astOperand2())
             continue;
 

diff --git a/lib/token.cpp b/lib/token.cpp
@@ -1894,6 +1894,16 @@ const ValueFlow::Value * Token::getValueGE(const MathLib::bigint val, const Sett
     });
 }
 
+const ValueFlow::Value * Token::getValueNE(MathLib::bigint val) const
+{
+    if (!mImpl->mValues)
+        return nullptr;
+    const auto it = std::find_if(mImpl->mValues->cbegin(), mImpl->mValues->cend(), [=](const ValueFlow::Value& value) {
+        return value.isIntValue() && !value.isImpossible() && value.intvalue != val;
+    });
+    return it == mImpl->mValues->end() ? nullptr : &*it;
+}
+
 const ValueFlow::Value * Token::getInvalidValue(const Token *ftok, nonneg int argnr, const Settings &settings) const
 {
     if (!mImpl->mValues)

diff --git a/lib/token.h b/lib/token.h
@@ -1300,6 +1300,7 @@ class CPPCHECKLIB Token {
 
     const ValueFlow::Value * getValueLE(MathLib::bigint val, const Settings &settings) const;
     const ValueFlow::Value * getValueGE(MathLib::bigint val, const Settings &settings) const;
+    const ValueFlow::Value * getValueNE(MathLib::bigint val) const;
 
     const ValueFlow::Value * getInvalidValue(const Token *ftok, nonneg int argnr, const Settings &settings) const;
 

diff --git a/test/testother.cpp b/test/testother.cpp
@@ -9369,6 +9369,35 @@ class TestOther : public TestFixture {
               "    }\n"
               "}\n");
         ASSERT_EQUALS("", errout_str());
+
+        // #13326
+        check("template<int b>\n"
+              "int f(int a)\n"
+              "{\n"
+              "    if constexpr (b >= 0) {\n"
+              "        return a << b;\n"
+              "    } else {\n"
+              "        return a << -b;\n"
+              "    }\n"
+              "}\n"
+              "int g() {\n"
+              "    return f<1>(2)\n"
+              "}\n");
+        ASSERT_EQUALS("", errout_str());
+
+        check("template<int b>\n"
+              "int f(int a)\n"
+              "{\n"
+              "    if constexpr (b >= 0) {\n"
+              "        return a << b;\n"
+              "    } else {\n"
+              "        return a << -b;\n"
+              "    }\n"
+              "}\n"
+              "int g() {\n"
+              "    return f<-1>(2)\n"
+              "}\n");
+        ASSERT_EQUALS("", errout_str());
     }
 
     void incompleteArrayFill() {
Original file line number	Diff line number	Diff line change
Expand Up		@@ -453,4 +453,6 @@ bool isExhaustiveSwitch(const Token *startbrace);

		bool isUnreachableOperand(const Token *tok);

		const Token skipUnreachableBranch(const Token tok);

		#endif // astutilsH