Skip to content

Commit 0070745

Browse files
Ken-PatrickKen-Patrick
authored
10110: Fix FP knownConditionTrueFalse (danmar#3053)
1 parent ff9d649 commit 0070745

4 files changed

Lines changed: 188 additions & 0 deletions

File tree

‎lib/astutils.cpp‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1119,6 +1119,73 @@ static bool isZeroBoundCond(const Token * const cond)
11191119
return false;
11201120
}
11211121

1122+
static bool isForLoopCondition(const Token * const tok)
1123+
{
1124+
if (!tok)
1125+
return false;
1126+
const Token *const parent = tok->astParent();
1127+
return Token::simpleMatch(parent, ";") && parent->astOperand1() == tok &&
1128+
Token::simpleMatch(parent->astParent(), ";") &&
1129+
Token::simpleMatch(parent->astParent()->astParent(), "(") &&
1130+
parent->astParent()->astParent()->astOperand1()->str() == "for";
1131+
}
1132+
1133+
/**
1134+
* Is token used a boolean (cast to a bool, or used as a condition somewhere)
1135+
* @param tok
1136+
* @param checkingParent true if we are checking a parent. This is used to know
1137+
* what we are checking. For instance in `if (i == 2)`, isUsedAsBool("==") is
1138+
* true whereas isUsedAsBool("i") is false, but it might call
1139+
* isUsedAsBool_internal("==") which must not return true
1140+
*/
1141+
static bool isUsedAsBool_internal(const Token * const tok, bool checkingParent)
1142+
{
1143+
if (!tok)
1144+
return false;
1145+
const Token::Type type = tok->tokType();
1146+
if (type == Token::eBitOp || type == Token::eIncDecOp || (type == Token::eArithmeticalOp && !tok->isUnaryOp("*")))
1147+
// those operators don't return a bool
1148+
return false;
1149+
if (type == Token::eComparisonOp) {
1150+
if (!checkingParent)
1151+
// this operator returns a bool
1152+
return true;
1153+
if (Token::Match(tok, "==|!="))
1154+
return astIsBool(tok->astOperand1()) || astIsBool(tok->astOperand2());
1155+
return false;
1156+
}
1157+
if (type == Token::eLogicalOp)
1158+
return true;
1159+
if (astIsBool(tok))
1160+
return true;
1161+
1162+
const Token * const parent = tok->astParent();
1163+
if (!parent)
1164+
return false;
1165+
if (parent->str() == "(" && parent->astOperand2() == tok) {
1166+
if (Token::Match(parent->astOperand1(), "if|while"))
1167+
return true;
1168+
1169+
if (!parent->isCast()) { // casts are handled via the recursive call, as astIsBool will be true
1170+
// is it a call to a function ?
1171+
int argnr;
1172+
const Token *const func = getTokenArgumentFunction(tok, argnr);
1173+
if (!func || !func->function())
1174+
return false;
1175+
const Variable *var = func->function()->getArgumentVar(argnr);
1176+
return var && (var->getTypeName() == "bool");
1177+
}
1178+
} else if (isForLoopCondition(tok))
1179+
return true;
1180+
1181+
return isUsedAsBool_internal(parent, true);
1182+
}
1183+
1184+
bool isUsedAsBool(const Token * const tok)
1185+
{
1186+
return isUsedAsBool_internal(tok, false);
1187+
}
1188+
11221189
bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token * const cond2, const Library& library, bool pure, bool followVar, ErrorPath* errors)
11231190
{
11241191
if (!cond1 || !cond2)
@@ -1146,6 +1213,8 @@ bool isOppositeCond(bool isNot, bool cpp, const Token * const cond1, const Token
11461213
if (cond2->astOperand2() && cond2->astOperand2()->str() == "0")
11471214
return isSameExpression(cpp, true, cond1->astOperand1(), cond2->astOperand1(), library, pure, followVar, errors);
11481215
}
1216+
if (!isUsedAsBool(cond2))
1217+
return false;
11491218
return isSameExpression(cpp, true, cond1->astOperand1(), cond2, library, pure, followVar, errors);
11501219
}
11511220

‎lib/astutils.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,11 @@ bool isEqualKnownValue(const Token * const tok1, const Token * const tok2);
146146

147147
bool isDifferentKnownValues(const Token * const tok1, const Token * const tok2);
148148

