From b455f847ba44ac66ab151c7c9ba26554da3b944e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sun, 12 Sep 2021 15:08:14 +0200 Subject: [PATCH] Fixed #10448 (FN compareValueOutOfTypeRangeError with int32_t) --- lib/checkcondition.cpp | 39 +++++++++++++++++++++++++++++++-------- lib/checkcondition.h | 4 ++-- test/testcondition.cpp | 11 ++++++++--- 3 files changed, 41 insertions(+), 13 deletions(-) diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index 29d127210..7e217b7f1 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -1809,26 +1809,49 @@ void CheckCondition::checkCompareValueOutOfTypeRange() const auto typeMinValue = (typeTok->valueType()->sign == ValueType::Sign::UNSIGNED) ? 0 : (-(1LL << (bits-1))); const auto unsignedTypeMaxValue = (1LL << bits) - 1LL; - const auto typeMaxValue = (typeTok->valueType()->sign != ValueType::Sign::SIGNED || bits >= mSettings->int_bit) ? - unsignedTypeMaxValue : // unsigned type. signed int/long/long long; comparing sign bit is ok. i.e. 'i == 0xffffffff' - (unsignedTypeMaxValue / 2); // signed char/short + long long typeMaxValue; + if (typeTok->valueType()->sign != ValueType::Sign::SIGNED) + typeMaxValue = unsignedTypeMaxValue; + else if (bits >= mSettings->int_bit && valueTok->valueType()->sign != ValueType::Sign::SIGNED) + typeMaxValue = unsignedTypeMaxValue; + else + typeMaxValue = unsignedTypeMaxValue / 2; - if (valueTok->getKnownIntValue() < typeMinValue) - compareValueOutOfTypeRangeError(valueTok, typeTok->valueType()->str(), valueTok->getKnownIntValue()); + bool result; + if (tok->str() == "==") + result = false; + else if (tok->str() == "!=") + result = true; + else if (tok->str()[0] == '>' && i == 0) + // num > var + result = (valueTok->getKnownIntValue() > 0); + else if (tok->str()[0] == '>' && i == 1) + // var > num + result = (valueTok->getKnownIntValue() < 0); + else if (tok->str()[0] == '<' && i == 0) + // num < var + result = (valueTok->getKnownIntValue() < 0); + else if (tok->str()[0] == '<' && i == 1) + // var < num + result = (valueTok->getKnownIntValue() > 0); + + if (valueTok->getKnownIntValue() < typeMinValue) { + compareValueOutOfTypeRangeError(valueTok, typeTok->valueType()->str(), valueTok->getKnownIntValue(), result); + } else if (valueTok->getKnownIntValue() > typeMaxValue) - compareValueOutOfTypeRangeError(valueTok, typeTok->valueType()->str(), valueTok->getKnownIntValue()); + compareValueOutOfTypeRangeError(valueTok, typeTok->valueType()->str(), valueTok->getKnownIntValue(), result); } } } } -void CheckCondition::compareValueOutOfTypeRangeError(const Token *comparison, const std::string &type, long long value) +void CheckCondition::compareValueOutOfTypeRangeError(const Token *comparison, const std::string &type, long long value, bool result) { reportError( comparison, Severity::style, "compareValueOutOfTypeRangeError", - "Comparing expression of type '" + type + "' against value " + std::to_string(value) + ". Condition is always true/false.", + "Comparing expression of type '" + type + "' against value " + std::to_string(value) + ". Condition is always " + (result ? "true" : "false") + ".", CWE398, Certainty::normal); } diff --git a/lib/checkcondition.h b/lib/checkcondition.h index dcf7501cd..964b1d67a 100644 --- a/lib/checkcondition.h +++ b/lib/checkcondition.h @@ -167,7 +167,7 @@ private: void assignmentInCondition(const Token *eq); void checkCompareValueOutOfTypeRange(); - void compareValueOutOfTypeRangeError(const Token *comparison, const std::string &type, long long value); + void compareValueOutOfTypeRangeError(const Token *comparison, const std::string &type, long long value, bool result); void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const OVERRIDE { CheckCondition c(nullptr, settings, errorLogger); @@ -192,7 +192,7 @@ private: c.pointerAdditionResultNotNullError(nullptr, nullptr); c.duplicateConditionalAssignError(nullptr, nullptr); c.assignmentInCondition(nullptr); - c.compareValueOutOfTypeRangeError(nullptr, "unsigned char", 256); + c.compareValueOutOfTypeRangeError(nullptr, "unsigned char", 256, true); } static std::string myName() { diff --git a/test/testcondition.cpp b/test/testcondition.cpp index 32af00bc9..2ef5055be 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -4586,7 +4586,7 @@ private: check("void f(unsigned char c) {\n" " if (c == 256) {}\n" "}", &settingsUnix64); - ASSERT_EQUALS("[test.cpp:2]: (style) Comparing expression of type 'unsigned char' against value 256. Condition is always true/false.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2]: (style) Comparing expression of type 'unsigned char' against value 256. Condition is always false.\n", errout.str()); check("void f(unsigned char c) {\n" " if (c == 255) {}\n" @@ -4602,12 +4602,12 @@ private: check("void f(signed char x) {\n" " if (x == 0xff) {}\n" "}", &settingsUnix64); - ASSERT_EQUALS("[test.cpp:2]: (style) Comparing expression of type 'signed char' against value 255. Condition is always true/false.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2]: (style) Comparing expression of type 'signed char' against value 255. Condition is always false.\n", errout.str()); check("void f(short x) {\n" " if (x == 0xffff) {}\n" "}", &settingsUnix64); - ASSERT_EQUALS("[test.cpp:2]: (style) Comparing expression of type 'signed short' against value 65535. Condition is always true/false.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2]: (style) Comparing expression of type 'signed short' against value 65535. Condition is always false.\n", errout.str()); check("void f(int x) {\n" " if (x == 0xffffffff) {}\n" @@ -4629,6 +4629,11 @@ private: " if ((c = foo()) != -1) {}\n" "}", &settingsUnix64); ASSERT_EQUALS("", errout.str()); + + check("void f(int x) {\n" + " if (x < 3000000000) {}\n" + "}", &settingsUnix64); + ASSERT_EQUALS("[test.cpp:2]: (style) Comparing expression of type 'signed int' against value 3000000000. Condition is always true.\n", errout.str()); } void knownConditionCast() { // #9976