Skip to content

Commit 8951560

Browse files
pfultz2pfultz2
authored
Fix 10538: FN: nullPointer (std::swap pointers) (danmar#3504)
1 parent 130d1ab commit 8951560

File tree

4 files changed

+71
-12
lines changed

4 files changed

+71
-12
lines changed

‎lib/astutils.cpp‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,8 @@ static int getArgumentPos(const Token* ftok, const Token* tokToFind){
144144
return findArgumentPos(startTok, tokToFind);
145145
}
146146

147-
static void astFlattenRecursive(const Token *tok, std::vector<const Token *> *result, const char* op, nonneg int depth = 0)
147+
template<class T, REQUIRES("T must be a Token class", std::is_convertible<T*, const Token*> )>
148+
static void astFlattenRecursive(T* tok, std::vector<T*>* result, const char* op, nonneg int depth = 0)
148149
{
149150
++depth;
150151
if (!tok || depth >= 100)
@@ -164,6 +165,12 @@ std::vector<const Token*> astFlatten(const Token* tok, const char* op)
164165
return result;
165166
}
166167

168+
std::vector<Token*> astFlatten(Token* tok, const char* op)
169+
{
170+
std::vector<Token*> result;
171+
astFlattenRecursive(tok, &result, op);
172+
return result;
173+
}
167174

168175
bool astHasToken(const Token* root, const Token * tok)
169176
{

‎lib/astutils.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ const Token* findExpression(const nonneg int exprid,
5858
const Token* findExpression(const Token* start, const nonneg int exprid);
5959

6060
std::vector<const Token*> astFlatten(const Token* tok, const char* op);
61+
std::vector<Token*> astFlatten(Token* tok, const char* op);
6162

6263
bool astHasToken(const Token* root, const Token * tok);
6364

‎lib/valueflow.cpp‎

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4805,6 +4805,41 @@ static void valueFlowAfterAssign(TokenList *tokenlist, SymbolDatabase* symboldat
48054805
}
48064806
}
48074807

4808+
static std::vector<const Variable*> getVariables(const Token* tok)
4809+
{
4810+
std::vector<const Variable*> result;
4811+
visitAstNodes(tok, [&](const Token* child) {
4812+
if (child->variable())
4813+
result.push_back(child->variable());
4814+
return ChildrenToVisit::op1_and_op2;
4815+
});
4816+
return result;
4817+
}
4818+
4819+
static void valueFlowAfterSwap(TokenList* tokenlist,
4820+
SymbolDatabase* symboldatabase,
4821+
ErrorLogger* errorLogger,
4822+
const Settings* settings)
4823+
{
4824+
for (const Scope* scope : symboldatabase->functionScopes) {
4825+
for (Token* tok = const_cast<Token*>(scope->bodyStart); tok != scope->bodyEnd; tok = tok->next()) {
4826+
if (!Token::simpleMatch(tok, "swap ("))
4827+
continue;
4828+
if (!Token::simpleMatch(tok->next()->astOperand2(), ","))
4829+
continue;
4830+
std::vector<Token*> args = astFlatten(tok->next()->astOperand2(), ",");
4831+
if (args.size() != 2)
4832+
continue;
4833+
for (int i = 0; i < 2; i++) {
4834+
std::vector<const Variable*> vars = getVariables(args[0]);
4835+
std::list<ValueFlow::Value> values = args[0]->values();
4836+
valueFlowForwardAssign(args[0], args[1], vars, values, false, tokenlist, errorLogger, settings);
4837+
std::swap(args[0], args[1]);
4838+
}
4839+
}
4840+
}
4841+
}
4842+
48084843
static void valueFlowSetConditionToKnown(const Token* tok, std::list<ValueFlow::Value>& values, bool then)
48094844
{
48104845
if (values.empty())
@@ -6559,17 +6594,6 @@ static bool isContainerSizeChanged(nonneg int varId,
65596594
return false;
65606595
}
65616596

6562-
static std::vector<const Variable*> getVariables(const Token* tok)
6563-
{
6564-
std::vector<const Variable*> result;
6565-
visitAstNodes(tok, [&](const Token* child) {
6566-
if (child->variable())
6567-
result.push_back(child->variable());
6568-
return ChildrenToVisit::op1_and_op2;
6569-
});
6570-
return result;
6571-
}
6572-
65736597
static void valueFlowSmartPointer(TokenList *tokenlist, ErrorLogger * errorLogger, const Settings *settings)
65746598
{
65756599
for (Token *tok = tokenlist->front(); tok; tok = tok->next()) {
@@ -7419,6 +7443,7 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase,
74197443
valueFlowArrayBool(tokenlist);
74207444
valueFlowRightShift(tokenlist, settings);
74217445
valueFlowAfterAssign(tokenlist, symboldatabase, errorLogger, settings);
7446+
valueFlowAfterSwap(tokenlist, symboldatabase, errorLogger, settings);
74227447
valueFlowCondition(SimpleConditionHandler{}, tokenlist, symboldatabase, errorLogger, settings);
74237448
valueFlowInferCondition(tokenlist, settings);
74247449
valueFlowSwitchVariable(tokenlist, symboldatabase, errorLogger, settings);

‎test/testvalueflow.cpp‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ class TestValueFlow : public TestFixture {
8383
TEST_CASE(valueFlowBeforeConditionForward);
8484

8585
TEST_CASE(valueFlowAfterAssign);
86+
TEST_CASE(valueFlowAfterSwap);
8687
TEST_CASE(valueFlowAfterCondition);
8788
TEST_CASE(valueFlowAfterConditionExpr);
8889
TEST_CASE(valueFlowAfterConditionSeveralNot);
@@ -2465,6 +2466,31 @@ class TestValueFlow : public TestFixture {
24652466
ASSERT_EQUALS(false, testValueOfXImpossible(code, 3U, 1));
24662467
}
24672468

2469+
void valueFlowAfterSwap()
2470+
{
2471+
const char* code;
2472+
2473+
code = "int f() {\n"
2474+
" int a = 1;\n"
2475+
" int b = 2;\n"
2476+
" std::swap(a, b);\n"
2477+
" int x = a;\n"
2478+
" return x;\n"
2479+
"}";
2480+
ASSERT_EQUALS(true, testValueOfXKnown(code, 6U, 2));
2481+
ASSERT_EQUALS(false, testValueOfXKnown(code, 6U, 1));
2482+
2483+
code = "int f() {\n"
2484+
" int a = 1;\n"
2485+
" int b = 2;\n"
2486+
" std::swap(a, b);\n"
2487+
" int x = b;\n"
2488+
" return x;\n"
2489+
"}";
2490+
ASSERT_EQUALS(true, testValueOfXKnown(code, 6U, 1));
2491+
ASSERT_EQUALS(false, testValueOfXKnown(code, 6U, 2));
2492+
}
2493+
24682494
void valueFlowAfterCondition() {
24692495
const char *code;
24702496
// in if

0 commit comments

Comments
 (0)