From 4b37f276c24c8301374e6e585b2e2c0826698bdc Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Mon, 21 Jan 2019 20:05:35 +0100 Subject: [PATCH] ValueFlow: Set arrays to true when converting to a boolean This sets it by checking the parent. It doesn't handle function parameters yet. --- lib/valueflow.cpp | 60 +++++++++++++++++++++++++++++++++----- test/testbufferoverrun.cpp | 7 +++++ test/testcondition.cpp | 54 +++++++++++++++++++++++++++++++++- 3 files changed, 112 insertions(+), 9 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 850432296..e94c7bb7a 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -1027,14 +1027,6 @@ static void valueFlowArray(TokenList *tokenlist) for (Token *tok = tokenlist->front(); tok; tok = tok->next()) { if (tok->varId() > 0U) { // array - if (tok->variable() && tok->variable()->isArray() && !tok->variable()->isArgument() && - !tok->variable()->isStlType()) { - ValueFlow::Value value{1}; - value.setKnown(); - // TODO : this leads to too many false positives so it is commented out. - // See for instance https://github.com/danmar/cppcheck/commit/025881cf35fdde1299d16a09059e7305f8c9bd13 - // setTokenValue(tok, value, tokenlist->getSettings()); - } const std::map::const_iterator it = constantArrays.find(tok->varId()); if (it != constantArrays.end()) { ValueFlow::Value value; @@ -1079,6 +1071,57 @@ static void valueFlowArray(TokenList *tokenlist) } } +static bool isNonZero(const Token *tok) +{ + return tok && (!tok->hasKnownIntValue() || tok->values().front().intvalue != 0); +} + +static const Token *getOtherOperand(const Token *tok) +{ + if (!tok) + return nullptr; + if (!tok->astParent()) + return nullptr; + if (tok->astParent()->astOperand1() != tok) + return tok->astParent()->astOperand1(); + if (tok->astParent()->astOperand2() != tok) + return tok->astParent()->astOperand2(); + return nullptr; +} + +static void valueFlowArrayBool(TokenList *tokenlist) +{ + for (Token *tok = tokenlist->front(); tok; tok = tok->next()) { + if (tok->hasKnownIntValue()) + continue; + const Variable *var = nullptr; + bool known = false; + std::list::const_iterator val = + std::find_if(tok->values().begin(), tok->values().end(), std::mem_fn(&ValueFlow::Value::isTokValue)); + if (val == tok->values().end()) { + var = tok->variable(); + known = true; + } else { + var = val->tokvalue->variable(); + known = val->isKnown(); + } + if (!var) + continue; + if (!var->isArray() || var->isArgument() || var->isStlType()) + continue; + if (isNonZero(getOtherOperand(tok)) && Token::Match(tok->astParent(), "%comp%")) + continue; + // TODO: Check for function argument + if ((astIsBool(tok->astParent()) && !Token::Match(tok->astParent(), "(|%name%")) || + (tok->astParent() && Token::Match(tok->astParent()->previous(), "if|while|for ("))) { + ValueFlow::Value value{1}; + if (known) + value.setKnown(); + setTokenValue(tok, value, tokenlist->getSettings()); + } + } +} + static void valueFlowPointerAlias(TokenList *tokenlist) { for (Token *tok = tokenlist->front(); tok; tok = tok->next()) { @@ -4792,6 +4835,7 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase, std::size_t values = 0; while (std::time(0) < timeout && values < getTotalValues(tokenlist)) { values = getTotalValues(tokenlist); + valueFlowArrayBool(tokenlist); valueFlowRightShift(tokenlist, settings); valueFlowOppositeCondition(symboldatabase, settings); valueFlowTerminatingCondition(tokenlist, symboldatabase, settings); diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index e5a870cee..673a4c5f0 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -2804,6 +2804,13 @@ private: " p = (unsigned char *) buf + sizeof (buf);\n" "}", false, "6350.c"); ASSERT_EQUALS("", errout.str()); + + check("int f() {\n" + " const char d[] = \"0123456789\";\n" + " char *cp = d + 3;\n" + " return cp - d;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); } void pointer_out_of_bounds_2() { diff --git a/test/testcondition.cpp b/test/testcondition.cpp index cd4c5be88..71accd870 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -2592,7 +2592,59 @@ private: " int buf[42];\n" " if( buf != 0) {}\n" "}\n"); - TODO_ASSERT_EQUALS("[test.cpp:3]: (style) Condition 'buf!=0' is always true\n", "", errout.str()); // #8924 + ASSERT_EQUALS("[test.cpp:3]: (style) Condition 'buf!=0' is always true\n", errout.str()); // #8924 + + check("void f() {\n" + " int buf[42];\n" + " if( !buf ) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (style) Condition '!buf' is always false\n", errout.str()); + + check("void f() {\n" + " int buf[42];\n" + " bool b = buf;\n" + " if( b ) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (style) Condition 'b' is always true\n", errout.str()); + + check("void f() {\n" + " int buf[42];\n" + " bool b = buf;\n" + " if( !b ) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (style) Condition '!b' is always false\n", errout.str()); + + check("void f() {\n" + " int buf[42];\n" + " int * p = nullptr;\n" + " if( buf == p ) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (style) Condition 'buf==p' is always false\n", errout.str()); + + check("void f(bool x) {\n" + " int buf[42];\n" + " if( buf || x ) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (style) Condition 'buf' is always true\n", errout.str()); + + check("void f(int * p) {\n" + " int buf[42];\n" + " if( buf == p ) {}\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " int buf[42];\n" + " int p[42];\n" + " if( buf == p ) {}\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " int buf[42];\n" + " if( buf == 1) {}\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); // Avoid FP when condition comes from macro check("#define NOT !\n"