Fix #11300 FP accessMoved with write access (inconclusive)

Author: web-flow

cfg/std.cfg

@@ -3993,7 +3993,7 @@ The obsolete function 'gets' is called. With 'gets' you'll get a buffer overrun
     <noreturn>false</noreturn>
     <leak-ignore/>
     <not-overlapping-data ptr1-arg="1" ptr2-arg="2" size-arg="3"/>
-    <arg nr="1" direction="out">
+    <arg nr="1" direction="out" indirect="1">
       <not-null/>
       <minsize type="argvalue" arg="3"/>
     </arg>
@@ -4100,7 +4100,7 @@ The obsolete function 'gets' is called. With 'gets' you'll get a buffer overrun
     <returnValue type="errno_t"/>
     <noreturn>false</noreturn>
     <leak-ignore/>
-    <arg nr="1" direction="out">
+    <arg nr="1" direction="out" indirect="1">
       <not-null/>
       <minsize type="argvalue" arg="2"/>
     </arg>
@@ -4121,7 +4121,7 @@ The obsolete function 'gets' is called. With 'gets' you'll get a buffer overrun
     <returnValue type="void *"/>
     <noreturn>false</noreturn>
     <leak-ignore/>
-    <arg nr="1" direction="out">
+    <arg nr="1" direction="out" indirect="1">
       <not-null/>
       <minsize type="argvalue" arg="3"/>
     </arg>
@@ -4911,7 +4911,7 @@ The obsolete function 'gets' is called. With 'gets' you'll get a buffer overrun
     <noreturn>false</noreturn>
     <not-overlapping-data ptr1-arg="1" ptr2-arg="2" strlen-arg="2"/>
     <leak-ignore/>
-    <arg nr="1" direction="out">
+    <arg nr="1" direction="out" indirect="1">
       <not-null/>
       <minsize type="strlen" arg="2"/>
     </arg>

cli/cmdlineparser.cpp

@@ -76,9 +76,7 @@ static bool addFilesToList(const std::string& fileList, std::vector<std::string>
         files = &infile;
     }
     std::string fileName;
