Skip to content

Commit 26da798

Browse files
firewavefirewave
authored
avoid some unchecked pointer dereferences (#6284)
1 parent 646f869 commit 26da798

16 files changed

Lines changed: 175 additions & 174 deletions

‎lib/astutils.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -771,7 +771,7 @@ std::vector<ValueType> getParentValueTypes(const Token* tok, const Settings* set
771771
if (Token::simpleMatch(tok->astParent(), "(") && ftok && !tok->astParent()->isCast() &&
772772
ftok->tokType() != Token::eType)
773773
return {};
774-
if (Token::Match(tok->astParent(), "return|(|{|%assign%") && parent) {
774+
if (parent && Token::Match(tok->astParent(), "return|(|{|%assign%")) {
775775
*parent = tok->astParent();
776776
}
777777
if (tok->astParent()->valueType())

‎lib/checkclass.cpp‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1740,7 +1740,7 @@ void CheckClass::operatorEqToSelf()
17401740
// find the parameter name
17411741
const Token *rhs = func.argumentList.cbegin()->nameToken();
17421742
const Token* out_ifStatementScopeStart = nullptr;
1743-
if (!hasAssignSelf(&func, rhs, &out_ifStatementScopeStart)) {
1743+
if (!hasAssignSelf(&func, rhs, out_ifStatementScopeStart)) {
17441744
if (hasAllocation(&func, scope))
17451745
operatorEqToSelfError(func.token);
17461746
} else if (out_ifStatementScopeStart != nullptr) {
@@ -1859,7 +1859,7 @@ const Token * CheckClass::getIfStmtBodyStart(const Token *tok, const Token *rhs)
18591859
return nullptr;
18601860
}
18611861

1862-
bool CheckClass::hasAssignSelf(const Function *func, const Token *rhs, const Token **out_ifStatementScopeStart)
1862+
bool CheckClass::hasAssignSelf(const Function *func, const Token *rhs, const Token *&out_ifStatementScopeStart)
18631863
{
18641864
if (!rhs)
18651865
return false;
@@ -1882,7 +1882,7 @@ bool CheckClass::hasAssignSelf(const Function *func, const Token *rhs, const Tok
18821882
if (tok2 && tok2->isUnaryOp("&") && tok2->astOperand1()->str() == rhs->str())
18831883
ret = true;
18841884
if (ret) {
1885-
*out_ifStatementScopeStart = getIfStmtBodyStart(tok2, rhs);
1885+
out_ifStatementScopeStart = getIfStmtBodyStart(tok2, rhs);
18861886
}
18871887
return ret ? ChildrenToVisit::done : ChildrenToVisit::op1_and_op2;
18881888
});
@@ -3400,13 +3400,13 @@ void CheckClass::checkThisUseAfterFree()
34003400

34013401
const Token * freeToken = nullptr;
34023402
std::set<const Function *> callstack;
3403-
checkThisUseAfterFreeRecursive(classScope, &func, &var, std::move(callstack), &freeToken);
3403+
checkThisUseAfterFreeRecursive(classScope, &func, &var, std::move(callstack), freeToken);
34043404
}
34053405
}
34063406
}
34073407
}
34083408

