From cbb07b6247ed9b6c85766b12011ffc783f5b7626 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Thu, 15 Jul 2021 09:43:38 +0200 Subject: [PATCH] misra; implement rule 14.3 --- lib/checkcondition.cpp | 71 ++++++++++++++++++++++++++++++++++++++++++ lib/checkcondition.h | 5 +++ test/testcondition.cpp | 32 +++++++++++++++---- 3 files changed, 102 insertions(+), 6 deletions(-) diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index 3f2bf0604..b7357c38e 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -1757,3 +1757,74 @@ void CheckCondition::assignmentInCondition(const Token *eq) Certainty::normal); } +void CheckCondition::checkCompareValueOutOfTypeRange() +{ + if (!mSettings->severity.isEnabled(Severity::style)) + return; + + if (mSettings->platformType == cppcheck::Platform::PlatformType::Native || + mSettings->platformType == cppcheck::Platform::PlatformType::Unspecified) + return; + + const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase(); + for (const Scope * scope : symbolDatabase->functionScopes) { + for (const Token* tok = scope->bodyStart; tok != scope->bodyEnd; tok = tok->next()) { + if (!tok->isComparisonOp() || !tok->isBinaryOp()) + continue; + + for (int i = 0; i < 2; ++i) { + const Token * const valueTok = (i == 0) ? tok->astOperand1() : tok->astOperand2(); + const Token * const typeTok = valueTok->astSibling(); + if (!valueTok->hasKnownIntValue() || !typeTok->valueType() || typeTok->valueType()->pointer) + continue; + if (valueTok->getKnownIntValue() < 0 && valueTok->valueType() && valueTok->valueType()->sign != ValueType::Sign::SIGNED) + continue; + int bits = 0; + switch (typeTok->valueType()->type) { + case ValueType::Type::BOOL: + bits = 1; + break; + case ValueType::Type::CHAR: + bits = mSettings->char_bit; + break; + case ValueType::Type::SHORT: + bits = mSettings->short_bit; + break; + case ValueType::Type::INT: + bits = mSettings->int_bit; + break; + case ValueType::Type::LONG: + bits = mSettings->long_bit; + break; + case ValueType::Type::LONGLONG: + bits = mSettings->long_long_bit; + break; + default: + break; + }; + if (bits == 0 || bits >= 64) + continue; + + const auto typeMinValue = (typeTok->valueType()->sign == ValueType::Sign::SIGNED) ? (-(1LL << (bits-1))) : 0; + const auto unsignedTypeMaxValue = (1LL << (bits-1)) - 1LL; + const auto typeMaxValue = (typeTok->valueType()->sign == ValueType::Sign::SIGNED) ? (unsignedTypeMaxValue / 2) : unsignedTypeMaxValue; + + if (valueTok->getKnownIntValue() < typeMinValue) + compareValueOutOfTypeRangeError(valueTok, typeTok->valueType()->str(), valueTok->getKnownIntValue()); + else if (valueTok->getKnownIntValue() > typeMaxValue) + compareValueOutOfTypeRangeError(valueTok, typeTok->valueType()->str(), valueTok->getKnownIntValue()); + } + } + } +} + +void CheckCondition::compareValueOutOfTypeRangeError(const Token *comparison, const std::string &type, long long value) +{ + reportError( + comparison, + Severity::style, + "compareValueOutOfTypeRangeError", + "Comparing expression of type '" + type + "' against value " + std::to_string(value) + ". Condition is always true/false.", + CWE398, + Certainty::normal); +} diff --git a/lib/checkcondition.h b/lib/checkcondition.h index a08994b16..cb492236f 100644 --- a/lib/checkcondition.h +++ b/lib/checkcondition.h @@ -71,6 +71,7 @@ public: checkCondition.comparison(); checkCondition.checkModuloAlwaysTrueFalse(); checkCondition.checkAssignmentInCondition(); + checkCondition.checkCompareValueOutOfTypeRange(); } /** mismatching assignment / comparison */ @@ -167,6 +168,9 @@ private: void assignmentInCondition(const Token *eq); + void checkCompareValueOutOfTypeRange(); + void compareValueOutOfTypeRangeError(const Token *comparison, const std::string &type, long long value); + void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const OVERRIDE { CheckCondition c(nullptr, settings, errorLogger); @@ -190,6 +194,7 @@ private: c.pointerAdditionResultNotNullError(nullptr, nullptr); c.duplicateConditionalAssignError(nullptr, nullptr); c.assignmentInCondition(nullptr); + c.compareValueOutOfTypeRangeError(nullptr, "unsigned char", 256); } static std::string myName() { diff --git a/test/testcondition.cpp b/test/testcondition.cpp index ae499cf50..8425ddf2c 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -38,6 +38,10 @@ private: Settings settings1; void run() OVERRIDE { + // known platform.. + settings0.platform(cppcheck::Platform::PlatformType::Native); + settings1.platform(cppcheck::Platform::PlatformType::Native); + LOAD_LIB_2(settings0.library, "qt.cfg"); LOAD_LIB_2(settings0.library, "std.cfg"); @@ -123,14 +127,14 @@ private: TEST_CASE(duplicateConditionalAssign); TEST_CASE(checkAssignmentInCondition); + + TEST_CASE(compareOutOfTypeRange); } - void check(const char code[], const char* filename = "test.cpp", bool inconclusive = false) { + void check(const char code[], Settings *settings, const char* filename = "test.cpp") { // Clear the error buffer.. errout.str(""); - settings0.certainty.setEnabled(Certainty::inconclusive, inconclusive); - // Raw tokens.. std::vector files(1, filename); std::istringstream istr(code); @@ -141,18 +145,23 @@ private: std::map filedata; simplecpp::preprocess(tokens2, tokens1, files, filedata, simplecpp::DUI()); - Preprocessor preprocessor(settings0, nullptr); + Preprocessor preprocessor(*settings, nullptr); preprocessor.setDirectives(tokens1); // Tokenizer.. - Tokenizer tokenizer(&settings0, this); + Tokenizer tokenizer(settings, this); tokenizer.createTokens(std::move(tokens2)); tokenizer.simplifyTokens1(""); tokenizer.setPreprocessor(&preprocessor); // Run checks.. CheckCondition checkCondition; - checkCondition.runChecks(&tokenizer, &settings0, this); + checkCondition.runChecks(&tokenizer, settings, this); + } + + void check(const char code[], const char* filename = "test.cpp", bool inconclusive = false) { + settings0.certainty.setEnabled(Certainty::inconclusive, inconclusive); + check(code, &settings0, filename); } void assignAndCompare() { @@ -4288,6 +4297,17 @@ private: "}"); ASSERT_EQUALS("", errout.str()); } + + void compareOutOfTypeRange() { + Settings settingsUnix64; + settingsUnix64.severity.enable(Severity::style); + settingsUnix64.platform(cppcheck::Platform::PlatformType::Unix64); + + check("void f(unsigned char c) {\n" + " if (c == 1234) {}\n" + "}", &settingsUnix64); + ASSERT_EQUALS("[test.cpp:2]: (style) Comparing expression of type 'unsigned char' against value 1234. Condition is always true/false.\n", errout.str()); + } }; REGISTER_TEST(TestCondition)