From 56f4845a1fca0f0c2d0e6a024f12fd49e1a41ee3 Mon Sep 17 00:00:00 2001 From: Paul Date: Wed, 7 Sep 2022 15:30:33 -0500 Subject: [PATCH 1/2] Fix 11194: FN: knownConditionTrueFalse --- lib/valueflow.cpp | 173 ++++++++++++++++++++++++++--------------- test/testvalueflow.cpp | 22 ++++++ 2 files changed, 131 insertions(+), 64 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index a885e12b545..0c57f6dfc43 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -5084,80 +5084,126 @@ static void valueFlowSymbolic(TokenList* tokenlist, SymbolDatabase* symboldataba } } -static void valueFlowSymbolicIdentity(TokenList* tokenlist) +static const Token* isStrlenOf(const Token* tok, const Token* expr, int depth=10) { - for (Token* tok = tokenlist->front(); tok; tok = tok->next()) { - if (tok->hasKnownIntValue()) - continue; - if (!Token::Match(tok, "*|/|<<|>>|^|+|-|%or%")) - continue; - if (!tok->astOperand1()) - continue; - if (!tok->astOperand2()) - continue; - if (!astIsIntegral(tok->astOperand1(), false) && !astIsIntegral(tok->astOperand2(), false)) - continue; - const ValueFlow::Value* constant = nullptr; - const Token* vartok = nullptr; - if (tok->astOperand1()->hasKnownIntValue()) { - constant = &tok->astOperand1()->values().front(); - vartok = tok->astOperand2(); - } - if (tok->astOperand2()->hasKnownIntValue()) { - constant = &tok->astOperand2()->values().front(); - vartok = tok->astOperand1(); - } - if (!constant) - continue; - if (!vartok) - continue; - if (vartok->exprId() == 0) - continue; - if (Token::Match(tok, "<<|>>|/") && !astIsLHS(vartok)) - continue; - if (Token::Match(tok, "<<|>>|^|+|-|%or%") && constant->intvalue != 0) - continue; - if (Token::Match(tok, "*|/") && constant->intvalue != 1) - continue; - std::vector values = {makeSymbolic(vartok)}; - std::unordered_set ids = {vartok->exprId()}; - std::copy_if( - vartok->values().begin(), vartok->values().end(), std::back_inserter(values), [&](const ValueFlow::Value& v) { + if (depth < 0) + return nullptr; + if (!tok) + return nullptr; + if (!expr) + return nullptr; + if (expr->exprId() == 0) + return nullptr; + if (Token::simpleMatch(tok->previous(), "strlen (")) { + if (tok->astOperand2()->exprId() == expr->exprId()) + return tok; + } else { + for(const ValueFlow::Value& v:tok->values()) { if (!v.isSymbolicValue()) - return false; - if (!v.tokvalue) - return false; - return ids.insert(v.tokvalue->exprId()).second; - }); - for (const ValueFlow::Value& v : values) - setTokenValue(tok, v, tokenlist->getSettings()); + continue; + if (!v.isKnown()) + continue; + if (v.intvalue != 0) + continue; + if (const Token* next = isStrlenOf(v.tokvalue, expr, depth-1)) + return next; + } } + return nullptr; } -static void valueFlowSymbolicAbs(TokenList* tokenlist, SymbolDatabase* symboldatabase) +static void valueFlowSymbolicOperators(TokenList* tokenlist, SymbolDatabase* symboldatabase) { for (const Scope* scope : symboldatabase->functionScopes) { for (Token* tok = const_cast(scope->bodyStart); tok != scope->bodyEnd; tok = tok->next()) { - if (!Token::Match(tok, "abs|labs|llabs|fabs|fabsf|fabsl (")) - continue; if (tok->hasKnownIntValue()) continue; - const Token* arg = tok->next()->astOperand2(); - if (!arg) - continue; - ValueFlow::Value c = inferCondition(">=", arg, 0); - if (!c.isKnown()) - continue; + if (Token::Match(tok, "abs|labs|llabs|fabs|fabsf|fabsl (")) { + const Token* arg = tok->next()->astOperand2(); + if (!arg) + continue; + ValueFlow::Value c = inferCondition(">=", arg, 0); + if (!c.isKnown()) + continue; + + ValueFlow::Value v = makeSymbolic(arg); + v.errorPath = c.errorPath; + v.errorPath.emplace_back(tok, "Passed to " + tok->str()); + if (c.intvalue == 0) + v.setImpossible(); + else + v.setKnown(); + setTokenValue(tok->next(), v, tokenlist->getSettings()); + } else if (Token::Match(tok, "*|/|<<|>>|^|+|-|%or%")) { + if (!tok->astOperand1()) + continue; + if (!tok->astOperand2()) + continue; + if (!astIsIntegral(tok->astOperand1(), false) && !astIsIntegral(tok->astOperand2(), false)) + continue; + const ValueFlow::Value* constant = nullptr; + const Token* vartok = nullptr; + if (tok->astOperand1()->hasKnownIntValue()) { + constant = &tok->astOperand1()->values().front(); + vartok = tok->astOperand2(); + } + if (tok->astOperand2()->hasKnownIntValue()) { + constant = &tok->astOperand2()->values().front(); + vartok = tok->astOperand1(); + } + if (!constant) + continue; + if (!vartok) + continue; + if (vartok->exprId() == 0) + continue; + if (Token::Match(tok, "<<|>>|/") && !astIsLHS(vartok)) + continue; + if (Token::Match(tok, "<<|>>|^|+|-|%or%") && constant->intvalue != 0) + continue; + if (Token::Match(tok, "*|/") && constant->intvalue != 1) + continue; + std::vector values = {makeSymbolic(vartok)}; + std::unordered_set ids = {vartok->exprId()}; + std::copy_if( + vartok->values().begin(), vartok->values().end(), std::back_inserter(values), [&](const ValueFlow::Value& v) { + if (!v.isSymbolicValue()) + return false; + if (!v.tokvalue) + return false; + return ids.insert(v.tokvalue->exprId()).second; + }); + for (const ValueFlow::Value& v : values) + setTokenValue(tok, v, tokenlist->getSettings()); + } else if (Token::simpleMatch(tok, "[")) { + const Token* arrayTok = tok->astOperand1(); + const Token* indexTok = tok->astOperand2(); + if (!arrayTok) + continue; + if (!indexTok) + continue; + for(const ValueFlow::Value& value:indexTok->values()) { + if (!value.isSymbolicValue()) + continue; + if (value.intvalue != 0) + continue; + if (value.bound == ValueFlow::Value::Bound::Upper) + continue; + if (value.isImpossible() && value.bound != ValueFlow::Value::Bound::Lower) + continue; + if (value.isKnown() && value.bound != ValueFlow::Value::Bound::Point) + continue; + const Token* strlenTok = isStrlenOf(value.tokvalue, arrayTok); + if (!strlenTok) + continue; + ValueFlow::Value v = value; + v.valueType = ValueFlow::Value::ValueType::INT; + v.errorPath.emplace_back(strlenTok, "Return index of string to the first element that is 0"); + setTokenValue(tok, v, tokenlist->getSettings()); + } + } - ValueFlow::Value v = makeSymbolic(arg); - v.errorPath = c.errorPath; - v.errorPath.emplace_back(tok, "Passed to " + tok->str()); - if (c.intvalue == 0) - v.setImpossible(); - else - v.setKnown(); - setTokenValue(tok->next(), v, tokenlist->getSettings()); } } } @@ -8760,8 +8806,7 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase, while (n > 0 && values < getTotalValues(tokenlist)) { values = getTotalValues(tokenlist); valueFlowImpossibleValues(tokenlist, settings); - valueFlowSymbolicIdentity(tokenlist); - valueFlowSymbolicAbs(tokenlist, symboldatabase); + valueFlowSymbolicOperators(tokenlist, symboldatabase); valueFlowCondition(SymbolicConditionHandler{}, tokenlist, symboldatabase, errorLogger, settings); valueFlowSymbolicInfer(tokenlist, symboldatabase); valueFlowArrayBool(tokenlist); diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index eab8eff4fb6..aea7a9df3e6 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -158,6 +158,7 @@ class TestValueFlow : public TestFixture { TEST_CASE(valueFlowNotNull); TEST_CASE(valueFlowSymbolic); TEST_CASE(valueFlowSymbolicIdentity); + TEST_CASE(valueFlowSymbolicStrlen); TEST_CASE(valueFlowSmartPointer); } @@ -7502,6 +7503,27 @@ class TestValueFlow : public TestFixture { ASSERT_EQUALS(false, testValueOfXKnown(code, 3U, "a", 0)); } + void valueFlowSymbolicStrlen() + { + const char* code; + + code = "int f(char *s) {\n" + " size_t len = strlen(s);\n" + " int x = s[len];\n" + " return x;\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfXKnown(code, 4U, 0)); + + code = "int f(char *s, size_t i) {\n" + " if (i < strlen(s)) {\n" + " int x = s[i];\n" + " return x;\n" + " }\n" + " return 0;\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfXImpossible(code, 4U, 0)); + } + void valueFlowSmartPointer() { const char* code; From e7d261db104b338818ebd5961aad39752d3beb03 Mon Sep 17 00:00:00 2001 From: Paul Date: Wed, 7 Sep 2022 15:31:34 -0500 Subject: [PATCH 2/2] Format --- lib/valueflow.cpp | 15 ++++++++------- test/testvalueflow.cpp | 20 ++++++++++---------- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 0c57f6dfc43..481e0bbc28c 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -5084,7 +5084,7 @@ static void valueFlowSymbolic(TokenList* tokenlist, SymbolDatabase* symboldataba } } -static const Token* isStrlenOf(const Token* tok, const Token* expr, int depth=10) +static const Token* isStrlenOf(const Token* tok, const Token* expr, int depth = 10) { if (depth < 0) return nullptr; @@ -5098,14 +5098,14 @@ static const Token* isStrlenOf(const Token* tok, const Token* expr, int depth=10 if (tok->astOperand2()->exprId() == expr->exprId()) return tok; } else { - for(const ValueFlow::Value& v:tok->values()) { + for (const ValueFlow::Value& v : tok->values()) { if (!v.isSymbolicValue()) continue; if (!v.isKnown()) continue; if (v.intvalue != 0) continue; - if (const Token* next = isStrlenOf(v.tokvalue, expr, depth-1)) + if (const Token* next = isStrlenOf(v.tokvalue, expr, depth - 1)) return next; } } @@ -5166,8 +5166,10 @@ static void valueFlowSymbolicOperators(TokenList* tokenlist, SymbolDatabase* sym continue; std::vector values = {makeSymbolic(vartok)}; std::unordered_set ids = {vartok->exprId()}; - std::copy_if( - vartok->values().begin(), vartok->values().end(), std::back_inserter(values), [&](const ValueFlow::Value& v) { + std::copy_if(vartok->values().begin(), + vartok->values().end(), + std::back_inserter(values), + [&](const ValueFlow::Value& v) { if (!v.isSymbolicValue()) return false; if (!v.tokvalue) @@ -5183,7 +5185,7 @@ static void valueFlowSymbolicOperators(TokenList* tokenlist, SymbolDatabase* sym continue; if (!indexTok) continue; - for(const ValueFlow::Value& value:indexTok->values()) { + for (const ValueFlow::Value& value : indexTok->values()) { if (!value.isSymbolicValue()) continue; if (value.intvalue != 0) @@ -5203,7 +5205,6 @@ static void valueFlowSymbolicOperators(TokenList* tokenlist, SymbolDatabase* sym setTokenValue(tok, v, tokenlist->getSettings()); } } - } } } diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index aea7a9df3e6..893aae70f4e 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -7508,19 +7508,19 @@ class TestValueFlow : public TestFixture { const char* code; code = "int f(char *s) {\n" - " size_t len = strlen(s);\n" - " int x = s[len];\n" - " return x;\n" - "}\n"; + " size_t len = strlen(s);\n" + " int x = s[len];\n" + " return x;\n" + "}\n"; ASSERT_EQUALS(true, testValueOfXKnown(code, 4U, 0)); code = "int f(char *s, size_t i) {\n" - " if (i < strlen(s)) {\n" - " int x = s[i];\n" - " return x;\n" - " }\n" - " return 0;\n" - "}\n"; + " if (i < strlen(s)) {\n" + " int x = s[i];\n" + " return x;\n" + " }\n" + " return 0;\n" + "}\n"; ASSERT_EQUALS(true, testValueOfXImpossible(code, 4U, 0)); }