From d05e224b5b371b6d303396183901fb713160c8b8 Mon Sep 17 00:00:00 2001 From: Rikard Falkeborn Date: Thu, 5 Jul 2018 12:03:46 +0200 Subject: [PATCH 1/7] Refactor loadLibErrors This reduces the amount of code slightly and will simplify adding more tests. --- test/testlibrary.cpp | 85 ++++++++++++++++++++------------------------ 1 file changed, 38 insertions(+), 47 deletions(-) diff --git a/test/testlibrary.cpp b/test/testlibrary.cpp index 1fedf43938e..fb7acae660f 100644 --- a/test/testlibrary.cpp +++ b/test/testlibrary.cpp @@ -724,54 +724,45 @@ class TestLibrary : public TestFixture { } } + void loadLibError(const char xmldata [], Library::ErrorCode errorcode, const char* file, unsigned line) const { + Library library; + assertEquals(file, line, errorcode, readLibrary(library, xmldata).errorcode); + } + +#define LOADLIBERROR(xmldata, errorcode) loadLibError(xmldata, errorcode, __FILE__, __LINE__) + void loadLibErrors() const { - // UNKNOWN_ELEMENT - { - const char xmldata [] = "\n" - "\n" - " \n" - ""; - Library library; - ASSERT_EQUALS(Library::UNKNOWN_ELEMENT, readLibrary(library, xmldata).errorcode); - } - // MISSING_ATTRIBUTE - { - // #define without attributes - { - const char xmldata [] = "\n" - "\n" - " \n" // no attributes provided at all - ""; - Library library; - ASSERT_EQUALS(Library::MISSING_ATTRIBUTE, readLibrary(library, xmldata).errorcode); - } - // #define with name but without value - { - const char xmldata [] = "\n" - "\n" - " \n" // no value provided - ""; - Library library; - ASSERT_EQUALS(Library::MISSING_ATTRIBUTE, readLibrary(library, xmldata).errorcode); - } - // #define with value but without a name - { - const char xmldata [] = "\n" - "\n" - " \n" // no name provided - ""; - Library library; - ASSERT_EQUALS(Library::MISSING_ATTRIBUTE, readLibrary(library, xmldata).errorcode); - } - } - // UNSUPPORTED_FORMAT - { - const char xmldata [] = "\n" - "\n" - ""; - Library library; - ASSERT_EQUALS(Library::UNSUPPORTED_FORMAT, readLibrary(library, xmldata).errorcode); - } + + LOADLIBERROR("\n" + "\n" + " \n" + "", + Library::UNKNOWN_ELEMENT); + + // #define without attributes + LOADLIBERROR("\n" + "\n" + " \n" // no attributes provided at all + "", + Library::MISSING_ATTRIBUTE); + + // #define with name but without value + LOADLIBERROR("\n" + "\n" + " \n" // no value provided + "", + Library::MISSING_ATTRIBUTE); + + LOADLIBERROR("\n" + "\n" + " \n" // no name provided + "", + Library::MISSING_ATTRIBUTE); + + LOADLIBERROR("\n" + "\n" + "", + Library::UNSUPPORTED_FORMAT); } }; From 1c9f46cd82f98c41a210322dc1ae609c49566749 Mon Sep 17 00:00:00 2001 From: Rikard Falkeborn Date: Tue, 3 Jul 2018 22:58:20 +0200 Subject: [PATCH 2/7] Add tests for invalid ranges --- test/testlibrary.cpp | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/test/testlibrary.cpp b/test/testlibrary.cpp index fb7acae660f..e44eb781049 100644 --- a/test/testlibrary.cpp +++ b/test/testlibrary.cpp @@ -730,6 +730,15 @@ class TestLibrary : public TestFixture { } #define LOADLIBERROR(xmldata, errorcode) loadLibError(xmldata, errorcode, __FILE__, __LINE__) +#define LOADLIB_ERROR_INVALID_RANGE(valid) LOADLIBERROR("\n" \ + "\n" \ + "\n" \ + "\n" \ + "" valid "\n" \ + "\n" \ + "\n" \ + "", \ + Library::BAD_ATTRIBUTE_VALUE) void loadLibErrors() const { @@ -763,6 +772,21 @@ class TestLibrary : public TestFixture { "\n" "", Library::UNSUPPORTED_FORMAT); + + // letter as range + LOADLIB_ERROR_INVALID_RANGE("a"); + + // letter and number as range + LOADLIB_ERROR_INVALID_RANGE("1a"); + + // digit followed by dash + LOADLIB_ERROR_INVALID_RANGE("0:2-1"); + + // single dash + LOADLIB_ERROR_INVALID_RANGE("-"); + + // range with multiple colons + LOADLIB_ERROR_INVALID_RANGE("1:2:3"); } }; From ff46ff1759b6283bfaa6cfc74c90f36d71a7257d Mon Sep 17 00:00:00 2001 From: Rikard Falkeborn Date: Fri, 6 Jul 2018 11:05:06 +0200 Subject: [PATCH 3/7] Handle empty valid field Before this change, the sequence in a config file would result in a segmentation fault. Now an empty field results in the error message: cppcheck: Failed to load library configuration file 'mycfg.cfg'. Bad attribute value '""' --- lib/library.cpp | 4 ++++ test/testlibrary.cpp | 3 +++ 2 files changed, 7 insertions(+) diff --git a/lib/library.cpp b/lib/library.cpp index 22cd75f6208..97bf2637c12 100644 --- a/lib/library.cpp +++ b/lib/library.cpp @@ -578,6 +578,10 @@ Library::Error Library::loadFunction(const tinyxml2::XMLElement * const node, co const char *p = argnode->GetText(); bool error = false; bool range = false; + + if (!p) + return Error(BAD_ATTRIBUTE_VALUE, "\"\""); + for (; *p; p++) { if (std::isdigit(*p)) error |= (*(p+1) == '-'); diff --git a/test/testlibrary.cpp b/test/testlibrary.cpp index e44eb781049..705c4e283e4 100644 --- a/test/testlibrary.cpp +++ b/test/testlibrary.cpp @@ -773,6 +773,9 @@ class TestLibrary : public TestFixture { "", Library::UNSUPPORTED_FORMAT); + // empty range + LOADLIB_ERROR_INVALID_RANGE(""); + // letter as range LOADLIB_ERROR_INVALID_RANGE("a"); From 09055eb5522a39a2c3a10416ecc209bdbe233abe Mon Sep 17 00:00:00 2001 From: Rikard Falkeborn Date: Sat, 7 Jul 2018 21:15:06 +0200 Subject: [PATCH 4/7] Add support for valid for floating point arguments Previously, it was not possible to add valid ranges to floating point arguments since it only handled integers. This made ranges not work well for floating point arguments since arguments were cast to integers before the ranges were handled. Fix this by using doubles instead of integers if the argument is a float. Add some tests for this and make sure errors are printed with enough precision (somewhat arbitrarily chosen). Note that it is still only possible to add integer ranges (i.e. -1:1). --- lib/checkfunctions.cpp | 7 ++-- lib/library.cpp | 41 +++++++++++++++++++----- lib/library.h | 1 + lib/token.cpp | 3 +- test/testlibrary.cpp | 72 +++++++++++++++++++++++++++++------------- 5 files changed, 90 insertions(+), 34 deletions(-) diff --git a/lib/checkfunctions.cpp b/lib/checkfunctions.cpp index b17541aa722..512f7d8b0f2 100644 --- a/lib/checkfunctions.cpp +++ b/lib/checkfunctions.cpp @@ -32,6 +32,7 @@ #include #include +#include #include #include @@ -119,9 +120,9 @@ void CheckFunctions::invalidFunctionUsage() invalidFunctionArgBoolError(argtok, functionToken->str(), argnr); // Are the values 0 and 1 valid? - else if (!mSettings->library.isargvalid(functionToken, argnr, 0)) + else if (!mSettings->library.isargvalid(functionToken, argnr, static_cast(0))) invalidFunctionArgError(argtok, functionToken->str(), argnr, nullptr, mSettings->library.validarg(functionToken, argnr)); - else if (!mSettings->library.isargvalid(functionToken, argnr, 1)) + else if (!mSettings->library.isargvalid(functionToken, argnr, static_cast(1))) invalidFunctionArgError(argtok, functionToken->str(), argnr, nullptr, mSettings->library.validarg(functionToken, argnr)); } } @@ -139,7 +140,7 @@ void CheckFunctions::invalidFunctionArgError(const Token *tok, const std::string else errmsg << "Invalid $symbol() argument nr " << argnr << '.'; if (invalidValue) - errmsg << " The value is " << invalidValue->intvalue << " but the valid values are '" << validstr << "'."; + errmsg << " The value is " << std::setprecision(10) << (invalidValue->isIntValue() ? invalidValue->intvalue : invalidValue->floatValue) << " but the valid values are '" << validstr << "'."; else errmsg << " The value is 0 or 1 (boolean) but the valid values are '" << validstr << "'."; if (invalidValue) diff --git a/lib/library.cpp b/lib/library.cpp index 97bf2637c12..16f2db43799 100644 --- a/lib/library.cpp +++ b/lib/library.cpp @@ -43,6 +43,18 @@ static std::vector getnames(const char *names) return ret; } +static void gettokenlistfromvalid(const std::string& valid, TokenList& tokenList) +{ + std::istringstream istr(valid + ','); + tokenList.createTokens(istr); + for (Token *tok = tokenList.front(); tok; tok = tok->next()) { + if (Token::Match(tok,"- %num%")) { + tok->str("-" + tok->strAt(1)); + tok->deleteNext(); + } + } +} + Library::Library() : mAllocId(0) { } @@ -709,15 +721,10 @@ bool Library::isargvalid(const Token *ftok, int argnr, const MathLib::bigint arg const ArgumentChecks *ac = getarg(ftok, argnr); if (!ac || ac->valid.empty()) return true; + else if (ac->valid.find('.') != std::string::npos) + return isargvalid(ftok, argnr, static_cast(argvalue)); TokenList tokenList(nullptr); - std::istringstream istr(ac->valid + ','); - tokenList.createTokens(istr); - for (Token *tok = tokenList.front(); tok; tok = tok->next()) { - if (Token::Match(tok,"- %num%")) { - tok->str("-" + tok->strAt(1)); - tok->deleteNext(); - } - } + gettokenlistfromvalid(ac->valid, tokenList); for (const Token *tok = tokenList.front(); tok; tok = tok->next()) { if (tok->isNumber() && argvalue == MathLib::toLongNumber(tok->str())) return true; @@ -731,6 +738,24 @@ bool Library::isargvalid(const Token *ftok, int argnr, const MathLib::bigint arg return false; } +bool Library::isargvalid(const Token *ftok, int argnr, double argvalue) const +{ + const ArgumentChecks *ac = getarg(ftok, argnr); + if (!ac || ac->valid.empty()) + return true; + TokenList tokenList(nullptr); + gettokenlistfromvalid(ac->valid, tokenList); + for (const Token *tok = tokenList.front(); tok; tok = tok->next()) { + if (Token::Match(tok, "%num% : %num%") && argvalue >= MathLib::toDoubleNumber(tok->str()) && argvalue <= MathLib::toDoubleNumber(tok->strAt(2))) + return true; + if (Token::Match(tok, "%num% : ,") && argvalue >= MathLib::toDoubleNumber(tok->str())) + return true; + if ((!tok->previous() || tok->previous()->str() == ",") && Token::Match(tok,": %num%") && argvalue <= MathLib::toDoubleNumber(tok->strAt(1))) + return true; + } + return false; +} + std::string Library::getFunctionName(const Token *ftok, bool *error) const { if (!ftok) { diff --git a/lib/library.h b/lib/library.h index 378075bca2a..6546e07188b 100644 --- a/lib/library.h +++ b/lib/library.h @@ -301,6 +301,7 @@ class CPPCHECKLIB Library { } bool isargvalid(const Token *ftok, int argnr, const MathLib::bigint argvalue) const; + bool isargvalid(const Token *ftok, int argnr, double argvalue) const; const std::string& validarg(const Token *ftok, int argnr) const { const ArgumentChecks *arg = getarg(ftok, argnr); diff --git a/lib/token.cpp b/lib/token.cpp index d6af64e74c3..c4632390fc3 100644 --- a/lib/token.cpp +++ b/lib/token.cpp @@ -1497,7 +1497,8 @@ const ValueFlow::Value * Token::getInvalidValue(const Token *ftok, unsigned int const ValueFlow::Value *ret = nullptr; std::list::const_iterator it; for (it = mValues->begin(); it != mValues->end(); ++it) { - if (it->isIntValue() && !settings->library.isargvalid(ftok, argnr, it->intvalue)) { + if ((it->isIntValue() && !settings->library.isargvalid(ftok, argnr, it->intvalue)) || + (it->isFloatValue() && !settings->library.isargvalid(ftok, argnr, it->floatValue))) { if (!ret || ret->isInconclusive() || (ret->condition && !it->isInconclusive())) ret = &(*it); if (!ret->isInconclusive() && !ret->condition) diff --git a/test/testlibrary.cpp b/test/testlibrary.cpp index 705c4e283e4..347e8715804 100644 --- a/test/testlibrary.cpp +++ b/test/testlibrary.cpp @@ -294,36 +294,64 @@ class TestLibrary : public TestFixture { tokenList.front()->next()->astOperand1(tokenList.front()); // 1- - ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 1, -10)); - ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 1, 0)); - ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 1, 1)); - ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 1, 10)); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 1, static_cast(-10))); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 1, -10.0)); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 1, static_cast(0))); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 1, 0.0)); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 1, static_cast(1))); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 1, 1.0)); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 1, static_cast(10))); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 1, 10.0)); // -7-0 - ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 2, -10)); - ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 2, -7)); - ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 2, -3)); - ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 2, 0)); - ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 2, 1)); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 2, static_cast(-10))); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 2, -10.0)); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 2, -7.5)); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 2, -7.1)); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 2, static_cast(-7))); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 2, -7.0)); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 2, static_cast(-3))); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 2, -3.0)); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 2, -3.5)); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 2, static_cast(0))); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 2, 0.0)); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 2, 0.5)); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 2, static_cast(1))); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 2, 1.0)); // 1-5,8 - ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 3, 0)); - ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 3, 1)); - ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 3, 3)); - ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 3, 5)); - ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 3, 6)); - ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 3, 7)); - ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 3, 8)); - ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 3, 9)); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 3, static_cast(0))); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 3, 0.0)); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 3, static_cast(1))); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 3, 1.0)); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 3, static_cast(3))); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 3, 3.0)); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 3, static_cast(5))); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 3, 5.0)); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 3, static_cast(6))); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 3, 6.0)); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 3, static_cast(7))); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 3, 7.0)); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 3, static_cast(8))); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 3, 8.0)); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 3, static_cast(9))); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 3, 9.0)); // -1,5 - ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 4, -10)); - ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 4, -1)); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 4, static_cast(-10))); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 4, -10.0)); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 4, static_cast(-1))); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 4, -1.0)); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 4, 5.000001)); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 4, 5.5)); // :1,5 - ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 5, -10)); - ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 5, 1)); - ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 5, 2)); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 5, static_cast(-10))); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 5, -10.0)); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 5, static_cast(1))); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 5, 1.0)); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 5, static_cast(2))); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 5, 2.0)); } void function_arg_minsize() const { From 34e5ba441db6ab8263c835acbd2f019c51806302 Mon Sep 17 00:00:00 2001 From: Rikard Falkeborn Date: Fri, 6 Jul 2018 20:06:17 +0200 Subject: [PATCH 5/7] Add support for floats in configuration valid range Now that it is possible to handle decimal arguments, there is no reason to not allow non-integer ranges. Take care to not allow broken configurations. --- cfg/cppcheck-cfg.rng | 2 +- lib/library.cpp | 17 ++++++-- man/manual.docbook | 3 +- test/testlibrary.cpp | 96 +++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 112 insertions(+), 6 deletions(-) diff --git a/cfg/cppcheck-cfg.rng b/cfg/cppcheck-cfg.rng index 0dafccad9eb..048f6b7d54e 100644 --- a/cfg/cppcheck-cfg.rng +++ b/cfg/cppcheck-cfg.rng @@ -154,7 +154,7 @@ - (-?[0-9]*[,:])*([-]?[0-9]+)? + (-?[0-9]*(\.[0-9]+)?[,:])*([-]?[0-9]+(\.[0-9]+)?)? diff --git a/lib/library.cpp b/lib/library.cpp index 16f2db43799..74152762842 100644 --- a/lib/library.cpp +++ b/lib/library.cpp @@ -590,19 +590,30 @@ Library::Error Library::loadFunction(const tinyxml2::XMLElement * const node, co const char *p = argnode->GetText(); bool error = false; bool range = false; + bool has_dot = false; if (!p) return Error(BAD_ATTRIBUTE_VALUE, "\"\""); + error = *p == '.'; for (; *p; p++) { if (std::isdigit(*p)) error |= (*(p+1) == '-'); - else if (*p == ':') - error |= range; + else if (*p == ':') { + error |= range | (*(p+1) == '.'); + has_dot = false; + } else if (*p == '-') error |= (!std::isdigit(*(p+1))); - else if (*p == ',') + else if (*p == ',') { range = false; + error |= *(p+1) == '.'; + has_dot = false; + } + else if (*p == '.') { + error |= has_dot | (!std::isdigit(*(p+1))); + has_dot = true; + } else error = true; diff --git a/man/manual.docbook b/man/manual.docbook index 4fed1f6be8b..c4e14108a94 100644 --- a/man/manual.docbook +++ b/man/manual.docbook @@ -1498,7 +1498,8 @@ Checking range.c... -10:20 => all values between -10 and 20 are valid :0 => all values that are less or equal to 0 are valid 0: => all values that are greater or equal to 0 are valid -0,2:32 => the value 0 and all values between 2 and 32 are valid +0,2:32 => the value 0 and all values between 2 and 32 are valid +-1.5:5.6 => all values between -1.5 and 5.6 are valid
diff --git a/test/testlibrary.cpp b/test/testlibrary.cpp index 347e8715804..590375befc6 100644 --- a/test/testlibrary.cpp +++ b/test/testlibrary.cpp @@ -282,6 +282,11 @@ class TestLibrary : public TestFixture { " 1:5,8\n" " -1,5\n" " :1,5\n" + " 1.5:\n" + " -6.7:-5.5,-3.3:-2.7\n" + " 0.0:\n" + " :2.0\n" + " 0.0\n" " \n" ""; @@ -289,7 +294,7 @@ class TestLibrary : public TestFixture { ASSERT_EQUALS(true, Library::OK == (readLibrary(library, xmldata)).errorcode); TokenList tokenList(nullptr); - std::istringstream istr("foo(a,b,c,d,e);"); + std::istringstream istr("foo(a,b,c,d,e,f,g,h,i,j);"); tokenList.createTokens(istr); tokenList.front()->next()->astOperand1(tokenList.front()); @@ -352,6 +357,62 @@ class TestLibrary : public TestFixture { ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 5, 1.0)); ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 5, static_cast(2))); ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 5, 2.0)); + + // 1.5: + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 6, static_cast(0))); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 6, 0.0)); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 6, static_cast(1))); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 6, 1.499999)); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 6, 1.5)); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 6, static_cast(2))); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 6, static_cast(10))); + + // -6.7:-5.5,-3.3:-2.7 + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 7, static_cast(-7))); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 7, -7.0)); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 7, -6.7000001)); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 7, -6.7)); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 7, static_cast(-6))); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 7, -6.0)); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 7, -5.5)); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 7, -5.4999999)); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 7, -3.3000001)); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 7, -3.3)); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 7, static_cast(-3))); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 7, -3.0)); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 7, -2.7)); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 7, -2.6999999)); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 7, static_cast(-2))); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 7, -2.0)); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 7, static_cast(0))); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 7, 0.0)); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 7, static_cast(3))); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 7, 3.0)); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 7, static_cast(6))); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 7, 6.0)); + + // 0.0: + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 8, static_cast(-1))); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 8, -1.0)); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 8, -0.00000001)); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 8, static_cast(0))); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 8, 0.0)); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 8, 0.000000001)); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 8, static_cast(1))); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 8, 1.0)); + + // :2.0 + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 9, static_cast(-1))); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 9, -1.0)); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 9, static_cast(2))); + ASSERT_EQUALS(true, library.isargvalid(tokenList.front(), 9, 2.0)); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 9, 2.00000001)); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 9, static_cast(200))); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 9, 200.0)); + + // 0.0 + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 10, static_cast(0))); + ASSERT_EQUALS(false, library.isargvalid(tokenList.front(), 10, 0.0)); } void function_arg_minsize() const { @@ -818,6 +879,39 @@ class TestLibrary : public TestFixture { // range with multiple colons LOADLIB_ERROR_INVALID_RANGE("1:2:3"); + + // extra dot + LOADLIB_ERROR_INVALID_RANGE("1.0.0:10"); + + // consecutive dots + LOADLIB_ERROR_INVALID_RANGE("1..0:10"); + + // dot followed by dash + LOADLIB_ERROR_INVALID_RANGE("1.-0:10"); + + // dot without preceding number + LOADLIB_ERROR_INVALID_RANGE(".5:10"); + + // dash followed by dot + LOADLIB_ERROR_INVALID_RANGE("-.5:10"); + + // colon followed by dot without preceding number + LOADLIB_ERROR_INVALID_RANGE("0:.5"); + + // colon followed by dash followed by dot + LOADLIB_ERROR_INVALID_RANGE("-10:-.5"); + + // dot not followed by number + LOADLIB_ERROR_INVALID_RANGE("1:5."); + + // dot not followed by number + LOADLIB_ERROR_INVALID_RANGE("1.:5"); + + // dot followed by comma + LOADLIB_ERROR_INVALID_RANGE("1:5.,6:10"); + + // comma followed by dot + LOADLIB_ERROR_INVALID_RANGE("-10:0,.5:"); } }; From 30dea8942d1a5555e80b247fcbfa00e2a72bec9f Mon Sep 17 00:00:00 2001 From: Rikard Falkeborn Date: Fri, 6 Jul 2018 21:09:01 +0200 Subject: [PATCH 6/7] Move check to within if-clause --- lib/library.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/library.cpp b/lib/library.cpp index 74152762842..353f3e8e444 100644 --- a/lib/library.cpp +++ b/lib/library.cpp @@ -601,6 +601,7 @@ Library::Error Library::loadFunction(const tinyxml2::XMLElement * const node, co error |= (*(p+1) == '-'); else if (*p == ':') { error |= range | (*(p+1) == '.'); + range = true; has_dot = false; } else if (*p == '-') @@ -616,8 +617,6 @@ Library::Error Library::loadFunction(const tinyxml2::XMLElement * const node, co } else error = true; - - range |= (*p == ':'); } if (error) return Error(BAD_ATTRIBUTE_VALUE, argnode->GetText()); From e6383b93f7330109e51e869d298b846d4870e0bc Mon Sep 17 00:00:00 2001 From: Rikard Falkeborn Date: Thu, 12 Jul 2018 23:31:25 +0200 Subject: [PATCH 7/7] Move asin{,f,l} and acos{,f,l} input checks to config file --- cfg/std.cfg | 6 ++++++ lib/checkfunctions.cpp | 5 ----- test/testfunctions.cpp | 24 ++++++++++++------------ 3 files changed, 18 insertions(+), 17 deletions(-) diff --git a/cfg/std.cfg b/cfg/std.cfg index 5a2456bc7da..8db8eb5d9b8 100644 --- a/cfg/std.cfg +++ b/cfg/std.cfg @@ -108,6 +108,7 @@ + -1.0:1.0 @@ -119,6 +120,7 @@ + -1.0:1.0 @@ -130,6 +132,7 @@ + -1.0:1.0 @@ -348,6 +351,7 @@ false + -1.0:1.0 @@ -359,6 +363,7 @@ false + -1.0:1.0 @@ -370,6 +375,7 @@ false + -1.0:1.0 diff --git a/lib/checkfunctions.cpp b/lib/checkfunctions.cpp index 512f7d8b0f2..22f7570a2b9 100644 --- a/lib/checkfunctions.cpp +++ b/lib/checkfunctions.cpp @@ -242,11 +242,6 @@ void CheckFunctions::checkMathFunctions() } } - // acos( x ), asin( x ) where x is defined for interval [-1,+1], but not beyond - else if (Token::Match(tok, "acos|acosl|acosf|asin|asinf|asinl ( %num% )")) { - if (std::fabs(MathLib::toDoubleNumber(tok->strAt(2))) > 1.0) - mathfunctionCallWarning(tok); - } // sqrt( x ): if x is negative the result is undefined else if (Token::Match(tok, "sqrt|sqrtf|sqrtl ( %num% )")) { if (MathLib::isNegative(tok->strAt(2))) diff --git a/test/testfunctions.cpp b/test/testfunctions.cpp index fb4936bec9c..cdc6cacb0aa 100644 --- a/test/testfunctions.cpp +++ b/test/testfunctions.cpp @@ -604,9 +604,9 @@ class TestFunctions : public TestFixture { " std::cout << acosf(1.1) << std::endl;\n" " std::cout << acosl(1.1) << std::endl;\n" "}"); - ASSERT_EQUALS("[test.cpp:3]: (warning) Passing value 1.1 to acos() leads to implementation-defined result.\n" - "[test.cpp:4]: (warning) Passing value 1.1 to acosf() leads to implementation-defined result.\n" - "[test.cpp:5]: (warning) Passing value 1.1 to acosl() leads to implementation-defined result.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (error) Invalid acos() argument nr 1. The value is 1.1 but the valid values are '-1.0:1.0'.\n" + "[test.cpp:4]: (error) Invalid acosf() argument nr 1. The value is 1.1 but the valid values are '-1.0:1.0'.\n" + "[test.cpp:5]: (error) Invalid acosl() argument nr 1. The value is 1.1 but the valid values are '-1.0:1.0'.\n", errout.str()); check("void foo()\n" "{\n" @@ -614,9 +614,9 @@ class TestFunctions : public TestFixture { " std::cout << acosf(-1.1) << std::endl;\n" " std::cout << acosl(-1.1) << std::endl;\n" "}"); - ASSERT_EQUALS("[test.cpp:3]: (warning) Passing value -1.1 to acos() leads to implementation-defined result.\n" - "[test.cpp:4]: (warning) Passing value -1.1 to acosf() leads to implementation-defined result.\n" - "[test.cpp:5]: (warning) Passing value -1.1 to acosl() leads to implementation-defined result.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (error) Invalid acos() argument nr 1. The value is -1.1 but the valid values are '-1.0:1.0'.\n" + "[test.cpp:4]: (error) Invalid acosf() argument nr 1. The value is -1.1 but the valid values are '-1.0:1.0'.\n" + "[test.cpp:5]: (error) Invalid acosl() argument nr 1. The value is -1.1 but the valid values are '-1.0:1.0'.\n", errout.str()); } void mathfunctionCall_asin() { @@ -665,9 +665,9 @@ class TestFunctions : public TestFixture { " std::cout << asinf(1.1) << std::endl;\n" " std::cout << asinl(1.1) << std::endl;\n" "}"); - ASSERT_EQUALS("[test.cpp:3]: (warning) Passing value 1.1 to asin() leads to implementation-defined result.\n" - "[test.cpp:4]: (warning) Passing value 1.1 to asinf() leads to implementation-defined result.\n" - "[test.cpp:5]: (warning) Passing value 1.1 to asinl() leads to implementation-defined result.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (error) Invalid asin() argument nr 1. The value is 1.1 but the valid values are '-1.0:1.0'.\n" + "[test.cpp:4]: (error) Invalid asinf() argument nr 1. The value is 1.1 but the valid values are '-1.0:1.0'.\n" + "[test.cpp:5]: (error) Invalid asinl() argument nr 1. The value is 1.1 but the valid values are '-1.0:1.0'.\n", errout.str()); check("void foo()\n" "{\n" @@ -675,9 +675,9 @@ class TestFunctions : public TestFixture { " std::cout << asinf(-1.1) << std::endl;\n" " std::cout << asinl(-1.1) << std::endl;\n" "}"); - ASSERT_EQUALS("[test.cpp:3]: (warning) Passing value -1.1 to asin() leads to implementation-defined result.\n" - "[test.cpp:4]: (warning) Passing value -1.1 to asinf() leads to implementation-defined result.\n" - "[test.cpp:5]: (warning) Passing value -1.1 to asinl() leads to implementation-defined result.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (error) Invalid asin() argument nr 1. The value is -1.1 but the valid values are '-1.0:1.0'.\n" + "[test.cpp:4]: (error) Invalid asinf() argument nr 1. The value is -1.1 but the valid values are '-1.0:1.0'.\n" + "[test.cpp:5]: (error) Invalid asinl() argument nr 1. The value is -1.1 but the valid values are '-1.0:1.0'.\n", errout.str()); } void mathfunctionCall_pow() {