Skip to content

avoid some unchecked pointer dereferences #6284

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chrchr-github merged 1 commit into from
Apr 13, 2024
Merged

avoid some unchecked pointer dereferences #6284

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/astutils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -771,7 +771,7 @@ std::vector<ValueType> getParentValueTypes(const Token* tok, const Settings* set
if (Token::simpleMatch(tok->astParent(), "(") && ftok && !tok->astParent()->isCast() &&
ftok->tokType() != Token::eType)
return {};
if (Token::Match(tok->astParent(), "return|(|{|%assign%") && parent) {
if (parent && Token::Match(tok->astParent(), "return|(|{|%assign%")) {
*parent = tok->astParent();
}
if (tok->astParent()->valueType())
Expand Down
22 changes: 11 additions & 11 deletions lib/checkclass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1739,7 +1739,7 @@ void CheckClass::operatorEqToSelf()
// find the parameter name
const Token *rhs = func.argumentList.cbegin()->nameToken();
const Token* out_ifStatementScopeStart = nullptr;
if (!hasAssignSelf(&func, rhs, &out_ifStatementScopeStart)) {
if (!hasAssignSelf(&func, rhs, out_ifStatementScopeStart)) {
if (hasAllocation(&func, scope))
operatorEqToSelfError(func.token);
} else if (out_ifStatementScopeStart != nullptr) {
Expand Down Expand Up @@ -1858,7 +1858,7 @@ const Token * CheckClass::getIfStmtBodyStart(const Token *tok, const Token *rhs)
return nullptr;
}

bool CheckClass::hasAssignSelf(const Function *func, const Token *rhs, const Token **out_ifStatementScopeStart)
bool CheckClass::hasAssignSelf(const Function *func, const Token *rhs, const Token *&out_ifStatementScopeStart)
{
if (!rhs)
return false;
Expand All @@ -1881,7 +1881,7 @@ bool CheckClass::hasAssignSelf(const Function *func, const Token *rhs, const Tok
if (tok2 && tok2->isUnaryOp("&") && tok2->astOperand1()->str() == rhs->str())
ret = true;
if (ret) {
*out_ifStatementScopeStart = getIfStmtBodyStart(tok2, rhs);
out_ifStatementScopeStart = getIfStmtBodyStart(tok2, rhs);
}
return ret ? ChildrenToVisit::done : ChildrenToVisit::op1_and_op2;
});
Expand Down Expand Up @@ -3399,13 +3399,13 @@ void CheckClass::checkThisUseAfterFree()

const Token * freeToken = nullptr;
std::set<const Function *> callstack;
checkThisUseAfterFreeRecursive(classScope, &func, &var, std::move(callstack), &freeToken);
checkThisUseAfterFreeRecursive(classScope, &func, &var, std::move(callstack), freeToken);
}
}
}
}

