Skip to content

Commit b0b3f7e

Browse files
pfultz2pfultz2
authored
Fix 10464: FP: knownConditionTrueFalse (#3452)
1 parent 47f5e5d commit b0b3f7e

File tree

3 files changed

+88
-29
lines changed

3 files changed

+88
-29
lines changed

‎lib/utils.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,15 @@ struct SelectMapValues {
4141
}
4242
};
4343

44+
// Enum hash for C++11. This is not needed in C++14
45+
struct EnumClassHash {
46+
template<typename T>
47+
std::size_t operator()(T t) const
48+
{
49+
return static_cast<std::size_t>(t);
50+
}
51+
};
52+
4453
inline bool endsWith(const std::string &str, char c)
4554
{
4655
return str[str.size()-1U] == c;

‎lib/valueflow.cpp‎

Lines changed: 73 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@
115115
#include <string>
116116
#include <tuple>
117117
#include <type_traits>
118+
#include <unordered_map>
118119
#include <unordered_set>
119120
#include <vector>
120121

@@ -306,6 +307,10 @@ static ValueFlow::Value castValue(ValueFlow::Value value, const ValueType::Sign
306307
return value;
307308
}
308309

310+
static bool isNumeric(const ValueFlow::Value& value) {
311+
return value.isIntValue() || value.isFloatValue();
312+
}
313+
309314
static void combineValueProperties(const ValueFlow::Value &value1, const ValueFlow::Value &value2, ValueFlow::Value *result)
310315
{
311316
if (value1.isKnown() && value2.isKnown())
@@ -316,6 +321,14 @@ static void combineValueProperties(const ValueFlow::Value &value1, const ValueFl
316321
result->setInconclusive();
317322
else
318323
result->setPossible();
324+
if (value1.isSymbolicValue()) {
325+
result->valueType = value1.valueType;
326+
result->tokvalue = value1.tokvalue;
327+
}
328+
if (value2.isSymbolicValue()) {
329+
result->valueType = value2.valueType;
330+
result->tokvalue = value2.tokvalue;
331+
}
319332
if (value1.isIteratorValue())
320333
result->valueType = value1.valueType;
321334
if (value2.isIteratorValue())
@@ -420,14 +433,14 @@ static R calculate(const std::string& s, const T& x, const T& y, bool* error = n
420433
case '<':
421434
return wrap(x < y);
422435
case '<<':
423-
if (y >= sizeof(MathLib::bigint) * 8) {
436+
if (y >= sizeof(MathLib::bigint) * 8 || y < 0 || x < 0) {
424437
if (error)
425438
*error = true;
426439
return R{};
427440
}
428441
return wrap(MathLib::bigint(x) << MathLib::bigint(y));
429442
case '>>':
430-
if (y >= sizeof(MathLib::bigint) * 8) {
443+
if (y >= sizeof(MathLib::bigint) * 8 || y < 0 || x < 0) {
431444
if (error)
432445
*error = true;
433446
return R{};
@@ -458,8 +471,35 @@ static T calculate(const std::string& s, const T& x, const T& y, bool* error = n
458471
/** Set token value for cast */
459472
static void setTokenValueCast(Token *parent, const ValueType &valueType, const ValueFlow::Value &value, const Settings *settings);
460473

474+
static bool isCompatibleValueTypes(ValueFlow::Value::ValueType x, ValueFlow::Value::ValueType y)
475+
{
476+
static const std::unordered_map<ValueFlow::Value::ValueType,
477+
std::unordered_set<ValueFlow::Value::ValueType, EnumClassHash>,
478+
EnumClassHash>
479+
compatibleTypes = {
480+
{ValueFlow::Value::ValueType::INT,
481+
{ValueFlow::Value::ValueType::FLOAT,
482+
ValueFlow::Value::ValueType::SYMBOLIC,
483+
ValueFlow::Value::ValueType::TOK}},
484+
{ValueFlow::Value::ValueType::FLOAT, {ValueFlow::Value::ValueType::INT}},
485+
{ValueFlow::Value::ValueType::TOK, {ValueFlow::Value::ValueType::INT}},
486+
{ValueFlow::Value::ValueType::ITERATOR_START, {ValueFlow::Value::ValueType::INT}},
487+
{ValueFlow::Value::ValueType::ITERATOR_END, {ValueFlow::Value::ValueType::INT}},
488+
};
489+
if (x == y)
490+
return true;
491+
auto it = compatibleTypes.find(x);
492+
if (it == compatibleTypes.end())
493+
return false;
494+
return it->second.count(y) > 0;
495+
}
496+
461497
static bool isCompatibleValues(const ValueFlow::Value& value1, const ValueFlow::Value& value2)
462498
{
499+
if (value1.isSymbolicValue() && value2.isSymbolicValue() && value1.tokvalue->exprId() != value2.tokvalue->exprId())
500+
return false;
501+
if (!isCompatibleValueTypes(value1.valueType, value2.valueType))
502+
return false;
463503
if (value1.isKnown() || value2.isKnown())
464504
return true;
465505
if (value1.isImpossible() || value2.isImpossible())
@@ -744,15 +784,18 @@ static void setTokenValue(Token* tok, ValueFlow::Value value, const Settings* se
744784
continue;
745785
ValueFlow::Value result(0);
746786
combineValueProperties(value1, value2, &result);
747-
const double floatValue1 = value1.isIntValue() ? value1.intvalue : value1.floatValue;
748-
const double floatValue2 = value2.isIntValue() ? value2.intvalue : value2.floatValue;
749-
const bool isFloat = value1.isFloatValue() || value2.isFloatValue();
750-
if (isFloat && Token::Match(parent, "&|^|%|<<|>>|%or%"))
751-
continue;
752-
if (Token::Match(parent, "<<|>>") &&
753-
!(value1.intvalue >= 0 && value2.intvalue >= 0 && value2.intvalue < MathLib::bigint_bits))
754-
continue;
755-
if (Token::Match(parent, "/|%") && isZero<double>(floatValue2))
787+
if (astIsFloat(parent, false)) {
788+
if (!result.isIntValue() && !result.isFloatValue())
789+
continue;
790+
result.valueType = ValueFlow::Value::ValueType::FLOAT;
791+
}
792+
const double floatValue1 = value1.isFloatValue() ? value1.floatValue : value1.intvalue;
793+
const double floatValue2 = value2.isFloatValue() ? value2.floatValue : value2.intvalue;
794+
const MathLib::bigint intValue1 =
795+
value1.isFloatValue() ? static_cast<MathLib::bigint>(value1.floatValue) : value1.intvalue;
796+
const MathLib::bigint intValue2 =
797+
value2.isFloatValue() ? static_cast<MathLib::bigint>(value2.floatValue) : value2.intvalue;
798+
if ((value1.isFloatValue() || value2.isFloatValue()) && Token::Match(parent, "&|^|%|<<|>>|==|!=|%or%"))
756799
continue;
757800
if (Token::Match(parent, "==|!=")) {
758801
if ((value1.isIntValue() && value2.isTokValue()) || (value1.isTokValue() && value2.isIntValue())) {
@@ -761,28 +804,30 @@ static void setTokenValue(Token* tok, ValueFlow::Value value, const Settings* se
761804
else if (parent->str() == "!=")
762805
result.intvalue = 1;
763806
} else if (value1.isIntValue() && value2.isIntValue()) {
764-
result.intvalue = calculate(parent->str(), value1.intvalue, value2.intvalue);
807+
bool error = false;
808+
result.intvalue = calculate(parent->str(), intValue1, intValue2, &error);
809+
if (error)
810+
continue;
765811
} else {
766812
continue;
767813
}
768814
setTokenValue(parent, result, settings);
769-
} else if (Token::Match(parent, "%comp%")) {
770-
if (!isFloat && !value1.isIntValue() && !value2.isIntValue())
771-
continue;
772-
if (isFloat)
773-
result.intvalue = calculate(parent->str(), floatValue1, floatValue2);
774-
else
775-
result.intvalue = calculate(parent->str(), value1.intvalue, value2.intvalue);
776-
setTokenValue(parent, result, settings);
777815
} else if (Token::Match(parent, "%op%")) {
778-
if (value1.isTokValue() || value2.isTokValue())
779-
break;
780-
if (isFloat) {
781-
result.valueType = ValueFlow::Value::ValueType::FLOAT;
782-
result.floatValue = calculate(parent->str(), floatValue1, floatValue2);
816+
if (Token::Match(parent, "%comp%")) {
817+
if (!result.isFloatValue() && !value1.isIntValue() && !value2.isIntValue())
818+
continue;
783819
} else {
784-
result.intvalue = calculate(parent->str(), value1.intvalue, value2.intvalue);
820+
if (value1.isTokValue() || value2.isTokValue())
821+
break;
785822
}
823+
bool error = false;
824+
if (result.isFloatValue()) {
825+
result.floatValue = calculate(parent->str(), floatValue1, floatValue2, &error);
826+
} else {
827+
result.intvalue = calculate(parent->str(), intValue1, intValue2, &error);
828+
}
829+
if (error)
830+
continue;
786831
// If the bound comes from the second value then invert the bound when subtracting
787832
if (Token::simpleMatch(parent, "-") && value2.bound == result.bound &&
788833
value2.bound != ValueFlow::Value::Bound::Point)
@@ -945,14 +990,13 @@ static void setTokenValueCast(Token *parent, const ValueType &valueType, const V
945990
setTokenValue(parent, castValue(value, valueType.sign, settings->long_bit), settings);
946991
else if (valueType.type == ValueType::Type::LONGLONG)
947992
setTokenValue(parent, castValue(value, valueType.sign, settings->long_long_bit), settings);
948-
else if (valueType.isFloat()) {
993+
else if (valueType.isFloat() && isNumeric(value)) {
949994
ValueFlow::Value floatValue = value;
950995
floatValue.valueType = ValueFlow::Value::ValueType::FLOAT;
951996
if (value.isIntValue())
952997
floatValue.floatValue = value.intvalue;
953998
setTokenValue(parent, floatValue, settings);
954-
}
955-
else if (value.isIntValue()) {
999+
} else if (value.isIntValue()) {
9561000
const long long charMax = settings->signedCharMax();
9571001
const long long charMin = settings->signedCharMin();
9581002
if (charMin <= value.intvalue && value.intvalue <= charMax) {

‎test/testvalueflow.cpp‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -826,6 +826,12 @@ class TestValueFlow : public TestFixture {
826826
ASSERT(tokenValues(";10>>-1;",">>").empty());
827827
ASSERT(tokenValues(";10>>64;",">>").empty());
828828

829+
code = "float f(const uint16_t& value) {\n"
830+
" const uint16_t uVal = value; \n"
831+
" return static_cast<float>(uVal) / 2;\n"
832+
"}\n";
833+
ASSERT_EQUALS(true, tokenValues(code, "/").empty());
834+
829835
// calculation using 1,2 variables/values
830836
code = "void f(int x) {\n"
831837
" a = x+456;\n"

0 commit comments

Comments
 (0)