Skip to content

Commit 95a7f7c

Browse files
chrchr-githubweb-flow
chrchr-github
and
web-flow
authored
Fix #14734 FP invalidFunctionArg with ternary expression (#8538)
Co-authored-by: chrchr-github <noreply@github.com>
1 parent 00d23ca commit 95a7f7c

3 files changed

Lines changed: 28 additions & 20 deletions

File tree

‎lib/programmemory.cpp‎

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -632,10 +632,6 @@ static MathLib::bigint asInt(const ValueFlow::Value& value)
632632
return value.isFloatValue() ? static_cast<MathLib::bigint>(value.floatValue) : value.intvalue;
633633
}
634634

635-
static std::string removeAssign(const std::string& assign) {
636-
return std::string{assign.cbegin(), assign.cend() - 1};
637-
}
638-
639635
namespace {
640636
struct assign {
641637
template<class T, class U>
@@ -651,22 +647,23 @@ static bool isIntegralValue(const ValueFlow::Value& value)
651647
return value.isIntValue() || value.isIteratorValue() || value.isSymbolicValue();
652648
}
653649

654-
static ValueFlow::Value evaluate(const std::string& op, const ValueFlow::Value& lhs, const ValueFlow::Value& rhs)
650+
static ValueFlow::Value evaluate(const Token* op, const ValueFlow::Value& lhs, const ValueFlow::Value& rhs, bool removeAssign = false)
655651
{
652+
const std::string opStr = removeAssign ? op->str().substr(0, op->str().size() - 1) : op->str();
656653
ValueFlow::Value result;
657654
if (lhs.isImpossible() && rhs.isImpossible())
658655
return ValueFlow::Value::unknown();
659656
if (lhs.isImpossible() || rhs.isImpossible()) {
660657
// noninvertible
661-
if (contains({"%", "/", "&", "|"}, op))
658+
if (contains({"%", "/", "&", "|"}, opStr))
662659
return ValueFlow::Value::unknown();
663660
result.setImpossible();
664661
}
665662
if (isNumericValue(lhs) && isNumericValue(rhs)) {
666663
if (lhs.isFloatValue() || rhs.isFloatValue()) {
667-
result.valueType = ValueFlow::Value::ValueType::FLOAT;
664+
result.valueType = op->isArithmeticalOp() ? ValueFlow::Value::ValueType::FLOAT : ValueFlow::Value::ValueType::INT;
668665
bool error = false;
669-
result.floatValue = calculate(op, asFloat(lhs), asFloat(rhs), &error);
666+
result.floatValue = calculate(opStr, asFloat(lhs), asFloat(rhs), &error);
670667
if (error)
671668
return ValueFlow::Value::unknown();
672669
return result;
@@ -678,12 +675,12 @@ static ValueFlow::Value evaluate(const std::string& op, const ValueFlow::Value&
678675
// If not the same type then one must be int
679676
if (lhs.valueType != rhs.valueType && !lhs.isIntValue() && !rhs.isIntValue())
680677
return ValueFlow::Value::unknown();
681-
const bool compareOp = contains({"==", "!=", "<", ">", ">=", "<="}, op);
678+
const bool compareOp = op->isComparisonOp();
682679
// Comparison must be the same type
683680
if (compareOp && lhs.valueType != rhs.valueType)
684681
return ValueFlow::Value::unknown();
685682
// Only add, subtract, and compare for non-integers
686-
if (!compareOp && !contains({"+", "-"}, op) && !lhs.isIntValue() && !rhs.isIntValue())
683+
if (!compareOp && !contains({"+", "-"}, opStr) && !lhs.isIntValue() && !rhs.isIntValue())
687684
return ValueFlow::Value::unknown();
688685
// Both can't be iterators for non-compare
689686
if (!compareOp && lhs.isIteratorValue() && rhs.isIteratorValue())
@@ -701,10 +698,10 @@ static ValueFlow::Value evaluate(const std::string& op, const ValueFlow::Value&
701698
result.valueType = ValueFlow::Value::ValueType::INT;
702699
}
703700
bool error = false;
704-
result.intvalue = calculate(op, lhs.intvalue, rhs.intvalue, &error);
701+
result.intvalue = calculate(opStr, lhs.intvalue, rhs.intvalue, &error);
705702
if (error)
706703
return ValueFlow::Value::unknown();
707-
if (result.isImpossible() && op == "!=") {
704+
if (result.isImpossible() && opStr == "!=") {
708705
if (isTrue(result)) {
709706
result.intvalue = 1;
710707
} else if (isFalse(result)) {
@@ -1501,7 +1498,7 @@ namespace {
15011498
if (!pm->hasValue(expr->astOperand1()->exprId()))
15021499
return unknown();
15031500
ValueFlow::Value& lhs = pm->at(expr->astOperand1()->exprId());
1504-
rhs = evaluate(removeAssign(expr->str()), lhs, rhs);
1501+
rhs = evaluate(expr, lhs, rhs, /*removeAssign*/ true);
15051502
if (lhs.isIntValue())
15061503
ValueFlow::Value::visitValue(rhs, std::bind(assign{}, std::ref(lhs.intvalue), std::placeholders::_1));
15071504
else if (lhs.isFloatValue())
@@ -1565,7 +1562,7 @@ namespace {
15651562
ValueFlow::Value rhs = execute(expr->astOperand2());
15661563
if (rhs.isUninitValue())
15671564
return unknown();
1568-
ValueFlow::Value r = evaluate(expr->str(), lhs, rhs);
1565+
ValueFlow::Value r = evaluate(expr, lhs, rhs);
15691566
if (expr->isComparisonOp() && (r.isUninitValue() || r.isImpossible())) {
15701567
if (rhs.isIntValue() && !expr->astOperand1()->values().empty()) {
15711568
std::vector<ValueFlow::Value> result = infer(makeIntegralInferModel(),

‎test/cfg/std.cpp‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1058,31 +1058,31 @@ void returnValue_std_isgreater(void)
10581058
{
10591059
// cppcheck-suppress knownConditionTrueFalse
10601060
if (std::isgreater(4,2) == 0) {}
1061-
// @todo support floats
1061+
// cppcheck-suppress knownConditionTrueFalse
10621062
if (std::isgreater(4.0f,2.0f) == 0) {}
10631063
}
10641064

10651065
void returnValue_std_isgreaterequal(void)
10661066
{
10671067
// cppcheck-suppress knownConditionTrueFalse
10681068
if (std::isgreaterequal(4,2) == 0) {}
1069-
// @todo support floats
1069+
// cppcheck-suppress knownConditionTrueFalse
10701070
if (std::isgreaterequal(4.0f,2.0f) == 0) {}
10711071
}
10721072

10731073
void returnValue_std_isless(void)
10741074
{
10751075
// cppcheck-suppress knownConditionTrueFalse
10761076
if (std::isless(4,2) == 0) {}
1077-
// @todo support floats
1077+
// cppcheck-suppress knownConditionTrueFalse
10781078
if (std::isless(4.0f,2.0f) == 0) {}
10791079
}
10801080

10811081
void returnValue_std_islessequal(void)
10821082
{
10831083
// cppcheck-suppress knownConditionTrueFalse
10841084
if (std::islessequal(4,2) == 0) {}
1085-
// @todo support floats
1085+
// cppcheck-suppress knownConditionTrueFalse
10861086
if (std::islessequal(4.0f,2.0f) == 0) {}
10871087
}
10881088

@@ -1093,8 +1093,10 @@ void returnValue_std_islessgreater(void)
10931093
// cppcheck-suppress knownConditionTrueFalse
10941094
if (std::islessgreater(2,4) == 0) {}
10951095

1096-
if (std::islessgreater(4.0f,2.0f) == 0) {} // @todo support floats
1097-
if (std::islessgreater(2.0f,4.0f) == 0) {} // @todo support floats
1096+
// cppcheck-suppress knownConditionTrueFalse
1097+
if (std::islessgreater(4.0f,2.0f) == 0) {}
1098+
// cppcheck-suppress knownConditionTrueFalse
1099+
if (std::islessgreater(2.0f,4.0f) == 0) {}
10981100
}
10991101

11001102
void bufferAccessOutOfBounds(void)

‎test/testvalueflow.cpp‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4119,6 +4119,15 @@ class TestValueFlow : public TestFixture {
41194119
"}\n";
41204120
ASSERT_EQUALS(true, testValueOfX(code, 4U, 0));
41214121
ASSERT_EQUALS(false, testValueOfXImpossible(code, 4U, 0));
4122+
4123+
code = "double f(double d, bool b) {\n" // #14734
4124+
" double s = 0.0;\n"
4125+
" if (b)\n"
4126+
" s += d;\n"
4127+
" return s > 0.0 ? s : 0.0;\n"
4128+
"}\n";
4129+
auto values = tokenValues(code, "s :", ValueFlow::Value::ValueType::FLOAT);
4130+
ASSERT_EQUALS(0, values.size());
41224131
}
41234132

41244133
void valueFlowForwardLambda() {

0 commit comments

Comments
 (0)