Fixed #8250 (New check: Pointer calculation result cant be NULL unless there is overflow)

This commit is contained in:
Daniel Marjamäki 2017-10-22 14:32:54 +02:00
parent 002f6679d5
commit 28cfee2d4f
3 changed files with 65 additions and 7 deletions

View File

@ -1338,6 +1338,50 @@ void CheckCondition::invalidTestForOverflow(const Token* tok, bool result)
(tok ? tok->expressionString() : std::string("x + u < x")) + (tok ? tok->expressionString() : std::string("x + u < x")) +
"'. Condition is always " + "'. Condition is always " +
std::string(result ? "true" : "false") + std::string(result ? "true" : "false") +
" unless there is overflow, and overflow is UB."; " unless there is overflow, and overflow is undefined behaviour.";
reportError(tok, Severity::warning, "invalidTestForOverflow", errmsg, (result ? CWE571 : CWE570), false); reportError(tok, Severity::warning, "invalidTestForOverflow", errmsg, (result ? CWE571 : CWE570), false);
} }
void CheckCondition::checkPointerAdditionResultNotNull()
{
if (!_settings->isEnabled(Settings::WARNING))
return;
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
const std::size_t functions = symbolDatabase->functionScopes.size();
for (std::size_t i = 0; i < functions; ++i) {
const Scope * scope = symbolDatabase->functionScopes[i];
for (const Token* tok = scope->classStart; tok != scope->classEnd; tok = tok->next()) {
if (!tok->isComparisonOp() || !tok->astOperand1() || !tok->astOperand2())
continue;
const Token *calcToken, *exprToken;
if (tok->astOperand1()->str() == "+") {
calcToken = tok->astOperand1();
exprToken = tok->astOperand2();
} else if (tok->astOperand2()->str() == "+") {
calcToken = tok->astOperand2();
exprToken = tok->astOperand1();
} else
continue;
// pointer comparison against NULL (ptr+12==0)
if (calcToken->hasKnownIntValue())
continue;
if (!calcToken->valueType() || calcToken->valueType()->pointer==0)
continue;
if (!exprToken->hasKnownIntValue() || !exprToken->getValue(0))
continue;
pointerAdditionResultNotNullError(tok, calcToken);
}
}
}
void CheckCondition::pointerAdditionResultNotNullError(const Token *tok, const Token *calc)
{
const std::string s = calc ? calc->expressionString() : "ptr+1";
reportError(tok, Severity::warning, "pointerAdditionResultNotNull", "Comparison is wrong. Result of '" + s + "' can't be 0 unless there is pointer overflow, and pointer overflow is undefined behaviour.");
}

View File

@ -59,6 +59,7 @@ public:
checkCondition.checkIncorrectLogicOperator(); checkCondition.checkIncorrectLogicOperator();
checkCondition.checkInvalidTestForOverflow(); checkCondition.checkInvalidTestForOverflow();
checkCondition.alwaysTrueFalse(); checkCondition.alwaysTrueFalse();
checkCondition.checkPointerAdditionResultNotNull();
} }
/** @brief Run checks against the simplified token list */ /** @brief Run checks against the simplified token list */
@ -113,6 +114,9 @@ public:
/** @brief %Check for invalid test for overflow 'x+100 < x' */ /** @brief %Check for invalid test for overflow 'x+100 < x' */
void checkInvalidTestForOverflow(); void checkInvalidTestForOverflow();
/** @brief Check if pointer addition result is NULL '(ptr + 1) == NULL' */
void checkPointerAdditionResultNotNull();
private: private:
bool isAliased(const std::set<unsigned int> &vars) const; bool isAliased(const std::set<unsigned int> &vars) const;
bool isOverlappingCond(const Token * const cond1, const Token * const cond2, bool pure) const; bool isOverlappingCond(const Token * const cond1, const Token * const cond2, bool pure) const;
@ -141,6 +145,7 @@ private:
void alwaysTrueFalseError(const Token *tok, const ValueFlow::Value *value); void alwaysTrueFalseError(const Token *tok, const ValueFlow::Value *value);
void invalidTestForOverflow(const Token* tok, bool result); void invalidTestForOverflow(const Token* tok, bool result);
void pointerAdditionResultNotNullError(const Token *tok, const Token *calc);
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const { void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
CheckCondition c(nullptr, settings, errorLogger); CheckCondition c(nullptr, settings, errorLogger);
@ -158,6 +163,7 @@ private:
c.clarifyConditionError(nullptr, true, false); c.clarifyConditionError(nullptr, true, false);
c.alwaysTrueFalseError(nullptr, nullptr); c.alwaysTrueFalseError(nullptr, nullptr);
c.invalidTestForOverflow(nullptr, false); c.invalidTestForOverflow(nullptr, false);
c.pointerAdditionResultNotNullError(nullptr, nullptr);
} }
static std::string myName() { static std::string myName() {
@ -177,7 +183,7 @@ private:
"- Mutual exclusion over || always evaluating to true\n" "- Mutual exclusion over || always evaluating to true\n"
"- Comparisons of modulo results that are always true/false.\n" "- Comparisons of modulo results that are always true/false.\n"
"- Known variable values => condition is always true/false\n" "- Known variable values => condition is always true/false\n"
"- Invalid test for overflow (for example 'ptr+u < ptr'). Condition is always false unless there is overflow, and overflow is UB.\n"; "- Invalid test for overflow (for example 'ptr+u < ptr'). Condition is always false unless there is overflow, and overflow is undefined behaviour.\n";
} }
}; };
/// @} /// @}

