From e4c538cc58cd79329f191ca2bfe4416f5830d8e5 Mon Sep 17 00:00:00 2001 From: Paul Date: Thu, 24 Dec 2020 17:50:06 -0600 Subject: [PATCH 01/11] Fix issue 8871: improve check: mismatching container size conditions --- lib/checkcondition.cpp | 2 +- lib/valueflow.cpp | 58 ++++++++++++++++++++++++++++++++++++++---- test/testvalueflow.cpp | 46 +++++++++++++++++++++++++++++++++ 3 files changed, 100 insertions(+), 6 deletions(-) diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index a5e24ca6e42..516add4e9a2 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -1384,7 +1384,7 @@ void CheckCondition::alwaysTrueFalse() const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase(); for (const Scope * scope : symbolDatabase->functionScopes) { for (const Token* tok = scope->bodyStart->next(); tok != scope->bodyEnd; tok = tok->next()) { - if (tok->link()) // don't write false positives when templates are used + if (Token::simpleMatch(tok, "<") && tok->link()) // don't write false positives when templates are used continue; if (!tok->hasKnownIntValue()) continue; diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index f27e97e7064..3da51d41c39 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -98,6 +98,7 @@ #include #include #include +#include #include #include #include @@ -105,6 +106,7 @@ #include #include #include +#include #include #include @@ -333,6 +335,43 @@ static bool isComputableValue(const Token* parent, const ValueFlow::Value& value return true; } +uint32_t convertToMultiCharInt(const std::string& x) +{ + assert(x.size() < 5); + uint32_t y = 0; + int shift = (x.size()*8)-8; + for(size_t i = 0;i < x.size();++i) + { + y |= (uint32_t(uint8_t(x[i])) << shift); + shift -= 8; + } + return y; +} + +template +static T calculate(const std::string& s, T x, U y) +{ + switch(convertToMultiCharInt(s)) { + case '+': return x + y; + case '-': return x - y; + case '*': return x * y; + case '/': return x / y; + case '%': return x % y; + case '&': return x & y; + case '|': return x | y; + case '^': return x ^ y; + case '>': return x > y; + case '<': return x < y; + case '&&': return x && y; + case '||': return x || y; + case '==': return x == y; + case '!=': return x != y; + case '>=': return x >= y; + case '<=': return x <= y; + } + throw std::runtime_error("Unknown operator: " + s); +} + /** Set token value for cast */ static void setTokenValueCast(Token *parent, const ValueType &valueType, const ValueFlow::Value &value, const Settings *settings); @@ -351,24 +390,33 @@ static void setTokenValue(Token* tok, const ValueFlow::Value &value, const Setti if (value.isContainerSizeValue()) { // .empty, .size, +"abc", +'a' - if (parent->str() == "+" && parent->astOperand1() && parent->astOperand2()) { + if (Token::Match(parent, "+|==|!=") && parent->astOperand1() && parent->astOperand2()) { for (const ValueFlow::Value &value1 : parent->astOperand1()->values()) { for (const ValueFlow::Value &value2 : parent->astOperand2()->values()) { if (value1.path != value2.path) continue; ValueFlow::Value result; - result.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE; + if (Token::Match(parent, "%comp%")) + result.valueType = ValueFlow::Value::ValueType::INT; + else + result.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE; + if (value1.isContainerSizeValue() && value2.isContainerSizeValue()) - result.intvalue = value1.intvalue + value2.intvalue; + result.intvalue = calculate(parent->str(), value1.intvalue, value2.intvalue); else if (value1.isContainerSizeValue() && value2.isTokValue() && value2.tokvalue->tokType() == Token::eString) - result.intvalue = value1.intvalue + Token::getStrLength(value2.tokvalue); + result.intvalue = calculate(parent->str(), value1.intvalue, Token::getStrLength(value2.tokvalue)); else if (value2.isContainerSizeValue() && value1.isTokValue() && value1.tokvalue->tokType() == Token::eString) - result.intvalue = Token::getStrLength(value1.tokvalue) + value2.intvalue; + result.intvalue = calculate(parent->str(), Token::getStrLength(value1.tokvalue), value2.intvalue); else continue; combineValueProperties(value1, value2, &result); + if (Token::simpleMatch(parent, "==") && result.intvalue) + continue; + if (Token::simpleMatch(parent, "!=") && !result.intvalue) + continue; + setTokenValue(parent, result, settings); } } diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index ccc2ab1e7a2..f07e7c6981d 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -4771,6 +4771,52 @@ class TestValueFlow : public TestFixture { "}"; ASSERT_EQUALS("", isKnownContainerSizeValue(tokenValues(code, "ints . front", ValueFlow::Value::CONTAINER_SIZE), 1)); + + code = "void f(std::string str) {\n" + " if (str == \"123\")\n" + " bool x = str.empty();\n" + "}\n"; + ASSERT_EQUALS("", isKnownContainerSizeValue(tokenValues(code, "str . empty", ValueFlow::Value::CONTAINER_SIZE), 3)); + + code = "void f(std::string str) {\n" + " if (str == \"123\") {\n" + " bool x = (str == \"\");\n" + " bool a = x;\n" + " }\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfXKnown(code, 4U, 0)); + + code = "void f(std::string str) {\n" + " if (str == \"123\") {\n" + " bool x = (str != \"\");\n" + " bool a = x;\n" + " }\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfXKnown(code, 4U, 1)); + + code = "void f(std::string str) {\n" + " if (str == \"123\") {\n" + " bool x = (str == \"321\");\n" + " bool a = x;\n" + " }\n" + "}\n"; + ASSERT_EQUALS(false, testValueOfXKnown(code, 4U, 1)); + + code = "void f(std::string str) {\n" + " if (str == \"123\") {\n" + " bool x = (str != \"321\");\n" + " bool a = x;\n" + " }\n" + "}\n"; + ASSERT_EQUALS(false, testValueOfXKnown(code, 4U, 0)); + + code = "void f(std::string str) {\n" + " if (str.size() == 1) {\n" + " bool x = (str == \"123\");\n" + " bool a = x;\n" + " }\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfXKnown(code, 4U, 0)); } void valueFlowDynamicBufferSize() { From 6ab32a5d2f3838fd686738df9b9a3d0c597cdd99 Mon Sep 17 00:00:00 2001 From: Paul Date: Fri, 25 Dec 2020 13:08:56 -0600 Subject: [PATCH 02/11] Reuse calculate function --- lib/valueflow.cpp | 327 +++++++++++++++++++++++++++------------------- 1 file changed, 192 insertions(+), 135 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 3da51d41c39..54a5427b1d0 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -348,6 +348,22 @@ uint32_t convertToMultiCharInt(const std::string& x) return y; } +template +T asInt(T x) +{ + return x; +} + +MathLib::bigint asInt(float x) +{ + return x; +} + +MathLib::bigint asInt(double x) +{ + return x; +} + template static T calculate(const std::string& s, T x, U y) { @@ -356,12 +372,14 @@ static T calculate(const std::string& s, T x, U y) case '-': return x - y; case '*': return x * y; case '/': return x / y; - case '%': return x % y; - case '&': return x & y; - case '|': return x | y; - case '^': return x ^ y; + case '%': return asInt(x) % asInt(y); + case '&': return asInt(x) & asInt(y); + case '|': return asInt(x) | asInt(y); + case '^': return asInt(x) ^ asInt(y); case '>': return x > y; case '<': return x < y; + case '<<': return asInt(x) << asInt(y); + case '>>': return asInt(x) >> asInt(y); case '&&': return x && y; case '||': return x || y; case '==': return x == y; @@ -585,148 +603,187 @@ static void setTokenValue(Token* tok, const ValueFlow::Value &value, const Setti combineValueProperties(value1, value2, &result); const double floatValue1 = value1.isIntValue() ? value1.intvalue : value1.floatValue; const double floatValue2 = value2.isIntValue() ? value2.intvalue : value2.floatValue; - switch (parent->str()[0]) { - case '+': - if (value1.isTokValue() || value2.isTokValue()) - break; - if (value1.isFloatValue() || value2.isFloatValue()) { - result.valueType = ValueFlow::Value::FLOAT; - result.floatValue = floatValue1 + floatValue2; + const bool isFloat = value1.isFloatValue() || value2.isFloatValue(); + if (isFloat && Token::Match(parent, "&|^|%|<<|>>|%or%")) + continue; + if (Token::Match(parent, "<<|>>") && !(value1.intvalue >= 0 && value2.intvalue >= 0 && value2.intvalue < MathLib::bigint_bits)) + continue; + if (Token::Match(parent, "/|%") && floatValue2 == 0) + continue; + if (Token::Match(parent, "==|!=")) { + if ((value1.isIntValue() && value2.isTokValue()) || + (value1.isTokValue() && value2.isIntValue())) { + if (parent->str() == "==") + result.intvalue = 0; + else if (parent->str() == "!=") + result.intvalue = 1; + } else if (value1.isIntValue() && value2.isIntValue()) { + result.intvalue = calculate(parent->str(), value1.intvalue, value2.intvalue); } else { - result.intvalue = value1.intvalue + value2.intvalue; + continue; } setTokenValue(parent, result, settings); - break; - case '-': - if (value1.isTokValue() || value2.isTokValue()) - break; - if (value1.isFloatValue() || value2.isFloatValue()) { - result.valueType = ValueFlow::Value::FLOAT; - result.floatValue = floatValue1 - floatValue2; - } else { - // Avoid overflow - if (value1.intvalue < 0 && value2.intvalue > value1.intvalue - LLONG_MIN) - break; - - result.intvalue = value1.intvalue - value2.intvalue; - } - // If the bound comes from the second value then invert the bound - if (value2.bound == result.bound && value2.bound != ValueFlow::Value::Bound::Point) - result.invertBound(); + } else if (Token::Match(parent, "%comp%")) { + if (!isFloat && !value1.isIntValue() && !value2.isIntValue()) + continue; + if (isFloat) + result.intvalue = calculate(parent->str(), floatValue1, floatValue2); + else + result.intvalue = calculate(parent->str(), value1.intvalue, value2.intvalue); setTokenValue(parent, result, settings); - break; - case '*': + } else if (Token::Match(parent, "%op%")) { if (value1.isTokValue() || value2.isTokValue()) break; - if (value1.isFloatValue() || value2.isFloatValue()) { - result.valueType = ValueFlow::Value::FLOAT; - result.floatValue = floatValue1 * floatValue2; - } else { - result.intvalue = value1.intvalue * value2.intvalue; - } - setTokenValue(parent, result, settings); - break; - case '/': - if (value1.isTokValue() || value2.isTokValue() || value2.intvalue == 0) - break; - if (value1.isFloatValue() || value2.isFloatValue()) { + if (isFloat) { result.valueType = ValueFlow::Value::FLOAT; - result.floatValue = floatValue1 / floatValue2; + result.floatValue = calculate(parent->str(), floatValue1, floatValue2); } else { - result.intvalue = value1.intvalue / value2.intvalue; + result.intvalue = calculate(parent->str(), value1.intvalue, value2.intvalue); } setTokenValue(parent, result, settings); - break; - case '%': - if (!value1.isIntValue() || !value2.isIntValue()) - break; - if (value2.intvalue == 0) - break; - result.intvalue = value1.intvalue % value2.intvalue; - setTokenValue(parent, result, settings); - break; - case '=': - if (parent->str() == "==") { - if ((value1.isIntValue() && value2.isTokValue()) || - (value1.isTokValue() && value2.isIntValue())) { - result.intvalue = 0; - setTokenValue(parent, result, settings); - } else if (value1.isIntValue() && value2.isIntValue()) { - result.intvalue = value1.intvalue == value2.intvalue; - setTokenValue(parent, result, settings); - } - } - break; - case '!': - if (parent->str() == "!=") { - if ((value1.isIntValue() && value2.isTokValue()) || - (value1.isTokValue() && value2.isIntValue())) { - result.intvalue = 1; - setTokenValue(parent, result, settings); - } else if (value1.isIntValue() && value2.isIntValue()) { - result.intvalue = value1.intvalue != value2.intvalue; - setTokenValue(parent, result, settings); - } - } - break; - case '>': { - const bool f = value1.isFloatValue() || value2.isFloatValue(); - if (!f && !value1.isIntValue() && !value2.isIntValue()) - break; - if (parent->str() == ">") - result.intvalue = f ? (floatValue1 > floatValue2) : (value1.intvalue > value2.intvalue); - else if (parent->str() == ">=") - result.intvalue = f ? (floatValue1 >= floatValue2) : (value1.intvalue >= value2.intvalue); - else if (!f && parent->str() == ">>" && value1.intvalue >= 0 && value2.intvalue >= 0 && value2.intvalue < MathLib::bigint_bits) - result.intvalue = value1.intvalue >> value2.intvalue; - else - break; - setTokenValue(parent, result, settings); - break; - } - case '<': { - const bool f = value1.isFloatValue() || value2.isFloatValue(); - if (!f && !value1.isIntValue() && !value2.isIntValue()) - break; - if (parent->str() == "<") - result.intvalue = f ? (floatValue1 < floatValue2) : (value1.intvalue < value2.intvalue); - else if (parent->str() == "<=") - result.intvalue = f ? (floatValue1 <= floatValue2) : (value1.intvalue <= value2.intvalue); - else if (!f && parent->str() == "<<" && value1.intvalue >= 0 && value2.intvalue >= 0 && value2.intvalue < MathLib::bigint_bits) - result.intvalue = value1.intvalue << value2.intvalue; - else - break; - setTokenValue(parent, result, settings); - break; - } - case '&': - if (!value1.isIntValue() || !value2.isIntValue()) - break; - if (parent->str() == "&") - result.intvalue = value1.intvalue & value2.intvalue; - else - result.intvalue = value1.intvalue && value2.intvalue; - setTokenValue(parent, result, settings); - break; - case '|': - if (!value1.isIntValue() || !value2.isIntValue()) - break; - if (parent->str() == "|") - result.intvalue = value1.intvalue | value2.intvalue; - else - result.intvalue = value1.intvalue || value2.intvalue; - setTokenValue(parent, result, settings); - break; - case '^': - if (!value1.isIntValue() || !value2.isIntValue()) - break; - result.intvalue = value1.intvalue ^ value2.intvalue; - setTokenValue(parent, result, settings); - break; - default: - // unhandled operator, do nothing - break; } + // switch (parent->str()[0]) { + // case '+': + // if (value1.isTokValue() || value2.isTokValue()) + // break; + // if (value1.isFloatValue() || value2.isFloatValue()) { + // result.valueType = ValueFlow::Value::FLOAT; + // result.floatValue = floatValue1 + floatValue2; + // } else { + // result.intvalue = value1.intvalue + value2.intvalue; + // } + // setTokenValue(parent, result, settings); + // break; + // case '-': + // if (value1.isTokValue() || value2.isTokValue()) + // break; + // if (value1.isFloatValue() || value2.isFloatValue()) { + // result.valueType = ValueFlow::Value::FLOAT; + // result.floatValue = floatValue1 - floatValue2; + // } else { + // // Avoid overflow + // if (value1.intvalue < 0 && value2.intvalue > value1.intvalue - LLONG_MIN) + // break; + + // result.intvalue = value1.intvalue - value2.intvalue; + // } + // // If the bound comes from the second value then invert the bound + // if (value2.bound == result.bound && value2.bound != ValueFlow::Value::Bound::Point) + // result.invertBound(); + // setTokenValue(parent, result, settings); + // break; + // case '*': + // if (value1.isTokValue() || value2.isTokValue()) + // break; + // if (value1.isFloatValue() || value2.isFloatValue()) { + // result.valueType = ValueFlow::Value::FLOAT; + // result.floatValue = floatValue1 * floatValue2; + // } else { + // result.intvalue = value1.intvalue * value2.intvalue; + // } + // setTokenValue(parent, result, settings); + // break; + // case '/': + // if (value1.isTokValue() || value2.isTokValue() || value2.intvalue == 0) + // break; + // if (value1.isFloatValue() || value2.isFloatValue()) { + // result.valueType = ValueFlow::Value::FLOAT; + // result.floatValue = floatValue1 / floatValue2; + // } else { + // result.intvalue = value1.intvalue / value2.intvalue; + // } + // setTokenValue(parent, result, settings); + // break; + // case '%': + // if (!value1.isIntValue() || !value2.isIntValue()) + // break; + // if (value2.intvalue == 0) + // break; + // result.intvalue = value1.intvalue % value2.intvalue; + // setTokenValue(parent, result, settings); + // break; + // case '=': + // if (parent->str() == "==") { + // if ((value1.isIntValue() && value2.isTokValue()) || + // (value1.isTokValue() && value2.isIntValue())) { + // result.intvalue = 0; + // setTokenValue(parent, result, settings); + // } else if (value1.isIntValue() && value2.isIntValue()) { + // result.intvalue = value1.intvalue == value2.intvalue; + // setTokenValue(parent, result, settings); + // } + // } + // break; + // case '!': + // if (parent->str() == "!=") { + // if ((value1.isIntValue() && value2.isTokValue()) || + // (value1.isTokValue() && value2.isIntValue())) { + // result.intvalue = 1; + // setTokenValue(parent, result, settings); + // } else if (value1.isIntValue() && value2.isIntValue()) { + // result.intvalue = value1.intvalue != value2.intvalue; + // setTokenValue(parent, result, settings); + // } + // } + // break; + // case '>': { + // const bool f = value1.isFloatValue() || value2.isFloatValue(); + // if (!f && !value1.isIntValue() && !value2.isIntValue()) + // break; + // if (parent->str() == ">") + // result.intvalue = f ? (floatValue1 > floatValue2) : (value1.intvalue > value2.intvalue); + // else if (parent->str() == ">=") + // result.intvalue = f ? (floatValue1 >= floatValue2) : (value1.intvalue >= value2.intvalue); + // else if (!f && parent->str() == ">>" && value1.intvalue >= 0 && value2.intvalue >= 0 && value2.intvalue < MathLib::bigint_bits) + // result.intvalue = value1.intvalue >> value2.intvalue; + // else + // break; + // setTokenValue(parent, result, settings); + // break; + // } + // case '<': { + // const bool f = value1.isFloatValue() || value2.isFloatValue(); + // if (!f && !value1.isIntValue() && !value2.isIntValue()) + // break; + // if (parent->str() == "<") + // result.intvalue = f ? (floatValue1 < floatValue2) : (value1.intvalue < value2.intvalue); + // else if (parent->str() == "<=") + // result.intvalue = f ? (floatValue1 <= floatValue2) : (value1.intvalue <= value2.intvalue); + // else if (!f && parent->str() == "<<" && value1.intvalue >= 0 && value2.intvalue >= 0 && value2.intvalue < MathLib::bigint_bits) + // result.intvalue = value1.intvalue << value2.intvalue; + // else + // break; + // setTokenValue(parent, result, settings); + // break; + // } + // case '&': + // if (!value1.isIntValue() || !value2.isIntValue()) + // break; + // if (parent->str() == "&") + // result.intvalue = value1.intvalue & value2.intvalue; + // else + // result.intvalue = value1.intvalue && value2.intvalue; + // setTokenValue(parent, result, settings); + // break; + // case '|': + // if (!value1.isIntValue() || !value2.isIntValue()) + // break; + // if (parent->str() == "|") + // result.intvalue = value1.intvalue | value2.intvalue; + // else + // result.intvalue = value1.intvalue || value2.intvalue; + // setTokenValue(parent, result, settings); + // break; + // case '^': + // if (!value1.isIntValue() || !value2.isIntValue()) + // break; + // result.intvalue = value1.intvalue ^ value2.intvalue; + // setTokenValue(parent, result, settings); + // break; + // default: + // // unhandled operator, do nothing + // break; + // } } } } From 2a05480fcf3bcd5988d5c4aa0ffbfa6acdd24a60 Mon Sep 17 00:00:00 2001 From: Paul Date: Fri, 25 Dec 2020 20:38:07 -0600 Subject: [PATCH 03/11] Remove unused code --- lib/valueflow.cpp | 145 +--------------------------------------------- 1 file changed, 3 insertions(+), 142 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 54a5427b1d0..0e9b3062962 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -640,150 +640,11 @@ static void setTokenValue(Token* tok, const ValueFlow::Value &value, const Setti } else { result.intvalue = calculate(parent->str(), value1.intvalue, value2.intvalue); } + // If the bound comes from the second value then invert the bound when subtracting + if (Token::simpleMatch(parent, "-") && value2.bound == result.bound && value2.bound != ValueFlow::Value::Bound::Point) + result.invertBound(); setTokenValue(parent, result, settings); } - // switch (parent->str()[0]) { - // case '+': - // if (value1.isTokValue() || value2.isTokValue()) - // break; - // if (value1.isFloatValue() || value2.isFloatValue()) { - // result.valueType = ValueFlow::Value::FLOAT; - // result.floatValue = floatValue1 + floatValue2; - // } else { - // result.intvalue = value1.intvalue + value2.intvalue; - // } - // setTokenValue(parent, result, settings); - // break; - // case '-': - // if (value1.isTokValue() || value2.isTokValue()) - // break; - // if (value1.isFloatValue() || value2.isFloatValue()) { - // result.valueType = ValueFlow::Value::FLOAT; - // result.floatValue = floatValue1 - floatValue2; - // } else { - // // Avoid overflow - // if (value1.intvalue < 0 && value2.intvalue > value1.intvalue - LLONG_MIN) - // break; - - // result.intvalue = value1.intvalue - value2.intvalue; - // } - // // If the bound comes from the second value then invert the bound - // if (value2.bound == result.bound && value2.bound != ValueFlow::Value::Bound::Point) - // result.invertBound(); - // setTokenValue(parent, result, settings); - // break; - // case '*': - // if (value1.isTokValue() || value2.isTokValue()) - // break; - // if (value1.isFloatValue() || value2.isFloatValue()) { - // result.valueType = ValueFlow::Value::FLOAT; - // result.floatValue = floatValue1 * floatValue2; - // } else { - // result.intvalue = value1.intvalue * value2.intvalue; - // } - // setTokenValue(parent, result, settings); - // break; - // case '/': - // if (value1.isTokValue() || value2.isTokValue() || value2.intvalue == 0) - // break; - // if (value1.isFloatValue() || value2.isFloatValue()) { - // result.valueType = ValueFlow::Value::FLOAT; - // result.floatValue = floatValue1 / floatValue2; - // } else { - // result.intvalue = value1.intvalue / value2.intvalue; - // } - // setTokenValue(parent, result, settings); - // break; - // case '%': - // if (!value1.isIntValue() || !value2.isIntValue()) - // break; - // if (value2.intvalue == 0) - // break; - // result.intvalue = value1.intvalue % value2.intvalue; - // setTokenValue(parent, result, settings); - // break; - // case '=': - // if (parent->str() == "==") { - // if ((value1.isIntValue() && value2.isTokValue()) || - // (value1.isTokValue() && value2.isIntValue())) { - // result.intvalue = 0; - // setTokenValue(parent, result, settings); - // } else if (value1.isIntValue() && value2.isIntValue()) { - // result.intvalue = value1.intvalue == value2.intvalue; - // setTokenValue(parent, result, settings); - // } - // } - // break; - // case '!': - // if (parent->str() == "!=") { - // if ((value1.isIntValue() && value2.isTokValue()) || - // (value1.isTokValue() && value2.isIntValue())) { - // result.intvalue = 1; - // setTokenValue(parent, result, settings); - // } else if (value1.isIntValue() && value2.isIntValue()) { - // result.intvalue = value1.intvalue != value2.intvalue; - // setTokenValue(parent, result, settings); - // } - // } - // break; - // case '>': { - // const bool f = value1.isFloatValue() || value2.isFloatValue(); - // if (!f && !value1.isIntValue() && !value2.isIntValue()) - // break; - // if (parent->str() == ">") - // result.intvalue = f ? (floatValue1 > floatValue2) : (value1.intvalue > value2.intvalue); - // else if (parent->str() == ">=") - // result.intvalue = f ? (floatValue1 >= floatValue2) : (value1.intvalue >= value2.intvalue); - // else if (!f && parent->str() == ">>" && value1.intvalue >= 0 && value2.intvalue >= 0 && value2.intvalue < MathLib::bigint_bits) - // result.intvalue = value1.intvalue >> value2.intvalue; - // else - // break; - // setTokenValue(parent, result, settings); - // break; - // } - // case '<': { - // const bool f = value1.isFloatValue() || value2.isFloatValue(); - // if (!f && !value1.isIntValue() && !value2.isIntValue()) - // break; - // if (parent->str() == "<") - // result.intvalue = f ? (floatValue1 < floatValue2) : (value1.intvalue < value2.intvalue); - // else if (parent->str() == "<=") - // result.intvalue = f ? (floatValue1 <= floatValue2) : (value1.intvalue <= value2.intvalue); - // else if (!f && parent->str() == "<<" && value1.intvalue >= 0 && value2.intvalue >= 0 && value2.intvalue < MathLib::bigint_bits) - // result.intvalue = value1.intvalue << value2.intvalue; - // else - // break; - // setTokenValue(parent, result, settings); - // break; - // } - // case '&': - // if (!value1.isIntValue() || !value2.isIntValue()) - // break; - // if (parent->str() == "&") - // result.intvalue = value1.intvalue & value2.intvalue; - // else - // result.intvalue = value1.intvalue && value2.intvalue; - // setTokenValue(parent, result, settings); - // break; - // case '|': - // if (!value1.isIntValue() || !value2.isIntValue()) - // break; - // if (parent->str() == "|") - // result.intvalue = value1.intvalue | value2.intvalue; - // else - // result.intvalue = value1.intvalue || value2.intvalue; - // setTokenValue(parent, result, settings); - // break; - // case '^': - // if (!value1.isIntValue() || !value2.isIntValue()) - // break; - // result.intvalue = value1.intvalue ^ value2.intvalue; - // setTokenValue(parent, result, settings); - // break; - // default: - // // unhandled operator, do nothing - // break; - // } } } } From ce28b8e4f191ee3acc3f0fe6c1b707127a281324 Mon Sep 17 00:00:00 2001 From: Paul Date: Fri, 25 Dec 2020 20:38:35 -0600 Subject: [PATCH 04/11] Formatting --- lib/valueflow.cpp | 92 ++++++++++++++++++++++++------------------ test/testvalueflow.cpp | 3 +- 2 files changed, 54 insertions(+), 41 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 0e9b3062962..202a659e43d 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -339,53 +339,64 @@ uint32_t convertToMultiCharInt(const std::string& x) { assert(x.size() < 5); uint32_t y = 0; - int shift = (x.size()*8)-8; - for(size_t i = 0;i < x.size();++i) - { + int shift = (x.size() * 8) - 8; + for (size_t i = 0; i < x.size(); ++i) { y |= (uint32_t(uint8_t(x[i])) << shift); shift -= 8; } return y; } -template +template T asInt(T x) { return x; } -MathLib::bigint asInt(float x) -{ - return x; -} +MathLib::bigint asInt(float x) { return x; } -MathLib::bigint asInt(double x) -{ - return x; -} +MathLib::bigint asInt(double x) { return x; } -template +template static T calculate(const std::string& s, T x, U y) { - switch(convertToMultiCharInt(s)) { - case '+': return x + y; - case '-': return x - y; - case '*': return x * y; - case '/': return x / y; - case '%': return asInt(x) % asInt(y); - case '&': return asInt(x) & asInt(y); - case '|': return asInt(x) | asInt(y); - case '^': return asInt(x) ^ asInt(y); - case '>': return x > y; - case '<': return x < y; - case '<<': return asInt(x) << asInt(y); - case '>>': return asInt(x) >> asInt(y); - case '&&': return x && y; - case '||': return x || y; - case '==': return x == y; - case '!=': return x != y; - case '>=': return x >= y; - case '<=': return x <= y; + switch (convertToMultiCharInt(s)) { + case '+': + return x + y; + case '-': + return x - y; + case '*': + return x * y; + case '/': + return x / y; + case '%': + return asInt(x) % asInt(y); + case '&': + return asInt(x) & asInt(y); + case '|': + return asInt(x) | asInt(y); + case '^': + return asInt(x) ^ asInt(y); + case '>': + return x > y; + case '<': + return x < y; + case '<<': + return asInt(x) << asInt(y); + case '>>': + return asInt(x) >> asInt(y); + case '&&': + return x && y; + case '||': + return x || y; + case '==': + return x == y; + case '!=': + return x != y; + case '>=': + return x >= y; + case '<=': + return x <= y; } throw std::runtime_error("Unknown operator: " + s); } @@ -440,8 +451,8 @@ static void setTokenValue(Token* tok, const ValueFlow::Value &value, const Setti } } - - else if (Token::Match(parent, ". %name% (") && parent->astParent() == parent->tokAt(2) && parent->astOperand1() && parent->astOperand1()->valueType()) { + else if (Token::Match(parent, ". %name% (") && parent->astParent() == parent->tokAt(2) && + parent->astOperand1() && parent->astOperand1()->valueType()) { const Library::Container *c = parent->astOperand1()->valueType()->container; const Library::Container::Yield yields = c ? c->getYield(parent->strAt(1)) : Library::Container::Yield::NO_YIELD; if (yields == Library::Container::Yield::SIZE) { @@ -606,17 +617,17 @@ static void setTokenValue(Token* tok, const ValueFlow::Value &value, const Setti const bool isFloat = value1.isFloatValue() || value2.isFloatValue(); if (isFloat && Token::Match(parent, "&|^|%|<<|>>|%or%")) continue; - if (Token::Match(parent, "<<|>>") && !(value1.intvalue >= 0 && value2.intvalue >= 0 && value2.intvalue < MathLib::bigint_bits)) + if (Token::Match(parent, "<<|>>") && + !(value1.intvalue >= 0 && value2.intvalue >= 0 && value2.intvalue < MathLib::bigint_bits)) continue; if (Token::Match(parent, "/|%") && floatValue2 == 0) continue; if (Token::Match(parent, "==|!=")) { - if ((value1.isIntValue() && value2.isTokValue()) || - (value1.isTokValue() && value2.isIntValue())) { + if ((value1.isIntValue() && value2.isTokValue()) || (value1.isTokValue() && value2.isIntValue())) { if (parent->str() == "==") - result.intvalue = 0; + result.intvalue = 0; else if (parent->str() == "!=") - result.intvalue = 1; + result.intvalue = 1; } else if (value1.isIntValue() && value2.isIntValue()) { result.intvalue = calculate(parent->str(), value1.intvalue, value2.intvalue); } else { @@ -641,7 +652,8 @@ static void setTokenValue(Token* tok, const ValueFlow::Value &value, const Setti result.intvalue = calculate(parent->str(), value1.intvalue, value2.intvalue); } // If the bound comes from the second value then invert the bound when subtracting - if (Token::simpleMatch(parent, "-") && value2.bound == result.bound && value2.bound != ValueFlow::Value::Bound::Point) + if (Token::simpleMatch(parent, "-") && value2.bound == result.bound && + value2.bound != ValueFlow::Value::Bound::Point) result.invertBound(); setTokenValue(parent, result, settings); } diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index f07e7c6981d..b2503c04ab5 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -4776,7 +4776,8 @@ class TestValueFlow : public TestFixture { " if (str == \"123\")\n" " bool x = str.empty();\n" "}\n"; - ASSERT_EQUALS("", isKnownContainerSizeValue(tokenValues(code, "str . empty", ValueFlow::Value::CONTAINER_SIZE), 3)); + ASSERT_EQUALS("", + isKnownContainerSizeValue(tokenValues(code, "str . empty", ValueFlow::Value::CONTAINER_SIZE), 3)); code = "void f(std::string str) {\n" " if (str == \"123\") {\n" From 74078a8ef0969dee0375146d8a2bba73fd70373b Mon Sep 17 00:00:00 2001 From: Paul Date: Sun, 27 Dec 2020 18:01:00 -0600 Subject: [PATCH 05/11] Use encodeMultiChar --- lib/mathlib.cpp | 2 +- lib/mathlib.h | 2 ++ lib/valueflow.cpp | 14 +------------- 3 files changed, 4 insertions(+), 14 deletions(-) diff --git a/lib/mathlib.cpp b/lib/mathlib.cpp index 881afc380e7..aa2668d4108 100644 --- a/lib/mathlib.cpp +++ b/lib/mathlib.cpp @@ -346,7 +346,7 @@ MathLib::biguint MathLib::toULongNumber(const std::string & str) return ret; } -static unsigned int encodeMultiChar(const std::string& str) +unsigned int MathLib::encodeMultiChar(const std::string& str) { unsigned int retval = 0; for (char it : str) { diff --git a/lib/mathlib.h b/lib/mathlib.h index 3b41d6c4506..a0834823558 100644 --- a/lib/mathlib.h +++ b/lib/mathlib.h @@ -125,6 +125,8 @@ class CPPCHECKLIB MathLib { */ static bool isOctalDigit(char c); + static unsigned int encodeMultiChar(const std::string& str); + /** * \param[in] str character literal * @return Number of internal representation of the character literal diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 202a659e43d..372724562a1 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -335,18 +335,6 @@ static bool isComputableValue(const Token* parent, const ValueFlow::Value& value return true; } -uint32_t convertToMultiCharInt(const std::string& x) -{ - assert(x.size() < 5); - uint32_t y = 0; - int shift = (x.size() * 8) - 8; - for (size_t i = 0; i < x.size(); ++i) { - y |= (uint32_t(uint8_t(x[i])) << shift); - shift -= 8; - } - return y; -} - template T asInt(T x) { @@ -360,7 +348,7 @@ MathLib::bigint asInt(double x) { return x; } template static T calculate(const std::string& s, T x, U y) { - switch (convertToMultiCharInt(s)) { + switch (MathLib::encodeMultiChar(s)) { case '+': return x + y; case '-': From 2c9d799bd4568c31d277f65b9b85a1eaa42d3287 Mon Sep 17 00:00:00 2001 From: Paul Date: Sun, 27 Dec 2020 18:04:19 -0600 Subject: [PATCH 06/11] Remove header --- lib/valueflow.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 372724562a1..9d091b7866e 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -98,7 +98,6 @@ #include #include #include -#include #include #include #include From a520e98497b315c1ea08003675ea6a262c18e079 Mon Sep 17 00:00:00 2001 From: Paul Date: Mon, 28 Dec 2020 16:50:10 -0600 Subject: [PATCH 07/11] Throw internal error instead --- lib/valueflow.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 9d091b7866e..8da49cb98c5 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -80,6 +80,7 @@ #include "analyzer.h" #include "astutils.h" #include "errorlogger.h" +#include "errortypes.h" #include "forwardanalyzer.h" #include "library.h" #include "mathlib.h" @@ -385,7 +386,7 @@ static T calculate(const std::string& s, T x, U y) case '<=': return x <= y; } - throw std::runtime_error("Unknown operator: " + s); + throw InternalError(nullptr, "Unknown operator: " + s); } /** Set token value for cast */ From 5cfeb200a8723beba51af06f8272c6268a140078 Mon Sep 17 00:00:00 2001 From: Paul Date: Fri, 8 Jan 2021 22:15:32 -0600 Subject: [PATCH 08/11] Remove useless branch --- lib/checkuninitvar.cpp | 8 -------- 1 file changed, 8 deletions(-) diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index c1b9d645498..57ad5c64ecc 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -266,14 +266,6 @@ static void conditionAlwaysTrueOrFalse(const Token *tok, const std::mapisComparisonOp()) { - if (tok->hasKnownIntValue()) { - if (tok->values().front().intvalue) - *alwaysTrue = true; - else - *alwaysFalse = true; - return; - } - if (variableValue.empty()) { return; } From 6d111712e2027d900de62f7473b4f6ef12e72b2f Mon Sep 17 00:00:00 2001 From: Paul Date: Sat, 9 Jan 2021 12:07:56 -0600 Subject: [PATCH 09/11] Dont warn on simple functions that always return true or false --- lib/checkcondition.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index 3aed363d2bb..3605b67cc97 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -1389,6 +1389,11 @@ void CheckCondition::alwaysTrueFalse() continue; if (!tok->hasKnownIntValue()) continue; + if (Token::Match(tok->previous(), "%name% (") && tok->previous()->function()) { + const Function* f = tok->previous()->function(); + if (f->functionScope && Token::Match(f->functionScope->bodyStart, "{ return true|false ;")) + continue; + } { // is this a condition.. const Token *parent = tok->astParent(); From afc27b5aeaed5ab6d721f31f838ecfad726a1190 Mon Sep 17 00:00:00 2001 From: Paul Date: Sat, 9 Jan 2021 12:26:37 -0600 Subject: [PATCH 10/11] Fix negative index --- lib/clangimport.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/clangimport.cpp b/lib/clangimport.cpp index 9ee156b508b..067b69a72b6 100644 --- a/lib/clangimport.cpp +++ b/lib/clangimport.cpp @@ -390,7 +390,7 @@ std::string clangimport::AstNode::getSpelling() const while (typeIndex > 0 && std::isalpha(mExtTokens[typeIndex][0])) typeIndex--; } - const std::string &str = mExtTokens[typeIndex - 1]; + const std::string &str = mExtTokens[typeIndex > 0 ? typeIndex - 1 : 0]; if (str.compare(0,4,"col:") == 0) return ""; if (str.compare(0,8," Date: Sat, 9 Jan 2021 12:29:10 -0600 Subject: [PATCH 11/11] Use std::max instead --- lib/clangimport.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/clangimport.cpp b/lib/clangimport.cpp index 067b69a72b6..ff54f8964bf 100644 --- a/lib/clangimport.cpp +++ b/lib/clangimport.cpp @@ -390,7 +390,7 @@ std::string clangimport::AstNode::getSpelling() const while (typeIndex > 0 && std::isalpha(mExtTokens[typeIndex][0])) typeIndex--; } - const std::string &str = mExtTokens[typeIndex > 0 ? typeIndex - 1 : 0]; + const std::string &str = mExtTokens[std::max(typeIndex - 1, 0)]; if (str.compare(0,4,"col:") == 0) return ""; if (str.compare(0,8,"