diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 4eaa571c4..fe44632d9 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2147,7 +2147,7 @@ static bool valueFlowForward(Token * const startToken, const ProgramMemory &programMemory = getProgramMemory(tok2, varid, v); if (subFunction && conditionIsTrue(condTok, programMemory)) truevalues.push_back(v); - else if (!subFunction && !conditionIsFalse(condTok, programMemory)) + else if (!conditionIsFalse(condTok, programMemory)) truevalues.push_back(v); if (condAlwaysFalse) 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.. else if (tok2->str() == "?") { + if (subFunction && (astIsPointer(tok2->astOperand1()) || astIsIntegral(tok2->astOperand1(), false))) { + tok2 = const_cast(nextAfterAstRightmostLeaf(tok2)); + if (settings->debugwarnings) + bailout(tokenlist, errorLogger, tok2, "variable " + var->name() + " valueFlowForward, skip ternary in subfunctions"); + continue; + } const Token *condition = tok2->astOperand1(); Token *op2 = tok2->astOperand2(); if (!condition || !op2) // Ticket #6713 @@ -4722,7 +4728,7 @@ static void valueFlowLibraryFunction(Token *tok, const std::string &returnValue, 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()) { if (!Token::Match(tok, "%name% (")) @@ -4777,8 +4783,10 @@ static void valueFlowSubFunction(TokenList *tokenlist, const Settings *settings) // passed values are not "known".. 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); valueFlowSwitchVariable(tokenlist, symboldatabase, errorLogger, settings); valueFlowForLoop(tokenlist, symboldatabase, errorLogger, settings); - valueFlowSubFunction(tokenlist, settings); + valueFlowSubFunction(tokenlist, errorLogger, settings); valueFlowFunctionDefaultParameter(tokenlist, symboldatabase, errorLogger, settings); valueFlowUninit(tokenlist, symboldatabase, errorLogger, settings); if (tokenlist->isCPP()) { diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index ecc7afa41..7df3bc8ba 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -76,6 +76,7 @@ private: TEST_CASE(nullpointerSwitch); // #2626 TEST_CASE(nullpointer_cast); // #4692 TEST_CASE(nullpointer_castToVoid); // #3771 + TEST_CASE(nullpointer_subfunction); TEST_CASE(pointerCheckAndDeRef); // check if pointer is null and then dereference it TEST_CASE(nullConstantDereference); // Dereference NULL constant TEST_CASE(gcc_statement_expression); // Don't crash @@ -1435,6 +1436,18 @@ private: 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 void pointerCheckAndDeRef() { check("void foo(char *p) {\n" @@ -2683,7 +2696,8 @@ private: " p = s - 20;\n" "}\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" " if (!s) {}\n" @@ -2695,7 +2709,8 @@ private: " s -= 20;\n" "}\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" " if (!s) {}\n" @@ -2715,7 +2730,7 @@ private: " char * p = s + 20;\n" "}\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" " if (!s) {}\n" @@ -2727,7 +2742,7 @@ private: " char * p = 20 + s;\n" "}\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" " if (!s) {}\n" @@ -2739,7 +2754,7 @@ private: " s += 20;\n" "}\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" " if (!s) {}\n" diff --git a/test/testother.cpp b/test/testother.cpp index 02bb88306..107f4f5cb 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -574,7 +574,10 @@ private: " f1(123,y);\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 check("void f() {\n" diff --git a/test/teststring.cpp b/test/teststring.cpp index 5411d7186..25664f8f4 100644 --- a/test/teststring.cpp +++ b/test/teststring.cpp @@ -101,7 +101,9 @@ private: " char* s = \"Y\";\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" " p[1] = 'B';\n" diff --git a/test/testtype.cpp b/test/testtype.cpp index 7450de4c6..e5a9366c8 100644 --- a/test/testtype.cpp +++ b/test/testtype.cpp @@ -200,13 +200,17 @@ private: " return x * y;\n" "}\n" "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) {" " return x * 5U;\n" "}\n" "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 " return 5U * (1234 - x);\n" // <- signed subtraction, x is not sign converted diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 8efde00cd..6e544111f 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -341,7 +341,7 @@ private: "}\n" "\n" "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() { @@ -982,10 +982,9 @@ private: " int x = 3;\n" " f1(x+1);\n" "}\n"; - TODO_ASSERT_EQUALS("5,Assignment 'x=3', assigned value is 3\n" - "6,Calling function 'f1', 1st argument 'x+1' value is 4\n", - "", - getErrorPathForX(code, 2U)); + ASSERT_EQUALS("5,Assignment 'x=3', assigned value is 3\n" + "6,Calling function 'f1', 1st argument 'x+1' value is 4\n", + getErrorPathForX(code, 2U)); code = "void f(int a) {\n" " int x;\n"