walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Fix issue 3695: Handle class pointers 

This switches to use lifetime analysis to check for assigning to non-local variables:

```cpp
class test
{
public:

  void f()
  {
    int x;
    this->ptr = &x;
  }

protected:
  int *ptr;
};
```
This commit is contained in:
Paul Fultz II 2019-07-07 10:16:19 +02:00 committed by Daniel Marjamäki
parent 28678e2fbf
commit b0d10273ed
6 changed files with 74 additions and 86 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
lib/astutils.cpp
View File

@ -1131,6 +1131,32 @@ bool isConstVarExpression(const Token *tok)
return false;
}
static const Variable *getLHSVariableRecursive(const Token *tok)
{
if (!tok)
return nullptr;
if (Token::Match(tok, "*|&|&&|[")) {
const Variable *var = getLHSVariableRecursive(tok->astOperand1());
if (var || Token::simpleMatch(tok, "["))
return var;
return getLHSVariableRecursive(tok->astOperand2());
}
if (Token::Match(tok->previous(), "this . %var%"))
return tok->next()->variable();
return tok->variable();
}
const Variable *getLHSVariable(const Token *tok)
{
if (!Token::Match(tok, "%assign%"))
return nullptr;
if (!tok->astOperand1())
return nullptr;
if (tok->astOperand1()->varId() > 0 && tok->astOperand1()->variable())
return tok->astOperand1()->variable();
return getLHSVariableRecursive(tok->astOperand1());
}
static bool nonLocal(const Variable* var, bool deref)
{
return !var || (!var->isLocal() && !var->isArgument()) || (deref && var->isArgument() && var->isPointer()) || var->isStatic() || var->isReference() || var->isExtern();

2
lib/astutils.h
View File

@ -166,6 +166,8 @@ bool isCPPCast(const Token* tok);
bool isConstVarExpression(const Token *tok);
const Variable *getLHSVariable(const Token *tok);
/**
* Forward data flow analysis for checks
* - unused value

66
lib/checkautovariables.cpp
View File

@ -53,12 +53,6 @@ static bool isPtrArg(const Token *tok)
return (var && var->isArgument() && var->isPointer());
}
static bool isGlobalPtr(const Token *tok)
{
const Variable *var = tok->variable();
return (var && var->isGlobal() && var->isPointer());
}
static bool isArrayArg(const Token *tok)
{
const Variable *var = tok->variable();
@ -236,17 +230,6 @@ void CheckAutoVariables::assignFunctionArg()
}
}
static bool reassignedGlobalPointer(const Token *vartok, unsigned int pointerVarId, const Settings *settings, bool cpp)
{
return isVariableChanged(vartok,
vartok->variable()->scope()->bodyEnd,
pointerVarId,
true,
settings,
cpp);
}
void CheckAutoVariables::autoVariables()
{
const bool printInconclusive = mSettings->inconclusive;
@ -266,17 +249,6 @@ void CheckAutoVariables::autoVariables()
errorAutoVariableAssignment(tok->next(), false);
} else if (Token::Match(tok, "[;{}] %var% . %var% =") && isPtrArg(tok->next()) && isAddressOfLocalVariable(tok->tokAt(4)->astOperand2())) {
errorAutoVariableAssignment(tok->next(), false);
} else if (mSettings->isEnabled(Settings::WARNING) && Token::Match(tok, "[;{}] %var% = &| %var% ;") && isGlobalPtr(tok->next())) {
const Token * const pointer = tok->next();
if (isAutoVarArray(tok->tokAt(3))) {
const Token * const array = tok->tokAt(3);
if (!reassignedGlobalPointer(array, pointer->varId(), mSettings, mTokenizer->isCPP()))
errorAssignAddressOfLocalArrayToGlobalPointer(pointer, array);
} else if (isAutoVar(tok->tokAt(4))) {
const Token * const variable = tok->tokAt(4);
if (!reassignedGlobalPointer(variable, pointer->varId(), mSettings, mTokenizer->isCPP()))
errorAssignAddressOfLocalVariableToGlobalPointer(pointer, variable);
}
} else if (Token::Match(tok, "[;{}] %var% . %var% = %var% ;")) {
// TODO: check if the parameter is only changed temporarily (#2969)
if (printInconclusive && isPtrArg(tok->next())) {
@ -357,22 +329,6 @@ void CheckAutoVariables::errorAutoVariableAssignment(const Token *tok, bool inco
}
}
void CheckAutoVariables::errorAssignAddressOfLocalArrayToGlobalPointer(const Token *pointer, const Token *array)
{
const std::string pointerName = pointer ? pointer->str() : std::string("pointer");
const std::string arrayName = array ? array->str() : std::string("array");
reportError(pointer, Severity::warning, "autoVariablesAssignGlobalPointer",
"$symbol:" + arrayName + "\nAddress of local array $symbol is assigned to global pointer " + pointerName +" and not reassigned before $symbol goes out of scope.", CWE562, false);
}
void CheckAutoVariables::errorAssignAddressOfLocalVariableToGlobalPointer(const Token *pointer, const Token *variable)
{
const std::string pointerName = pointer ? pointer->str() : std::string("pointer");
const std::string variableName = variable ? variable->str() : std::string("variable");
reportError(pointer, Severity::warning, "autoVariablesAssignGlobalPointer",
"$symbol:" + variableName + "\nAddress of local variable $symbol is assigned to global pointer " + pointerName +" and not reassigned before $symbol goes out of scope.", CWE562, false);
}
void CheckAutoVariables::errorReturnAddressOfFunctionParameter(const Token *tok, const std::string &varname)
{
reportError(tok, Severity::error, "returnAddressOfFunctionParameter",
@ -628,11 +584,21 @@ void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token
} else if (isDeadScope(val.tokvalue->variable()->nameToken(), tok->scope())) {
errorInvalidLifetime(tok, &val);
break;
} else if (tok->variable() && tok->variable()->declarationId() == tok->varId() &&
!tok->variable()->isLocal() && !tok->variable()->isArgument() &&
isInScope(val.tokvalue->variable()->nameToken(), tok->scope())) {
errorDanglngLifetime(tok, &val);
break;
} else if (isInScope(val.tokvalue->variable()->nameToken(), tok->scope())) {
const Variable * var = nullptr;
const Token * tok2 = tok;
if (Token::simpleMatch(tok->astParent(), "=")) {
if (tok->astParent()->astOperand2() == tok) {
var = getLHSVariable(tok->astParent());
tok2 = tok->astParent()->astOperand1();
}
} else if (tok->variable() && tok->variable()->declarationId() == tok->varId()) {
var = tok->variable();
}
if (var && !var->isLocal() && !var->isArgument() && !isVariableChanged(tok->next(), tok->scope()->bodyEnd, var->declarationId(), var->isGlobal(), mSettings, mTokenizer->isCPP())) {
errorDanglngLifetime(tok2, &val);
break;
}
}
}
const Token *lambdaEndToken = findLambdaEndToken(tok);
@ -716,7 +682,7 @@ void CheckAutoVariables::errorInvalidLifetime(const Token *tok, const ValueFlow:
void CheckAutoVariables::errorDanglngLifetime(const Token *tok, const ValueFlow::Value *val)
{
ErrorPath errorPath = val ? val->errorPath : ErrorPath();
std::string tokName = tok ? tok->str() : "x";
std::string tokName = tok ? tok->expressionString() : "x";
std::string msg = "Non-local variable '" + tokName + "' will use " + lifetimeMessage(tok, val, errorPath);
errorPath.emplace_back(tok, "");
reportError(errorPath, Severity::error, "danglingLifetime", msg + ".", CWE562, false);

3
lib/checkautovariables.h
View File

@ -80,8 +80,6 @@ private:
void errorReturnAddressToAutoVariable(const Token *tok);
void errorReturnAddressToAutoVariable(const Token *tok, const ValueFlow::Value *value);
void errorAssignAddressOfLocalArrayToGlobalPointer(const Token *pointer, const Token *array);
void errorAssignAddressOfLocalVariableToGlobalPointer(const Token *pointer, const Token *variable);
void errorReturnPointerToLocalArray(const Token *tok);
void errorAutoVariableAssignment(const Token *tok, bool inconclusive);
void errorReturnDanglingLifetime(const Token *tok, const ValueFlow::Value* val);
@ -100,7 +98,6 @@ private:
CheckAutoVariables c(nullptr,settings,errorLogger);
c.errorAutoVariableAssignment(nullptr, false);
c.errorReturnAddressToAutoVariable(nullptr);
c.errorAssignAddressOfLocalArrayToGlobalPointer(nullptr, nullptr);
c.errorReturnPointerToLocalArray(nullptr);
c.errorReturnReference(nullptr, errorPath);
c.errorDanglingReference(nullptr, nullptr, errorPath);

37
lib/valueflow.cpp
View File

@ -2819,35 +2819,6 @@ static bool isNotLifetimeValue(const ValueFlow::Value& val)
return !val.isLifetimeValue();
}
static const Variable *getLHSVariableRecursive(const Token *tok)
{
if (!tok)
return nullptr;
if (Token::Match(tok, "*|&|&&|[")) {
const Variable *var1 = getLHSVariableRecursive(tok->astOperand1());
if (var1 || Token::simpleMatch(tok, "["))
return var1;
const Variable *var2 = getLHSVariableRecursive(tok->astOperand2());
return var2;
}
if (!tok->variable())
return nullptr;
if (tok->variable()->nameToken() == tok)
return tok->variable();
return nullptr;
}
static const Variable *getLHSVariable(const Token *tok)
{
if (!Token::Match(tok, "%assign%"))
return nullptr;
if (!tok->astOperand1())
return nullptr;
if (tok->astOperand1()->varId() > 0 && tok->astOperand1()->variable())
return tok->astOperand1()->variable();
return getLHSVariableRecursive(tok->astOperand1());
}
static bool isLifetimeOwned(const ValueType *vt, const ValueType *vtParent)
{
if (!vtParent)
@ -2942,10 +2913,14 @@ static void valueFlowForwardLifetime(Token * tok, TokenList *tokenlist, ErrorLog
// Assignment
if (parent->str() == "=" && (!parent->astParent() || Token::simpleMatch(parent->astParent(), ";"))) {
const Variable *var = getLHSVariable(parent);
if (!var || (!var->isLocal() && !var->isGlobal() && !var->isArgument()))
if (!var)
return;
const Token *const endOfVarScope = var->typeStartToken()->scope()->bodyEnd;
const Token * endOfVarScope = nullptr;
if (!var->isLocal())
endOfVarScope = tok->scope()->bodyEnd;
else
endOfVarScope = var->typeStartToken()->scope()->bodyEnd;
// Rhs values..
if (!parent->astOperand2() || parent->astOperand2()->values().empty())

26
test/testautovariables.cpp
View File

@ -83,6 +83,7 @@ private:
TEST_CASE(assignAddressOfLocalArrayToGlobalPointer);
TEST_CASE(assignAddressOfLocalVariableToGlobalPointer);
TEST_CASE(assignAddressOfLocalVariableToMemberVariable);
TEST_CASE(returnLocalVariable1);
TEST_CASE(returnLocalVariable2);
@ -700,7 +701,7 @@ private:
" int x[10];\n"
" p = x;\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (warning) Address of local array x is assigned to global pointer p and not reassigned before x goes out of scope.\n", errout.str());
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:3] -> [test.cpp:4]: (error) Non-local variable 'p' will use pointer to local variable 'x'.\n", errout.str());
check("int *p;\n"
"void f() {\n"
@ -717,7 +718,7 @@ private:
" int x;\n"
" p = &x;\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (warning) Address of local variable x is assigned to global pointer p and not reassigned before x goes out of scope.\n", errout.str());
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:3] -> [test.cpp:4]: (error) Non-local variable 'p' will use pointer to local variable 'x'.\n", errout.str());
check("int *p;\n"
"void f() {\n"
@ -728,6 +729,27 @@ private:
ASSERT_EQUALS("", errout.str());
}
void assignAddressOfLocalVariableToMemberVariable() {
check("struct A {\n"
" void f() {\n"
" int x;\n"
" ptr = &x;\n"
" }\n"
" int *ptr;\n"
"};\n");
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:3] -> [test.cpp:4]: (error) Non-local variable 'ptr' will use pointer to local variable 'x'.\n", errout.str());
check("struct A {\n"
" void f() {\n"
" int x;\n"
" ptr = &x;\n"
" ptr = 0;\n"
" }\n"
" int *ptr;\n"
"};\n");
ASSERT_EQUALS("", errout.str());
}
void returnLocalVariable1() {
check("char *foo()\n"
"{\n"