walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Re-enable valueFlowSubFunction (#2063) 

* Re-enable valueFlowSubFunction

* Formatting

* Skip ternary operators in subfunctions

* Fix test with iostreams

* Fix FP with multiple parameters
This commit is contained in:
Paul Fultz II 2019-08-05 09:26:32 -05:00 committed by Daniel Marjamäki
parent ebcca4edd1
commit aaeec462e6
6 changed files with 50 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
lib/valueflow.cpp
View File

@ -2147,7 +2147,7 @@ static bool valueFlowForward(Token * const startToken,
const ProgramMemory &programMemory = getProgramMemory(tok2, varid, v); const ProgramMemory &programMemory = getProgramMemory(tok2, varid, v);
if (subFunction && conditionIsTrue(condTok, programMemory)) if (subFunction && conditionIsTrue(condTok, programMemory))
truevalues.push_back(v); truevalues.push_back(v);
else if (!subFunction && !conditionIsFalse(condTok, programMemory)) else if (!conditionIsFalse(condTok, programMemory))
truevalues.push_back(v); truevalues.push_back(v);
if (condAlwaysFalse) if (condAlwaysFalse)
falsevalues.push_back(v); falsevalues.push_back(v);
@ -2412,6 +2412,12 @@ static bool valueFlowForward(Token * const startToken,
// If a ? is seen and it's known that the condition is true/false.. // If a ? is seen and it's known that the condition is true/false..
else if (tok2->str() == "?") { else if (tok2->str() == "?") {
if (subFunction && (astIsPointer(tok2->astOperand1()) || astIsIntegral(tok2->astOperand1(), false))) {
tok2 = const_cast<Token*>(nextAfterAstRightmostLeaf(tok2));
if (settings->debugwarnings)
bailout(tokenlist, errorLogger, tok2, "variable " + var->name() + " valueFlowForward, skip ternary in subfunctions");
continue;
}
const Token *condition = tok2->astOperand1(); const Token *condition = tok2->astOperand1();
Token *op2 = tok2->astOperand2(); Token *op2 = tok2->astOperand2();
if (!condition || !op2) // Ticket #6713 if (!condition || !op2) // Ticket #6713
@ -4722,7 +4728,7 @@ static void valueFlowLibraryFunction(Token *tok, const std::string &returnValue,
setTokenValues(tok, results, settings); setTokenValues(tok, results, settings);
} }
static void valueFlowSubFunction(TokenList *tokenlist, const Settings *settings) static void valueFlowSubFunction(TokenList* tokenlist, ErrorLogger* errorLogger, const Settings* settings)
{ {
for (Token *tok = tokenlist->front(); tok; tok = tok->next()) { for (Token *tok = tokenlist->front(); tok; tok = tok->next()) {
if (!Token::Match(tok, "%name% (")) if (!Token::Match(tok, "%name% ("))
@ -4777,8 +4783,10 @@ static void valueFlowSubFunction(TokenList *tokenlist, const Settings *settings)
// passed values are not "known".. // passed values are not "known"..
changeKnownToPossible(argvalues); changeKnownToPossible(argvalues);
// FIXME: We need to rewrite the valueflow analysis of function calls. This does not work well. valueFlowInjectParameter(tokenlist, errorLogger, settings, argvar, calledFunctionScope, argvalues);
//valueFlowInjectParameter(tokenlist, errorLogger, settings, argvar, calledFunctionScope, argvalues); // FIXME: We need to rewrite the valueflow analysis to better handle multiple arguments
if (!argvalues.empty())
break;
} }
} }
} }
@ -5696,7 +5704,7 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase,
valueFlowAfterCondition(tokenlist, symboldatabase, errorLogger, settings); valueFlowAfterCondition(tokenlist, symboldatabase, errorLogger, settings);
valueFlowSwitchVariable(tokenlist, symboldatabase, errorLogger, settings); valueFlowSwitchVariable(tokenlist, symboldatabase, errorLogger, settings);
valueFlowForLoop(tokenlist, symboldatabase, errorLogger, settings); valueFlowForLoop(tokenlist, symboldatabase, errorLogger, settings);
valueFlowSubFunction(tokenlist, settings); valueFlowSubFunction(tokenlist, errorLogger, settings);
valueFlowFunctionDefaultParameter(tokenlist, symboldatabase, errorLogger, settings); valueFlowFunctionDefaultParameter(tokenlist, symboldatabase, errorLogger, settings);
valueFlowUninit(tokenlist, symboldatabase, errorLogger, settings); valueFlowUninit(tokenlist, symboldatabase, errorLogger, settings);
if (tokenlist->isCPP()) { if (tokenlist->isCPP()) {

25
test/testnullpointer.cpp
View File

@ -76,6 +76,7 @@ private:
TEST_CASE(nullpointerSwitch); // #2626 TEST_CASE(nullpointerSwitch); // #2626
TEST_CASE(nullpointer_cast); // #4692 TEST_CASE(nullpointer_cast); // #4692
TEST_CASE(nullpointer_castToVoid); // #3771 TEST_CASE(nullpointer_castToVoid); // #3771
TEST_CASE(nullpointer_subfunction);
TEST_CASE(pointerCheckAndDeRef); // check if pointer is null and then dereference it TEST_CASE(pointerCheckAndDeRef); // check if pointer is null and then dereference it
TEST_CASE(nullConstantDereference); // Dereference NULL constant TEST_CASE(nullConstantDereference); // Dereference NULL constant
TEST_CASE(gcc_statement_expression); // Don't crash TEST_CASE(gcc_statement_expression); // Don't crash
@ -1435,6 +1436,18 @@ private:
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
void nullpointer_subfunction() {
check("int f(int* x, int* y) {\n"
" if (!x)\n"
" return;\n"
" return *x + *y;\n"
"}\n"
"void g() {\n"
" f(nullptr, nullptr);\n"
"}\n", true);
ASSERT_EQUALS("", errout.str());
}
// Check if pointer is null and the dereference it // Check if pointer is null and the dereference it
void pointerCheckAndDeRef() { void pointerCheckAndDeRef() {
check("void foo(char *p) {\n" check("void foo(char *p) {\n"
@ -2683,7 +2696,8 @@ private:
" p = s - 20;\n" " p = s - 20;\n"
"}\n" "}\n"
"void bar() { foo(0); }\n"); "void bar() { foo(0); }\n");
TODO_ASSERT_EQUALS("[test.cpp:2]: (error) Overflow in pointer arithmetic, NULL pointer is subtracted.\n", "", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (error) Overflow in pointer arithmetic, NULL pointer is subtracted.\n",
errout.str());
check("void foo(char *s) {\n" check("void foo(char *s) {\n"
" if (!s) {}\n" " if (!s) {}\n"
@ -2695,7 +2709,8 @@ private:
" s -= 20;\n" " s -= 20;\n"
"}\n" "}\n"
"void bar() { foo(0); }\n"); "void bar() { foo(0); }\n");
TODO_ASSERT_EQUALS("[test.cpp:2]: (error) Overflow in pointer arithmetic, NULL pointer is subtracted.\n", "", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (error) Overflow in pointer arithmetic, NULL pointer is subtracted.\n",
errout.str());
check("void foo(char *s) {\n" check("void foo(char *s) {\n"
" if (!s) {}\n" " if (!s) {}\n"
@ -2715,7 +2730,7 @@ private:
" char * p = s + 20;\n" " char * p = s + 20;\n"
"}\n" "}\n"
"void bar() { foo(0); }\n"); "void bar() { foo(0); }\n");
TODO_ASSERT_EQUALS("[test.cpp:2]: (error) Pointer addition with NULL pointer.\n", "", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (error) Pointer addition with NULL pointer.\n", errout.str());
check("void foo(char *s) {\n" check("void foo(char *s) {\n"
" if (!s) {}\n" " if (!s) {}\n"
@ -2727,7 +2742,7 @@ private:
" char * p = 20 + s;\n" " char * p = 20 + s;\n"
"}\n" "}\n"
"void bar() { foo(0); }\n"); "void bar() { foo(0); }\n");
TODO_ASSERT_EQUALS("[test.cpp:2]: (error) Pointer addition with NULL pointer.\n", "", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (error) Pointer addition with NULL pointer.\n", errout.str());
check("void foo(char *s) {\n" check("void foo(char *s) {\n"
" if (!s) {}\n" " if (!s) {}\n"
@ -2739,7 +2754,7 @@ private:
" s += 20;\n" " s += 20;\n"
"}\n" "}\n"
"void bar() { foo(0); }\n"); "void bar() { foo(0); }\n");
TODO_ASSERT_EQUALS("[test.cpp:2]: (error) Pointer addition with NULL pointer.\n", "", errout.str()); ASSERT_EQUALS("[test.cpp:2]: (error) Pointer addition with NULL pointer.\n", errout.str());
check("void foo(char *s) {\n" check("void foo(char *s) {\n"
" if (!s) {}\n" " if (!s) {}\n"

5
test/testother.cpp
View File

@ -574,7 +574,10 @@ private:
" f1(123,y);\n" " f1(123,y);\n"
" if (y>0){}\n" " if (y>0){}\n"
"}"); "}");
TODO_ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:1]: (warning) Either the condition 'y>0' is redundant or there is division by zero at line 1.\n", "", errout.str()); TODO_ASSERT_EQUALS(
"[test.cpp:4] -> [test.cpp:1]: (warning) Either the condition 'y>0' is redundant or there is division by zero at line 1.\n",
"",
errout.str());
// avoid false positives when variable is changed after division // avoid false positives when variable is changed after division
check("void f() {\n" check("void f() {\n"

4
test/teststring.cpp
View File

@ -101,7 +101,9 @@ private:
" char* s = \"Y\";\n" " char* s = \"Y\";\n"
" foo_FP1(s);\n" " foo_FP1(s);\n"
"}"); "}");
TODO_ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:5]: (error) Modifying string literal \"Y\" directly or indirectly is undefined behaviour.\n", "", errout.str()); ASSERT_EQUALS(
"[test.cpp:2] -> [test.cpp:5]: (error) Modifying string literal \"Y\" directly or indirectly is undefined behaviour.\n",
errout.str());
check("void foo_FP1(char *p) {\n" check("void foo_FP1(char *p) {\n"
" p[1] = 'B';\n" " p[1] = 'B';\n"

8
test/testtype.cpp
View File

@ -200,13 +200,17 @@ private:
" return x * y;\n" " return x * y;\n"
"}\n" "}\n"
"void f2() { f1(-4,4); }"); "void f2() { f1(-4,4); }");
TODO_ASSERT_EQUALS("error", "", errout.str()); ASSERT_EQUALS(
"[test.cpp:1]: (warning) Expression 'x' can have a negative value. That is converted to an unsigned value and used in an unsigned calculation.\n",
errout.str());
check("unsigned int f1(int x) {" check("unsigned int f1(int x) {"
" return x * 5U;\n" " return x * 5U;\n"
"}\n" "}\n"
"void f2() { f1(-4); }"); "void f2() { f1(-4); }");
TODO_ASSERT_EQUALS("error", "", errout.str()); ASSERT_EQUALS(
"[test.cpp:1]: (warning) Expression 'x' can have a negative value. That is converted to an unsigned value and used in an unsigned calculation.\n",
errout.str());
check("unsigned int f1(int x) {" // #6168: FP for inner calculation check("unsigned int f1(int x) {" // #6168: FP for inner calculation
" return 5U * (1234 - x);\n" // <- signed subtraction, x is not sign converted " return 5U * (1234 - x);\n" // <- signed subtraction, x is not sign converted

9
test/testvalueflow.cpp
View File

@ -341,7 +341,7 @@ private:
"}\n" "}\n"
"\n" "\n"
"void test() { dostuff(\"abc\"); }"; "void test() { dostuff(\"abc\"); }";
TODO_ASSERT_EQUALS(true, false, testValueOfX(code, 2, "\"abc\"", ValueFlow::Value::TOK)); ASSERT_EQUALS(true, testValueOfX(code, 2, "\"abc\"", ValueFlow::Value::TOK));
} }
void valueFlowPointerAlias() { void valueFlowPointerAlias() {
@ -982,10 +982,9 @@ private:
" int x = 3;\n" " int x = 3;\n"
" f1(x+1);\n" " f1(x+1);\n"
"}\n"; "}\n";
TODO_ASSERT_EQUALS("5,Assignment 'x=3', assigned value is 3\n" ASSERT_EQUALS("5,Assignment 'x=3', assigned value is 3\n"
"6,Calling function 'f1', 1st argument 'x+1' value is 4\n", "6,Calling function 'f1', 1st argument 'x+1' value is 4\n",
"", getErrorPathForX(code, 2U));
getErrorPathForX(code, 2U));
code = "void f(int a) {\n" code = "void f(int a) {\n"
" int x;\n" " int x;\n"