diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 6c9d43ca3..4a6793da8 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -303,10 +303,12 @@ static bool checkMinSizes(const std::list &min error = true; } break; - case Library::ArgumentChecks::MinSize::STRLEN: - if (argtok->type() == Token::eString && Token::getStrLength(argtok) >= arraySize) + case Library::ArgumentChecks::MinSize::STRLEN: { + const Token *strtoken = argtok->getValueTokenMaxStrLength(); + if (strtoken && Token::getStrLength(strtoken) >= arraySize) error = true; - break; + } + break; case Library::ArgumentChecks::MinSize::SIZEOF: if (argtok->type() == Token::eString && Token::getStrLength(argtok) >= arraySize) error = true; @@ -573,7 +575,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const std::vectorstr() == "&" || - Token::simpleMatch(tok3->previous(), "& ("))); + Token::simpleMatch(tok3->previous(), "& ("))); if (addr) continue; } diff --git a/lib/token.cpp b/lib/token.cpp index 18a39fbd9..3db800b5b 100644 --- a/lib/token.cpp +++ b/lib/token.cpp @@ -1213,15 +1213,23 @@ void Token::printValueFlow(bool xml, std::ostream &out) const out << " " << tok->str() << ":{"; for (std::list::const_iterator it=tok->values.begin(); it!=tok->values.end(); ++it) { if (xml) { - out << " intvalue << "\""; - if (it->condition) { + out << " tokvalue) + out << "tokvalue=\"" << it->tokvalue << '\"'; + else + out << "intvalue=\"" << it->intvalue << '\"'; + if (it->condition) out << " condition-line=\"" << it->condition->linenr() << '\"'; - } out << "/>" << std::endl; } else { - out << (it == tok->values.begin() ? "" : ",") << it->intvalue; + if (it != tok->values.begin()) + out << ","; + if (it->tokvalue) + out << it->tokvalue->str(); + else + out << it->intvalue; } } if (xml) @@ -1238,7 +1246,7 @@ const ValueFlow::Value * Token::getValueLE(const MathLib::bigint val, const Sett const ValueFlow::Value *ret = nullptr; std::list::const_iterator it; for (it = values.begin(); it != values.end(); ++it) { - if (it->intvalue <= val) { + if (it->intvalue <= val && !it->tokvalue) { if (!ret || ret->inconclusive || (ret->condition && !it->inconclusive)) ret = &(*it); if (!ret->inconclusive && !ret->condition) @@ -1259,7 +1267,7 @@ const ValueFlow::Value * Token::getValueGE(const MathLib::bigint val, const Sett const ValueFlow::Value *ret = nullptr; std::list::const_iterator it; for (it = values.begin(); it != values.end(); ++it) { - if (it->intvalue >= val) { + if (it->intvalue >= val && !it->tokvalue) { if (!ret || ret->inconclusive || (ret->condition && !it->inconclusive)) ret = &(*it); if (!ret->inconclusive && !ret->condition) diff --git a/lib/token.h b/lib/token.h index ed35d586a..93cdb21a5 100644 --- a/lib/token.h +++ b/lib/token.h @@ -648,7 +648,7 @@ public: const ValueFlow::Value * getValue(const MathLib::bigint val) const { std::list::const_iterator it; for (it = values.begin(); it != values.end(); ++it) { - if (it->intvalue == val) + if (it->intvalue == val && !it->tokvalue) return &(*it); } return NULL; @@ -658,6 +658,8 @@ public: const ValueFlow::Value *ret = nullptr; std::list::const_iterator it; for (it = values.begin(); it != values.end(); ++it) { + if (it->tokvalue) + continue; if ((!ret || it->intvalue > ret->intvalue) && ((it->condition != NULL) == condition)) ret = &(*it); @@ -668,6 +670,22 @@ public: const ValueFlow::Value * getValueLE(const MathLib::bigint val, const Settings *settings) const; const ValueFlow::Value * getValueGE(const MathLib::bigint val, const Settings *settings) const; + const Token *getValueTokenMaxStrLength() const { + const Token *ret = nullptr; + std::size_t maxlength = 0U; + std::list::const_iterator it; + for (it = values.begin(); it != values.end(); ++it) { + if (it->tokvalue && it->tokvalue->type() == Token::eString) { + std::size_t length = Token::getStrLength(it->tokvalue); + if (!ret || length > maxlength) { + maxlength = length; + ret = it->tokvalue; + } + } + } + return ret; + } + private: void next(Token *nextToken) { diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 817104488..d6340fa14 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -254,13 +254,24 @@ static void setTokenValue(Token* tok, const ValueFlow::Value &value) // if value already exists, don't add it again std::list::iterator it; for (it = tok->values.begin(); it != tok->values.end(); ++it) { - if (it->intvalue == value.intvalue) { - if (it->inconclusive && !value.inconclusive) { - *it = value; - break; - } - return; + // different intvalue => continue + if (it->intvalue != value.intvalue) + continue; + + // different tokvalue => continue + if ((it->tokvalue == nullptr) != (value.tokvalue == nullptr)) + continue; + if ((value.tokvalue != nullptr) && (it->tokvalue != value.tokvalue) && (it->tokvalue->str() != value.tokvalue->str())) + continue; + + // same value, but old value is inconclusive so replace it + if (it->inconclusive && !value.inconclusive) { + *it = value; + break; } + + // Same value already exists, don't add new value + return; } if (it == tok->values.end()) { @@ -330,6 +341,38 @@ static void valueFlowNumber(TokenList *tokenlist) } } +static void valueFlowString(TokenList *tokenlist) +{ + for (Token *tok = tokenlist->front(); tok; tok = tok->next()) { + if (tok->type() == Token::eString) { + ValueFlow::Value strvalue; + strvalue.tokvalue = tok; + setTokenValue(tok, strvalue); + } + } +} + +static void valueFlowPointerAlias(TokenList *tokenlist) +{ + for (Token *tok = tokenlist->front(); tok; tok = tok->next()) { + // not address of + if (tok->str() != "&" || tok->astOperand2()) + continue; + + // parent should be a '=' + if (!Token::simpleMatch(tok->astParent(), "=")) + continue; + + // child should be some buffer or variable + if (!Token::Match(tok->astOperand1(), "%var%|.|[")) + continue; + + ValueFlow::Value value; + value.tokvalue = tok; + setTokenValue(tok, value); + } +} + static void valueFlowBitAnd(TokenList *tokenlist) { for (Token *tok = tokenlist->front(); tok; tok = tok->next()) { @@ -1414,6 +1457,8 @@ void ValueFlow::setValues(TokenList *tokenlist, ErrorLogger *errorLogger, const tok->values.clear(); valueFlowNumber(tokenlist); + valueFlowString(tokenlist); + valueFlowPointerAlias(tokenlist); valueFlowFunctionReturn(tokenlist, errorLogger, settings); valueFlowBitAnd(tokenlist); valueFlowForLoop(tokenlist, errorLogger, settings); diff --git a/lib/valueflow.h b/lib/valueflow.h index b3ddbe793..b17ebd44d 100644 --- a/lib/valueflow.h +++ b/lib/valueflow.h @@ -29,12 +29,15 @@ class Settings; namespace ValueFlow { class Value { public: - Value(long long val = 0) : intvalue(val), varvalue(val), condition(0), varId(0U), conditional(false), inconclusive(false) {} - Value(const Token *c, long long val) : intvalue(val), varvalue(val), condition(c), varId(0U), conditional(false), inconclusive(false) {} + Value(long long val = 0) : intvalue(val), tokvalue(nullptr), varvalue(val), condition(0), varId(0U), conditional(false), inconclusive(false) {} + Value(const Token *c, long long val) : intvalue(val), tokvalue(nullptr), varvalue(val), condition(c), varId(0U), conditional(false), inconclusive(false) {} /** int value */ long long intvalue; + /** token value - the token that has the value. this is used for pointer aliases, strings, etc. */ + const Token *tokvalue; + /** For calculated values - variable value that calculated value depends on */ long long varvalue; diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index bfe3a6145..625689728 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -35,6 +35,8 @@ private: void run() { TEST_CASE(valueFlowNumber); + TEST_CASE(valueFlowString); + TEST_CASE(valueFlowPointerAlias); TEST_CASE(valueFlowBitAnd); @@ -81,7 +83,7 @@ private: if (tok->str() == "x" && tok->linenr() == linenr) { std::list::const_iterator it; for (it = tok->values.begin(); it != tok->values.end(); ++it) { - if (it->intvalue == value) + if (it->intvalue == value && !it->tokvalue) return true; } } @@ -91,6 +93,34 @@ private: } + bool testValueOfX(const char code[], unsigned int linenr, const char value[]) { + Settings settings; + + // strcpy cfg + const char cfg[] = "\n" + "\n" + " \n" + ""; + settings.library.loadxmldata(cfg, sizeof(cfg)); + + // 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) { + std::list::const_iterator it; + for (it = tok->values.begin(); it != tok->values.end(); ++it) { + if (Token::simpleMatch(it->tokvalue, value)) + return true; + } + } + } + + return false; + } + void bailout(const char code[]) { Settings settings; settings.debugwarnings = true; @@ -114,7 +144,7 @@ private: ValueFlow::Value valueOfTok(const char code[], const char tokstr[]) { std::list values = tokenValues(code, tokstr); - return values.size() == 1U ? values.front() : ValueFlow::Value(); + return values.size() == 1U && !values.front().tokvalue ? values.front() : ValueFlow::Value(); } void valueFlowNumber() { @@ -126,6 +156,29 @@ private: ASSERT_EQUALS(123, valueOfTok(code, "123").intvalue); } + void valueFlowString() { + const char *code; + + code = "const char * f() {\n" + " static const char *x;\n" + " if (a) x = \"123\";\n" + " return x;\n" + "}"; + ASSERT_EQUALS(true, testValueOfX(code, 4, "\"123\"")); + } + + void valueFlowPointerAlias() { + const char *code; + + code = "const char * f() {\n" + " static const char *x;\n" + " static char ret[10];\n" + " if (a) x = &ret[0];\n" + " return x;\n" + "}"; + ASSERT_EQUALS(true, testValueOfX(code, 5, "& ret [ 0 ]")); + } + void valueFlowCalculations() { const char *code; /* @@ -602,6 +655,7 @@ private: " a = 2 + x;\n" "}"; ASSERT_EQUALS(true, testValueOfX(code, 4U, 1)); + ASSERT_EQUALS(true, testValueOfX(code, 4U, 2)); code = "void f() {\n" " int x = 123;\n"