Overlapping sprintf, improve handling of casts (#1945)

* Overlapping sprintf, improve handling of casts

If there is a cast of the argument buffer, cppcheck would print out the
expression including the cast, which looks a bit strange to talk about

    Variable (char*)buf is used as...

Instead, only print the variable name without the cast.

Also, handle arbitrary many casts (the previous code only handled one).
Multiple casts of the input arguments is probably an unusual case in
real code, but can perhaps occur if macros are used.

* Fix printing of variable

... and add a test.

* Simplify testcase

lib/checkstring.cpp

@@ -436,12 +436,12 @@ void CheckString::sprintfOverlappingData()
             const int formatString = Token::simpleMatch(tok, "sprintf") ? 1 : 2;
             for (unsigned int argnr = formatString + 1; argnr < args.size(); ++argnr) {
                 const Token *dest = args[0];
-                if (dest->isCast())
+                while (dest->isCast())
                     dest = dest->astOperand2() ? dest->astOperand2() : dest->astOperand1();
                 const Token *arg = args[argnr];
                 if (!arg->valueType() || arg->valueType()->pointer != 1)
                     continue;
-                if (arg->isCast())
+                while (arg->isCast())
                     arg = arg->astOperand2() ? arg->astOperand2() : arg->astOperand1();
 
                 const bool same = isSameExpression(mTokenizer->isCPP(),
@@ -452,7 +452,7 @@ void CheckString::sprintfOverlappingData()
                                                    true,
                                                    false);
                 if (same) {
-                    sprintfOverlappingDataError(tok, args[argnr], args[argnr]->expressionString());
+                    sprintfOverlappingDataError(tok, args[argnr], arg->expressionString());
                 }
             }
         }

test/teststring.cpp

@@ -51,6 +51,9 @@ private:
         TEST_CASE(sprintf2);
         TEST_CASE(sprintf3);
         TEST_CASE(sprintf4);        // struct member
+        TEST_CASE(sprintf5);        // another struct member
+        TEST_CASE(sprintf6);        // (char*)
+        TEST_CASE(sprintf7);        // (char*)(void*)
         TEST_CASE(wsprintf1);       // Dangerous usage of wsprintf
 
         TEST_CASE(incorrectStringCompare);
@@ -496,6 +499,37 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    void sprintf5() {
+        check("struct A\n"
+              "{\n"
+              "    char filename[128];\n"
+              "};\n"
+              "\n"
+              "void foo(struct A *a)\n"
+              "{\n"
+              "    snprintf(a->filename, 128, \"%s\", a->filename);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:8]: (error) Undefined behavior: Variable 'a->filename' is used as parameter and destination in snprintf().\n", errout.str());
+    }
+
+    void sprintf6() {
+        check("void foo()\n"
+              "{\n"
+              "    char buf[100];\n"
+              "    sprintf((char*)buf,\"%s\",(char*)buf);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:4]: (error) Undefined behavior: Variable 'buf' is used as parameter and destination in sprintf().\n", errout.str());
+    }
+
+    void sprintf7() {
+        check("void foo()\n"
+              "{\n"
+              "    char buf[100];\n"
+              "    sprintf((char*)(void*)buf,\"%s\",(void*)(char*)buf);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:4]: (error) Undefined behavior: Variable 'buf' is used as parameter and destination in sprintf().\n", errout.str());
+    }
+
     void wsprintf1() {
         check("void foo()\n"
               "{\n"