From 8945c151f59824ba3e2686e62c2756f68b2e1131 Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Sat, 11 Jun 2022 04:02:04 -0500 Subject: [PATCH] Fix 11124: FP knownConditionTrueFalse with fruit (#4196) * Update valueType * Allow comparisons * Fix compare op * Add test * Format * Fix FP * Format --- lib/programmemory.cpp | 38 +++++++++++++++++++++++++++++++------- test/testcondition.cpp | 26 ++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 7 deletions(-) diff --git a/lib/programmemory.cpp b/lib/programmemory.cpp index 96dc48714..165f41cd4 100644 --- a/lib/programmemory.cpp +++ b/lib/programmemory.cpp @@ -548,6 +548,11 @@ struct assign { } }; +static bool isIntegralValue(const ValueFlow::Value& value) +{ + return value.isIntValue() || value.isIteratorValue() || value.isSymbolicValue(); +} + static ValueFlow::Value evaluate(const std::string& op, const ValueFlow::Value& lhs, const ValueFlow::Value& rhs) { ValueFlow::Value result; @@ -569,14 +574,33 @@ static ValueFlow::Value evaluate(const std::string& op, const ValueFlow::Value& return result; } } - result.valueType = ValueFlow::Value::ValueType::INT; - if (op == "+") { - if (lhs.isIteratorValue()) - result.valueType = lhs.valueType; - else if (rhs.isIteratorValue()) - result.valueType = rhs.valueType; - } else if (lhs.valueType != rhs.valueType) { + // Must be integral types + if (!isIntegralValue(lhs) && !isIntegralValue(rhs)) return ValueFlow::Value::unknown(); + // If not the same type then one must be int + if (lhs.valueType != rhs.valueType && !lhs.isIntValue() && !rhs.isIntValue()) + return ValueFlow::Value::unknown(); + bool compareOp = contains({"==", "!=", "<", ">", ">=", "<="}, op); + // Comparison must be the same type + if (compareOp && lhs.valueType != rhs.valueType) + return ValueFlow::Value::unknown(); + // Only add, subtract, and compare for non-integers + if (!compareOp && !contains({"+", "-"}, op) && !lhs.isIntValue() && !rhs.isIntValue()) + return ValueFlow::Value::unknown(); + // Both cant be iterators for non-compare + if (!compareOp && lhs.isIteratorValue() && rhs.isIteratorValue()) + return ValueFlow::Value::unknown(); + // Symbolic values must be in the same ring + if (lhs.isSymbolicValue() && rhs.isSymbolicValue() && lhs.tokvalue != rhs.tokvalue) + return ValueFlow::Value::unknown(); + if (!lhs.isIntValue() && !compareOp) { + result.valueType = lhs.valueType; + result.tokvalue = lhs.tokvalue; + } else if (!rhs.isIntValue() && !compareOp) { + result.valueType = rhs.valueType; + result.tokvalue = rhs.tokvalue; + } else { + result.valueType = ValueFlow::Value::ValueType::INT; } bool error = false; result.intvalue = calculate(op, lhs.intvalue, rhs.intvalue, &error); diff --git a/test/testcondition.cpp b/test/testcondition.cpp index 71305295b..b3176619d 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -4364,6 +4364,32 @@ private: " return 0;\n" "}\n"); ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Condition 'b' is always false\n", errout.str()); + + // #11124 + check("struct Basket {\n" + " std::vector getApples() const;\n" + " std::vector getBananas() const; \n" + "};\n" + "int getFruit(const Basket & b, bool preferApples)\n" + "{\n" + " std::vector apples = b.getApples();\n" + " int apple = apples.empty() ? -1 : apples.front();\n" + " std::vector bananas = b.getBananas();\n" + " int banana = bananas.empty() ? -1 : bananas.front();\n" + " int fruit = std::max(apple, banana);\n" + " if (fruit == -1)\n" + " return fruit;\n" + " if (std::min(apple, banana) != -1)\n" + " fruit = preferApples ? apple : banana;\n" + " return fruit;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void f(const std::string & s, int i) {\n" + " const char c = s[i];\n" + " if (!std::isalnum(c)) {}\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); } void alwaysTrueInfer() {