From f2ae295f1efb945ee212114b4df0a7250bf48223 Mon Sep 17 00:00:00 2001 From: PKEuS Date: Tue, 24 May 2016 15:08:07 +0200 Subject: [PATCH] Support char literals in CheckCondition::checkIncorrectLogicOperator() (#5912) --- lib/checkcondition.cpp | 35 +++++++++++++++++++++-------------- lib/checkcondition.h | 8 ++++---- test/testcondition.cpp | 25 +++++++++++++++++++++---- 3 files changed, 46 insertions(+), 22 deletions(-) diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index 55cfe91bb..f9a256eb2 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -603,7 +603,7 @@ static inline T getvalue(const int test, const T value1, const T value2) return 0; } -static bool parseComparison(const Token *comp, bool *not1, std::string *op, std::string *value, const Token **expr) +static bool parseComparison(const Token *comp, bool *not1, std::string *op, std::string *value, const Token **expr, bool* inconclusive) { *not1 = false; while (comp && comp->str() == "!") { @@ -636,8 +636,10 @@ static bool parseComparison(const Token *comp, bool *not1, std::string *op, std: *expr = comp; } + *inconclusive = *inconclusive || ((*value)[0] == '\'' && !(*op == "!=" || *op == "==")); + // Only float and int values are currently handled - if (!MathLib::isInt(*value) && !MathLib::isFloat(*value)) + if (!MathLib::isInt(*value) && !MathLib::isFloat(*value) && (*value)[0] != '\'') return false; return true; @@ -679,7 +681,7 @@ void CheckCondition::checkIncorrectLogicOperator() isOppositeCond(true, _tokenizer->isCPP(), tok->astOperand1(), tok->astOperand2(), _settings->library.functionpure)) { const bool alwaysTrue(tok->str() == "||"); - incorrectLogicOperatorError(tok, tok->expressionString(), alwaysTrue); + incorrectLogicOperatorError(tok, tok->expressionString(), alwaysTrue, false); continue; } @@ -710,7 +712,7 @@ void CheckCondition::checkIncorrectLogicOperator() const std::string cond1 = expr1 + " " + tok->str() + " (" + expr2 + " " + tok->astOperand2()->str() + " " + expr3 + ")"; const std::string cond2 = expr1 + " " + tok->str() + " " + expr3; - redundantConditionError(tok, tok2->expressionString() + ". '" + cond1 + "' is equivalent to '" + cond2 + "'"); + redundantConditionError(tok, tok2->expressionString() + ". '" + cond1 + "' is equivalent to '" + cond2 + "'", false); continue; } } @@ -723,18 +725,23 @@ void CheckCondition::checkIncorrectLogicOperator() // Comparison #2 (RHS) const Token *comp2 = tok->astOperand2(); + bool inconclusive = false; + // Parse LHS bool not1; std::string op1, value1; const Token *expr1; - if (!parseComparison(comp1, ¬1, &op1, &value1, &expr1)) + if (!parseComparison(comp1, ¬1, &op1, &value1, &expr1, &inconclusive)) continue; // Parse RHS bool not2; std::string op2, value2; const Token *expr2; - if (!parseComparison(comp2, ¬2, &op2, &value2, &expr2)) + if (!parseComparison(comp2, ¬2, &op2, &value2, &expr2, &inconclusive)) + continue; + + if (inconclusive && !_settings->inconclusive) continue; if (isSameExpression(_tokenizer->isCPP(), true, comp1, comp2, _settings->library.functionpure)) @@ -799,40 +806,40 @@ void CheckCondition::checkIncorrectLogicOperator() const std::string cond2str = conditionString(not2, expr2, op2, value2); if (printWarning && (alwaysTrue || alwaysFalse)) { const std::string text = cond1str + " " + tok->str() + " " + cond2str; - incorrectLogicOperatorError(tok, text, alwaysTrue); + incorrectLogicOperatorError(tok, text, alwaysTrue, inconclusive); } else if (printStyle && secondTrue) { const std::string text = "If '" + cond1str + "', the comparison '" + cond2str + "' is always " + (secondTrue ? "true" : "false") + "."; - redundantConditionError(tok, text); + redundantConditionError(tok, text, inconclusive); } else if (printStyle && firstTrue) { //const std::string text = "The comparison " + cond1str + " is always " + // (firstTrue ? "true" : "false") + " when " + // cond2str + "."; const std::string text = "If '" + cond2str + "', the comparison '" + cond1str + "' is always " + (firstTrue ? "true" : "false") + "."; - redundantConditionError(tok, text); + redundantConditionError(tok, text, inconclusive); } } } } -void CheckCondition::incorrectLogicOperatorError(const Token *tok, const std::string &condition, bool always) +void CheckCondition::incorrectLogicOperatorError(const Token *tok, const std::string &condition, bool always, bool inconclusive) { if (always) reportError(tok, Severity::warning, "incorrectLogicOperator", "Logical disjunction always evaluates to true: " + condition + ".\n" "Logical disjunction always evaluates to true: " + condition + ". " - "Are these conditions necessary? Did you intend to use && instead? Are the numbers correct? Are you comparing the correct variables?", CWE571, false); + "Are these conditions necessary? Did you intend to use && instead? Are the numbers correct? Are you comparing the correct variables?", CWE571, inconclusive); else reportError(tok, Severity::warning, "incorrectLogicOperator", "Logical conjunction always evaluates to false: " + condition + ".\n" "Logical conjunction always evaluates to false: " + condition + ". " - "Are these conditions necessary? Did you intend to use || instead? Are the numbers correct? Are you comparing the correct variables?", CWE570, false); + "Are these conditions necessary? Did you intend to use || instead? Are the numbers correct? Are you comparing the correct variables?", CWE570, inconclusive); } -void CheckCondition::redundantConditionError(const Token *tok, const std::string &text) +void CheckCondition::redundantConditionError(const Token *tok, const std::string &text, bool inconclusive) { - reportError(tok, Severity::style, "redundantCondition", "Redundant condition: " + text, CWE398, false); + reportError(tok, Severity::style, "redundantCondition", "Redundant condition: " + text, CWE398, inconclusive); } //----------------------------------------------------------------------------- diff --git a/lib/checkcondition.h b/lib/checkcondition.h index 75519ceea..ec8ca1885 100644 --- a/lib/checkcondition.h +++ b/lib/checkcondition.h @@ -117,8 +117,8 @@ private: void oppositeInnerConditionError(const Token *tok1, const Token* tok2); - void incorrectLogicOperatorError(const Token *tok, const std::string &condition, bool always); - void redundantConditionError(const Token *tok, const std::string &text); + void incorrectLogicOperatorError(const Token *tok, const std::string &condition, bool always, bool inconclusive); + void redundantConditionError(const Token *tok, const std::string &text, bool inconclusive); void moduloAlwaysTrueFalseError(const Token* tok, const std::string& maxVal); @@ -137,8 +137,8 @@ private: c.multiConditionError(nullptr,1); c.mismatchingBitAndError(nullptr, 0xf0, 0, 1); c.oppositeInnerConditionError(nullptr, 0); - c.incorrectLogicOperatorError(nullptr, "foo > 3 && foo < 4", true); - c.redundantConditionError(nullptr, "If x > 11 the condition x > 10 is always true."); + c.incorrectLogicOperatorError(nullptr, "foo > 3 && foo < 4", true, false); + c.redundantConditionError(nullptr, "If x > 11 the condition x > 10 is always true.", false); c.moduloAlwaysTrueFalseError(nullptr, "1"); c.clarifyConditionError(nullptr, true, false); c.alwaysTrueFalseError(nullptr, true); diff --git a/test/testcondition.cpp b/test/testcondition.cpp index dad7a9b05..2b21a152d 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -84,10 +84,12 @@ private: TEST_CASE(checkInvalidTestForOverflow); } - void check(const char code[], const char* filename = "test.cpp") { + void check(const char code[], const char* filename = "test.cpp", bool inconclusive = false) { // Clear the error buffer.. errout.str(""); + settings0.inconclusive = inconclusive; + CheckCondition checkCondition; // Tokenize.. @@ -946,13 +948,28 @@ private: void incorrectLogicOperator6() { // char literals check("void f(char x) {\n" " if (x == '1' || x == '2') {}\n" - "}"); + "}", "test.cpp", true); ASSERT_EQUALS("", errout.str()); check("void f(char x) {\n" " if (x == '1' && x == '2') {}\n" - "}"); - TODO_ASSERT_EQUALS("error", "", errout.str()); + "}", "test.cpp", true); + ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x == '1' && x == '2'.\n", errout.str()); + + check("int f(char c) {\n" + " return (c >= 'a' && c <= 'z');\n" + "}", "test.cpp", true); + ASSERT_EQUALS("", errout.str()); + + check("int f(char c) {\n" + " return (c <= 'a' && c >= 'z');\n" + "}", "test.cpp", true); + ASSERT_EQUALS("[test.cpp:2]: (warning, inconclusive) Logical conjunction always evaluates to false: c <= 'a' && c >= 'z'.\n", errout.str()); + + check("int f(char c) {\n" + " return (c <= 'a' && c >= 'z');\n" + "}", "test.cpp", false); + ASSERT_EQUALS("", errout.str()); } void incorrectLogicOperator7() { // opposite expressions