Skip to content

Commit

Permalink
avoid some unchecked pointer dereferences
Browse files Browse the repository at this point in the history
  • Loading branch information
@firewave
firewave committed Apr 12, 2024
1 parent 686e28d commit 17c6f40
Show file tree
Hide file tree
Showing 16 changed files with 175 additions and 174 deletions.
2 changes: 1 addition & 1 deletion lib/astutils.cpp
View file
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
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
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
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
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
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
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
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
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

0 comments on commit 17c6f40

Please sign in to comment.