From 7c8da17c44143072f0ddb43f7ae7de88c8c1cef9 Mon Sep 17 00:00:00 2001
From: Arpit Chaudhary <arpchaudhary@paypal.com>
Date: Wed, 22 Aug 2012 15:44:20 +0200
Subject: [PATCH] Added check for detecting if a variable or number is shifted
 by negative right operand. Statements like:

 int i = -1;
 a << i;

 would result in an error message stating undefined behavior.
---
 lib/checkother.cpp | 20 ++++++++++++++++++++
 lib/checkother.h   |  8 +++++++-
 test/testother.cpp | 43 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/lib/checkother.cpp b/lib/checkother.cpp
index 5c5762c87..fa0b228f3 100644
--- a/lib/checkother.cpp
+++ b/lib/checkother.cpp
@@ -3096,3 +3096,23 @@ void CheckOther::redundantCopyError(const Token *tok,const std::string& varname)
                 "Use const reference for "+varname+" to avoid unnecessary data copying.\n"
                 "The const "+varname+" gets a copy of the data since const reference is not used. You can avoid the unnecessary data copying by converting "+varname+" to const reference instead of just const.");
 }
+
+//---------------------------------------------------------------------------
+// Checking for shift by negative values
+//---------------------------------------------------------------------------
+
+void CheckOther::checkNegativeBitwiseShift()
+{
+    for (const Token *tok = _tokenizer->tokens(); tok ; tok = tok->next()) {
+        if (Token::Match(tok,"%var% >>|<< %num%") || Token::Match(tok,"%num >>|<< %num%")) {
+            if ((tok->strAt(2))[0] == '-')
+                negativeBitwiseShiftError(tok);
+        }
+    }
+}
+
+
+void CheckOther::negativeBitwiseShiftError(const Token *tok)
+{
+    reportError(tok, Severity::error, "shiftNegative", "Shifting by a negative value.");
+}
diff --git a/lib/checkother.h b/lib/checkother.h
index 01a87836c..98c3f5ba3 100644
--- a/lib/checkother.h
+++ b/lib/checkother.h
@@ -104,6 +104,7 @@ public:
         checkOther.checkBitwiseOnBoolean();
         checkOther.checkDoubleFree();
         checkOther.checkRedundantCopy();
+        checkOther.checkNegativeBitwiseShift();
     }
 
     /** @brief Clarify calculation for ".. a * b ? .." */
@@ -240,6 +241,9 @@ public:
     /** @brief %Check for code creating redundant copies */
     void checkRedundantCopy();
 
+    /** @brief %Check for bitwise operation with negative right operand */
+    void checkNegativeBitwiseShift();
+
 private:
     // Error messages..
     void clarifyCalculationError(const Token *tok, const std::string &op);
@@ -296,7 +300,7 @@ private:
     void SuspiciousSemicolonError(const Token *tok);
     void doubleCloseDirError(const Token *tok, const std::string &varname);
     void moduloAlwaysTrueFalseError(const Token* tok, const std::string& maxVal);
-
+    void negativeBitwiseShiftError(const Token *tok);
     void redundantCopyError(const Token *tok, const std::string &varname);
 
     void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
@@ -314,6 +318,7 @@ private:
         c.sizeofForNumericParameterError(0);
         c.doubleFreeError(0, "varname");
         c.invalidPointerCastError(0, "float", "double", false);
+        c.negativeBitwiseShiftError(0);
 
         //performance
         c.redundantCopyError(0, "varname");
@@ -378,6 +383,7 @@ private:
                "* using sizeof(pointer) instead of the size of pointed data\n"
                "* incorrect length arguments for 'substr' and 'strncmp'\n"
                "* double free() or double closedir()\n"
+               "* bitwise operation with negative right operand\n"
 
                //performance
                "* redundant data copying for const variable\n"
diff --git a/test/testother.cpp b/test/testother.cpp
index abf806838..fc7fa3730 100644
--- a/test/testother.cpp
+++ b/test/testother.cpp
@@ -162,6 +162,8 @@ private:
         TEST_CASE(checkDoubleFree);
 
         TEST_CASE(checkRedundantCopy);
+
+        TEST_CASE(checkNegativeShift);
     }
 
     void check(const char code[], const char *filename = NULL, bool experimental = false, bool inconclusive = true) {
@@ -5794,6 +5796,47 @@ private:
                              "}\n");
         ASSERT_EQUALS("", errout.str());
     }
+
+    void checkNegativeShift() {
+        check("void foo()\n"
+              "{\n"
+              "   int a = 123;\n"
+              "   a << -1;\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:4]: (error) Shifting by a negative value.\n", errout.str());
+        check("void foo()\n"
+              "{\n"
+              "   int a = 123;\n"
+              "   int i = -1;\n"
+              "   a << i;\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:5]: (error) Shifting by a negative value.\n", errout.str());
+        check("void foo()\n"
+              "{\n"
+              "   int a = 123;\n"
+              "   a >> -1;\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:4]: (error) Shifting by a negative value.\n", errout.str());
+        check("void foo()\n"
+              "{\n"
+              "   int a = 123;\n"
+              "   int i = -1;\n"
+              "   a >> i;\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:5]: (error) Shifting by a negative value.\n",errout.str());
+        check("void foo()\n"
+              "{\n"
+              "   int a = 123;\n"
+              "   a <<= -1;\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:4]: (error) Shifting by a negative value.\n", errout.str());
+        check("void foo()\n"
+              "{\n"
+              "   int a = 123;\n"
+              "   a >>= -1;\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:4]: (error) Shifting by a negative value.\n", errout.str());s
+    }
 };
 
 REGISTER_TEST(TestOther)