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

Fix issue 9711: FP knownConditionTrueFalse for variable modified via pointer (#2813)

This commit is contained in:
Paul Fultz II 2020-09-20 07:27:09 -05:00 committed by GitHub
parent 9f690fb478
commit 857722f859
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 125 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
lib/astutils.cpp
View File

@ -523,17 +523,22 @@ bool precedes(const Token * tok1, const Token * tok2)
return tok1->index() < tok2->index();
}
bool isAliasOf(const Token *tok, nonneg int varid)
bool isAliasOf(const Token *tok, nonneg int varid, bool* inconclusive)
{
if (tok->varId() == varid)
return false;
for (const ValueFlow::Value &val : tok->values()) {
if (!val.isLocalLifetimeValue())
continue;
if (val.isInconclusive())
continue;
if (val.tokvalue->varId() == varid)
if (val.tokvalue->varId() == varid) {
if (val.isInconclusive()) {
if (inconclusive)
*inconclusive = true;
else
continue;
}
return true;
}
}
return false;
}

2
lib/astutils.h
View File

@ -202,7 +202,7 @@ const Token* findVariableChanged(const Token *start, const Token *end, int indir
Token* findVariableChanged(Token *start, const Token *end, int indirect, const nonneg int exprid, bool globalvar, const Settings *settings, bool cpp, int depth = 20);
/// If token is an alias if another variable
bool isAliasOf(const Token *tok, nonneg int varid);
bool isAliasOf(const Token *tok, nonneg int varid, bool* inconclusive = nullptr);
bool isAliased(const Variable *var);

7
lib/checkautovariables.cpp
View File

@ -494,6 +494,7 @@ static bool isDanglingSubFunction(const Token* tokvalue, const Token* tok)
void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token * end)
{
const bool printInconclusive = (mSettings->inconclusive);
if (!start)
return;
const Scope * scope = start->scope();
@ -507,6 +508,8 @@ void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token
// Return reference from function
if (returnRef && Token::simpleMatch(tok->astParent(), "return")) {
for (const LifetimeToken& lt : getLifetimeTokens(tok, true)) {
if (!printInconclusive && lt.inconclusive)
continue;
const Variable* var = lt.token->variable();
if (var && !var->isGlobal() && !var->isStatic() && !var->isReference() && !var->isRValueReference() &&
isInScope(var->nameToken(), tok->scope())) {
@ -531,6 +534,8 @@ void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token
// Reference to temporary
} else if (tok->variable() && (tok->variable()->isReference() || tok->variable()->isRValueReference())) {
for (const LifetimeToken& lt : getLifetimeTokens(getParentLifetime(tok))) {
if (!printInconclusive && lt.inconclusive)
continue;
const Token * tokvalue = lt.token;
if (isDeadTemporary(mTokenizer->isCPP(), tokvalue, tok, &mSettings->library)) {
errorDanglingTempReference(tok, lt.errorPath, lt.inconclusive);
@ -542,6 +547,8 @@ void CheckAutoVariables::checkVarLifetimeScope(const Token * start, const Token
for (const ValueFlow::Value& val:tok->values()) {
if (!val.isLocalLifetimeValue() && !val.isSubFunctionLifetimeValue())
continue;
if (!printInconclusive && val.isInconclusive())
continue;
const bool escape = Token::Match(tok->astParent(), "return|throw");
for (const LifetimeToken& lt : getLifetimeTokens(getParentLifetime(val.tokvalue), escape)) {
const Token * tokvalue = lt.token;

107
lib/valueflow.cpp
View File

@ -2193,42 +2193,13 @@ static bool evalAssignment(ValueFlow::Value &lhsValue, const std::string &assign
}
// Check if its an alias of the variable or is being aliased to this variable
static bool isAliasOf(const Variable* var, const Token* tok, nonneg int varid, const ValueFlow::Value& val)
static bool isAliasOf(const Variable * var, const Token *tok, nonneg int varid, const std::list<ValueFlow::Value>& values, bool* inconclusive = nullptr)
{
if (tok->varId() == varid)
return false;
if (tok->varId() == 0)
return false;
if (isAliasOf(tok, varid))
return true;
if (var && !var->isPointer())
return false;
// Search through non value aliases
if (!val.isNonValue())
return false;
if (val.isInconclusive())
return false;
if (val.isLifetimeValue() && !val.isLocalLifetimeValue())
return false;
if (val.isLifetimeValue() && val.lifetimeKind != ValueFlow::Value::LifetimeKind::Address)
return false;
if (!Token::Match(val.tokvalue, ".|&|*|%var%"))
return false;
if (astHasVar(val.tokvalue, tok->varId()))
return true;
return false;
}
// Check if its an alias of the variable or is being aliased to this variable
static bool isAliasOf(const Variable * var, const Token *tok, nonneg int varid, const std::list<ValueFlow::Value>& values)
{
if (tok->varId() == varid)
return false;
if (tok->varId() == 0)
return false;
if (isAliasOf(tok, varid))
if (isAliasOf(tok, varid, inconclusive))
return true;
if (var && !var->isPointer())
return false;
@ -2328,7 +2299,7 @@ struct ValueFlowForwardAnalyzer : ForwardAnalyzer {
virtual bool match(const Token* tok) const = 0;
virtual bool isAlias(const Token* tok) const = 0;
virtual bool isAlias(const Token* tok, bool& inconclusive) const = 0;
using ProgramState = std::unordered_map<nonneg int, ValueFlow::Value>;
@ -2448,6 +2419,7 @@ struct ValueFlowForwardAnalyzer : ForwardAnalyzer {
virtual Action analyze(const Token* tok) const OVERRIDE {
if (invalid())
return Action::Invalid;
bool inconclusive = false;
if (match(tok)) {
const Token* parent = tok->astParent();
if (astIsPointer(tok) && (Token::Match(parent, "*|[") || (parent && parent->originalName() == "->")) && getIndirect(tok) <= 0)
@ -2479,8 +2451,12 @@ struct ValueFlowForwardAnalyzer : ForwardAnalyzer {
}
return Action::None;
} else if (isAlias(tok)) {
return isAliasModified(tok);
} else if (isAlias(tok, inconclusive)) {
Action a = isAliasModified(tok);
if (inconclusive && a.isModified())
return Action::Inconclusive;
else
return a;
} else if (Token::Match(tok, "%name% (") && !Token::simpleMatch(tok->linkAt(1), ") {")) {
// bailout: global non-const variables
if (isGlobal()) {
@ -2568,7 +2544,7 @@ struct SingleValueFlowForwardAnalyzer : ValueFlowForwardAnalyzer {
value.errorPath.emplace_back(tok, s);
}
virtual bool isAlias(const Token* tok) const OVERRIDE {
virtual bool isAlias(const Token* tok, bool& inconclusive) const OVERRIDE {
if (value.isLifetimeValue())
return false;
for (const auto& m: {
@ -2579,7 +2555,7 @@ struct SingleValueFlowForwardAnalyzer : ValueFlowForwardAnalyzer {
const Variable* var = p.second;
if (tok->varId() == varid)
return true;
if (isAliasOf(var, tok, varid, value))
if (isAliasOf(var, tok, varid, {value}, &inconclusive))
return true;
}
}
@ -3293,8 +3269,32 @@ static void valueFlowLifetimeConstructor(Token *tok,
ErrorLogger *errorLogger,
const Settings *settings);
static const Token* getEndOfVarScope(const Token* tok, const std::vector<const Variable*>& vars)
{
const Token* endOfVarScope = nullptr;
for (const Variable* var : vars) {
if (var && var->isLocal())
endOfVarScope = var->typeStartToken()->scope()->bodyEnd;
else if (!endOfVarScope)
endOfVarScope = tok->scope()->bodyEnd;
}
return endOfVarScope;
}
static void valueFlowForwardLifetime(Token * tok, TokenList *tokenlist, ErrorLogger *errorLogger, const Settings *settings)
{
// Forward lifetimes to constructed variable
if (Token::Match(tok->previous(), "%var% {")) {
std::list<ValueFlow::Value> values = tok->values();
values.remove_if(&isNotLifetimeValue);
valueFlowForward(nextAfterAstRightmostLeaf(tok),
getEndOfVarScope(tok, {tok->variable()}),
tok->previous(),
values,
tokenlist,
settings);
return;
}
Token *parent = tok->astParent();
while (parent && (parent->isArithmeticalOp() || parent->str() == ","))
parent = parent->astParent();
@ -3311,13 +3311,7 @@ static void valueFlowForwardLifetime(Token * tok, TokenList *tokenlist, ErrorLog
std::vector<const Variable*> vars = getLHSVariables(parent);
const Token* endOfVarScope = nullptr;
for (const Variable* var : vars) {
if (var && var->isLocal())
endOfVarScope = var->typeStartToken()->scope()->bodyEnd;
else if (!endOfVarScope)
endOfVarScope = tok->scope()->bodyEnd;
}
const Token* endOfVarScope = getEndOfVarScope(tok, vars);
// Only forward lifetime values
std::list<ValueFlow::Value> values = parent->astOperand2()->values();
@ -3427,8 +3421,6 @@ struct LifetimeStore {
template <class Predicate>
void byRef(Token *tok, TokenList *tokenlist, ErrorLogger *errorLogger, const Settings *settings, Predicate pred) const {
if (!settings->inconclusive && inconclusive)
return;
if (!argtok)
return;
for (const LifetimeToken& lt : getLifetimeTokens(argtok)) {
@ -3465,8 +3457,6 @@ struct LifetimeStore {
template <class Predicate>
void byVal(Token *tok, TokenList *tokenlist, ErrorLogger *errorLogger, const Settings *settings, Predicate pred) const {
if (!settings->inconclusive && inconclusive)
return;
if (!argtok)
return;
if (argtok->values().empty()) {
@ -3647,10 +3637,21 @@ static void valueFlowLifetimeConstructor(Token* tok,
ErrorLogger* errorLogger,
const Settings* settings)
{
if (!t)
return;
if (!Token::Match(tok, "(|{"))
return;
if (!t) {
if (tok->valueType() && tok->valueType()->type != ValueType::RECORD)
return;
// If the type is unknown then assume it captures by value in the
// constructor, but make each lifetime inconclusive
std::vector<const Token*> args = getArguments(tok);
for (const Token *argtok : args) {
LifetimeStore ls{argtok, "Passed to initializer list.", ValueFlow::Value::LifetimeKind::Object};
ls.inconclusive = true;
ls.byVal(tok, tokenlist, errorLogger, settings);
}
return;
}
const Scope* scope = t->classScope;
if (!scope)
return;
@ -3710,8 +3711,8 @@ static void valueFlowLifetimeConstructor(Token* tok, TokenList* tokenlist, Error
ls.byVal(tok, tokenlist, errorLogger, settings);
}
}
} else if (const Type* t = Token::typeOf(tok->previous())) {
valueFlowLifetimeConstructor(tok, t, tokenlist, errorLogger, settings);
} else {
valueFlowLifetimeConstructor(tok, Token::typeOf(tok->previous()), tokenlist, errorLogger, settings);
}
}
@ -5037,7 +5038,7 @@ struct MultiValueFlowForwardAnalyzer : ValueFlowForwardAnalyzer {
}
}
virtual bool isAlias(const Token* tok) const OVERRIDE {
virtual bool isAlias(const Token* tok, bool& inconclusive) const OVERRIDE {
std::list<ValueFlow::Value> vals;
std::transform(values.begin(), values.end(), std::back_inserter(vals), SelectMapValues{});
@ -5046,7 +5047,7 @@ struct MultiValueFlowForwardAnalyzer : ValueFlowForwardAnalyzer {
const Variable* var = p.second;
if (tok->varId() == varid)
return true;
if (isAliasOf(var, tok, varid, vals))
if (isAliasOf(var, tok, varid, vals, &inconclusive))
return true;
}
return false;

6
test/testautovariables.cpp
View File

@ -2611,7 +2611,6 @@ private:
"[test.cpp:7] -> [test.cpp:6] -> [test.cpp:7]: (error) Returning object that points to local variable 'i' that will be invalid when returning.\n",
errout.str());
// TODO: Ast is missing for this case
check("struct A {\n"
" const int& x;\n"
" int y;\n"
@ -2621,9 +2620,8 @@ private:
" A r{i, i};\n"
" return r;\n"
"}\n");
TODO_ASSERT_EQUALS(
"[test.cpp:7] -> [test.cpp:6] -> [test.cpp:7]: (error) Returning object that points to local variable 'i' that will be invalid when returning.\n",
"",
ASSERT_EQUALS(
"[test.cpp:7] -> [test.cpp:6] -> [test.cpp:8]: (error) Returning object that points to local variable 'i' that will be invalid when returning.\n",
errout.str());
check("struct A {\n"

12
test/testcondition.cpp
View File

@ -3441,6 +3441,18 @@ private:
"}\n");
ASSERT_EQUALS("", errout.str());
// #9711
check("int main(int argc, char* argv[]) {\n"
" int foo = 0;\n"
" struct option options[] = {\n"
" {\"foo\", no_argument, &foo, \'f\'},\n"
" {NULL, 0, NULL, 0},\n"
" };\n"
" getopt_long(argc, argv, \"f\", options, NULL);\n"
" if (foo) {}\n"
"}\n");
ASSERT_EQUALS("", errout.str());
}
void alwaysTrueInfer() {

40
test/testvalueflow.cpp
View File

@ -182,6 +182,24 @@ private:
return false;
}
bool testValueOfXInconclusive(const char code[], unsigned int linenr, int value) {
// Tokenize..
Tokenizer tokenizer(&settings, this);
std::istringstream istr(code);
tokenizer.tokenize(istr, "test.cpp");
for (const Token *tok = tokenizer.tokens(); tok; tok = tok->next()) {
if (tok->str() == "x" && tok->linenr() == linenr) {
for (const ValueFlow::Value& val:tok->values()) {
if (val.isInconclusive() && val.intvalue == value)
return true;
}
}
}
return false;
}
bool testValueOfX(const char code[], unsigned int linenr, int value) {
// Tokenize..
Tokenizer tokenizer(&settings, this);
@ -2106,6 +2124,28 @@ private:
"Fred::Fred(std::unique_ptr<Wilma> wilma)\n"
" : mWilma(std::move(wilma)) {}\n";
ASSERT_EQUALS(0, tokenValues(code, "mWilma (").size());
code = "void g(unknown*);\n"
"int f() {\n"
" int a = 1;\n"
" unknown c[] = {{&a}};\n"
" g(c);\n"
" int x = a;\n"
" return x;\n"
"}\n";
ASSERT_EQUALS(false, testValueOfXKnown(code, 7U, 1));
ASSERT_EQUALS(true, testValueOfXInconclusive(code, 7U, 1));
code = "void g(unknown&);\n"
"int f() {\n"
" int a = 1;\n"
" unknown c{&a};\n"
" g(c);\n"
" int x = a;\n"
" return x;\n"
"}\n";
ASSERT_EQUALS(false, testValueOfXKnown(code, 7U, 1));
ASSERT_EQUALS(true, testValueOfXInconclusive(code, 7U, 1));
}
void valueFlowAfterCondition() {