From 0e55f12140fe02a890187e830cedb56cf465a19c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Fri, 12 Sep 2014 16:59:16 +0200 Subject: [PATCH] Fixed #6165 (Remove old checkUnsignedDivision checker that uses neither AST nor ValueFlow. The CheckType::checkSignConversion should be much more accurate) --- Makefile | 4 - lib/checkother.cpp | 57 --------- lib/checkother.h | 10 -- test/testdivision.cpp | 209 -------------------------------- test/testfiles.pri | 1 - test/testrunner.vcxproj | 1 - test/testrunner.vcxproj.filters | 3 - 7 files changed, 285 deletions(-) delete mode 100644 test/testdivision.cpp diff --git a/Makefile b/Makefile index 768a62d1a..6ff34db02 100644 --- a/Makefile +++ b/Makefile @@ -179,7 +179,6 @@ TESTOBJ = test/options.o \ test/testcondition.o \ test/testconstructors.o \ test/testcppcheck.o \ - test/testdivision.o \ test/testerrorlogger.o \ test/testexceptionsafety.o \ test/testfilelister.o \ @@ -462,9 +461,6 @@ test/testconstructors.o: test/testconstructors.cpp lib/cxx11emu.h lib/tokenize.h test/testcppcheck.o: test/testcppcheck.cpp lib/cxx11emu.h lib/cppcheck.h lib/config.h lib/settings.h lib/library.h lib/path.h lib/mathlib.h lib/token.h lib/valueflow.h lib/suppressions.h lib/standards.h lib/timer.h lib/errorlogger.h test/testsuite.h test/redirect.h lib/check.h lib/tokenize.h lib/tokenlist.h $(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CFG) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -std=c++0x -c -o test/testcppcheck.o test/testcppcheck.cpp -test/testdivision.o: test/testdivision.cpp lib/cxx11emu.h lib/tokenize.h lib/errorlogger.h lib/config.h lib/suppressions.h lib/tokenlist.h lib/checkother.h lib/check.h lib/token.h lib/valueflow.h lib/mathlib.h lib/settings.h lib/library.h lib/path.h lib/standards.h lib/timer.h test/testsuite.h test/redirect.h - $(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CFG) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -std=c++0x -c -o test/testdivision.o test/testdivision.cpp - test/testerrorlogger.o: test/testerrorlogger.cpp lib/cxx11emu.h lib/cppcheck.h lib/config.h lib/settings.h lib/library.h lib/path.h lib/mathlib.h lib/token.h lib/valueflow.h lib/suppressions.h lib/standards.h lib/timer.h lib/errorlogger.h test/testsuite.h test/redirect.h $(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CFG) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -std=c++0x -c -o test/testerrorlogger.o test/testerrorlogger.cpp diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 7eeac0ca2..92e2fba3a 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -1211,63 +1211,6 @@ void CheckOther::unreachableCodeError(const Token *tok, bool inconclusive) "Statements following return, break, continue, goto or throw will never be executed.", inconclusive); } -//--------------------------------------------------------------------------- -// Check for unsigned divisions -//--------------------------------------------------------------------------- -bool CheckOther::isUnsigned(const Variable* var) const -{ - return (var && var->typeStartToken()->isUnsigned() && !var->isPointer() && !var->isArray() && _tokenizer->sizeOfType(var->typeStartToken()) >= _settings->sizeof_int); -} -bool CheckOther::isSigned(const Variable* var) -{ - return (var && !var->typeStartToken()->isUnsigned() && var->isIntegralType() && !var->isPointer() && !var->isArray()); -} - -void CheckOther::checkUnsignedDivision() -{ - bool warning = _settings->isEnabled("warning"); - - const SymbolDatabase* symbolDatabase = _tokenizer->getSymbolDatabase(); - const std::size_t functions = symbolDatabase->functionScopes.size(); - for (std::size_t i = 0; i < functions; ++i) { - const Scope * scope = symbolDatabase->functionScopes[i]; - const Token* ifTok = 0; - // Check for "ivar / uvar" and "uvar / ivar" - for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) { - - if (Token::Match(tok, "[).]")) // Don't check members or casted variables - continue; - - if (Token::Match(tok->next(), "%var% / %num%")) { - if (tok->strAt(3)[0] == '-' && isUnsigned(tok->next()->variable())) { - udivError(tok->next(), false); - } - } else if (Token::Match(tok->next(), "%num% / %var%")) { - if (tok->strAt(1)[0] == '-' && isUnsigned(tok->tokAt(3)->variable())) { - udivError(tok->next(), false); - } - } else if (_settings->inconclusive && warning && !ifTok && Token::Match(tok->next(), "%var% / %var%")) { - const Variable* var1 = tok->next()->variable(); - const Variable* var2 = tok->tokAt(3)->variable(); - if ((isUnsigned(var1) && isSigned(var2)) || (isUnsigned(var2) && isSigned(var1))) { - udivError(tok->next(), true); - } - } else if (!ifTok && Token::simpleMatch(tok, "if (")) - ifTok = tok->next()->link()->next()->link(); - else if (ifTok == tok) - ifTok = 0; - } - } -} - -void CheckOther::udivError(const Token *tok, bool inconclusive) -{ - if (inconclusive) - reportError(tok, Severity::warning, "udivError", "Division with signed and unsigned operators. The result might be wrong.", true); - else - reportError(tok, Severity::error, "udivError", "Unsigned division. The result will be wrong."); -} - //--------------------------------------------------------------------------- // memset(p, y, 0 /* bytes to fill */) <- 2nd and 3rd arguments inverted //--------------------------------------------------------------------------- diff --git a/lib/checkother.h b/lib/checkother.h index 6bb7811e0..f45d08425 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -59,7 +59,6 @@ public: // Checks checkOther.warningOldStylePointerCast(); checkOther.invalidPointerCast(); - checkOther.checkUnsignedDivision(); checkOther.checkCharVariable(); checkOther.checkRedundantAssignment(); checkOther.checkRedundantAssignmentInSwitch(); @@ -128,9 +127,6 @@ public: */ void invalidFunctionUsage(); - /** @brief %Check for unsigned division */ - void checkUnsignedDivision(); - /** @brief %Check scope of variables */ void checkVariableScope(); static bool checkInnerScope(const Token *tok, const Variable* var, bool& used); @@ -234,9 +230,6 @@ public: void checkComparisonFunctionIsAlwaysTrueOrFalse(); private: - bool isUnsigned(const Variable *var) const; - static bool isSigned(const Variable *var); - // Error messages.. void checkComparisonFunctionIsAlwaysTrueOrFalseError(const Token* tok, const std::string &strFunctionName, const std::string &varName, const bool result); void checkCastIntToCharAndBackError(const Token *tok, const std::string &strFunctionName); @@ -248,7 +241,6 @@ private: void invalidPointerCastError(const Token* tok, const std::string& from, const std::string& to, bool inconclusive); void invalidFunctionArgError(const Token *tok, const std::string &functionName, int argnr, const std::string &validstr); void invalidFunctionArgBoolError(const Token *tok, const std::string &functionName, int argnr); - void udivError(const Token *tok, bool inconclusive); void passedByValueError(const Token *tok, const std::string &parname); void constStatementError(const Token *tok, const std::string &type); void charArrayIndexError(const Token *tok); @@ -297,7 +289,6 @@ private: // error c.invalidFunctionArgError(0, "func_name", 1, "1-4"); c.invalidFunctionArgBoolError(0, "func_name", 1); - c.udivError(0, false); c.zerodivError(0, false); c.zerodivcondError(0,0,false); c.misusedScopeObjectError(NULL, "varname"); @@ -382,7 +373,6 @@ private: "* C-style pointer cast in cpp file\n" "* casting between incompatible pointer types\n" "* redundant if\n" - "* [[CheckUnsignedDivision|unsigned division]]\n" "* passing parameter by value\n" "* [[IncompleteStatement|Incomplete statement]]\n" "* [[charvar|check how signed char variables are used]]\n" diff --git a/test/testdivision.cpp b/test/testdivision.cpp deleted file mode 100644 index ff8ba2a46..000000000 --- a/test/testdivision.cpp +++ /dev/null @@ -1,209 +0,0 @@ -/* - * Cppcheck - A tool for static C/C++ code analysis - * Copyright (C) 2007-2014 Daniel Marjamäki and Cppcheck team. - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - */ - - -// Check for dangerous division.. -// such as "svar / uvar". Treating "svar" as unsigned data is not good - - -#include "tokenize.h" -#include "checkother.h" -#include "testsuite.h" - -#include - -extern std::ostringstream errout; - -class TestDivision : public TestFixture { -public: - TestDivision() : TestFixture("TestDivision") { - } - -private: - void check(const char code[], bool inconclusive = false) { - // Clear the error buffer.. - errout.str(""); - - Settings settings; - settings.addEnabled("warning"); - settings.inconclusive = inconclusive; - - // Tokenize.. - Tokenizer tokenizer(&settings, this); - std::istringstream istr(code); - tokenizer.tokenize(istr, "test.cpp"); - - // Check for unsigned divisions.. - CheckOther checkOther(&tokenizer, &settings, this); - checkOther.checkUnsignedDivision(); - } - - void run() { - TEST_CASE(division1); - TEST_CASE(division2); - TEST_CASE(division4); - TEST_CASE(division5); - TEST_CASE(division6); - TEST_CASE(division7); - TEST_CASE(division8); - TEST_CASE(division9); - TEST_CASE(division10); - TEST_CASE(division11); // no error when using "unsigned char" (it is promoted) - } - - void division1() { - check("void f() {\n" - " int ivar = -2;\n" - " unsigned int uvar = 2;\n" - " return ivar / uvar;\n" - "}", false); - ASSERT_EQUALS("", errout.str()); - - check("void f()\n" - "{\n" - " int ivar = -2;\n" - " unsigned int uvar = 2;\n" - " return ivar / uvar;\n" - "}", true); - ASSERT_EQUALS("[test.cpp:5]: (warning, inconclusive) Division with signed and unsigned operators. The result might be wrong.\n", errout.str()); - } - - void division2() { - check("void f()\n" - "{\n" - " int ivar = -2;\n" - " unsigned int uvar = 2;\n" - " return uvar / ivar;\n" - "}", true); - ASSERT_EQUALS("[test.cpp:5]: (warning, inconclusive) Division with signed and unsigned operators. The result might be wrong.\n", errout.str()); - } - - void division4() { - check("void f1()\n" - "{\n" - " int i1;\n" - "}\n" - "\n" - "void f2(unsigned int i1)\n" - "{\n" - " unsigned int i2;\n" - " result = i2 / i1;" - "}", true); - ASSERT_EQUALS("", errout.str()); - - check("void f1()\n" - "{\n" - " unsigned int num = 0;\n" - "}\n" - "\n" - "void f2(int X)\n" - "{\n" - " X = X / z;" - "}", true); - ASSERT_EQUALS("", errout.str()); - } - - void division5() { - check("void foo()\n" - "{\n" - " unsigned int val = 32;\n" - " val = val / (16);\n" - "}", true); - ASSERT_EQUALS("", errout.str()); - } - - void division6() { - check("void foo()\n" - "{\n" - " unsigned int val = 32;\n" - " int i = val / -2; }"); - ASSERT_EQUALS("[test.cpp:4]: (error) Unsigned division. The result will be wrong.\n", errout.str()); - } - - void division7() { - check("void foo()\n" - "{\n" - " unsigned int val = 32;\n" - " int i = -96 / val; }"); - ASSERT_EQUALS("[test.cpp:4]: (error) Unsigned division. The result will be wrong.\n", errout.str()); - } - - void division8() { - check("void foo(int b)\n" - "{\n" - " if (b > 0)\n" - " {\n" - " unsigned int a;\n" - " unsigned int c = a / b;\n" - " }\n" - "}", true); - ASSERT_EQUALS("", errout.str()); - - check("void foo(int b)\n" - "{\n" - " if (b < 0)\n" - " {\n" - " unsigned int a;\n" - " unsigned int c = a / b;\n" - " }\n" - "}", true); - TODO_ASSERT_EQUALS("[test.cpp:6]: (warning) Division with signed and unsigned operators. The result might be wrong.\n", - "", errout.str()); - - check("void a(int i) { }\n" - "int foo( unsigned int sz )\n" - "{\n" - " register unsigned int i=1;\n" - " return i/sz;\n" - "}", true); - ASSERT_EQUALS("", errout.str()); - } - - void division9() { - check("void f()\n" - "{\n" - " int ivar = -2;\n" - " unsigned long uvar = 2;\n" - " return ivar / uvar;\n" - "}", true); - ASSERT_EQUALS("[test.cpp:5]: (warning, inconclusive) Division with signed and unsigned operators. The result might be wrong.\n", errout.str()); - - check("void f()\n" - "{\n" - " int ivar = -2;\n" - " unsigned long long uvar = 2;\n" - " return ivar / uvar;\n" - "}", true); - ASSERT_EQUALS("[test.cpp:5]: (warning, inconclusive) Division with signed and unsigned operators. The result might be wrong.\n", errout.str()); - } - - void division10() { - // Ticket: #2932 - don't segfault - check("i / i", true); - ASSERT_EQUALS("", errout.str()); - } - - void division11() { - check("void f(int x, unsigned char y) {\n" - " int z = x / y;\n" // no error, y is promoted - "}", true); - ASSERT_EQUALS("", errout.str()); - } -}; - -REGISTER_TEST(TestDivision) diff --git a/test/testfiles.pri b/test/testfiles.pri index 42e3c6c47..606f4de6e 100644 --- a/test/testfiles.pri +++ b/test/testfiles.pri @@ -15,7 +15,6 @@ SOURCES += $${BASEPATH}/test64bit.cpp \ $${BASEPATH}/testcondition.cpp \ $${BASEPATH}/testconstructors.cpp \ $${BASEPATH}/testcppcheck.cpp \ - $${BASEPATH}/testdivision.cpp \ $${BASEPATH}/testerrorlogger.cpp \ $${BASEPATH}/testexceptionsafety.cpp \ $${BASEPATH}/testfilelister.cpp \ diff --git a/test/testrunner.vcxproj b/test/testrunner.vcxproj index 36e80e7ea..974768cce 100644 --- a/test/testrunner.vcxproj +++ b/test/testrunner.vcxproj @@ -42,7 +42,6 @@ - diff --git a/test/testrunner.vcxproj.filters b/test/testrunner.vcxproj.filters index b92e8aaff..8d91cf650 100644 --- a/test/testrunner.vcxproj.filters +++ b/test/testrunner.vcxproj.filters @@ -46,9 +46,6 @@ Source Files - - Source Files - Source Files