Fix #9478: Valueflow: printf does not change value (#2388)

Format-string arguments are now marked to have `in` direction, except
for `scan`-functions (like `scanf`) where these arguments are explicitly
marked to have `out` direction.

lib/library.cpp

@@ -1263,6 +1263,23 @@ bool Library::hasminsize(const Token *ftok) const
     return false;
 }
 
+Library::ArgumentChecks::Direction Library::getArgDirection(const Token* ftok, int argnr) const
+{
+    const ArgumentChecks* arg = getarg(ftok, argnr);
+    if (arg)
+        return arg->direction;
+    if (formatstr_function(ftok)) {
+        const int fs_argno = formatstr_argno(ftok);
+        if (fs_argno >= 0 && argnr >= fs_argno) {
+            if (formatstr_scan(ftok))
+                return ArgumentChecks::Direction::DIR_OUT;
+            else
+                return ArgumentChecks::Direction::DIR_IN;
+        }
+    }
+    return ArgumentChecks::Direction::DIR_UNKNOWN;
+}
+
 bool Library::ignorefunction(const std::string& functionName) const
 {
     const std::map<std::string, Function>::const_iterator it = functions.find(functionName);

lib/library.h

@@ -352,10 +352,7 @@ public:
         return arg ? &arg->minsizes : nullptr;
     }
 
-    ArgumentChecks::Direction getArgDirection(const Token *ftok, int argnr) const {
+    ArgumentChecks::Direction getArgDirection(const Token* ftok, int argnr) const;
-        const ArgumentChecks *arg = getarg(ftok, argnr);
-        return arg ? arg->direction : ArgumentChecks::Direction::DIR_UNKNOWN;
-    }
 
     bool markupFile(const std::string &path) const;
 

test/testbufferoverrun.cpp

@@ -126,6 +126,7 @@ private:
         TEST_CASE(array_index_45); // #4207 - calling function with variable number of parameters (...)
         TEST_CASE(array_index_46); // #4840 - two-statement for loop
         TEST_CASE(array_index_47); // #5849
+        TEST_CASE(array_index_48); // #9478
         TEST_CASE(array_index_multidim);
         TEST_CASE(array_index_switch_in_for);
         TEST_CASE(array_index_for_in_for);   // FP: #2634
@@ -1470,6 +1471,29 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    void array_index_48() {
+        // #9478
+        check("void test(void)\n"
+              "{\n"
+              "    int array[4] = { 1,2,3,4 };\n"
+              "    for (int i = 1; i <= 4; i++) {\n"
+              "        printf(\" %i\", i);\n"
+              "        array[i] = 0;\n"
+              "    }\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:6]: (error) Array 'array[4]' accessed at index 4, which is out of bounds.\n", errout.str());
+
+        check("void test(void)\n"
+              "{\n"
+              "    int array[4] = { 1,2,3,4 };\n"
+              "    for (int i = 1; i <= 4; i++) {\n"
+              "        scanf(\"%i\", &i);\n"
+              "        array[i] = 0;\n"
+              "    }\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+    }
+
     void array_index_multidim() {
         check("void f()\n"
               "{\n"

test/testnullpointer.cpp

@@ -2866,7 +2866,7 @@ private:
               "    printf(\"%p\", p);\n"
               "    *p = 0;\n"
               "}", true);
-        ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) Possible null pointer dereference if the default parameter value is used: p\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:3]: (warning) Possible null pointer dereference if the default parameter value is used: p\n", errout.str());
 
         // The init() function may or may not initialize p, but since the address
         // of p is passed in, it's a good bet that p may be modified and