From c3c3d6770c5bdca3d3ba8becf7d315339dec0355 Mon Sep 17 00:00:00 2001
From: Sebastian <versat@users.noreply.github.com>
Date: Sun, 24 Nov 2019 01:40:31 +0100
Subject: [PATCH] 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            | 17 +++++++++++++++++
 lib/library.h              |  5 +----
 test/testbufferoverrun.cpp | 24 ++++++++++++++++++++++++
 test/testnullpointer.cpp   |  2 +-
 4 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/lib/library.cpp b/lib/library.cpp
index 5f8c86b51..d48d59baf 100644
--- a/lib/library.cpp
+++ b/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);
diff --git a/lib/library.h b/lib/library.h
index 0d8ff4735..75ba07605 100644
--- a/lib/library.h
+++ b/lib/library.h
@@ -352,10 +352,7 @@ public:
         return arg ? &arg->minsizes : nullptr;
     }
 
-    ArgumentChecks::Direction getArgDirection(const Token *ftok, int argnr) const {
-        const ArgumentChecks *arg = getarg(ftok, argnr);
-        return arg ? arg->direction : ArgumentChecks::Direction::DIR_UNKNOWN;
-    }
+    ArgumentChecks::Direction getArgDirection(const Token* ftok, int argnr) const;
 
     bool markupFile(const std::string &path) const;
 
diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp
index 8cc5ce60c..844567316 100644
--- a/test/testbufferoverrun.cpp
+++ b/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"
diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp
index 2a7fc2c63..f576874cf 100644
--- a/test/testnullpointer.cpp
+++ b/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