3409-
bool CheckClass::checkThisUseAfterFreeRecursive(const Scope *classScope, const Function *func, const Variable *selfPointer, std::set<const Function *> callstack, const Token **freeToken)
3409+
bool CheckClass::checkThisUseAfterFreeRecursive(const Scope *classScope, const Function *func, const Variable *selfPointer, std::set<const Function *> callstack, const Token *&freeToken)
34103410
{
34113411
if (!func || !func->functionScope)
34123412
return false;
@@ -3419,23 +3419,23 @@ bool CheckClass::checkThisUseAfterFreeRecursive(const Scope *classScope, const F
34193419
const Token * const bodyStart = func->functionScope->bodyStart;
34203420
const Token * const bodyEnd = func->functionScope->bodyEnd;
34213421
for (const Token *tok = bodyStart; tok != bodyEnd; tok = tok->next()) {
3422-
const bool isDestroyed = *freeToken != nullptr && !func->isStatic();
3422+
const bool isDestroyed = freeToken != nullptr && !func->isStatic();
34233423
if (Token::Match(tok, "delete %var% ;") && selfPointer == tok->next()->variable()) {
3424-
*freeToken = tok;
3424+
freeToken = tok;
34253425
tok = tok->tokAt(2);
34263426
} else if (Token::Match(tok, "%var% . reset ( )") && selfPointer == tok->variable())
3427-
*freeToken = tok;
3427+
freeToken = tok;
34283428
else if (Token::Match(tok->previous(), "!!. %name% (") && tok->function() && tok->function()->nestedIn == classScope) {
34293429
if (isDestroyed) {
3430-
thisUseAfterFree(selfPointer->nameToken(), *freeToken, tok);
3430+
thisUseAfterFree(selfPointer->nameToken(), freeToken, tok);
34313431
return true;
34323432
}
34333433
if (checkThisUseAfterFreeRecursive(classScope, tok->function(), selfPointer, callstack, freeToken))
34343434
return true;
34353435
} else if (isDestroyed && Token::Match(tok->previous(), "!!. %name%") && tok->variable() && tok->variable()->scope() == classScope && !tok->variable()->isStatic() && !tok->variable()->isArgument()) {
3436-
thisUseAfterFree(selfPointer->nameToken(), *freeToken, tok);
3436+
thisUseAfterFree(selfPointer->nameToken(), freeToken, tok);
34373437
return true;
3438-
} else if (*freeToken && Token::Match(tok, "return|throw")) {
3438+
} else if (freeToken && Token::Match(tok, "return|throw")) {
34393439
// TODO
34403440
return tok->str() == "throw";
34413441
} else if (tok->str() == "{" && tok->scope()->type == Scope::ScopeType::eLambda) {

‎lib/checkclass.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,7 @@ class CPPCHECKLIB CheckClass : public Check {
298298
bool hasAllocation(const Function *func, const Scope* scope) const;
299299
bool hasAllocation(const Function *func, const Scope* scope, const Token *start, const Token *end) const;
300300
bool hasAllocationInIfScope(const Function *func, const Scope* scope, const Token *ifStatementScopeStart) const;
301-
static bool hasAssignSelf(const Function *func, const Token *rhs, const Token **out_ifStatementScopeStart);
301+
static bool hasAssignSelf(const Function *func, const Token *rhs, const Token *&out_ifStatementScopeStart);
302302
enum class Bool { TRUE, FALSE, BAILOUT };
303303
static Bool isInverted(const Token *tok, const Token *rhs);
304304
static const Token * getIfStmtBodyStart(const Token *tok, const Token *rhs);
@@ -411,7 +411,7 @@ class CPPCHECKLIB CheckClass : public Check {
411411
/**
412412
* @brief Helper for checkThisUseAfterFree
413413
*/
414-
bool checkThisUseAfterFreeRecursive(const Scope *classScope, const Function *func, const Variable *selfPointer, std::set<const Function *> callstack, const Token **freeToken);
414+
bool checkThisUseAfterFreeRecursive(const Scope *classScope, const Function *func, const Variable *selfPointer, std::set<const Function *> callstack, const Token *&freeToken);
415415
};
416416
/// @}
417417
//---------------------------------------------------------------------------

‎lib/checkcondition.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1573,8 +1573,8 @@ void CheckCondition::alwaysTrueFalse()
15731573
{
15741574
const ValueFlow::Value *zeroValue = nullptr;
15751575
const Token *nonZeroExpr = nullptr;
1576-
if (CheckOther::comparisonNonZeroExpressionLessThanZero(tok, &zeroValue, &nonZeroExpr, /*suppress*/ true) ||
1577-
CheckOther::testIfNonZeroExpressionIsPositive(tok, &zeroValue, &nonZeroExpr))
1576+
if (CheckOther::comparisonNonZeroExpressionLessThanZero(tok, zeroValue, nonZeroExpr, /*suppress*/ true) ||
1577+
CheckOther::testIfNonZeroExpressionIsPositive(tok, zeroValue, nonZeroExpr))
15781578
continue;
15791579
}
15801580

‎lib/checkio.cpp‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -487,16 +487,16 @@ void CheckIO::invalidScanfError(const Token *tok)
487487
//---------------------------------------------------------------------------
488488

489489
static bool findFormat(nonneg int arg, const Token *firstArg,
490-
const Token **formatStringTok, const Token **formatArgTok)
490+
const Token *&formatStringTok, const Token *&formatArgTok)
491491
{
492492
const Token* argTok = firstArg;
493493

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

497497
if (Token::Match(argTok, "%str% [,)]")) {
498-
*formatArgTok = argTok->nextArgument();
499-
*formatStringTok = argTok;
498+
formatArgTok = argTok->nextArgument();
499+
formatStringTok = argTok;
500500
return true;
501501
}
502502
if (Token::Match(argTok, "%var% [,)]") &&
@@ -506,13 +506,13 @@ static bool findFormat(nonneg int arg, const Token *firstArg,
506506
(argTok->variable()->dimensions().size() == 1 &&
507507
argTok->variable()->dimensionKnown(0) &&
508508
argTok->variable()->dimension(0) != 0))) {
509-
*formatArgTok = argTok->nextArgument();
509+
formatArgTok = argTok->nextArgument();
510510
if (!argTok->values().empty()) {
511511
const std::list<ValueFlow::Value>::const_iterator value = std::find_if(
512512
argTok->values().cbegin(), argTok->values().cend(), std::mem_fn(&ValueFlow::Value::isTokValue));
513513
if (value != argTok->values().cend() && value->isTokValue() && value->tokvalue &&
514514
value->tokvalue->tokType() == Token::eString) {
515-
*formatStringTok = value->tokvalue;
515+
formatStringTok = value->tokvalue;
516516
}
517517
}
518518
return true;
@@ -552,37 +552,37 @@ void CheckIO::checkWrongPrintfScanfArguments()
552552

553553
if (formatStringArgNo >= 0) {
554554
// formatstring found in library. Find format string and first argument belonging to format string.
555-
if (!findFormat(formatStringArgNo, tok->tokAt(2), &formatStringTok, &argListTok))
555+
if (!findFormat(formatStringArgNo, tok->tokAt(2), formatStringTok, argListTok))
556556
continue;
557557
} else if (Token::simpleMatch(tok, "swprintf (")) {
558558
if (Token::Match(tok->tokAt(2)->nextArgument(), "%str%")) {
559559
// Find third parameter and format string
560-
if (!findFormat(1, tok->tokAt(2), &formatStringTok, &argListTok))
560+
if (!findFormat(1, tok->tokAt(2), formatStringTok, argListTok))
561561
continue;
562562
} else {
563563
// Find fourth parameter and format string
564-
if (!findFormat(2, tok->tokAt(2), &formatStringTok, &argListTok))
564+
if (!findFormat(2, tok->tokAt(2), formatStringTok, argListTok))
565565
continue;
566566
}
567567
} else if (isWindows && Token::Match(tok, "sprintf_s|swprintf_s (")) {
568568
// template <size_t size> int sprintf_s(char (&buffer)[size], const char *format, ...);
569-
if (findFormat(1, tok->tokAt(2), &formatStringTok, &argListTok)) {
569+
if (findFormat(1, tok->tokAt(2), formatStringTok, argListTok)) {
570570
if (!formatStringTok)
571571
continue;
572572
}
573573
// int sprintf_s(char *buffer, size_t sizeOfBuffer, const char *format, ...);
574-
else if (findFormat(2, tok->tokAt(2), &formatStringTok, &argListTok)) {
574+
else if (findFormat(2, tok->tokAt(2), formatStringTok, argListTok)) {
575575
if (!formatStringTok)
576576
continue;
577577
}
578578
} else if (isWindows && Token::Match(tok, "_snprintf_s|_snwprintf_s (")) {
579579
// template <size_t size> int _snprintf_s(char (&buffer)[size], size_t count, const char *format, ...);
580-
if (findFormat(2, tok->tokAt(2), &formatStringTok, &argListTok)) {
580+
if (findFormat(2, tok->tokAt(2), formatStringTok, argListTok)) {
581581
if (!formatStringTok)
582582
continue;
583583
}
584584
// int _snprintf_s(char *buffer, size_t sizeOfBuffer, size_t count, const char *format, ...);
585-
else if (findFormat(3, tok->tokAt(2), &formatStringTok, &argListTok)) {
585+
else if (findFormat(3, tok->tokAt(2), formatStringTok, argListTok)) {
586586
if (!formatStringTok)
587587
continue;
588588
}

‎lib/checkother.cpp‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2713,13 +2713,13 @@ void CheckOther::checkSignOfUnsignedVariable()
27132713
for (const Token *tok = scope->bodyStart->next(); tok != scope->bodyEnd; tok = tok->next()) {
27142714
const ValueFlow::Value *zeroValue = nullptr;
27152715
const Token *nonZeroExpr = nullptr;
2716-
if (comparisonNonZeroExpressionLessThanZero(tok, &zeroValue, &nonZeroExpr)) {
2716+
if (comparisonNonZeroExpressionLessThanZero(tok, zeroValue, nonZeroExpr)) {
27172717
const ValueType* vt = nonZeroExpr->valueType();
27182718
if (vt->pointer)
27192719
pointerLessThanZeroError(tok, zeroValue);
27202720
else
27212721
unsignedLessThanZeroError(tok, zeroValue, nonZeroExpr->expressionString());
2722-
} else if (testIfNonZeroExpressionIsPositive(tok, &zeroValue, &nonZeroExpr)) {
2722+
} else if (testIfNonZeroExpressionIsPositive(tok, zeroValue, nonZeroExpr)) {
27232723
const ValueType* vt = nonZeroExpr->valueType();
27242724
if (vt->pointer)
27252725
pointerPositiveError(tok, zeroValue);
@@ -2730,7 +2730,7 @@ void CheckOther::checkSignOfUnsignedVariable()
27302730
}
27312731
}
27322732

2733-
bool CheckOther::comparisonNonZeroExpressionLessThanZero(const Token *tok, const ValueFlow::Value **zeroValue, const Token **nonZeroExpr, bool suppress)
2733+
bool CheckOther::comparisonNonZeroExpressionLessThanZero(const Token *tok, const ValueFlow::Value *&zeroValue, const Token *&nonZeroExpr, bool suppress)
27342734
{
27352735
if (!tok->isComparisonOp() || !tok->astOperand1() || !tok->astOperand2())
27362736
return false;
@@ -2739,24 +2739,24 @@ bool CheckOther::comparisonNonZeroExpressionLessThanZero(const Token *tok, const
27392739
const ValueFlow::Value *v2 = tok->astOperand2()->getValue(0);
27402740

27412741
if (Token::Match(tok, "<|<=") && v2 && v2->isKnown()) {
2742-
*zeroValue = v2;
2743-
*nonZeroExpr = tok->astOperand1();
2742+
zeroValue = v2;
2743+
nonZeroExpr = tok->astOperand1();
27442744
} else if (Token::Match(tok, ">|>=") && v1 && v1->isKnown()) {
2745-
*zeroValue = v1;
2746-
*nonZeroExpr = tok->astOperand2();
2745+
zeroValue = v1;
2746+
nonZeroExpr = tok->astOperand2();
27472747
} else {
27482748
return false;
27492749
}
27502750

2751-
if (const Variable* var = (*nonZeroExpr)->variable())
2751+
if (const Variable* var = nonZeroExpr->variable())
27522752
if (var->typeStartToken()->isTemplateArg())
27532753
return suppress;
27542754

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

2759-
bool CheckOther::testIfNonZeroExpressionIsPositive(const Token *tok, const ValueFlow::Value **zeroValue, const Token **nonZeroExpr)
2759+
bool CheckOther::testIfNonZeroExpressionIsPositive(const Token *tok, const ValueFlow::Value *&zeroValue, const Token *&nonZeroExpr)
27602760
{
27612761
if (!tok->isComparisonOp() || !tok->astOperand1() || !tok->astOperand2())
27622762
return false;
@@ -2765,16 +2765,16 @@ bool CheckOther::testIfNonZeroExpressionIsPositive(const Token *tok, const Value
27652765
const ValueFlow::Value *v2 = tok->astOperand2()->getValue(0);
27662766

27672767
if (Token::simpleMatch(tok, ">=") && v2 && v2->isKnown()) {
2768-
*zeroValue = v2;
2769-
*nonZeroExpr = tok->astOperand1();
2768+
zeroValue = v2;
2769+
nonZeroExpr = tok->astOperand1();
27702770
} else if (Token::simpleMatch(tok, "<=") && v1 && v1->isKnown()) {
2771-
*zeroValue = v1;
2772-
*nonZeroExpr = tok->astOperand2();
2771+
zeroValue = v1;
2772+
nonZeroExpr = tok->astOperand2();
27732773
} else {
27742774
return false;
27752775
}
27762776

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

@@ -3863,7 +3863,7 @@ void CheckOther::checkModuloOfOneError(const Token *tok)
38633863
//-----------------------------------------------------------------------------
38643864
// Overlapping write (undefined behavior)
38653865
//-----------------------------------------------------------------------------
3866-
static bool getBufAndOffset(const Token *expr, const Token **buf, MathLib::bigint *offset)
3866+
static bool getBufAndOffset(const Token *expr, const Token *&buf, MathLib::bigint *offset)
38673867
{
38683868
if (!expr)
38693869
return false;
@@ -3884,7 +3884,7 @@ static bool getBufAndOffset(const Token *expr, const Token **buf, MathLib::bigin
38843884
return false;
38853885
}
38863886
} else if (expr->valueType() && expr->valueType()->pointer > 0) {
3887-
*buf = expr;
3887+
buf = expr;
38883888
*offset = 0;
38893889
return true;
38903890
} else {
@@ -3894,7 +3894,7 @@ static bool getBufAndOffset(const Token *expr, const Token **buf, MathLib::bigin
38943894
return false;
38953895
if (!offsetToken->hasKnownIntValue())
38963896
return false;
3897-
*buf = bufToken;
3897+
buf = bufToken;
38983898
*offset = offsetToken->getKnownIntValue();
38993899
return true;
39003900
}
@@ -3973,9 +3973,9 @@ void CheckOther::checkOverlappingWrite()
39733973
const MathLib::bigint sizeValue = args[nonOverlappingData->sizeArg-1]->getKnownIntValue();
39743974
const Token *buf1, *buf2;
39753975
MathLib::bigint offset1, offset2;
3976-
if (!getBufAndOffset(ptr1, &buf1, &offset1))
3976+
if (!getBufAndOffset(ptr1, buf1, &offset1))
39773977
continue;
3978-
if (!getBufAndOffset(ptr2, &buf2, &offset2))
3978+
if (!getBufAndOffset(ptr2, buf2, &offset2))
39793979
continue;
39803980

39813981
if (offset1 < offset2 && offset1 + sizeValue <= offset2)

‎lib/checkother.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,10 @@ class CPPCHECKLIB CheckOther : public Check {
5656
CheckOther() : Check(myName()) {}
5757

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

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

6464
private:
6565
/** @brief This constructor is used when running checks. */

‎lib/ctu.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ std::list<CTU::FileInfo::UnsafeUsage> CTU::loadUnsafeUsageListFromXml(const tiny
276276
return ret;
277277
}
278278

279-
static int isCallFunction(const Scope *scope, int argnr, const Token **tok)
279+
static int isCallFunction(const Scope *scope, int argnr, const Token *&tok)
280280
{
281281
const Variable * const argvar = scope->function->getArgumentVar(argnr);
282282
if (!argvar->isPointer())
@@ -299,7 +299,7 @@ static int isCallFunction(const Scope *scope, int argnr, const Token **tok)
299299
break;
300300
if (!prev->astOperand1() || !prev->astOperand1()->function())
301301
break;
302-
*tok = prev->previous();
302+
tok = prev->previous();
303303
return argnr2;
304304
}
305305
return -1;
@@ -424,7 +424,7 @@ CTU::FileInfo *CTU::getFileInfo(const Tokenizer *tokenizer)
424424
// Nested function calls
425425
for (int argnr = 0; argnr < scopeFunction->argCount(); ++argnr) {
426426
const Token *tok;
427-
const int argnr2 = isCallFunction(&scope, argnr, &tok);
427+
const int argnr2 = isCallFunction(&scope, argnr, tok);
428428
if (argnr2 > 0) {
429429
FileInfo::NestedCall nestedCall(tokenizer, scopeFunction, tok);
430430
nestedCall.myArgNr = argnr + 1;

‎lib/programmemory.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,11 @@ void ProgramMemory::setIntValue(const Token* expr, MathLib::bigint value, bool i
9797
setValue(expr, v);
9898
}
9999

100-
bool ProgramMemory::getTokValue(nonneg int exprid, const Token** result) const
100+
bool ProgramMemory::getTokValue(nonneg int exprid, const Token*& result) const
101101
{
102102
const ValueFlow::Value* value = getValue(exprid);
103103
if (value && value->isTokValue()) {
104-
*result = value->tokvalue;
104+
result = value->tokvalue;
105105
return true;
106106
}
107107
return false;
@@ -1469,7 +1469,7 @@ namespace {
14691469
return lhs;
14701470
} else if (expr->str() == "[" && expr->astOperand1() && expr->astOperand2()) {
14711471
const Token* tokvalue = nullptr;
1472-
if (!pm->getTokValue(expr->astOperand1()->exprId(), &tokvalue)) {
1472+
if (!pm->getTokValue(expr->astOperand1()->exprId(), tokvalue)) {
14731473
auto tokvalue_it = std::find_if(expr->astOperand1()->values().cbegin(),
14741474
expr->astOperand1()->values().cend(),
14751475
std::mem_fn(&ValueFlow::Value::isTokValue));

0 commit comments

Comments
 (0)