149+
/**
150+
* Is token used a boolean, that is to say cast to a bool, or used as a condition in a if/while/for
151+
*/
152+
bool isUsedAsBool(const Token * const tok);
153+
149154
/**
150155
* Are two conditions opposite
151156
* @param isNot do you want to know if cond1 is !cond2 or if cond1 and cond2 are non-overlapping. true: cond1==!cond2 false: cond1==true => cond2==false

‎test/testastutils.cpp‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ class TestAstUtils : public TestFixture {
4242
TEST_CASE(isVariableChanged);
4343
TEST_CASE(isVariableChangedByFunctionCall);
4444
TEST_CASE(nextAfterAstRightmostLeaf);
45+
TEST_CASE(isUsedAsBool);
4546
}
4647

4748
bool findLambdaEndToken(const char code[]) {
@@ -265,6 +266,52 @@ class TestAstUtils : public TestFixture {
265266
ASSERT_EQUALS(true, nextAfterAstRightmostLeaf("int * g(int); void f(int a, int b) { int x = g(a)[b + 1]; }", "+", "] ; }"));
266267
ASSERT_EQUALS(true, nextAfterAstRightmostLeaf("int * g(int); void f(int a, int b) { int x = g(a + 1)[b]; }", "+", ") ["));
267268
}
269+
270+
enum class Result {False, True, Fail};
271+
272+
Result isUsedAsBool(const char code[], const char pattern[]) {
273+
Settings settings;
274+
Tokenizer tokenizer(&settings, this);
275+
std::istringstream istr(code);
276+
if (!tokenizer.tokenize(istr, "test.cpp"))
277+
return Result::Fail;
278+
const Token * const argtok = Token::findmatch(tokenizer.tokens(), pattern);
279+
if (!argtok)
280+
return Result::Fail;
281+
return ::isUsedAsBool(argtok) ? Result::True : Result::False;
282+
}
283+
284+
void isUsedAsBool() {
285+
ASSERT(Result::True == isUsedAsBool("void f() { bool b = true; }", "b"));
286+
ASSERT(Result::False ==isUsedAsBool("void f() { int i = true; }", "i"));
287+
ASSERT(Result::True == isUsedAsBool("void f() { int i; if (i) {} }", "i )"));
288+
ASSERT(Result::True == isUsedAsBool("void f() { int i; while (i) {} }", "i )"));
289+
ASSERT(Result::True == isUsedAsBool("void f() { int i; for (;i;) {} }", "i ; )"));
290+
ASSERT(Result::False == isUsedAsBool("void f() { int i; for (;;i) {} }", "i )"));
291+
ASSERT(Result::False == isUsedAsBool("void f() { int i; for (i;;) {} }", "i ; ; )"));
292+
ASSERT(Result::True == isUsedAsBool("void f() { int i; for (int j=0; i; ++j) {} }", "i ; ++"));
293+
ASSERT(Result::False == isUsedAsBool("void f() { int i; if (i == 2) {} }", "i =="));
294+
ASSERT(Result::True == isUsedAsBool("void f() { int i; if (i == true) {} }", "i =="));
295+
ASSERT(Result::True == isUsedAsBool("void f() { int i,j; if (i == (j&&f())) {} }", "i =="));
296+
ASSERT(Result::True == isUsedAsBool("void f() { int i; if (!i == 0) {} }", "i =="));
297+
ASSERT(Result::True == isUsedAsBool("void f() { int i; if (!i) {} }", "i )"));
298+
ASSERT(Result::True == isUsedAsBool("void f() { int i; if (!!i) {} }", "i )"));
299+
ASSERT(Result::True == isUsedAsBool("void f() { int i; if (i && f()) {} }", "i &&"));
300+
ASSERT(Result::True == isUsedAsBool("void f() { int i; int j = i && f(); }", "i &&"));
301+
ASSERT(Result::False == isUsedAsBool("void f() { int i; if (i & f()) {} }", "i &"));
302+
ASSERT(Result::True == isUsedAsBool("void f() { int i; if (static_cast<bool>(i)) {} }", "i )"));
303+
ASSERT(Result::True == isUsedAsBool("void f() { int i; if ((bool)i) {} }", "i )"));
304+
ASSERT(Result::True == isUsedAsBool("void f() { int i; if (1+static_cast<bool>(i)) {} }", "i )"));
305+
ASSERT(Result::True == isUsedAsBool("void f() { int i; if (1+(bool)i) {} }", "i )"));
306+
ASSERT(Result::False == isUsedAsBool("void f() { int i; if (1+static_cast<int>(i)) {} }", "i )"));
307+
ASSERT(Result::True == isUsedAsBool("void f() { int i; if (1+!static_cast<int>(i)) {} }", "i )"));
308+
ASSERT(Result::False == isUsedAsBool("void f() { int i; if (1+(int)i) {} }", "i )"));
309+
ASSERT(Result::False == isUsedAsBool("void f() { int i; if (i + 2) {} }", "i +"));
310+
ASSERT(Result::True == isUsedAsBool("void f() { int i; bool b = i; }", "i ; }"));
311+
ASSERT(Result::True == isUsedAsBool("void f(bool b); void f() { int i; f(i); }","i )"));
312+
ASSERT(Result::True == isUsedAsBool("void f() { int *i; if (*i) {} }", "i )"));
313+
ASSERT(Result::True == isUsedAsBool("void f() { int *i; if (*i) {} }", "* i )"));
314+
}
268315
};
269316

270317
REGISTER_TEST(TestAstUtils)

‎test/testvalueflow.cpp‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4359,6 +4359,73 @@ class TestValueFlow : public TestFixture {
43594359
"}\n";
43604360
ASSERT_EQUALS(true, valueOfTok(code, "> 0").isKnown());
43614361
ASSERT_EQUALS(true, valueOfTok(code, "> 0").intvalue == 1);
4362+
4363+
// FP #10110
4364+
code = "enum FENUMS { NONE = 0, CB = 8 };\n"
4365+
"bool calc(int x) {\n"
4366+
" if (!x) {\n"
4367+
" return false;\n"
4368+
" }\n"
4369+
"\n"
4370+
" if (x & CB) {\n"
4371+
" return true;\n"
4372+
" }\n"
4373+
" return false;\n"
4374+
"}\n";
4375+
ASSERT_EQUALS(false, valueOfTok(code, "& CB").isKnown());
4376+
ASSERT_EQUALS(true, testValueOfXImpossible(code, 7U, 0));
4377+
4378+
code = "enum FENUMS { NONE = 0, CB = 8 };\n"
4379+
"bool calc(int x) {\n"
4380+
" if (x) {\n"
4381+
" return false;\n"
4382+
" }\n"
4383+
"\n"
4384+
" if ((!x) & CB) {\n"
4385+
" return true;\n"
4386+
" }\n"
4387+
" return false;\n"
4388+
"}\n";
4389+
ASSERT_EQUALS(true, valueOfTok(code, "& CB").isKnown());
4390+
ASSERT_EQUALS(true, testValueOfXKnown(code, 7U, 0));
4391+
4392+
code = "enum FENUMS { NONE = 0, CB = 8 };\n"
4393+
"bool calc(int x) {\n"
4394+
" if (!!x) {\n"
4395+
" return false;\n"
4396+
" }\n"
4397+
"\n"
4398+
" if (x & CB) {\n"
4399+
" return true;\n"
4400+
" }\n"
4401+
" return false;\n"
4402+
"}\n";
4403+
ASSERT_EQUALS(true, valueOfTok(code, "& CB").isKnown());
4404+
ASSERT_EQUALS(true, testValueOfXKnown(code, 7U, 0));
4405+
4406+
code = "bool calc(bool x) {\n"
4407+
" if (!x) {\n"
4408+
" return false;\n"
4409+
" }\n"
4410+
"\n"
4411+
" if (x) {\n"
4412+
" return true;\n"
4413+
" }\n"
4414+
" return false;\n"
4415+
"}\n";
4416+
ASSERT_EQUALS(true, testValueOfXKnown(code, 6U, 1));
4417+
4418+
code = "bool calc(bool x) {\n"
4419+
" if (x) {\n"
4420+
" return false;\n"
4421+
" }\n"
4422+
"\n"
4423+
" if (!x) {\n"
4424+
" return true;\n"
4425+
" }\n"
4426+
" return false;\n"
4427+
"}\n";
4428+
ASSERT_EQUALS(true, testValueOfXKnown(code, 6U, 0));
43624429
}
43634430

43644431
static std::string isPossibleContainerSizeValue(const std::list<ValueFlow::Value> &values, MathLib::bigint i) {

0 commit comments

Comments
 (0)