diff --git a/lib/utils.h b/lib/utils.h index d66a3f4c1..db4d2a3e7 100644 --- a/lib/utils.h +++ b/lib/utils.h @@ -62,6 +62,12 @@ bool contains(const std::initializer_list& r, const U& x) return std::find(r.begin(), r.end(), x) != r.end(); } +template +inline std::array makeArray(T x, Ts... xs) +{ + return {std::move(x), std::move(xs)...}; +} + // Enum hash for C++11. This is not needed in C++14 struct EnumClassHash { template diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 5306eb45e..662487f3c 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -1731,6 +1731,15 @@ static bool isConvertedToIntegral(const Token* tok, const Settings* settings) return vt.type != ValueType::UNKNOWN_INT && vt.isIntegral(); } +static bool isSameToken(const Token* tok1, const Token* tok2) +{ + if (tok1->exprId() != 0 && tok1->exprId() == tok2->exprId()) + return true; + if (tok1->hasKnownIntValue() && tok2->hasKnownIntValue()) + return tok1->values().front().intvalue == tok2->values().front().intvalue; + return false; +} + static void valueFlowImpossibleValues(TokenList* tokenList, const Settings* settings) { for (Token* tok = tokenList->front(); tok; tok = tok->next()) { @@ -1757,7 +1766,50 @@ static void valueFlowImpossibleValues(TokenList* tokenList, const Settings* sett value.setImpossible(); setTokenValue(tok, value, settings); } - if (Token::simpleMatch(tok, "%") && tok->astOperand2() && tok->astOperand2()->hasKnownIntValue()) { + if (Token::simpleMatch(tok, "?") && Token::Match(tok->astOperand1(), "<|<=|>|>=")) { + const Token* condTok = tok->astOperand1(); + const Token* branchesTok = tok->astOperand2(); + + auto tokens = makeArray(condTok->astOperand1(), condTok->astOperand2()); + auto branches = makeArray(branchesTok->astOperand1(), branchesTok->astOperand2()); + bool flipped = false; + if (std::equal(tokens.begin(), tokens.end(), branches.rbegin(), &isSameToken)) + flipped = true; + else if (!std::equal(tokens.begin(), tokens.end(), branches.begin(), &isSameToken)) + continue; + const bool isMin = Token::Match(condTok, "<|<=") ^ flipped; + std::vector values; + for (const Token* tok2 : tokens) { + if (tok2->hasKnownIntValue()) { + values.emplace_back(); + } else { + ValueFlow::Value symValue{}; + symValue.valueType = ValueFlow::Value::ValueType::SYMBOLIC; + symValue.tokvalue = tok2; + values.push_back(symValue); + std::copy_if(tok2->values().begin(), + tok2->values().end(), + std::back_inserter(values), + [](const ValueFlow::Value& v) { + if (!v.isKnown()) + return false; + return v.isSymbolicValue(); + }); + } + } + for (ValueFlow::Value& value : values) { + value.setImpossible(); + if (isMin) { + value.intvalue++; + value.bound = ValueFlow::Value::Bound::Lower; + } else { + value.intvalue--; + value.bound = ValueFlow::Value::Bound::Upper; + } + setTokenValue(tok, value, settings); + } + + } else if (Token::simpleMatch(tok, "%") && tok->astOperand2() && tok->astOperand2()->hasKnownIntValue()) { ValueFlow::Value value{tok->astOperand2()->values().front()}; value.bound = ValueFlow::Value::Bound::Lower; value.setImpossible(); diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index f63e4e2e2..065e6e7a1 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -205,6 +205,7 @@ private: TEST_CASE(array_index_negative3); TEST_CASE(array_index_negative4); TEST_CASE(array_index_negative5); // #10526 + TEST_CASE(array_index_negative6); // #11349 TEST_CASE(array_index_for_decr); TEST_CASE(array_index_varnames); // FP: struct member #1576, FN: #1586 TEST_CASE(array_index_for_continue); // for,continue @@ -2227,6 +2228,19 @@ private: ASSERT_EQUALS("", errout.str()); } + // #11349 + void array_index_negative6() + { + check("void f(int i) {\n" + " int j = i;\n" + " const int a[5] = {};\n" + " const int k = j < 0 ? 0 : j;\n" + " (void)a[k];\n" + " if (i == -3) {}\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + void array_index_for_decr() { check("void f()\n" "{\n" diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 5b2cb58e4..236a24f05 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -160,6 +160,7 @@ private: TEST_CASE(valueFlowSymbolicIdentity); TEST_CASE(valueFlowSymbolicStrlen); TEST_CASE(valueFlowSmartPointer); + TEST_CASE(valueFlowImpossibleMinMax); } static bool isNotTokValue(const ValueFlow::Value &val) { @@ -7631,6 +7632,67 @@ private: "}\n"; ASSERT_EQUALS(false, testValueOfX(code, 5U, 0)); } + + void valueFlowImpossibleMinMax() + { + const char* code; + + code = "void f(int a, int b) {\n" + " int x = a < b ? a : b;\n" + " return x;\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, "a", 1)); + ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, "b", 1)); + + code = "void f(int a, int b) {\n" + " int x = a > b ? a : b;\n" + " return x;\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, "a", -1)); + ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, "b", -1)); + + code = "void f(int a, int b) {\n" + " int x = a > b ? b : a;\n" + " return x;\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, "a", 1)); + ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, "b", 1)); + + code = "void f(int a, int b) {\n" + " int x = a < b ? b : a;\n" + " return x;\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, "a", -1)); + ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, "b", -1)); + + code = "void f(int a) {\n" + " int x = a < 0 ? a : 0;\n" + " return x;\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, "a", 1)); + ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, 1)); + + code = "void f(int a) {\n" + " int x = a > 0 ? a : 0;\n" + " return x;\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, "a", -1)); + ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, -1)); + + code = "void f(int a) {\n" + " int x = a > 0 ? 0 : a;\n" + " return x;\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, "a", 1)); + ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, 1)); + + code = "void f(int a) {\n" + " int x = a < 0 ? 0 : a;\n" + " return x;\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, "a", -1)); + ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, -1)); + } }; REGISTER_TEST(TestValueFlow)