View File

@ -98,6 +98,7 @@ private:
TEST_CASE(checkInvalidTestForOverflow); TEST_CASE(checkInvalidTestForOverflow);
TEST_CASE(checkConditionIsAlwaysTrueOrFalseInsideIfWhile); TEST_CASE(checkConditionIsAlwaysTrueOrFalseInsideIfWhile);
TEST_CASE(pointerAdditionResultNotNull);
} }
void check(const char code[], const char* filename = "test.cpp", bool inconclusive = false) { void check(const char code[], const char* filename = "test.cpp", bool inconclusive = false) {
@ -2212,27 +2213,27 @@ private:
check("void f(char *p, unsigned int x) {\n" check("void f(char *p, unsigned int x) {\n"
" assert((p + x) < p);\n" " assert((p + x) < p);\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Invalid test for overflow '(p+x)<p'. Condition is always false unless there is overflow, and overflow is UB.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (warning) Invalid test for overflow '(p+x)<p'. Condition is always false unless there is overflow, and overflow is undefined behaviour.\n", errout.str());
check("void f(char *p, unsigned int x) {\n" check("void f(char *p, unsigned int x) {\n"
" assert((p + x) >= p);\n" " assert((p + x) >= p);\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Invalid test for overflow '(p+x)>=p'. Condition is always true unless there is overflow, and overflow is UB.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (warning) Invalid test for overflow '(p+x)>=p'. Condition is always true unless there is overflow, and overflow is undefined behaviour.\n", errout.str());
check("void f(char *p, unsigned int x) {\n" check("void f(char *p, unsigned int x) {\n"
" assert(p > (p + x));\n" " assert(p > (p + x));\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Invalid test for overflow 'p>(p+x)'. Condition is always false unless there is overflow, and overflow is UB.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (warning) Invalid test for overflow 'p>(p+x)'. Condition is always false unless there is overflow, and overflow is undefined behaviour.\n", errout.str());
check("void f(char *p, unsigned int x) {\n" check("void f(char *p, unsigned int x) {\n"
" assert(p <= (p + x));\n" " assert(p <= (p + x));\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Invalid test for overflow 'p<=(p+x)'. Condition is always true unless there is overflow, and overflow is UB.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (warning) Invalid test for overflow 'p<=(p+x)'. Condition is always true unless there is overflow, and overflow is undefined behaviour.\n", errout.str());
check("void f(signed int x) {\n" check("void f(signed int x) {\n"
" assert(x + 100 < x);\n" " assert(x + 100 < x);\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Invalid test for overflow 'x+100<x'. Condition is always false unless there is overflow, and overflow is UB.\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (warning) Invalid test for overflow 'x+100<x'. Condition is always false unless there is overflow, and overflow is undefined behaviour.\n", errout.str());
check("void f(signed int x) {\n" // unsigned overflow => don't warn check("void f(signed int x) {\n" // unsigned overflow => don't warn
" assert(x + 100U < x);\n" " assert(x + 100U < x);\n"
@ -2266,6 +2267,13 @@ private:
"}"); "}");
ASSERT_EQUALS("[test.cpp:2]: (style) Condition 'a+1' is always true\n", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (style) Condition 'a+1' is always true\n", errout.str());
} }
void pointerAdditionResultNotNull() {
check("void f(char *ptr) {\n"
" if (ptr + 1 != 0);\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison is wrong. Result of 'ptr+1' can't be 0 unless there is pointer overflow, and pointer overflow is undefined behaviour.\n", errout.str());
}
}; };
REGISTER_TEST(TestCondition) REGISTER_TEST(TestCondition)