-    // cppcheck-suppress accessMoved - FP
     while (std::getline(*files, fileName)) { // next line
-        // cppcheck-suppress accessMoved - FP
         if (!fileName.empty()) {
             pathNames.emplace_back(std::move(fileName));
         }
@@ -92,7 +90,6 @@ static bool addIncludePathsToList(const std::string& fileList, std::list<std::st
     std::ifstream files(fileList);
     if (files) {
         std::string pathName;
-        // cppcheck-suppress accessMoved - FP
         while (std::getline(files, pathName)) { // next line
             if (!pathName.empty()) {
                 pathName = Path::removeQuotationMarks(std::move(pathName));

cli/cppcheckexecutor.cpp

@@ -174,10 +174,8 @@ namespace {
             {
                 std::set<std::string> activeCheckers;
                 std::string line;
-                // cppcheck-suppress accessMoved - FP
                 while (std::getline(fin, line))
                 {
-                    // cppcheck-suppress accessMoved - FP
                     activeCheckers.emplace(std::move(line));
                 }
                 mActiveCheckers = std::move(activeCheckers);

lib/astutils.cpp

@@ -2548,10 +2548,11 @@ bool isVariableChangedByFunctionCall(const Token *tok, int indirect, const Setti
         const Library::ArgumentChecks::Direction argDirection = settings.library.getArgDirection(tok, 1 + argnr, indirect);
         if (argDirection == Library::ArgumentChecks::Direction::DIR_IN)
             return false;
+        if (argDirection == Library::ArgumentChecks::Direction::DIR_OUT)
+            return true;
 
         const bool requireNonNull = settings.library.isnullargbad(tok, 1 + argnr);
-        if (argDirection == Library::ArgumentChecks::Direction::DIR_OUT ||
-            argDirection == Library::ArgumentChecks::Direction::DIR_INOUT) {
+        if (argDirection == Library::ArgumentChecks::Direction::DIR_INOUT) {
             if (indirect == 0 && isArray(tok1))
                 return true;
             const bool requireInit = settings.library.isuninitargbad(tok, 1 + argnr);
@@ -3471,6 +3472,9 @@ static ExprUsage getFunctionUsage(const Token* tok, int indirect, const Settings
         const bool isuninitbad = settings.library.isuninitargbad(ftok, argnr + 1, indirect, &hasIndirect);
         if (isuninitbad && (!addressOf || isnullbad))
             return ExprUsage::Used;
+        const Library::ArgumentChecks::Direction dir = settings.library.getArgDirection(ftok, argnr + 1, indirect);
+        if (dir == Library::ArgumentChecks::Direction::DIR_OUT)
+            return ExprUsage::NotUsed;
     }
     return ExprUsage::Inconclusive;
 }

lib/fwdanalysis.cpp

@@ -362,7 +362,7 @@ FwdAnalysis::Result FwdAnalysis::checkRecursive(const Token *expr, const Token *
                     while (argnr < args.size() && args[argnr] != parent)
                         argnr++;
                     if (argnr < args.size()) {
-                        if (mSettings.library.getArgDirection(ftok->astOperand1(), argnr + 1) == Library::ArgumentChecks::Direction::DIR_OUT)
+                        if (mSettings.library.getArgDirection(ftok->astOperand1(), argnr + 1, /*indirect*/ 1) == Library::ArgumentChecks::Direction::DIR_OUT)
                             continue;
                     }
                 }

test/cfg/std.cpp

@@ -586,7 +586,6 @@ void bufferAccessOutOfBounds_std_ofstream_write(std::ofstream &os, const char* s
     (void)os.write(s,n);
 }
 
-// cppcheck-suppress constParameterReference // TODO: FP
 void bufferAccessOutOfBounds_std_ifstream_get(std::ifstream& in, std::streambuf& sb)
 {
     char cBuf[10];

test/testother.cpp

@@ -304,6 +304,7 @@ class TestOther : public TestFixture {
         TEST_CASE(moveForRange);
         TEST_CASE(moveTernary);
         TEST_CASE(movePointerAlias);
+        TEST_CASE(moveOutparam);
 
         TEST_CASE(funcArgNamesDifferent);
         TEST_CASE(funcArgOrderDifferent);
@@ -12550,7 +12551,8 @@ class TestOther : public TestFixture {
               "    A c = a;\n"
               "}");
         ASSERT_EQUALS("[test.cpp:4:7]: (warning, inconclusive) Access of moved variable 'a'. [accessMoved]\n"
-                      "[test.cpp:5:11]: (warning, inconclusive) Access of moved variable 'a'. [accessMoved]\n", errout_str());
+                      "[test.cpp:5:11]: (warning, inconclusive) Access of moved variable 'a'. [accessMoved]\n",
+                      errout_str());
     }
 
     void moveAndReturn() {
@@ -12730,6 +12732,27 @@ class TestOther : public TestFixture {
         ASSERT_EQUALS("[test.cpp:5:8]: (warning) Access of moved variable '.'. [accessMoved]\n", errout_str());
     }
 
+    void moveOutparam()
+    {
+        check("void f(std::vector<std::string>& v) {\n" // #11300
+              "    std::string l;\n"
+              "    while (std::getline(std::cin, l)) {\n"
+              "        if (!l.empty()) {\n"
+              "            v.emplace_back(std::move(l));\n"
+              "        }\n"
+              "    }\n"
+              "}\n");
+        ASSERT_EQUALS("", errout_str());
+
+        check("void f(std::ifstream& fin, std::set<std::string>& s) {\n"
+	        "    std::string line;\n"
+	        "    while (std::getline(fin, line)) {\n"
+	        "        s.emplace(std::move(line));\n"
+	        "    }\n"
+              "}\n");
+        ASSERT_EQUALS("", errout_str());
+    }
+
     void funcArgNamesDifferent() {
         check("void func1(int a, int b, int c);\n"
               "void func1(int a, int b, int c) { }\n"