Fix issue 1777: Undefined Behavior: Comparing pointers to different objects
This uses the lifetime analysis to check when comparing pointer that point to different objects: ```cpp int main(void) { int foo[10]; int bar[10]; int diff; if(foo > bar) // Undefined Behavior { diff = 1; } return 0; } ```
This commit is contained in:
parent
1b703ce58e
commit
fd3c1fd040
|
@ -2836,3 +2836,69 @@ void CheckOther::constArgumentError(const Token *tok, const Token *ftok, const V
|
||||||
const ErrorPath errorPath = getErrorPath(tok, value, errmsg);
|
const ErrorPath errorPath = getErrorPath(tok, value, errmsg);
|
||||||
reportError(errorPath, Severity::style, "constArgument", errmsg, CWE570, false);
|
reportError(errorPath, Severity::style, "constArgument", errmsg, CWE570, false);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
static ValueFlow::Value getLifetimeObjValue(const Token *tok)
|
||||||
|
{
|
||||||
|
ValueFlow::Value result;
|
||||||
|
auto pred = [](const ValueFlow::Value &v) {
|
||||||
|
if (!v.isLocalLifetimeValue())
|
||||||
|
return false;
|
||||||
|
if (!v.tokvalue->variable())
|
||||||
|
return false;
|
||||||
|
return true;
|
||||||
|
};
|
||||||
|
auto it = std::find_if(tok->values().begin(), tok->values().end(), pred);
|
||||||
|
if (it == tok->values().end())
|
||||||
|
return result;
|
||||||
|
result = *it;
|
||||||
|
// There should only be one lifetime
|
||||||
|
if (std::find_if(std::next(it), tok->values().end(), pred) != tok->values().end())
|
||||||
|
return result;
|
||||||
|
return result;
|
||||||
|
}
|
||||||
|
|
||||||
|
void CheckOther::checkComparePointers()
|
||||||
|
{
|
||||||
|
const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();
|
||||||
|
for (const Scope *functionScope : symbolDatabase->functionScopes) {
|
||||||
|
for (const Token *tok = functionScope->bodyStart; tok != functionScope->bodyEnd; tok = tok->next()) {
|
||||||
|
if (!Token::Match(tok, "<|>|<=|>="))
|
||||||
|
continue;
|
||||||
|
const Token *tok1 = tok->astOperand1();
|
||||||
|
const Token *tok2 = tok->astOperand2();
|
||||||
|
if (!astIsPointer(tok1) || !astIsPointer(tok2))
|
||||||
|
continue;
|
||||||
|
ValueFlow::Value v1 = getLifetimeObjValue(tok1);
|
||||||
|
ValueFlow::Value v2 = getLifetimeObjValue(tok2);
|
||||||
|
if (!v1.isLocalLifetimeValue() || !v2.isLocalLifetimeValue())
|
||||||
|
continue;
|
||||||
|
const Variable *var1 = v1.tokvalue->variable();
|
||||||
|
const Variable *var2 = v2.tokvalue->variable();
|
||||||
|
if (!var1 || !var2)
|
||||||
|
continue;
|
||||||
|
if (v1.tokvalue->varId() == v2.tokvalue->varId())
|
||||||
|
continue;
|
||||||
|
if (var1->isReference() || var2->isReference())
|
||||||
|
continue;
|
||||||
|
if (var1->isRValueReference() || var2->isRValueReference())
|
||||||
|
continue;
|
||||||
|
comparePointersError(tok, &v1, &v2);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
void CheckOther::comparePointersError(const Token *tok, const ValueFlow::Value *v1, const ValueFlow::Value *v2)
|
||||||
|
{
|
||||||
|
ErrorPath errorPath;
|
||||||
|
if (v1) {
|
||||||
|
errorPath.emplace_back(v1->tokvalue->variable()->nameToken(), "Variable declared here.");
|
||||||
|
errorPath.insert(errorPath.end(), v1->errorPath.begin(), v1->errorPath.end());
|
||||||
|
}
|
||||||
|
if (v2) {
|
||||||
|
errorPath.emplace_back(v2->tokvalue->variable()->nameToken(), "Variable declared here.");
|
||||||
|
errorPath.insert(errorPath.end(), v2->errorPath.begin(), v2->errorPath.end());
|
||||||
|
}
|
||||||
|
errorPath.emplace_back(tok, "");
|
||||||
|
reportError(
|
||||||
|
errorPath, Severity::error, "comparePointers", "Comparing pointers that point to different objects", CWE570, false);
|
||||||
|
}
|
||||||
|
|
|
@ -83,6 +83,7 @@ public:
|
||||||
checkOther.checkFuncArgNamesDifferent();
|
checkOther.checkFuncArgNamesDifferent();
|
||||||
checkOther.checkShadowVariables();
|
checkOther.checkShadowVariables();
|
||||||
checkOther.checkConstArgument();
|
checkOther.checkConstArgument();
|
||||||
|
checkOther.checkComparePointers();
|
||||||
checkOther.checkIncompleteStatement();
|
checkOther.checkIncompleteStatement();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -214,6 +215,8 @@ public:
|
||||||
|
|
||||||
void checkConstArgument();
|
void checkConstArgument();
|
||||||
|
|
||||||
|
void checkComparePointers();
|
||||||
|
|
||||||
private:
|
private:
|
||||||
// Error messages..
|
// Error messages..
|
||||||
void checkComparisonFunctionIsAlwaysTrueOrFalseError(const Token* tok, const std::string &functionName, const std::string &varName, const bool result);
|
void checkComparisonFunctionIsAlwaysTrueOrFalseError(const Token* tok, const std::string &functionName, const std::string &varName, const bool result);
|
||||||
|
@ -267,6 +270,7 @@ private:
|
||||||
void funcArgOrderDifferent(const std::string & functionName, const Token * declaration, const Token * definition, const std::vector<const Token*> & declarations, const std::vector<const Token*> & definitions);
|
void funcArgOrderDifferent(const std::string & functionName, const Token * declaration, const Token * definition, const std::vector<const Token*> & declarations, const std::vector<const Token*> & definitions);
|
||||||
void shadowError(const Token *var, const Token *shadowed, bool shadowVar);
|
void shadowError(const Token *var, const Token *shadowed, bool shadowVar);
|
||||||
void constArgumentError(const Token *tok, const Token *ftok, const ValueFlow::Value *value);
|
void constArgumentError(const Token *tok, const Token *ftok, const ValueFlow::Value *value);
|
||||||
|
void comparePointersError(const Token *tok, const ValueFlow::Value *v1, const ValueFlow::Value *v2);
|
||||||
|
|
||||||
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const OVERRIDE {
|
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const OVERRIDE {
|
||||||
CheckOther c(nullptr, settings, errorLogger);
|
CheckOther c(nullptr, settings, errorLogger);
|
||||||
|
@ -331,6 +335,7 @@ private:
|
||||||
c.shadowError(nullptr, nullptr, false);
|
c.shadowError(nullptr, nullptr, false);
|
||||||
c.shadowError(nullptr, nullptr, true);
|
c.shadowError(nullptr, nullptr, true);
|
||||||
c.constArgumentError(nullptr, nullptr, nullptr);
|
c.constArgumentError(nullptr, nullptr, nullptr);
|
||||||
|
c.comparePointersError(nullptr, nullptr, nullptr);
|
||||||
|
|
||||||
const std::vector<const Token *> nullvec;
|
const std::vector<const Token *> nullvec;
|
||||||
c.funcArgOrderDifferent("function", nullptr, nullptr, nullvec, nullvec);
|
c.funcArgOrderDifferent("function", nullptr, nullptr, nullvec, nullvec);
|
||||||
|
|
|
@ -2727,8 +2727,8 @@ const Token *getLifetimeToken(const Token *tok, ValueFlow::Value::ErrorPath &err
|
||||||
if (!vartok)
|
if (!vartok)
|
||||||
return tok;
|
return tok;
|
||||||
const Variable *tokvar = vartok->variable();
|
const Variable *tokvar = vartok->variable();
|
||||||
if (!astIsContainer(vartok) && !(tokvar && tokvar->isArray()) && Token::Match(vartok->astParent(), "[|*|.") &&
|
if (!astIsContainer(vartok) && !(tokvar && tokvar->isArray()) &&
|
||||||
vartok->astParent()->originalName() != ".") {
|
(Token::Match(vartok->astParent(), "[|*") || vartok->astParent()->originalName() == "->")) {
|
||||||
for (const ValueFlow::Value &v : vartok->values()) {
|
for (const ValueFlow::Value &v : vartok->values()) {
|
||||||
if (!v.isLocalLifetimeValue())
|
if (!v.isLocalLifetimeValue())
|
||||||
continue;
|
continue;
|
||||||
|
@ -3090,6 +3090,8 @@ static bool isDecayedPointer(const Token *tok, const Settings *settings)
|
||||||
return false;
|
return false;
|
||||||
if (astIsPointer(tok->astParent()) && !Token::simpleMatch(tok->astParent(), "return"))
|
if (astIsPointer(tok->astParent()) && !Token::simpleMatch(tok->astParent(), "return"))
|
||||||
return true;
|
return true;
|
||||||
|
if (Token::Match(tok->astParent(), "%cop%"))
|
||||||
|
return true;
|
||||||
if (!Token::simpleMatch(tok->astParent(), "return"))
|
if (!Token::simpleMatch(tok->astParent(), "return"))
|
||||||
return false;
|
return false;
|
||||||
if (!tok->scope())
|
if (!tok->scope())
|
||||||
|
|
|
@ -223,6 +223,7 @@ private:
|
||||||
|
|
||||||
TEST_CASE(shadowVariables);
|
TEST_CASE(shadowVariables);
|
||||||
TEST_CASE(constArgument);
|
TEST_CASE(constArgument);
|
||||||
|
TEST_CASE(checkComparePointers);
|
||||||
}
|
}
|
||||||
|
|
||||||
void check(const char code[], const char *filename = nullptr, bool experimental = false, bool inconclusive = true, bool runSimpleChecks=true, bool verbose=false, Settings* settings = 0) {
|
void check(const char code[], const char *filename = nullptr, bool experimental = false, bool inconclusive = true, bool runSimpleChecks=true, bool verbose=false, Settings* settings = 0) {
|
||||||
|
@ -7590,6 +7591,110 @@ private:
|
||||||
"}\n");
|
"}\n");
|
||||||
ASSERT_EQUALS("", errout.str());
|
ASSERT_EQUALS("", errout.str());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void checkComparePointers() {
|
||||||
|
check("int f() {\n"
|
||||||
|
" int foo[1] = {0};\n"
|
||||||
|
" int bar[1] = {0};\n"
|
||||||
|
" int diff = 0;\n"
|
||||||
|
" if(foo > bar) {\n"
|
||||||
|
" diff = 1;\n"
|
||||||
|
" }\n"
|
||||||
|
" return diff;\n"
|
||||||
|
"}\n");
|
||||||
|
ASSERT_EQUALS(
|
||||||
|
"[test.cpp:2] -> [test.cpp:5] -> [test.cpp:3] -> [test.cpp:5] -> [test.cpp:5]: (error) Comparing pointers that point to different objects\n",
|
||||||
|
errout.str());
|
||||||
|
|
||||||
|
check("bool f() {\n"
|
||||||
|
" int x = 0;\n"
|
||||||
|
" int y = 0;\n"
|
||||||
|
" int* xp = &x;\n"
|
||||||
|
" int* yp = &y;\n"
|
||||||
|
" return xp > yp;\n"
|
||||||
|
"}\n");
|
||||||
|
ASSERT_EQUALS(
|
||||||
|
"[test.cpp:2] -> [test.cpp:4] -> [test.cpp:3] -> [test.cpp:5] -> [test.cpp:6]: (error) Comparing pointers that point to different objects\n",
|
||||||
|
errout.str());
|
||||||
|
|
||||||
|
check("bool f() {\n"
|
||||||
|
" int x = 0;\n"
|
||||||
|
" int y = 1;\n"
|
||||||
|
" return &x > &y;\n"
|
||||||
|
"}\n");
|
||||||
|
ASSERT_EQUALS(
|
||||||
|
"[test.cpp:2] -> [test.cpp:4] -> [test.cpp:3] -> [test.cpp:4] -> [test.cpp:4]: (error) Comparing pointers that point to different objects\n",
|
||||||
|
errout.str());
|
||||||
|
|
||||||
|
check("struct A {int data;};\n"
|
||||||
|
"bool f() {\n"
|
||||||
|
" A x;\n"
|
||||||
|
" A y;\n"
|
||||||
|
" int* xp = &x.data;\n"
|
||||||
|
" int* yp = &y.data;\n"
|
||||||
|
" return xp > yp;\n"
|
||||||
|
"}\n");
|
||||||
|
ASSERT_EQUALS(
|
||||||
|
"[test.cpp:1] -> [test.cpp:5] -> [test.cpp:1] -> [test.cpp:6] -> [test.cpp:7]: (error) Comparing pointers that point to different objects\n",
|
||||||
|
errout.str());
|
||||||
|
|
||||||
|
check("struct A {int data;};\n"
|
||||||
|
"bool f(A ix, A iy) {\n"
|
||||||
|
" A* x = &ix;\n"
|
||||||
|
" A* y = &iy;\n"
|
||||||
|
" int* xp = &x->data;\n"
|
||||||
|
" int* yp = &y->data;\n"
|
||||||
|
" return xp > yp;\n"
|
||||||
|
"}\n");
|
||||||
|
ASSERT_EQUALS(
|
||||||
|
"[test.cpp:2] -> [test.cpp:3] -> [test.cpp:5] -> [test.cpp:2] -> [test.cpp:4] -> [test.cpp:6] -> [test.cpp:7]: (error) Comparing pointers that point to different objects\n",
|
||||||
|
errout.str());
|
||||||
|
|
||||||
|
check("bool f(int * xp, int* yp) {\n"
|
||||||
|
" return &xp > &yp;\n"
|
||||||
|
"}\n");
|
||||||
|
ASSERT_EQUALS(
|
||||||
|
"[test.cpp:1] -> [test.cpp:2] -> [test.cpp:1] -> [test.cpp:2] -> [test.cpp:2]: (error) Comparing pointers that point to different objects\n",
|
||||||
|
errout.str());
|
||||||
|
|
||||||
|
check("bool f() {\n"
|
||||||
|
" int x[2] = {1, 2}m;\n"
|
||||||
|
" int* xp = &x[0];\n"
|
||||||
|
" int* yp = &x[1];\n"
|
||||||
|
" return xp > yp;\n"
|
||||||
|
"}\n");
|
||||||
|
ASSERT_EQUALS("", errout.str());
|
||||||
|
|
||||||
|
check("bool f(int * xp, int* yp) {\n"
|
||||||
|
" return xp > yp;\n"
|
||||||
|
"}\n");
|
||||||
|
ASSERT_EQUALS("", errout.str());
|
||||||
|
|
||||||
|
check("bool f(int & x, int& y) {\n"
|
||||||
|
" return &x > &y;\n"
|
||||||
|
"}\n");
|
||||||
|
ASSERT_EQUALS("", errout.str());
|
||||||
|
|
||||||
|
check("int& g();\n"
|
||||||
|
"bool f() {\n"
|
||||||
|
" int& x = g();\n"
|
||||||
|
" int& y = g();\n"
|
||||||
|
" int* xp = &x;\n"
|
||||||
|
" int* yp = &y;\n"
|
||||||
|
" return xp > yp;\n"
|
||||||
|
"}\n");
|
||||||
|
ASSERT_EQUALS("", errout.str());
|
||||||
|
|
||||||
|
check("struct A {int data;};\n"
|
||||||
|
"bool f(A ix) {\n"
|
||||||
|
" A* x = &ix;\n"
|
||||||
|
" A* y = x;\n"
|
||||||
|
" int* xp = &x->data;\n"
|
||||||
|
" int* yp = &y->data;\n"
|
||||||
|
" return xp > yp;\n"
|
||||||
|
"}\n");
|
||||||
|
ASSERT_EQUALS("", errout.str());
|
||||||
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
REGISTER_TEST(TestOther)
|
REGISTER_TEST(TestOther)
|
||||||
|
|
Loading…
Reference in New Issue