Fix 6577: Detect pointer to uninitialised memory with clock_settime() (#3993)

* Fix 6577: Detect pointer to uninitialised memory with clock_settime()

* Format
This commit is contained in:
Paul Fultz II 2022-04-11 00:23:44 -05:00 committed by GitHub
parent 45b4580554
commit d97942d3c6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 37 additions and 16 deletions

View File

@ -2129,7 +2129,8 @@ bool isVariableChangedByFunctionCall(const Token *tok, int indirect, const Setti
const Token * const tok1 = tok; const Token * const tok1 = tok;
// address of variable // address of variable
const bool addressOf = tok->astParent() && tok->astParent()->isUnaryOp("&"); if (tok->astParent() && tok->astParent()->isUnaryOp("&"))
indirect++;
int argnr; int argnr;
tok = getTokenArgumentFunction(tok, argnr); tok = getTokenArgumentFunction(tok, argnr);
@ -2148,25 +2149,27 @@ bool isVariableChangedByFunctionCall(const Token *tok, int indirect, const Setti
const bool possiblyPassedByReference = (parenTok->next() == tok1 || Token::Match(tok1->previous(), ", %name% [,)}]")); const bool possiblyPassedByReference = (parenTok->next() == tok1 || Token::Match(tok1->previous(), ", %name% [,)}]"));
if (!tok->function() && !tok->variable() && Token::Match(tok, "%name%")) { if (!tok->function() && !tok->variable() && Token::Match(tok, "%name%")) {
if (settings) {
const bool requireInit = settings->library.isuninitargbad(tok, 1 + argnr);
const bool requireNonNull = settings->library.isnullargbad(tok, 1 + argnr);
// Check if direction (in, out, inout) is specified in the library configuration and use that // Check if direction (in, out, inout) is specified in the library configuration and use that
if (!addressOf && settings) {
const Library::ArgumentChecks::Direction argDirection = settings->library.getArgDirection(tok, 1 + argnr); const Library::ArgumentChecks::Direction argDirection = settings->library.getArgDirection(tok, 1 + argnr);
if (argDirection == Library::ArgumentChecks::Direction::DIR_IN) if (argDirection == Library::ArgumentChecks::Direction::DIR_IN)
return false; return false;
else if (argDirection == Library::ArgumentChecks::Direction::DIR_OUT || else if (argDirection == Library::ArgumentChecks::Direction::DIR_OUT ||
argDirection == Library::ArgumentChecks::Direction::DIR_INOUT) { argDirection == Library::ArgumentChecks::Direction::DIR_INOUT) {
// With out or inout the direction of the content is specified, not a pointer itself, so ignore pointers for now if (indirect == 0 && isArray(tok1))
const ValueType * const valueType = tok1->valueType(); return true;
if ((valueType && valueType->pointer == indirect) || (indirect == 0 && isArray(tok1))) { // Assume that if the variable must be initialized then the indirection is 1
if (indirect > 0 && requireInit && requireNonNull)
return true; return true;
}
}
} }
// if the library says 0 is invalid // if the library says 0 is invalid
// => it is assumed that parameter is an in parameter (TODO: this is a bad heuristic) // => it is assumed that parameter is an in parameter (TODO: this is a bad heuristic)
if (!addressOf && settings && settings->library.isnullargbad(tok, 1+argnr)) if (indirect == 0 && requireNonNull)
return false; return false;
}
// possible pass-by-reference => inconclusive // possible pass-by-reference => inconclusive
if (possiblyPassedByReference) { if (possiblyPassedByReference) {
if (inconclusive != nullptr) if (inconclusive != nullptr)
@ -2183,7 +2186,7 @@ bool isVariableChangedByFunctionCall(const Token *tok, int indirect, const Setti
if (!arg) if (!arg)
continue; continue;
conclusive = true; conclusive = true;
if (addressOf || indirect > 0) { if (indirect > 0) {
if (!arg->isConst() && arg->isPointer()) if (!arg->isConst() && arg->isPointer())
return true; return true;
// If const is applied to the pointer, then the value can still be modified // If const is applied to the pointer, then the value can still be modified

View File

@ -1539,7 +1539,7 @@ void CheckUninitVar::uninitStructMemberError(const Token *tok, const std::string
"$symbol:" + membername + "\nUninitialized struct member: $symbol", CWE_USE_OF_UNINITIALIZED_VARIABLE, Certainty::normal); "$symbol:" + membername + "\nUninitialized struct member: $symbol", CWE_USE_OF_UNINITIALIZED_VARIABLE, Certainty::normal);
} }
enum class ExprUsage { None, PassedByReference, Used }; enum class ExprUsage { None, NotUsed, PassedByReference, Used };
static ExprUsage getFunctionUsage(const Token* tok, int indirect, const Settings* settings) static ExprUsage getFunctionUsage(const Token* tok, int indirect, const Settings* settings)
{ {
@ -1571,6 +1571,12 @@ static ExprUsage getFunctionUsage(const Token* tok, int indirect, const Settings
static ExprUsage getExprUsage(const Token* tok, int indirect, const Settings* settings) static ExprUsage getExprUsage(const Token* tok, int indirect, const Settings* settings)
{ {
if (indirect > 0 && tok->astParent()) {
if (Token::Match(tok->astParent(), "%assign%") && astIsRhs(tok))
return ExprUsage::NotUsed;
if (tok->astParent()->isCast())
return ExprUsage::NotUsed;
}
if (indirect == 0 && Token::Match(tok->astParent(), "%cop%|%assign%") && tok->astParent()->str() != "=") if (indirect == 0 && Token::Match(tok->astParent(), "%cop%|%assign%") && tok->astParent()->str() != "=")
return ExprUsage::Used; return ExprUsage::Used;
return getFunctionUsage(tok, indirect, settings); return getFunctionUsage(tok, indirect, settings);
@ -1603,7 +1609,7 @@ void CheckUninitVar::valueFlowUninit()
} }
if (ids.count(tok->exprId()) > 0) if (ids.count(tok->exprId()) > 0)
continue; continue;
if (!tok->variable() && !tok->isUnaryOp("*")) if (!tok->variable() && !tok->isUnaryOp("*") && !tok->isUnaryOp("&"))
continue; continue;
if (Token::Match(tok, "%name% (")) if (Token::Match(tok, "%name% ("))
continue; continue;
@ -1642,6 +1648,8 @@ void CheckUninitVar::valueFlowUninit()
continue; continue;
} }
ExprUsage usage = getExprUsage(tok, v->indirect, mSettings); ExprUsage usage = getExprUsage(tok, v->indirect, mSettings);
if (usage == ExprUsage::NotUsed)
continue;
if (!v->subexpressions.empty() && usage == ExprUsage::PassedByReference) if (!v->subexpressions.empty() && usage == ExprUsage::PassedByReference)
continue; continue;
if (usage != ExprUsage::Used) { if (usage != ExprUsage::Used) {

View File

@ -576,6 +576,10 @@ static void setTokenValue(Token* tok, ValueFlow::Value value, const Settings* se
if (value.isUninitValue()) { if (value.isUninitValue()) {
if (Token::Match(tok, ". %var%")) if (Token::Match(tok, ". %var%"))
setTokenValue(tok->next(), value, settings); setTokenValue(tok->next(), value, settings);
if (parent->isCast()) {
setTokenValue(parent, value, settings);
return;
}
ValueFlow::Value pvalue = value; ValueFlow::Value pvalue = value;
if (!value.subexpressions.empty() && Token::Match(parent, ". %var%")) { if (!value.subexpressions.empty() && Token::Match(parent, ". %var%")) {
if (contains(value.subexpressions, parent->next()->str())) if (contains(value.subexpressions, parent->next()->str()))

View File

@ -593,7 +593,7 @@ void timet_h(struct timespec* ptp1)
clock_settime(clk_id2, ptp1); clock_settime(clk_id2, ptp1);
struct timespec tp; struct timespec tp;
// TODO cppcheck-suppress uninitvar // cppcheck-suppress uninitvar
clock_settime(CLOCK_REALTIME, &tp); // #6577 - false negative clock_settime(CLOCK_REALTIME, &tp); // #6577 - false negative
// cppcheck-suppress uninitvar // cppcheck-suppress uninitvar
clock_settime(clk_id3, &tp); clock_settime(clk_id3, &tp);

View File

@ -5241,6 +5241,12 @@ private:
" return u.u16;\n" " return u.u16;\n"
"}\n"); "}\n");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
valueFlowUninit("void f() {\n"
" char src, dest;\n"
" std::memcpy(&dest, &src, 1);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3]: (error) Uninitialized variable: &src\n", errout.str());
} }
void valueFlowUninitBreak() { // Do not show duplicate warnings about the same uninitialized value void valueFlowUninitBreak() { // Do not show duplicate warnings about the same uninitialized value