fix #11845: FP variableScope if buffer is passed to a conditionally c…

…alled function (#6834)
danmar · Oct 7, 2024 · b56ce53 · b56ce53
1 parent 62a92a6
commit b56ce53
Show file tree

Hide file tree

Showing 2 changed files with 111 additions and 0 deletions.
diff --git a/lib/checkother.cpp b/lib/checkother.cpp
@@ -1136,6 +1136,32 @@ void CheckOther::checkVariableScope()
     }
 }
 
+// used to check if an argument to a function might depend on another argument
+static bool mayDependOn(const ValueType *other, const ValueType *original)
+{
+    if (!other || !original)
+        return false;
+
+    // other must be pointer
+    if (!other->pointer)
+        return false;
+
+    // must be same underlying type
+    if (other->type != original->type)
+        return false;
+
+    const int otherPtr = other->pointer + (other->reference == Reference::LValue ? 1 : 0);
+    const int originalPtr = original->pointer;
+
+    if (otherPtr == originalPtr) {
+        // if other is not const than original may be copied to other
+        return !other->isConst(otherPtr);
+    }
+
+    // other may be reassigned to original
+    return otherPtr > originalPtr;
+}
+
 bool CheckOther::checkInnerScope(const Token *tok, const Variable* var, bool& used) const
 {
     const Scope* scope = tok->next()->scope();
@@ -1226,6 +1252,13 @@ bool CheckOther::checkInnerScope(const Token *tok, const Variable* var, bool& us
                         if (!ret.empty() && ret.back() == '*')
                             return false;
                     }
+                    if (ftok->function()) {
+                        const std::list<Variable> &argvars = ftok->function()->argumentList;
+                        const Variable *argvar = ftok->function()->getArgumentVar(argn);
+                        if (!std::all_of(argvars.cbegin(), argvars.cend(), [&](const Variable &other) {
+                            return &other == argvar || !mayDependOn(other.valueType(), argvar->valueType());
+                        })) return false;
+                    }
                 }
             }
             const auto yield = astContainerYield(tok);

diff --git a/test/testother.cpp b/test/testother.cpp
@@ -103,6 +103,8 @@ class TestOther : public TestFixture {
         TEST_CASE(varScope38);
         TEST_CASE(varScope39);
         TEST_CASE(varScope40);
+        TEST_CASE(varScope41);      // #11845
+        TEST_CASE(varScope42);
 
         TEST_CASE(oldStylePointerCast);
         TEST_CASE(invalidPointerCast);
@@ -1767,6 +1769,82 @@ class TestOther : public TestFixture {
                       errout_str());
     }
 
+    void varScope41() {    // #11845
+        check("void get_errmsg(const char **msg, char *buf, size_t bufsiz, int err);\n"
+              "void test(int err)\n"
+              "{\n"
+              "    const char *msg = \"Success\";\n"
+              "    char buf[42];\n"
+              "    if (err != 0)\n"
+              "        get_errmsg(&msg, buf, sizeof(buf), err);\n"
+              "    printf(\"%d: %s\\n\", err, msg);\n"
+              "}\n");
+        ASSERT_EQUALS("", errout_str());
+
+        check("void get_errmsg(char *buf, size_t bufsiz, int err);\n"
+              "void test(int err)\n"
+              "{\n"
+              "    const char *msg = \"Success\";\n"
+              "    char buf[42];\n"
+              "    if (err != 0)\n"
+              "        get_errmsg(buf, sizeof(buf), err);\n"
+              "    printf(\"%d: %s\\n\", err, msg);\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:5]: (style) The scope of the variable 'buf' can be reduced.\n", errout_str());
+    }
+
+    void varScope42() {
+        check("void f(const char **, char *);\n"
+              "void g(int e) {\n"
+              "    const char *msg = \"Something\";\n"
+              "    char buf[42];\n"
+              "    if (e != 0)\n"
+              "        f(&msg, buf);\n"
+              "    printf(\"result: %s\\n\", msg);\n"
+              "}\n");
+        ASSERT_EQUALS("", errout_str());
+
+        check("void f(char *, char *);\n"
+              "void g(int e) {\n"
+              "    char msg [42] = \"Something\";\n"
+              "    char buf[42];\n"
+              "    if (e != 0)\n"
+              "        f(msg, buf);\n"
+              "    printf(\"result: %s\\n\", msg);\n"
+              "}\n");
+        ASSERT_EQUALS("", errout_str());
+
+        check("void f(const char *, char *);\n"
+              "void g(int e) {\n"
+              "    const char *msg = \"Something\";\n"
+              "    char buf[42];\n"
+              "    if (e != 0)\n"
+              "        f(msg, buf);\n"
+              "    printf(\"result: %s\\n\", msg);\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:4]: (style) The scope of the variable 'buf' can be reduced.\n", errout_str());
+
+        check("void f(int **, char *);\n"
+              "void g(int e) {\n"
+              "    int *msg = calloc(0, sizeof(*msg));\n"
+              "    char buf[42];\n"
+              "    if (e != 0)\n"
+              "        f(&msg, buf);\n"
+              "    printf(\"result: %d\\n\", *msg);\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:4]: (style) The scope of the variable 'buf' can be reduced.\n", errout_str());
+
+        check("void f(const char *&, const char *&);\n"
+              "void g(int e) {\n"
+              "    const char *msg = \"Something\";\n"
+              "    char *buf = malloc(42);\n"
+              "    if (e != 0)\n"
+              "        f(msg, buf);\n"
+              "    printf(\"result: %d\\n\", msg);\n"
+              "}\n");
+        ASSERT_EQUALS("", errout_str());
+    }
+
 #define checkOldStylePointerCast(...) checkOldStylePointerCast_(__FILE__, __LINE__, __VA_ARGS__)
     template<size_t size>
     void checkOldStylePointerCast_(const char* file, int line, const char (&code)[size], Standards::cppstd_t std = Standards::CPPLatest) {