bool CheckClass::checkThisUseAfterFreeRecursive(const Scope *classScope, const Function *func, const Variable *selfPointer, std::set<const Function *> callstack, const Token **freeToken)
bool CheckClass::checkThisUseAfterFreeRecursive(const Scope *classScope, const Function *func, const Variable *selfPointer, std::set<const Function *> callstack, const Token *&freeToken)
{
if (!func || !func->functionScope)
return false;
Expand All @@ -3418,23 +3418,23 @@ bool CheckClass::checkThisUseAfterFreeRecursive(const Scope *classScope, const F
const Token * const bodyStart = func->functionScope->bodyStart;
const Token * const bodyEnd = func->functionScope->bodyEnd;
for (const Token *tok = bodyStart; tok != bodyEnd; tok = tok->next()) {
const bool isDestroyed = *freeToken != nullptr && !func->isStatic();
const bool isDestroyed = freeToken != nullptr && !func->isStatic();
if (Token::Match(tok, "delete %var% ;") && selfPointer == tok->next()->variable()) {
*freeToken = tok;
freeToken = tok;
tok = tok->tokAt(2);
} else if (Token::Match(tok, "%var% . reset ( )") && selfPointer == tok->variable())
*freeToken = tok;
freeToken = tok;
else if (Token::Match(tok->previous(), "!!. %name% (") && tok->function() && tok->function()->nestedIn == classScope) {
if (isDestroyed) {
thisUseAfterFree(selfPointer->nameToken(), *freeToken, tok);
thisUseAfterFree(selfPointer->nameToken(), freeToken, tok);
return true;
}
if (checkThisUseAfterFreeRecursive(classScope, tok->function(), selfPointer, callstack, freeToken))
return true;
} else if (isDestroyed && Token::Match(tok->previous(), "!!. %name%") && tok->variable() && tok->variable()->scope() == classScope && !tok->variable()->isStatic() && !tok->variable()->isArgument()) {
thisUseAfterFree(selfPointer->nameToken(), *freeToken, tok);
thisUseAfterFree(selfPointer->nameToken(), freeToken, tok);
return true;
} else if (*freeToken && Token::Match(tok, "return|throw")) {
} else if (freeToken && Token::Match(tok, "return|throw")) {
// TODO
return tok->str() == "throw";
} else if (tok->str() == "{" && tok->scope()->type == Scope::ScopeType::eLambda) {
Expand Down
4 changes: 2 additions & 2 deletions lib/checkclass.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ class CPPCHECKLIB CheckClass : public Check {
bool hasAllocation(const Function *func, const Scope* scope) const;
bool hasAllocation(const Function *func, const Scope* scope, const Token *start, const Token *end) const;
bool hasAllocationInIfScope(const Function *func, const Scope* scope, const Token *ifStatementScopeStart) const;
static bool hasAssignSelf(const Function *func, const Token *rhs, const Token **out_ifStatementScopeStart);
static bool hasAssignSelf(const Function *func, const Token *rhs, const Token *&out_ifStatementScopeStart);
enum class Bool { TRUE, FALSE, BAILOUT };
static Bool isInverted(const Token *tok, const Token *rhs);
static const Token * getIfStmtBodyStart(const Token *tok, const Token *rhs);
Expand Down Expand Up @@ -411,7 +411,7 @@ class CPPCHECKLIB CheckClass : public Check {
/**
* @brief Helper for checkThisUseAfterFree
*/
bool checkThisUseAfterFreeRecursive(const Scope *classScope, const Function *func, const Variable *selfPointer, std::set<const Function *> callstack, const Token **freeToken);
bool checkThisUseAfterFreeRecursive(const Scope *classScope, const Function *func, const Variable *selfPointer, std::set<const Function *> callstack, const Token *&freeToken);
};
/// @}
//---------------------------------------------------------------------------
Expand Down
4 changes: 2 additions & 2 deletions lib/checkcondition.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1573,8 +1573,8 @@ void CheckCondition::alwaysTrueFalse()
{
const ValueFlow::Value *zeroValue = nullptr;
const Token *nonZeroExpr = nullptr;
if (CheckOther::comparisonNonZeroExpressionLessThanZero(tok, &zeroValue, &nonZeroExpr, /*suppress*/ true) ||
CheckOther::testIfNonZeroExpressionIsPositive(tok, &zeroValue, &nonZeroExpr))
if (CheckOther::comparisonNonZeroExpressionLessThanZero(tok, zeroValue, nonZeroExpr, /*suppress*/ true) ||
CheckOther::testIfNonZeroExpressionIsPositive(tok, zeroValue, nonZeroExpr))
continue;
}

Expand Down
24 changes: 12 additions & 12 deletions lib/checkio.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -487,16 +487,16 @@ void CheckIO::invalidScanfError(const Token *tok)
//---------------------------------------------------------------------------

static bool findFormat(nonneg int arg, const Token *firstArg,
const Token **formatStringTok, const Token **formatArgTok)
const Token *&formatStringTok, const Token *&formatArgTok)
{
const Token* argTok = firstArg;

for (int i = 0; i < arg && argTok; ++i)
argTok = argTok->nextArgument();

if (Token::Match(argTok, "%str% [,)]")) {
*formatArgTok = argTok->nextArgument();
*formatStringTok = argTok;
formatArgTok = argTok->nextArgument();
formatStringTok = argTok;
return true;
}
if (Token::Match(argTok, "%var% [,)]") &&
Expand All @@ -506,13 +506,13 @@ static bool findFormat(nonneg int arg, const Token *firstArg,
(argTok->variable()->dimensions().size() == 1 &&
argTok->variable()->dimensionKnown(0) &&
argTok->variable()->dimension(0) != 0))) {
*formatArgTok = argTok->nextArgument();
formatArgTok = argTok->nextArgument();
if (!argTok->values().empty()) {
const std::list<ValueFlow::Value>::const_iterator value = std::find_if(
argTok->values().cbegin(), argTok->values().cend(), std::mem_fn(&ValueFlow::Value::isTokValue));
if (value != argTok->values().cend() && value->isTokValue() && value->tokvalue &&
value->tokvalue->tokType() == Token::eString) {
*formatStringTok = value->tokvalue;
formatStringTok = value->tokvalue;
}
}
return true;
Expand Down Expand Up @@ -552,37 +552,37 @@ void CheckIO::checkWrongPrintfScanfArguments()

if (formatStringArgNo >= 0) {
// formatstring found in library. Find format string and first argument belonging to format string.
if (!findFormat(formatStringArgNo, tok->tokAt(2), &formatStringTok, &argListTok))
if (!findFormat(formatStringArgNo, tok->tokAt(2), formatStringTok, argListTok))
continue;
} else if (Token::simpleMatch(tok, "swprintf (")) {
if (Token::Match(tok->tokAt(2)->nextArgument(), "%str%")) {
// Find third parameter and format string
if (!findFormat(1, tok->tokAt(2), &formatStringTok, &argListTok))
if (!findFormat(1, tok->tokAt(2), formatStringTok, argListTok))
continue;
} else {
// Find fourth parameter and format string
if (!findFormat(2, tok->tokAt(2), &formatStringTok, &argListTok))
if (!findFormat(2, tok->tokAt(2), formatStringTok, argListTok))
continue;
}
} else if (isWindows && Token::Match(tok, "sprintf_s|swprintf_s (")) {
// template <size_t size> int sprintf_s(char (&buffer)[size], const char *format, ...);
if (findFormat(1, tok->tokAt(2), &formatStringTok, &argListTok)) {
if (findFormat(1, tok->tokAt(2), formatStringTok, argListTok)) {
if (!formatStringTok)
continue;
}
// int sprintf_s(char *buffer, size_t sizeOfBuffer, const char *format, ...);
else if (findFormat(2, tok->tokAt(2), &formatStringTok, &argListTok)) {
else if (findFormat(2, tok->tokAt(2), formatStringTok, argListTok)) {
if (!formatStringTok)
continue;
}
} else if (isWindows && Token::Match(tok, "_snprintf_s|_snwprintf_s (")) {
// template <size_t size> int _snprintf_s(char (&buffer)[size], size_t count, const char *format, ...);
if (findFormat(2, tok->tokAt(2), &formatStringTok, &argListTok)) {
if (findFormat(2, tok->tokAt(2), formatStringTok, argListTok)) {
if (!formatStringTok)
continue;
}
// int _snprintf_s(char *buffer, size_t sizeOfBuffer, size_t count, const char *format, ...);
else if (findFormat(3, tok->tokAt(2), &formatStringTok, &argListTok)) {
else if (findFormat(3, tok->tokAt(2), formatStringTok, argListTok)) {
if (!formatStringTok)
continue;
}
Expand Down
40 changes: 20 additions & 20 deletions lib/checkother.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2714,13 +2714,13 @@ void CheckOther::checkSignOfUnsignedVariable()
for (const Token *tok = scope->bodyStart->next(); tok != scope->bodyEnd; tok = tok->next()) {
const ValueFlow::Value *zeroValue = nullptr;
const Token *nonZeroExpr = nullptr;
if (comparisonNonZeroExpressionLessThanZero(tok, &zeroValue, &nonZeroExpr)) {
if (comparisonNonZeroExpressionLessThanZero(tok, zeroValue, nonZeroExpr)) {
const ValueType* vt = nonZeroExpr->valueType();
if (vt->pointer)
pointerLessThanZeroError(tok, zeroValue);
else
unsignedLessThanZeroError(tok, zeroValue, nonZeroExpr->expressionString());
} else if (testIfNonZeroExpressionIsPositive(tok, &zeroValue, &nonZeroExpr)) {
} else if (testIfNonZeroExpressionIsPositive(tok, zeroValue, nonZeroExpr)) {
const ValueType* vt = nonZeroExpr->valueType();
if (vt->pointer)
pointerPositiveError(tok, zeroValue);
Expand All @@ -2731,7 +2731,7 @@ void CheckOther::checkSignOfUnsignedVariable()
}
}

bool CheckOther::comparisonNonZeroExpressionLessThanZero(const Token *tok, const ValueFlow::Value **zeroValue, const Token **nonZeroExpr, bool suppress)
bool CheckOther::comparisonNonZeroExpressionLessThanZero(const Token *tok, const ValueFlow::Value *&zeroValue, const Token *&nonZeroExpr, bool suppress)
{
if (!tok->isComparisonOp() || !tok->astOperand1() || !tok->astOperand2())
return false;
Expand All @@ -2740,24 +2740,24 @@ bool CheckOther::comparisonNonZeroExpressionLessThanZero(const Token *tok, const
const ValueFlow::Value *v2 = tok->astOperand2()->getValue(0);

if (Token::Match(tok, "<|<=") && v2 && v2->isKnown()) {
*zeroValue = v2;
*nonZeroExpr = tok->astOperand1();
zeroValue = v2;
nonZeroExpr = tok->astOperand1();
} else if (Token::Match(tok, ">|>=") && v1 && v1->isKnown()) {
*zeroValue = v1;
*nonZeroExpr = tok->astOperand2();
zeroValue = v1;
nonZeroExpr = tok->astOperand2();
} else {
return false;
}

if (const Variable* var = (*nonZeroExpr)->variable())
if (const Variable* var = nonZeroExpr->variable())
if (var->typeStartToken()->isTemplateArg())
return suppress;

const ValueType* vt = (*nonZeroExpr)->valueType();
const ValueType* vt = nonZeroExpr->valueType();
return vt && (vt->pointer || vt->sign == ValueType::UNSIGNED);
}

bool CheckOther::testIfNonZeroExpressionIsPositive(const Token *tok, const ValueFlow::Value **zeroValue, const Token **nonZeroExpr)
bool CheckOther::testIfNonZeroExpressionIsPositive(const Token *tok, const ValueFlow::Value *&zeroValue, const Token *&nonZeroExpr)
{
if (!tok->isComparisonOp() || !tok->astOperand1() || !tok->astOperand2())
return false;
Expand All @@ -2766,16 +2766,16 @@ bool CheckOther::testIfNonZeroExpressionIsPositive(const Token *tok, const Value
const ValueFlow::Value *v2 = tok->astOperand2()->getValue(0);

if (Token::simpleMatch(tok, ">=") && v2 && v2->isKnown()) {
*zeroValue = v2;
*nonZeroExpr = tok->astOperand1();
zeroValue = v2;
nonZeroExpr = tok->astOperand1();
} else if (Token::simpleMatch(tok, "<=") && v1 && v1->isKnown()) {
*zeroValue = v1;
*nonZeroExpr = tok->astOperand2();
zeroValue = v1;
nonZeroExpr = tok->astOperand2();
} else {
return false;
}

const ValueType* vt = (*nonZeroExpr)->valueType();
const ValueType* vt = nonZeroExpr->valueType();
return vt && (vt->pointer || vt->sign == ValueType::UNSIGNED);
}

Expand Down Expand Up @@ -3864,7 +3864,7 @@ void CheckOther::checkModuloOfOneError(const Token *tok)
//-----------------------------------------------------------------------------
// Overlapping write (undefined behavior)
//-----------------------------------------------------------------------------
static bool getBufAndOffset(const Token *expr, const Token **buf, MathLib::bigint *offset)
static bool getBufAndOffset(const Token *expr, const Token *&buf, MathLib::bigint *offset)
{
if (!expr)
return false;
Expand All @@ -3885,7 +3885,7 @@ static bool getBufAndOffset(const Token *expr, const Token **buf, MathLib::bigin
return false;
}
} else if (expr->valueType() && expr->valueType()->pointer > 0) {
*buf = expr;
buf = expr;
*offset = 0;
return true;
} else {
Expand All @@ -3895,7 +3895,7 @@ static bool getBufAndOffset(const Token *expr, const Token **buf, MathLib::bigin
return false;
if (!offsetToken->hasKnownIntValue())
return false;
*buf = bufToken;
buf = bufToken;
*offset = offsetToken->getKnownIntValue();
return true;
}
Expand Down Expand Up @@ -3974,9 +3974,9 @@ void CheckOther::checkOverlappingWrite()
const MathLib::bigint sizeValue = args[nonOverlappingData->sizeArg-1]->getKnownIntValue();
const Token *buf1, *buf2;
MathLib::bigint offset1, offset2;
if (!getBufAndOffset(ptr1, &buf1, &offset1))
if (!getBufAndOffset(ptr1, buf1, &offset1))
continue;
if (!getBufAndOffset(ptr2, &buf2, &offset2))
if (!getBufAndOffset(ptr2, buf2, &offset2))
continue;

if (offset1 < offset2 && offset1 + sizeValue <= offset2)
Expand Down
4 changes: 2 additions & 2 deletions lib/checkother.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,10 @@ class CPPCHECKLIB CheckOther : public Check {
CheckOther() : Check(myName()) {}

/** Is expression a comparison that checks if a nonzero (unsigned/pointer) expression is less than zero? */
static bool comparisonNonZeroExpressionLessThanZero(const Token *tok, const ValueFlow::Value **zeroValue, const Token **nonZeroExpr, bool suppress = false);
static bool comparisonNonZeroExpressionLessThanZero(const Token *tok, const ValueFlow::Value *&zeroValue, const Token *&nonZeroExpr, bool suppress = false);

/** Is expression a comparison that checks if a nonzero (unsigned/pointer) expression is positive? */
static bool testIfNonZeroExpressionIsPositive(const Token *tok, const ValueFlow::Value **zeroValue, const Token **nonZeroExpr);
static bool testIfNonZeroExpressionIsPositive(const Token *tok, const ValueFlow::Value *&zeroValue, const Token *&nonZeroExpr);

private:
/** @brief This constructor is used when running checks. */
Expand Down
6 changes: 3 additions & 3 deletions lib/ctu.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ std::list<CTU::FileInfo::UnsafeUsage> CTU::loadUnsafeUsageListFromXml(const tiny
return ret;
}

static int isCallFunction(const Scope *scope, int argnr, const Token **tok)
static int isCallFunction(const Scope *scope, int argnr, const Token *&tok)
{
const Variable * const argvar = scope->function->getArgumentVar(argnr);
if (!argvar->isPointer())
Expand All @@ -299,7 +299,7 @@ static int isCallFunction(const Scope *scope, int argnr, const Token **tok)
break;
if (!prev->astOperand1() || !prev->astOperand1()->function())
break;
*tok = prev->previous();
tok = prev->previous();
return argnr2;
}
return -1;
Expand Down Expand Up @@ -424,7 +424,7 @@ CTU::FileInfo *CTU::getFileInfo(const Tokenizer *tokenizer)
// Nested function calls
for (int argnr = 0; argnr < scopeFunction->argCount(); ++argnr) {
const Token *tok;
const int argnr2 = isCallFunction(&scope, argnr, &tok);
const int argnr2 = isCallFunction(&scope, argnr, tok);
if (argnr2 > 0) {
FileInfo::NestedCall nestedCall(tokenizer, scopeFunction, tok);
nestedCall.myArgNr = argnr + 1;
Expand Down
6 changes: 3 additions & 3 deletions lib/programmemory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,11 @@ void ProgramMemory::setIntValue(const Token* expr, MathLib::bigint value, bool i
setValue(expr, v);
}

bool ProgramMemory::getTokValue(nonneg int exprid, const Token** result) const
bool ProgramMemory::getTokValue(nonneg int exprid, const Token*& result) const
{
const ValueFlow::Value* value = getValue(exprid);
if (value && value->isTokValue()) {
*result = value->tokvalue;
result = value->tokvalue;
return true;
}
return false;
Expand Down Expand Up @@ -1469,7 +1469,7 @@ namespace {
return lhs;
} else if (expr->str() == "[" && expr->astOperand1() && expr->astOperand2()) {
const Token* tokvalue = nullptr;
if (!pm->getTokValue(expr->astOperand1()->exprId(), &tokvalue)) {
if (!pm->getTokValue(expr->astOperand1()->exprId(), tokvalue)) {
auto tokvalue_it = std::find_if(expr->astOperand1()->values().cbegin(),
expr->astOperand1()->values().cend(),
std::mem_fn(&ValueFlow::Value::isTokValue));
Expand Down
Loading