From ef35b86b4a1f266b334256951617e0ebaa9821a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sat, 3 Nov 2018 15:53:24 +0100 Subject: [PATCH] Multipass Valueflow --- lib/valueflow.cpp | 47 +++++++++++++++++++++++++++++----------- test/testnullpointer.cpp | 13 +++++++++++ 2 files changed, 47 insertions(+), 13 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index c392123d9..2cab566b0 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -100,6 +100,8 @@ #include #include +static const int TIMEOUT = 10; // Do not repeat ValueFlow analysis more than 10 seconds + namespace { struct ProgramMemory { std::map values; @@ -2404,7 +2406,7 @@ static void valueFlowAfterAssign(TokenList *tokenlist, SymbolDatabase* symboldat continue; // Lhs should be a variable - if (!tok->astOperand1() || !tok->astOperand1()->varId()) + if (!tok->astOperand1() || !tok->astOperand1()->varId() || tok->astOperand1()->hasKnownValue()) continue; const unsigned int varid = tok->astOperand1()->varId(); if (aliased.find(varid) != aliased.end()) @@ -3367,6 +3369,9 @@ static void valueFlowFunctionReturn(TokenList *tokenlist, ErrorLogger *errorLogg if (tok->str() != "(" || !tok->astOperand1() || !tok->astOperand1()->function()) continue; + if (tok->hasKnownValue()) + continue; + // Arguments.. std::vector parvalues; if (tok->astOperand2()) { @@ -3623,6 +3628,8 @@ static void valueFlowContainerSize(TokenList *tokenlist, SymbolDatabase* symbold continue; if (!Token::Match(var->nameToken(), "%name% ;")) continue; + if (var->nameToken()->hasKnownValue()) + continue; ValueFlow::Value value(0); if (var->valueType()->container->size_templateArgNo >= 0) { if (var->dimensions().size() == 1 && var->dimensions().front().known) @@ -3760,6 +3767,13 @@ const ValueFlow::Value *ValueFlow::valueFlowConstantFoldAST(const Token *expr, c return expr && expr->hasKnownValue() ? &expr->values().front() : nullptr; } +static std::size_t getTotalValues(TokenList *tokenlist) +{ + std::size_t n = 1; + for (Token *tok = tokenlist->front(); tok; tok = tok->next()) + n += tok->values().size(); + return n; +} void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase, ErrorLogger *errorLogger, const Settings *settings) { @@ -3773,18 +3787,25 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase, valueFlowPointerAlias(tokenlist); valueFlowFunctionReturn(tokenlist, errorLogger); valueFlowBitAnd(tokenlist); - valueFlowOppositeCondition(symboldatabase, settings); - valueFlowBeforeCondition(tokenlist, symboldatabase, errorLogger, settings); - valueFlowAfterMove(tokenlist, symboldatabase, errorLogger, settings); - valueFlowAfterAssign(tokenlist, symboldatabase, errorLogger, settings); - valueFlowAfterCondition(tokenlist, symboldatabase, errorLogger, settings); - valueFlowSwitchVariable(tokenlist, symboldatabase, errorLogger, settings); - valueFlowForLoop(tokenlist, symboldatabase, errorLogger, settings); - valueFlowSubFunction(tokenlist, errorLogger, settings); - valueFlowFunctionDefaultParameter(tokenlist, symboldatabase, errorLogger, settings); - valueFlowUninit(tokenlist, symboldatabase, errorLogger, settings); - if (tokenlist->isCPP()) - valueFlowContainerSize(tokenlist, symboldatabase, errorLogger, settings); + + // Temporary hack.. run valueflow until there is nothing to update or timeout expires + const std::time_t timeout = std::time(0) + TIMEOUT; + std::size_t values = 0; + while (std::time(0) < timeout && values < getTotalValues(tokenlist)) { + values = getTotalValues(tokenlist); + valueFlowOppositeCondition(symboldatabase, settings); + valueFlowBeforeCondition(tokenlist, symboldatabase, errorLogger, settings); + valueFlowAfterMove(tokenlist, symboldatabase, errorLogger, settings); + valueFlowAfterAssign(tokenlist, symboldatabase, errorLogger, settings); + valueFlowAfterCondition(tokenlist, symboldatabase, errorLogger, settings); + valueFlowSwitchVariable(tokenlist, symboldatabase, errorLogger, settings); + valueFlowForLoop(tokenlist, symboldatabase, errorLogger, settings); + valueFlowSubFunction(tokenlist, errorLogger, settings); + valueFlowFunctionDefaultParameter(tokenlist, symboldatabase, errorLogger, settings); + valueFlowUninit(tokenlist, symboldatabase, errorLogger, settings); + if (tokenlist->isCPP()) + valueFlowContainerSize(tokenlist, symboldatabase, errorLogger, settings); + } } diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index f50d9972e..68df44819 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -82,6 +82,7 @@ private: TEST_CASE(nullpointer28); // #6491 TEST_CASE(nullpointer30); // #6392 TEST_CASE(nullpointer31); // #8482 + TEST_CASE(nullpointer32); // #8460 TEST_CASE(nullpointer_addressOf); // address of TEST_CASE(nullpointerSwitch); // #2626 TEST_CASE(nullpointer_cast); // #4692 @@ -1376,6 +1377,18 @@ private: ASSERT_EQUALS("", errout.str()); } + void nullpointer32() { // #8460 + check("int f(int * ptr) {\n" + " if(ptr)\n" + " { return 0;}\n" + " else{\n" + " int *p1 = ptr;\n" + " return *p1;\n" + " }\n" + "}\n", true); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:6]: (warning) Either the condition 'ptr' is redundant or there is possible null pointer dereference: p1.\n", errout.str()); + } + void nullpointer_addressOf() { // address of check("void f() {\n" " struct X *x = 0;\n"