From a114bf02930849d24b161848ac47a2af47b66c45 Mon Sep 17 00:00:00 2001 From: Ken-Patrick Lehrmann Date: Thu, 10 Sep 2020 16:18:30 +0200 Subject: [PATCH] Fix false positives with condition with || and && The value of something in the middle of a condition with mixed || and && gives no information on which branch will be taken. For instance with: ``` int f(int a, int b, bool x) {\n" if (a == 1 && (!(b == 2 && x))) { } else { if (x) { } } return 0; } ``` We can enter the if part whether x is true or false, and similarly, enter the else part whether x is true or false. Same thing with the value of b. This fixes the following false positive with above code: ``` :4:13: style: Condition 'x' is always true [knownConditionTrueFalse] if (x) { ^ :2:33: note: Assuming that condition 'x' is not redundant if (a == 6 && (!(b == 21 && x))) { ^ ``` --- lib/valueflow.cpp | 25 +++++++++++++++++++++++++ test/testvalueflow.cpp | 16 ++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index a030003aa..31409ad7b 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -4424,6 +4424,31 @@ struct ValueFlowConditionHandler { } } + { + const Token *tok2 = tok; + std::string op; + bool mixedOperators = false; + while (tok2->astParent()) { + const Token *parent = tok2->astParent(); + if (Token::Match(parent, "%oror%|&&")) { + if (op.empty()) { + op = parent->str() == "&&" ? "&&" : "||"; + } else if (op != parent->str()) { + mixedOperators = true; + break; + } + } + if (parent->str()=="!") { + op = (op == "&&" ? "||" : "&&"); + } + tok2 = parent; + } + + if (mixedOperators) { + continue; + } + } + if (top && Token::Match(top->previous(), "if|while (") && !top->previous()->isExpandedMacro()) { // does condition reassign variable? if (tok != top->astOperand2() && Token::Match(top->astOperand2(), "%oror%|&&") && diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index a149df581..d3ffb4834 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -138,6 +138,8 @@ private: TEST_CASE(valueFlowCrash); TEST_CASE(valueFlowHang); TEST_CASE(valueFlowCrashConstructorInitialization); + + TEST_CASE(valueFlowUnknownMixedOperators); } static bool isNotTokValue(const ValueFlow::Value &val) { @@ -4804,6 +4806,20 @@ private: "}"; valueOfTok(code, "path"); } + + void valueFlowUnknownMixedOperators() { + const char *code= "int f(int a, int b, bool x) {\n" + " if (a == 1 && (!(b == 2 && x))) {\n" + " } else {\n" + " if (x) {\n" + " }\n" + " }\n" + "\n" + " return 0;\n" + "}" ; + + ASSERT_EQUALS(false, testValueOfXKnown(code, 4U, 1)); + } }; REGISTER_TEST(TestValueFlow)