Fixed #6165 (Remove old checkUnsignedDivision checker that uses neither AST nor ValueFlow. The CheckType::checkSignConversion should be much more accurate)
This commit is contained in:
Daniel Marjamäki
parent
d60cf16eb8
commit
0e55f12140
4
Makefile
4
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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
//---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
|
|
@ -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 <http://www.gnu.org/licenses/>.
|
||||
*/
|
||||
|
||||
|
||||
// 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 <sstream>
|
||||
|
||||
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)
|
||||
|
|
@ -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 \
|
||||
|
|
|
|||
|
|
@ -42,7 +42,6 @@
|
|||
<ClCompile Include="testcondition.cpp" />
|
||||
<ClCompile Include="testconstructors.cpp" />
|
||||
<ClCompile Include="testcppcheck.cpp" />
|
||||
<ClCompile Include="testdivision.cpp" />
|
||||
<ClCompile Include="testerrorlogger.cpp" />
|
||||
<ClCompile Include="testexceptionsafety.cpp" />
|
||||
<ClCompile Include="testfilelister.cpp" />
|
||||
|
|
|
|||
|
|
@ -46,9 +46,6 @@
|
|||
<ClCompile Include="testcppcheck.cpp">
|
||||
<Filter>Source Files</Filter>
|
||||
</ClCompile>
|
||||
<ClCompile Include="testdivision.cpp">
|
||||
<Filter>Source Files</Filter>
|
||||
</ClCompile>
|
||||
<ClCompile Include="testerrorlogger.cpp">
|
||||
<Filter>Source Files</Filter>
|
||||
</ClCompile>
|
||||
|
|
|
|||
Loading…
Reference in New Issue