New check: Check public interface of classes (#8248)

lib/checkclass.cpp

@@ -2440,3 +2440,41 @@ void CheckClass::copyCtorAndEqOperatorError(const Token *tok, const std::string
 
     reportError(tok, Severity::warning, "copyCtorAndEqOperator", message);
 }
+
+void CheckClass::checkPublicInterfaceDivZero(bool test)
+{
+    if (!_settings->isEnabled(Settings::WARNING))
+        return;
+
+    const std::size_t classes = symbolDatabase->classAndStructScopes.size();
+    for (std::size_t i = 0; i < classes; ++i) {
+        const Scope * scope = symbolDatabase->classAndStructScopes[i];
+        if (!test && scope->classDef->fileIndex() != 1)
+            continue;
+        std::list<Function>::const_iterator func;
+        for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) {
+            if (func->access != AccessControl::Public)
+                continue;
+            if (!func->hasBody())
+                continue;
+            for (const Token *tok = func->functionScope->classStart; tok; tok = tok->next()) {
+                if (tok->str() == "if")
+                    break;
+                if (tok->str() != "/")
+                    continue;
+                if (!tok->valueType() || !tok->valueType()->isIntegral())
+                    continue;
+                if (!tok->astOperand2())
+                    continue;
+                const Variable *var = tok->astOperand2()->variable();
+                if (var && var->isArgument())
+                    publicInterfaceDivZeroError(tok, scope->className + "::" + func->name());
+            }
+        }
+    }
+}
+
+void CheckClass::publicInterfaceDivZeroError(const Token *tok, const std::string &functionName)
+{
+    reportError(tok, Severity::warning, "classPublicInterfaceDivZero", "Arbitrary usage of public method " + functionName + "() could result in division by zero.");
+}

lib/checkclass.h

@@ -61,6 +61,7 @@ public:
 
         // can't be a simplified check .. the 'sizeof' is used.
         checkClass.checkMemset();
+        checkClass.checkPublicInterfaceDivZero();
     }
 
     /** @brief Run checks on the simplified token list */
@@ -151,6 +152,9 @@ public:
     /** @brief Check that copy constructor and operator defined together */
     void checkCopyCtorAndEqOperator();
 
+    /** @brief Check that arbitrary usage of the public interface does not result in division by zero */
+    void checkPublicInterfaceDivZero(bool test=false);
+
 private:
     const SymbolDatabase *symbolDatabase;
 
@@ -183,6 +187,7 @@ private:
     void callsPureVirtualFunctionError(const Function & scopeFunction, const std::list<const Token *> & tokStack, const std::string &purefuncname);
     void duplInheritedMembersError(const Token* tok1, const Token* tok2, const std::string &derivedname, const std::string &basename, const std::string &variablename, bool derivedIsStruct, bool baseIsStruct);
     void copyCtorAndEqOperatorError(const Token *tok, const std::string &classname, bool isStruct, bool hasCopyCtor);
+    void publicInterfaceDivZeroError(const Token *tok, const std::string &functionName);
 
     void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
         CheckClass c(nullptr, settings, errorLogger);
@@ -213,6 +218,7 @@ private:
         c.selfInitializationError(nullptr, "var");
         c.duplInheritedMembersError(nullptr, nullptr, "class", "class", "variable", false, false);
         c.copyCtorAndEqOperatorError(nullptr, "class", false, false);
+        c.publicInterfaceDivZeroError(nullptr, "Class::dostuff");
     }
 
     static std::string myName() {
@@ -239,7 +245,8 @@ private:
                "- Suspicious subtraction from 'this'\n"
                "- Call of pure virtual function in constructor/destructor\n"
                "- Duplicated inherited data members\n"
-               "- If 'copy constructor' defined, 'operator=' also should be defined and vice versa\n";
+               "- If 'copy constructor' defined, 'operator=' also should be defined and vice versa\n"
+               "- Check that arbitrary usage of public interface does not result in division by zero\n";
     }
 
     // operatorEqRetRefThis helper functions

test/testclass.cpp

@@ -188,6 +188,8 @@ private:
         TEST_CASE(duplInheritedMembers);
         TEST_CASE(explicitConstructors);
         TEST_CASE(copyCtorAndEqOperator);
+
+        TEST_CASE(publicInterfaceDivZero);
     }
 
     void checkCopyCtorAndEqOperator(const char code[]) {
@@ -6492,6 +6494,31 @@ private:
                                      "{nonpure(false);}\n");
         ASSERT_EQUALS("", errout.str());
     }
+
+    void checkPublicInterfaceDivZero(const char code[]) {
+        // Clear the error log
+        errout.str("");
+        Settings settings;
+        settings.addEnabled("warning");
+
+        // Tokenize..
+        Tokenizer tokenizer(&settings, this);
+        std::istringstream istr(code);
+        tokenizer.tokenize(istr, "test.cpp");
+
+        // Check..
+        CheckClass checkClass(&tokenizer, &settings, this);
+        checkClass.checkPublicInterfaceDivZero(true);
+    }
+
+    void publicInterfaceDivZero() {
+        checkPublicInterfaceDivZero("class A {\n"
+                                    "public:\n"
+                                    "  void dostuff(int x);\n"
+                                    "}\n"
+                                    "void A::dostuff(int x) { int a = 1000 / x; }");
+        ASSERT_EQUALS("[test.cpp:5]: (warning) Arbitrary usage of public method A::dostuff() could result in division by zero.\n", errout.str());
+    }
 };
 
 REGISTER_TEST(TestClass)