From 86c84029e385c1813f811d11c6b300d2f1202511 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Fri, 20 Oct 2017 02:02:51 +0200 Subject: [PATCH] New check: Check public interface of classes (#8248) --- lib/checkclass.cpp | 38 ++++++++++++++++++++++++++++++++++++++ lib/checkclass.h | 9 ++++++++- test/testclass.cpp | 27 +++++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 1 deletion(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index aa192b300..3c86bddd7 100644 --- a/lib/checkclass.cpp +++ b/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::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."); +} diff --git a/lib/checkclass.h b/lib/checkclass.h index c6d0dd773..8c8746513 100644 --- a/lib/checkclass.h +++ b/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 & 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 diff --git a/test/testclass.cpp b/test/testclass.cpp index 83f33b8ae..f1c08eeea 100644 --- a/test/testclass.cpp +++ b/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)