Fix issue 8313 and 7326: Track values of pointer aliases in valueflow

This commit is contained in:
Paul Fultz II 2019-08-12 12:58:53 +02:00 committed by Daniel Marjamäki
parent aec217fede
commit 68e8253920
4 changed files with 92 additions and 20 deletions

View File

@ -51,17 +51,32 @@ static const struct CWE CWE788(788U); // Access of Memory Location After End o
static const struct CWE CWE825(825U); // Expired Pointer Dereference static const struct CWE CWE825(825U); // Expired Pointer Dereference
static const struct CWE CWE834(834U); // Excessive Iteration static const struct CWE CWE834(834U); // Excessive Iteration
static const Library::Container * getLibraryContainer(const Token * tok)
{
if (!tok)
return nullptr;
if (tok->isUnaryOp("*") && astIsPointer(tok->astOperand1())) {
for (const ValueFlow::Value& v:tok->astOperand1()->values()) {
if (!v.isLocalLifetimeValue())
continue;
return getLibraryContainer(v.tokvalue);
}
}
if (!tok->valueType())
return nullptr;
return tok->valueType()->container;
}
void CheckStl::outOfBounds() void CheckStl::outOfBounds()
{ {
for (const Scope *function : mTokenizer->getSymbolDatabase()->functionScopes) { for (const Scope *function : mTokenizer->getSymbolDatabase()->functionScopes) {
for (const Token *tok = function->bodyStart; tok != function->bodyEnd; tok = tok->next()) { for (const Token *tok = function->bodyStart; tok != function->bodyEnd; tok = tok->next()) {
if (!tok->isName() || !tok->valueType()) const Library::Container *container = getLibraryContainer(tok);
continue;
const Library::Container *container = tok->valueType()->container;
if (!container) if (!container)
continue; continue;
const Token * parent = tok->astParent();
while (Token::simpleMatch(parent, "(") && !Token::Match(parent->previous(), "%name%"))
parent = parent->astParent();
for (const ValueFlow::Value &value : tok->values()) { for (const ValueFlow::Value &value : tok->values()) {
if (!value.isContainerSizeValue()) if (!value.isContainerSizeValue())
continue; continue;
@ -69,41 +84,41 @@ void CheckStl::outOfBounds()
continue; continue;
if (!value.errorSeverity() && !mSettings->isEnabled(Settings::WARNING)) if (!value.errorSeverity() && !mSettings->isEnabled(Settings::WARNING))
continue; continue;
if (value.intvalue == 0 && Token::Match(tok, "%name% . %name% (") && container->getYield(tok->strAt(2)) == Library::Container::Yield::ITEM) { if (value.intvalue == 0 && Token::Match(parent, ". %name% (") && container->getYield(parent->strAt(1)) == Library::Container::Yield::ITEM) {
outOfBoundsError(tok->tokAt(3), tok->str(), &value, tok->strAt(2), nullptr); outOfBoundsError(parent->tokAt(2), tok->expressionString(), &value, parent->strAt(1), nullptr);
continue; continue;
} }
if (Token::Match(tok, "%name% . %name% (") && container->getYield(tok->strAt(2)) == Library::Container::Yield::START_ITERATOR) { if (Token::Match(tok, "%name% . %name% (") && container->getYield(tok->strAt(2)) == Library::Container::Yield::START_ITERATOR) {
const Token *parent = tok->tokAt(3)->astParent(); const Token *fparent = tok->tokAt(3)->astParent();
const Token *other = nullptr; const Token *other = nullptr;
if (Token::simpleMatch(parent, "+") && parent->astOperand1() == tok->tokAt(3)) if (Token::simpleMatch(fparent, "+") && fparent->astOperand1() == tok->tokAt(3))
other = parent->astOperand2(); other = fparent->astOperand2();
else if (Token::simpleMatch(parent, "+") && parent->astOperand2() == tok->tokAt(3)) else if (Token::simpleMatch(fparent, "+") && fparent->astOperand2() == tok->tokAt(3))
other = parent->astOperand1(); other = fparent->astOperand1();
if (other && other->hasKnownIntValue() && other->getKnownIntValue() > value.intvalue) { if (other && other->hasKnownIntValue() && other->getKnownIntValue() > value.intvalue) {
outOfBoundsError(parent, tok->str(), &value, other->expressionString(), &other->values().back()); outOfBoundsError(fparent, tok->expressionString(), &value, other->expressionString(), &other->values().back());
continue; continue;
} else if (other && !other->hasKnownIntValue() && value.isKnown() && value.intvalue==0) { } else if (other && !other->hasKnownIntValue() && value.isKnown() && value.intvalue==0) {
outOfBoundsError(parent, tok->str(), &value, other->expressionString(), nullptr); outOfBoundsError(fparent, tok->expressionString(), &value, other->expressionString(), nullptr);
continue; continue;
} }
} }
if (!container->arrayLike_indexOp && !container->stdStringLike) if (!container->arrayLike_indexOp && !container->stdStringLike)
continue; continue;
if (value.intvalue == 0 && Token::Match(tok, "%name% [")) { if (value.intvalue == 0 && Token::Match(parent, "[")) {
outOfBoundsError(tok->next(), tok->str(), &value, "", nullptr); outOfBoundsError(parent, tok->expressionString(), &value, "", nullptr);
continue; continue;
} }
if (container->arrayLike_indexOp && Token::Match(tok, "%name% [")) { if (container->arrayLike_indexOp && Token::Match(parent, "[")) {
const ValueFlow::Value *indexValue = tok->next()->astOperand2() ? tok->next()->astOperand2()->getMaxValue(false) : nullptr; const ValueFlow::Value *indexValue = parent->astOperand2() ? parent->astOperand2()->getMaxValue(false) : nullptr;
if (indexValue && indexValue->intvalue >= value.intvalue) { if (indexValue && indexValue->intvalue >= value.intvalue) {
outOfBoundsError(tok->next(), tok->str(), &value, tok->next()->astOperand2()->expressionString(), indexValue); outOfBoundsError(parent, tok->expressionString(), &value, parent->astOperand2()->expressionString(), indexValue);
continue; continue;
} }
if (mSettings->isEnabled(Settings::WARNING)) { if (mSettings->isEnabled(Settings::WARNING)) {
indexValue = tok->next()->astOperand2() ? tok->next()->astOperand2()->getMaxValue(true) : nullptr; indexValue = parent->astOperand2() ? parent->astOperand2()->getMaxValue(true) : nullptr;
if (indexValue && indexValue->intvalue >= value.intvalue) { if (indexValue && indexValue->intvalue >= value.intvalue) {
outOfBoundsError(tok->next(), tok->str(), &value, tok->next()->astOperand2()->expressionString(), indexValue); outOfBoundsError(parent, tok->expressionString(), &value, parent->astOperand2()->expressionString(), indexValue);
continue; continue;
} }
} }

View File

@ -1155,6 +1155,41 @@ static void valueFlowPointerAlias(TokenList *tokenlist)
} }
} }
static void valueFlowPointerAliasDeref(TokenList *tokenlist)
{
for (Token *tok = tokenlist->front(); tok; tok = tok->next()) {
if (!tok->isUnaryOp("*"))
continue;
if (!astIsPointer(tok->astOperand1()))
continue;
const Token* lifeTok = nullptr;
ErrorPath errorPath;
for (const ValueFlow::Value& v:tok->astOperand1()->values()) {
if (!v.isLocalLifetimeValue())
continue;
lifeTok = v.tokvalue;
errorPath = v.errorPath;
}
if (!lifeTok)
continue;
if (lifeTok->varId() == 0)
continue;
const Variable * var = lifeTok->variable();
if (!var)
continue;
if (!var->isConst() && isVariableChanged(lifeTok->next(), tok, lifeTok->varId(), !var->isLocal(), tokenlist->getSettings(), tokenlist->isCPP()))
continue;
for (const ValueFlow::Value& v:lifeTok->values()) {
if (v.isLifetimeValue())
continue;
ValueFlow::Value value = v;
value.errorPath.insert(value.errorPath.begin(), errorPath.begin(), errorPath.end());
setTokenValue(tok, value, tokenlist->getSettings());
}
}
}
static void valueFlowBitAnd(TokenList *tokenlist) static void valueFlowBitAnd(TokenList *tokenlist)
{ {
for (Token *tok = tokenlist->front(); tok; tok = tok->next()) { for (Token *tok = tokenlist->front(); tok; tok = tok->next()) {
@ -5735,6 +5770,7 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase,
std::size_t values = 0; std::size_t values = 0;
while (std::time(nullptr) < timeout && values < getTotalValues(tokenlist)) { while (std::time(nullptr) < timeout && values < getTotalValues(tokenlist)) {
values = getTotalValues(tokenlist); values = getTotalValues(tokenlist);
valueFlowPointerAliasDeref(tokenlist);
valueFlowArrayBool(tokenlist); valueFlowArrayBool(tokenlist);
valueFlowRightShift(tokenlist, settings); valueFlowRightShift(tokenlist, settings);
valueFlowOppositeCondition(symboldatabase, settings); valueFlowOppositeCondition(symboldatabase, settings);

View File

@ -289,6 +289,13 @@ private:
" x = s.begin() + x;\n" " x = s.begin() + x;\n"
"}\n"); "}\n");
ASSERT_EQUALS("test.cpp:3:error:Out of bounds access in expression 's.begin()+x' because 's' is empty and 'x' may be non-zero.\n", errout.str()); ASSERT_EQUALS("test.cpp:3:error:Out of bounds access in expression 's.begin()+x' because 's' is empty and 'x' may be non-zero.\n", errout.str());
checkNormal("int f() {\n"
" std::vector<int> v;\n"
" std::vector<int> * pv = &v;\n"
" return (*pv)[42];\n"
"}\n");
ASSERT_EQUALS("test.cpp:4:error:Out of bounds access in expression '(*pv)[42]' because '*pv' is empty.\n", errout.str());
} }
void outOfBoundsIndexExpression() { void outOfBoundsIndexExpression() {

View File

@ -123,6 +123,8 @@ private:
TEST_CASE(valueFlowSafeFunctionParameterValues); TEST_CASE(valueFlowSafeFunctionParameterValues);
TEST_CASE(valueFlowUnknownFunctionReturn); TEST_CASE(valueFlowUnknownFunctionReturn);
TEST_CASE(valueFlowPointerAliasDeref);
} }
static bool isNotTokValue(const ValueFlow::Value &val) { static bool isNotTokValue(const ValueFlow::Value &val) {
@ -4065,6 +4067,18 @@ private:
ASSERT_EQUALS(INT_MIN, values.front().intvalue); ASSERT_EQUALS(INT_MIN, values.front().intvalue);
ASSERT_EQUALS(INT_MAX, values.back().intvalue); ASSERT_EQUALS(INT_MAX, values.back().intvalue);
} }
void valueFlowPointerAliasDeref() {
const char* code;
code = "int f() {\n"
" int a = 123;\n"
" int *p = &a;\n"
" int x = *p;\n"
" return x;\n"
"}\n";
ASSERT_EQUALS(true, testValueOfX(code, 5U, 123));
}
}; };
REGISTER_TEST(TestValueFlow) REGISTER_TEST(TestValueFlow)