From 5a50522c9c01893397b2edd8367bb4f2cce01ef3 Mon Sep 17 00:00:00 2001 From: firewave Date: Tue, 21 Mar 2023 12:24:57 +0100 Subject: [PATCH 01/11] added `strToInt()` for non-literal string-to-integer conversions --- lib/mathlib.cpp | 3 +- lib/mathlib.h | 3 + lib/utils.h | 82 +++++++++++++++++++++++++ test/fixture.h | 1 + test/testutils.cpp | 149 +++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 237 insertions(+), 1 deletion(-) diff --git a/lib/mathlib.cpp b/lib/mathlib.cpp index 446b7dc4675..e332d2b8d4f 100644 --- a/lib/mathlib.cpp +++ b/lib/mathlib.cpp @@ -286,7 +286,7 @@ MathLib::value MathLib::value::shiftRight(const MathLib::value &v) const return ret; } - +// TODO: remove handling of non-literal stuff MathLib::biguint MathLib::toULongNumber(const std::string & str) { // hexadecimal numbers: @@ -365,6 +365,7 @@ unsigned int MathLib::encodeMultiChar(const std::string& str) }); } +// TODO: remove handling of non-literal stuff MathLib::bigint MathLib::toLongNumber(const std::string & str) { // hexadecimal numbers: diff --git a/lib/mathlib.h b/lib/mathlib.h index 8e0e790f4d0..1ed77f06c6a 100644 --- a/lib/mathlib.h +++ b/lib/mathlib.h @@ -69,12 +69,15 @@ class CPPCHECKLIB MathLib { using biguint = unsigned long long; static const int bigint_bits; + /** @brief for conversion of numeric literals - for atoi-like conversions please use strToInt() */ static bigint toLongNumber(const std::string & str); + /** @brief for conversion of numeric literals - for atoi-like conversions please use strToInt() */ static biguint toULongNumber(const std::string & str); template static std::string toString(T value) { return std::to_string(value); } + /** @brief for conversion of numeric literals */ static double toDoubleNumber(const std::string & str); static bool isInt(const std::string & str); diff --git a/lib/utils.h b/lib/utils.h index 5cd2a823c0b..e942c82f1da 100644 --- a/lib/utils.h +++ b/lib/utils.h @@ -27,6 +27,8 @@ #include #include #include +#include +#include #include #include @@ -177,6 +179,86 @@ CPPCHECKLIB bool matchglobs(const std::vector &patterns, const std: CPPCHECKLIB void strTolower(std::string& str); +template::value, bool>::type=true> +bool strToInt(const std::string& str, T &num, std::string* err = nullptr) +{ + long long tmp; + try { + std::size_t idx = 0; + tmp = std::stoll(str, &idx); + if (idx != str.size()) { + if (err) + *err = "not an integer"; + return false; + } + } catch (const std::out_of_range&) { + if (err) + *err = "out of range (stoll)"; + return false; + } catch (const std::invalid_argument &) { + if (err) + *err = "not an integer"; + return false; + } + if (str.front() == '-' && std::numeric_limits::min() == 0) { + if (err) + *err = "needs to be positive"; + return false; + } + if (tmp < std::numeric_limits::min() || tmp > std::numeric_limits::max()) { + if (err) + *err = "out of range (limits)"; + return false; + } + num = static_cast(tmp); + return true; +} + +template::value, bool>::type=true> +bool strToInt(const std::string& str, T &num, std::string* err = nullptr) +{ + unsigned long long tmp; + try { + std::size_t idx = 0; + tmp = std::stoull(str, &idx); + if (idx != str.size()) { + if (err) + *err = "not an integer"; + return false; + } + } catch (const std::out_of_range&) { + if (err) + *err = "out of range (stoull)"; + return false; + } catch (const std::invalid_argument &) { + if (err) + *err = "not an integer"; + return false; + } + if (str.front() == '-') { + if (err) + *err = "needs to be positive"; + return false; + } + if (tmp > std::numeric_limits::max()) { + if (err) + *err = "out of range (limits)"; + return false; + } + num = tmp; + return true; +} + +template +T strToInt(const std::string& str) +{ + T tmp = 0; + std::string err; + if (!strToInt(str, tmp, &err)) + throw std::runtime_error("converting '" + str + "' to integer failed - " + err); + return tmp; +} + /** * Simple helper function: * \return size of array diff --git a/test/fixture.h b/test/fixture.h index 47fc5e20ecc..d37a949d44a 100644 --- a/test/fixture.h +++ b/test/fixture.h @@ -157,6 +157,7 @@ extern std::ostringstream output; #define ASSERT_EQUALS_MSG( EXPECTED, ACTUAL, MSG ) assertEquals(__FILE__, __LINE__, EXPECTED, ACTUAL, MSG) #define ASSERT_THROW( CMD, EXCEPTION ) do { try { CMD; assertThrowFail(__FILE__, __LINE__); } catch (const EXCEPTION&) {} catch (...) { assertThrowFail(__FILE__, __LINE__); } } while (false) #define ASSERT_THROW_EQUALS( CMD, EXCEPTION, EXPECTED ) do { try { CMD; assertThrowFail(__FILE__, __LINE__); } catch (const EXCEPTION&e) { assertEquals(__FILE__, __LINE__, EXPECTED, e.errorMessage); } catch (...) { assertThrowFail(__FILE__, __LINE__); } } while (false) +#define ASSERT_THROW_EQUALS_2( CMD, EXCEPTION, EXPECTED ) do { try { CMD; assertThrowFail(__FILE__, __LINE__); } catch (const EXCEPTION&e) { assertEquals(__FILE__, __LINE__, EXPECTED, e.what()); } catch (...) { assertThrowFail(__FILE__, __LINE__); } } while (false) #define ASSERT_NO_THROW( CMD ) do { try { CMD; } catch (...) { assertNoThrowFail(__FILE__, __LINE__); } } while (false) #define TODO_ASSERT_THROW( CMD, EXCEPTION ) do { try { CMD; } catch (const EXCEPTION&) {} catch (...) { assertThrow(__FILE__, __LINE__); } } while (false) #define TODO_ASSERT( CONDITION ) do { const bool condition=(CONDITION); todoAssertEquals(__FILE__, __LINE__, true, false, condition); } while (false) diff --git a/test/testutils.cpp b/test/testutils.cpp index 6ef95fa9eb5..612b92da412 100644 --- a/test/testutils.cpp +++ b/test/testutils.cpp @@ -33,6 +33,7 @@ class TestUtils : public TestFixture { TEST_CASE(matchglob); TEST_CASE(isStringLiteral); TEST_CASE(isCharLiteral); + TEST_CASE(strToInt); } void isValidGlobPattern() const { @@ -179,6 +180,154 @@ class TestUtils : public TestFixture { ASSERT_EQUALS(true, ::isCharLiteral("U'test'")); ASSERT_EQUALS(true, ::isCharLiteral("L'test'")); } + + void strToInt() { + ASSERT_EQUALS(1, ::strToInt("1")); + ASSERT_EQUALS(-1, ::strToInt("-1")); + ASSERT_EQUALS(1, ::strToInt("1")); + ASSERT_THROW_EQUALS_2(::strToInt(""), std::runtime_error, "converting '' to integer failed - not an integer"); + ASSERT_THROW_EQUALS_2(::strToInt(""), std::runtime_error, "converting '' to integer failed - not an integer"); + ASSERT_THROW_EQUALS_2(::strToInt(" "), std::runtime_error, "converting ' ' to integer failed - not an integer"); + ASSERT_THROW_EQUALS_2(::strToInt(" "), std::runtime_error, "converting ' ' to integer failed - not an integer"); + ASSERT_THROW_EQUALS_2(::strToInt("-1"), std::runtime_error, "converting '-1' to integer failed - needs to be positive"); + ASSERT_THROW_EQUALS_2(::strToInt("1ms"), std::runtime_error, "converting '1ms' to integer failed - not an integer"); + ASSERT_THROW_EQUALS_2(::strToInt("1.0"), std::runtime_error, "converting '1.0' to integer failed - not an integer"); + ASSERT_THROW_EQUALS_2(::strToInt("one"), std::runtime_error, "converting 'one' to integer failed - not an integer"); + ASSERT_THROW_EQUALS_2(::strToInt("1ms"), std::runtime_error, "converting '1ms' to integer failed - not an integer"); + ASSERT_THROW_EQUALS_2(::strToInt("1.0"), std::runtime_error, "converting '1.0' to integer failed - not an integer"); + ASSERT_THROW_EQUALS_2(::strToInt("one"), std::runtime_error, "converting 'one' to integer failed - not an integer"); + ASSERT_THROW_EQUALS_2(::strToInt(std::to_string(static_cast(std::numeric_limits::max()) + 1)), std::runtime_error, "converting '2147483648' to integer failed - out of range (limits)"); + ASSERT_THROW_EQUALS_2(::strToInt(std::to_string(static_cast(std::numeric_limits::min()) - 1)), std::runtime_error, "converting '-2147483649' to integer failed - out of range (limits)"); + ASSERT_THROW_EQUALS_2(::strToInt(std::to_string(static_cast(std::numeric_limits::max()) + 1)), std::runtime_error, "converting '128' to integer failed - out of range (limits)"); + ASSERT_THROW_EQUALS_2(::strToInt(std::to_string(static_cast(std::numeric_limits::min()) - 1)), std::runtime_error, "converting '-129' to integer failed - out of range (limits)"); + ASSERT_THROW_EQUALS_2(::strToInt(std::to_string(static_cast(std::numeric_limits::max()) + 1)), std::runtime_error, "converting '4294967296' to integer failed - out of range (limits)"); + ASSERT_THROW_EQUALS_2(::strToInt("9223372036854775808"), std::runtime_error, "converting '9223372036854775808' to integer failed - out of range (stoll)"); // LLONG_MAX + 1 + ASSERT_THROW_EQUALS_2(::strToInt("18446744073709551616"), std::runtime_error, "converting '18446744073709551616' to integer failed - out of range (stoull)"); // ULLONG_MAX + 1 + + { + long tmp; + ASSERT(::strToInt("1", tmp)); + ASSERT_EQUALS(1, tmp); + } + { + long tmp; + ASSERT(::strToInt("-1", tmp)); + ASSERT_EQUALS(-1, tmp); + } + { + std::size_t tmp; + ASSERT(::strToInt("1", tmp)); + } + { + std::size_t tmp; + ASSERT(!::strToInt("-1", tmp)); + } + { + long tmp; + ASSERT(!::strToInt("1ms", tmp)); + } + { + unsigned long tmp; + ASSERT(!::strToInt("1ms", tmp)); + } + { + long tmp; + ASSERT(!::strToInt("", tmp)); + } + { + unsigned long tmp; + ASSERT(!::strToInt("", tmp)); + } + { + long tmp; + ASSERT(!::strToInt(" ", tmp)); + } + { + unsigned long tmp; + ASSERT(!::strToInt(" ", tmp)); + } + + { + long tmp; + std::string err; + ASSERT(::strToInt("1", tmp, &err)); + ASSERT_EQUALS(1, tmp); + ASSERT_EQUALS("", err); + } + { + std::size_t tmp; + std::string err; + ASSERT(::strToInt("1", tmp, &err)); + ASSERT_EQUALS(1, tmp); + ASSERT_EQUALS("", err); + } + { + unsigned long tmp; + std::string err; + ASSERT(!::strToInt("-1", tmp, &err)); + ASSERT_EQUALS("needs to be positive", err); + } + { + long tmp; + std::string err; + ASSERT(!::strToInt("1ms", tmp, &err)); + ASSERT_EQUALS("not an integer", err); + } + { + long tmp; + std::string err; + ASSERT(!::strToInt("1.0", tmp, &err)); + ASSERT_EQUALS("not an integer", err); + } + { + long tmp; + std::string err; + ASSERT(!::strToInt("one", tmp, &err)); + ASSERT_EQUALS("not an integer", err); + } + { + std::size_t tmp; + std::string err; + ASSERT(!::strToInt("1ms", tmp, &err)); + ASSERT_EQUALS("not an integer", err); + } + { + long tmp; + std::string err; + ASSERT(!::strToInt("9223372036854775808", tmp, &err)); // LLONG_MAX + 1 + ASSERT_EQUALS("out of range (stoll)", err); + } + { + int tmp; + std::string err; + ASSERT(!::strToInt(std::to_string(static_cast(std::numeric_limits::max()) + 1), tmp, &err)); + ASSERT_EQUALS("out of range (limits)", err); + } + { + int tmp; + std::string err; + ASSERT(!::strToInt(std::to_string(static_cast(std::numeric_limits::min()) - 1), tmp, &err)); + ASSERT_EQUALS("out of range (limits)", err); + } + { + int8_t tmp; + std::string err; + ASSERT(!::strToInt(std::to_string(static_cast(std::numeric_limits::max()) + 1), tmp, &err)); + ASSERT_EQUALS("out of range (limits)", err); + } + { + int8_t tmp; + std::string err; + ASSERT(!::strToInt(std::to_string(static_cast(std::numeric_limits::min()) - 1), tmp, &err)); + ASSERT_EQUALS("out of range (limits)", err); + } + { + std::size_t tmp; + std::string err; + ASSERT(!::strToInt("18446744073709551616", tmp, &err)); // ULLONG_MAX + 1 + ASSERT_EQUALS("out of range (stoull)", err); + } + } }; REGISTER_TEST(TestUtils) From 65f902dc1f93af1f0120de0d9a985a778d25b3e9 Mon Sep 17 00:00:00 2001 From: firewave Date: Tue, 21 Mar 2023 12:56:53 +0100 Subject: [PATCH 02/11] CmdLineParser: do not use string-to-integer conversion without error handling --- cli/cmdlineparser.cpp | 89 ++++++++-------- cli/cmdlineparser.h | 20 ++++ test/testcmdlineparser.cpp | 203 +++++++++++++++++++++++++++++++++++-- 3 files changed, 254 insertions(+), 58 deletions(-) diff --git a/cli/cmdlineparser.cpp b/cli/cmdlineparser.cpp index 5f2cf23c527..2c606cf87db 100644 --- a/cli/cmdlineparser.cpp +++ b/cli/cmdlineparser.cpp @@ -260,7 +260,8 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[]) } else if (std::strncmp(argv[i], "--checks-max-time=", 18) == 0) { - mSettings.checksMaxTime = std::atoi(argv[i] + 18); + if (!parseNumberArg(argv[i], 18, mSettings.checksMaxTime)) + return false; } else if (std::strcmp(argv[i], "--clang") == 0) { @@ -365,12 +366,8 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[]) // --error-exitcode=1 else if (std::strncmp(argv[i], "--error-exitcode=", 17) == 0) { - const std::string temp = argv[i]+17; - std::istringstream iss(temp); - if (!(iss >> mSettings.exitCode)) { - printError("argument must be an integer. Try something like '--error-exitcode=1'."); + if (!parseNumberArg(argv[i], 17, mSettings.exitCode)) return false; - } } // Exception handling inside cppcheck client @@ -505,18 +502,19 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[]) else numberString = argv[i]+2; - std::istringstream iss(numberString); - if (!(iss >> mSettings.jobs)) { - printError("argument to '-j' is not a number."); + unsigned int tmp; + std::string err; + if (!strToInt(numberString, tmp, &err)) { + printError("argument to '-j' is not valid - " + err + "."); return false; } - - if (mSettings.jobs > 10000) { + if (tmp > 10000) { // This limit is here just to catch typos. If someone has // need for more jobs, this value should be increased. printError("argument for '-j' is allowed to be 10000 at max."); return false; } + mSettings.jobs = tmp; } #ifdef THREADING_MODEL_FORK @@ -538,11 +536,13 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[]) else numberString = argv[i]+2; - std::istringstream iss(numberString); - if (!(iss >> mSettings.loadAverage)) { - printError("argument to '-l' is not a number."); + int tmp; + std::string err; + if (!strToInt(numberString, tmp, &err)) { + printError("argument to '-l' is not valid - " + err + "."); return false; } + mSettings.loadAverage = tmp; } #endif @@ -577,25 +577,24 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[]) // Set maximum number of #ifdef configurations to check else if (std::strncmp(argv[i], "--max-configs=", 14) == 0) { - mSettings.force = false; - - std::istringstream iss(14+argv[i]); - if (!(iss >> mSettings.maxConfigs)) { - printError("argument to '--max-configs=' is not a number."); + int tmp; + if (!parseNumberArg(argv[i], 14, tmp)) return false; - } - - if (mSettings.maxConfigs < 1) { + if (tmp < 1) { printError("argument to '--max-configs=' must be greater than 0."); return false; } + mSettings.maxConfigs = tmp; + mSettings.force = false; maxconfigs = true; } // max ctu depth - else if (std::strncmp(argv[i], "--max-ctu-depth=", 16) == 0) - mSettings.maxCtuDepth = std::atoi(argv[i] + 16); + else if (std::strncmp(argv[i], "--max-ctu-depth=", 16) == 0) { + if (!parseNumberArg(argv[i], 16, mSettings.maxCtuDepth)) + return false; + } // Write results in file else if (std::strncmp(argv[i], "--output-file=", 14) == 0) @@ -603,11 +602,15 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[]) // Experimental: limit execution time for extended valueflow analysis. basic valueflow analysis // is always executed. - else if (std::strncmp(argv[i], "--performance-valueflow-max-time=", 33) == 0) - mSettings.performanceValueFlowMaxTime = std::atoi(argv[i] + 33); + else if (std::strncmp(argv[i], "--performance-valueflow-max-time=", 33) == 0) { + if (!parseNumberArg(argv[i], 33, mSettings.performanceValueFlowMaxTime, true)) + return false; + } - else if (std::strncmp(argv[i], "--performance-valueflow-max-if-count=", 37) == 0) - mSettings.performanceValueFlowMaxIfCount = std::atoi(argv[i] + 37); + else if (std::strncmp(argv[i], "--performance-valueflow-max-if-count=", 37) == 0) { + if (!parseNumberArg(argv[i], 37, mSettings.performanceValueFlowMaxIfCount, true)) + return false; + } // Specify platform else if (std::strncmp(argv[i], "--platform=", 11) == 0) { @@ -937,26 +940,18 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[]) } else if (std::strncmp(argv[i], "--template-max-time=", 20) == 0) { - mSettings.templateMaxTime = std::atoi(argv[i] + 20); + if (!parseNumberArg(argv[i], 20, mSettings.templateMaxTime)) + return false; } else if (std::strncmp(argv[i], "--typedef-max-time=", 19) == 0) { - mSettings.typedefMaxTime = std::atoi(argv[i] + 19); + if (!parseNumberArg(argv[i], 19, mSettings.typedefMaxTime)) + return false; } else if (std::strncmp(argv[i], "--valueflow-max-iterations=", 27) == 0) { - long tmp; - try { - tmp = std::stol(argv[i] + 27); - } catch (const std::invalid_argument &) { - printError("argument to '--valueflow-max-iteration' is invalid."); + if (!parseNumberArg(argv[i], 27, mSettings.valueFlowMaxIterations)) return false; - } - if (tmp < 0) { - printError("argument to '--valueflow-max-iteration' needs to be at least 0."); - return false; - } - mSettings.valueFlowMaxIterations = static_cast(tmp); } else if (std::strcmp(argv[i], "-v") == 0 || std::strcmp(argv[i], "--verbose") == 0) @@ -975,20 +970,16 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[]) // Define the XML file version (and enable XML output) else if (std::strncmp(argv[i], "--xml-version=", 14) == 0) { - const std::string numberString(argv[i]+14); - - std::istringstream iss(numberString); - if (!(iss >> mSettings.xml_version)) { - printError("argument to '--xml-version' is not a number."); + int tmp; + if (!parseNumberArg(argv[i], 14, tmp)) return false; - } - - if (mSettings.xml_version != 2) { + if (tmp != 2) { // We only have xml version 2 printError("'--xml-version' can only be 2."); return false; } + mSettings.xml_version = tmp; // Enable also XML if version is set mSettings.xml = true; } diff --git a/cli/cmdlineparser.h b/cli/cmdlineparser.h index d7be0f7bea1..6f92aaaf1c0 100644 --- a/cli/cmdlineparser.h +++ b/cli/cmdlineparser.h @@ -22,6 +22,8 @@ #include #include +#include "utils.h" + class Settings; /// @addtogroup CLI @@ -118,6 +120,24 @@ class CmdLineParser { private: bool isCppcheckPremium() const; + // TODO: get rid of is_signed + template + bool parseNumberArg(const char* const arg, std::size_t offset, T& num, bool is_signed = false) + { + T tmp; + std::string err; + if (!strToInt(arg + offset, tmp, &err)) { + printError("argument to '" + std::string(arg, offset) + "' is not valid - " + err + "."); + return false; + } + if (is_signed && tmp < 0) { + printError("argument to '" + std::string(arg, offset) + "' needs to be a positive integer."); + return false; + } + num = tmp; + return true; + } + std::vector mPathNames; std::vector mIgnoredPaths; Settings &mSettings; diff --git a/test/testcmdlineparser.cpp b/test/testcmdlineparser.cpp index 36776694044..48eab1817ad 100644 --- a/test/testcmdlineparser.cpp +++ b/test/testcmdlineparser.cpp @@ -124,8 +124,10 @@ class TestCmdlineParser : public TestFixture { TEST_CASE(fileListInvalid); TEST_CASE(inlineSuppr); TEST_CASE(jobs); + TEST_CASE(jobs2); TEST_CASE(jobsMissingCount); TEST_CASE(jobsInvalid); + TEST_CASE(jobsTooBig); TEST_CASE(maxConfigs); TEST_CASE(maxConfigsMissingCount); TEST_CASE(maxConfigsInvalid); @@ -187,6 +189,28 @@ class TestCmdlineParser : public TestFixture { TEST_CASE(valueFlowMaxIterationsInvalid); TEST_CASE(valueFlowMaxIterationsInvalid2); TEST_CASE(valueFlowMaxIterationsInvalid3); + TEST_CASE(checksMaxTime); + TEST_CASE(checksMaxTime2); + TEST_CASE(checksMaxTimeInvalid); +#ifdef THREADING_MODEL_FORK + TEST_CASE(loadAverage); + TEST_CASE(loadAverage2); + TEST_CASE(loadAverageInvalid); +#endif + TEST_CASE(maxCtuDepth); + TEST_CASE(maxCtuDepthInvalid); + TEST_CASE(performanceValueflowMaxTime); + TEST_CASE(performanceValueflowMaxTimeInvalid); + TEST_CASE(performanceValueFlowMaxIfCount); + TEST_CASE(performanceValueFlowMaxIfCountInvalid); + TEST_CASE(templateMaxTime); + TEST_CASE(templateMaxTimeInvalid); + TEST_CASE(templateMaxTimeInvalid2); + TEST_CASE(typedefMaxTime); + TEST_CASE(typedefMaxTimeInvalid); + TEST_CASE(typedefMaxTimeInvalid2); + TEST_CASE(templateMaxTime); + TEST_CASE(templateMaxTime); TEST_CASE(ignorepaths1); TEST_CASE(ignorepaths2); @@ -839,7 +863,7 @@ class TestCmdlineParser : public TestFixture { const char * const argv[] = {"cppcheck", "--error-exitcode=", "file.cpp"}; // Fails since exit code not given ASSERT_EQUALS(false, defParser.parseFromArgs(3, argv)); - ASSERT_EQUALS("cppcheck: error: argument must be an integer. Try something like '--error-exitcode=1'.\n", GET_REDIRECT_OUTPUT); + ASSERT_EQUALS("cppcheck: error: argument to '--error-exitcode=' is not valid - not an integer.\n", GET_REDIRECT_OUTPUT); } void errorExitcodeStr() { @@ -847,7 +871,7 @@ class TestCmdlineParser : public TestFixture { const char * const argv[] = {"cppcheck", "--error-exitcode=foo", "file.cpp"}; // Fails since invalid exit code ASSERT_EQUALS(false, defParser.parseFromArgs(3, argv)); - ASSERT_EQUALS("cppcheck: error: argument must be an integer. Try something like '--error-exitcode=1'.\n", GET_REDIRECT_OUTPUT); + ASSERT_EQUALS("cppcheck: error: argument to '--error-exitcode=' is not valid - not an integer.\n", GET_REDIRECT_OUTPUT); } void exitcodeSuppressionsOld() { @@ -923,12 +947,21 @@ class TestCmdlineParser : public TestFixture { ASSERT_EQUALS("", GET_REDIRECT_OUTPUT); } + void jobs2() { + REDIRECT; + const char * const argv[] = {"cppcheck", "-j3", "file.cpp"}; + settings.jobs = 0; + ASSERT(defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS(3, settings.jobs); + ASSERT_EQUALS("", GET_REDIRECT_OUTPUT); + } + void jobsMissingCount() { REDIRECT; const char * const argv[] = {"cppcheck", "-j", "file.cpp"}; // Fails since -j is missing thread count ASSERT_EQUALS(false, defParser.parseFromArgs(3, argv)); - ASSERT_EQUALS("cppcheck: error: argument to '-j' is not a number.\n", GET_REDIRECT_OUTPUT); + ASSERT_EQUALS("cppcheck: error: argument to '-j' is not valid - not an integer.\n", GET_REDIRECT_OUTPUT); } void jobsInvalid() { @@ -936,7 +969,14 @@ class TestCmdlineParser : public TestFixture { const char * const argv[] = {"cppcheck", "-j", "e", "file.cpp"}; // Fails since invalid count given for -j ASSERT_EQUALS(false, defParser.parseFromArgs(4, argv)); - ASSERT_EQUALS("cppcheck: error: argument to '-j' is not a number.\n", GET_REDIRECT_OUTPUT); + ASSERT_EQUALS("cppcheck: error: argument to '-j' is not valid - not an integer.\n", GET_REDIRECT_OUTPUT); + } + + void jobsTooBig() { + REDIRECT; + const char * const argv[] = {"cppcheck", "-j10001", "file.cpp"}; + ASSERT(!defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS("cppcheck: error: argument for '-j' is allowed to be 10000 at max.\n", GET_REDIRECT_OUTPUT); } void maxConfigs() { @@ -955,7 +995,7 @@ class TestCmdlineParser : public TestFixture { const char * const argv[] = {"cppcheck", "--max-configs=", "file.cpp"}; // Fails since --max-configs= is missing limit ASSERT_EQUALS(false, defParser.parseFromArgs(3, argv)); - ASSERT_EQUALS("cppcheck: error: argument to '--max-configs=' is not a number.\n", GET_REDIRECT_OUTPUT); + ASSERT_EQUALS("cppcheck: error: argument to '--max-configs=' is not valid - not an integer.\n", GET_REDIRECT_OUTPUT); } void maxConfigsInvalid() { @@ -963,7 +1003,7 @@ class TestCmdlineParser : public TestFixture { const char * const argv[] = {"cppcheck", "--max-configs=e", "file.cpp"}; // Fails since invalid count given for --max-configs= ASSERT_EQUALS(false, defParser.parseFromArgs(3, argv)); - ASSERT_EQUALS("cppcheck: error: argument to '--max-configs=' is not a number.\n", GET_REDIRECT_OUTPUT); + ASSERT_EQUALS("cppcheck: error: argument to '--max-configs=' is not valid - not an integer.\n", GET_REDIRECT_OUTPUT); } void maxConfigsTooSmall() { @@ -1343,7 +1383,7 @@ class TestCmdlineParser : public TestFixture { const char * const argv[] = {"cppcheck", "--xml", "--xml-version=a", "file.cpp"}; // FAils since unknown XML format version ASSERT_EQUALS(false, defParser.parseFromArgs(4, argv)); - ASSERT_EQUALS("cppcheck: error: argument to '--xml-version' is not a number.\n", GET_REDIRECT_OUTPUT); + ASSERT_EQUALS("cppcheck: error: argument to '--xml-version=' is not valid - not an integer.\n", GET_REDIRECT_OUTPUT); } void doc() { @@ -1503,15 +1543,160 @@ class TestCmdlineParser : public TestFixture { REDIRECT; const char * const argv[] = {"cppcheck", "--valueflow-max-iterations=seven"}; ASSERT(!defParser.parseFromArgs(2, argv)); - ASSERT_EQUALS("cppcheck: error: argument to '--valueflow-max-iteration' is invalid.\n", GET_REDIRECT_OUTPUT); + ASSERT_EQUALS("cppcheck: error: argument to '--valueflow-max-iterations=' is not valid - not an integer.\n", GET_REDIRECT_OUTPUT); } void valueFlowMaxIterationsInvalid3() { REDIRECT; const char * const argv[] = {"cppcheck", "--valueflow-max-iterations=-1"}; ASSERT(!defParser.parseFromArgs(2, argv)); - ASSERT_EQUALS("cppcheck: error: argument to '--valueflow-max-iteration' needs to be at least 0.\n", GET_REDIRECT_OUTPUT); + ASSERT_EQUALS("cppcheck: error: argument to '--valueflow-max-iterations=' is not valid - needs to be positive.\n", GET_REDIRECT_OUTPUT); + } + + void checksMaxTime() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--checks-max-time=12", "file.cpp"}; + settings.checksMaxTime = SIZE_MAX; + ASSERT(defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS(12, settings.checksMaxTime); + ASSERT_EQUALS("", GET_REDIRECT_OUTPUT); + } + + void checksMaxTime2() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--checks-max-time=-1", "file.cpp"}; + ASSERT(!defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS("cppcheck: error: argument to '--checks-max-time=' is not valid - needs to be positive.\n", GET_REDIRECT_OUTPUT); + } + + void checksMaxTimeInvalid() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--checks-max-time=one", "file.cpp"}; + ASSERT(!defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS("cppcheck: error: argument to '--checks-max-time=' is not valid - not an integer.\n", GET_REDIRECT_OUTPUT); + } + +#ifdef THREADING_MODEL_FORK + void loadAverage() { + REDIRECT; + const char * const argv[] = {"cppcheck", "-l", "12", "file.cpp"}; + settings.loadAverage = 0; + ASSERT(defParser.parseFromArgs(4, argv)); + ASSERT_EQUALS(12, settings.loadAverage); + ASSERT_EQUALS("", GET_REDIRECT_OUTPUT); + } + + void loadAverage2() { + REDIRECT; + const char * const argv[] = {"cppcheck", "-l12", "file.cpp"}; + settings.loadAverage = 0; + ASSERT(defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS(12, settings.loadAverage); + ASSERT_EQUALS("", GET_REDIRECT_OUTPUT); + } + + void loadAverageInvalid() { + REDIRECT; + const char * const argv[] = {"cppcheck", "-l", "one", "file.cpp"}; + ASSERT(!defParser.parseFromArgs(4, argv)); + ASSERT_EQUALS("cppcheck: error: argument to '-l' is not valid - not an integer.\n", GET_REDIRECT_OUTPUT); + } +#endif + + void maxCtuDepth() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--max-ctu-depth=12", "file.cpp"}; + settings.maxCtuDepth = 0; + ASSERT(defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS(12, settings.maxCtuDepth); + ASSERT_EQUALS("", GET_REDIRECT_OUTPUT); + } + + void maxCtuDepthInvalid() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--max-ctu-depth=one", "file.cpp"}; + ASSERT(!defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS("cppcheck: error: argument to '--max-ctu-depth=' is not valid - not an integer.\n", GET_REDIRECT_OUTPUT); + } + + void performanceValueflowMaxTime() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--performance-valueflow-max-time=12", "file.cpp"}; + settings.performanceValueFlowMaxTime = 0; + ASSERT(defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS(12, settings.performanceValueFlowMaxTime); + ASSERT_EQUALS("", GET_REDIRECT_OUTPUT); + } + + void performanceValueflowMaxTimeInvalid() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--performance-valueflow-max-time=one", "file.cpp"}; + ASSERT(!defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS("cppcheck: error: argument to '--performance-valueflow-max-time=' is not valid - not an integer.\n", GET_REDIRECT_OUTPUT); + } + + void performanceValueFlowMaxIfCount() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--performance-valueflow-max-if-count=12", "file.cpp"}; + settings.performanceValueFlowMaxIfCount = 0; + ASSERT(defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS(12, settings.performanceValueFlowMaxIfCount); + ASSERT_EQUALS("", GET_REDIRECT_OUTPUT); + } + + void performanceValueFlowMaxIfCountInvalid() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--performance-valueflow-max-if-count=one", "file.cpp"}; + ASSERT(!defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS("cppcheck: error: argument to '--performance-valueflow-max-if-count=' is not valid - not an integer.\n", GET_REDIRECT_OUTPUT); } + + void templateMaxTime() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--template-max-time=12", "file.cpp"}; + settings.templateMaxTime = 0; + ASSERT(defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS(12, settings.templateMaxTime); + ASSERT_EQUALS("", GET_REDIRECT_OUTPUT); + } + + void templateMaxTimeInvalid() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--template-max-time=one", "file.cpp"}; + ASSERT(!defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS("cppcheck: error: argument to '--template-max-time=' is not valid - not an integer.\n", GET_REDIRECT_OUTPUT); + } + + void templateMaxTimeInvalid2() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--template-max-time=-1", "file.cpp"}; + ASSERT(!defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS("cppcheck: error: argument to '--template-max-time=' is not valid - needs to be positive.\n", GET_REDIRECT_OUTPUT); + } + + void typedefMaxTime() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--typedef-max-time=12", "file.cpp"}; + settings.typedefMaxTime = 0; + ASSERT(defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS(12, settings.typedefMaxTime); + ASSERT_EQUALS("", GET_REDIRECT_OUTPUT); + } + + void typedefMaxTimeInvalid() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--typedef-max-time=one", "file.cpp"}; + ASSERT(!defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS("cppcheck: error: argument to '--typedef-max-time=' is not valid - not an integer.\n", GET_REDIRECT_OUTPUT); + } + + void typedefMaxTimeInvalid2() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--typedef-max-time=-1", "file.cpp"}; + ASSERT(!defParser.parseFromArgs(3, argv)); + ASSERT_EQUALS("cppcheck: error: argument to '--typedef-max-time=' is not valid - needs to be positive.\n", GET_REDIRECT_OUTPUT); + } + void ignorepaths1() { REDIRECT; const char * const argv[] = {"cppcheck", "-isrc", "file.cpp"}; From f449a45e937312cd0c7faaffda83320076a53998 Mon Sep 17 00:00:00 2001 From: firewave Date: Tue, 21 Mar 2023 13:44:00 +0100 Subject: [PATCH 03/11] use `strToInt()` instead of string-to-integer conversion without error handling --- lib/checkbufferoverrun.cpp | 4 ++-- lib/checkclass.cpp | 6 +++--- lib/checkio.cpp | 6 +++--- lib/checkunusedfunctions.cpp | 2 +- lib/clangimport.cpp | 10 +++++----- lib/cppcheck.cpp | 8 ++++---- lib/errorlogger.cpp | 33 ++++++++++++--------------------- lib/importproject.cpp | 6 +++--- lib/library.cpp | 24 ++++++++++++------------ lib/suppressions.cpp | 4 ++-- test/testerrorlogger.cpp | 8 ++++---- 11 files changed, 51 insertions(+), 60 deletions(-) diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 0755926bbf2..4887f6d065e 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -143,13 +143,13 @@ static int getMinFormatStringOutputLength(const std::vector ¶m outputStringSize++; if (handleNextParameter) { - int tempDigits = std::abs(std::atoi(digits_string.c_str())); + int tempDigits = std::abs(strToInt(digits_string)); if (i_d_x_f_found) tempDigits = std::max(tempDigits, 1); if (digits_string.find('.') != std::string::npos) { const std::string endStr = digits_string.substr(digits_string.find('.') + 1); - const int maxLen = std::max(std::abs(std::atoi(endStr.c_str())), 1); + const int maxLen = std::max(std::abs(strToInt(endStr)), 1); if (formatString[i] == 's') { // For strings, the length after the dot "%.2s" will limit diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 4dc0ac55496..fdf0bcdf308 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -3224,9 +3224,9 @@ Check::FileInfo * CheckClass::loadFileInfoFromXml(const tinyxml2::XMLElement *xm MyFileInfo::NameLoc nameLoc; nameLoc.className = name; nameLoc.fileName = file; - nameLoc.lineNumber = std::atoi(line); - nameLoc.column = std::atoi(col); - nameLoc.hash = MathLib::toULongNumber(hash); + nameLoc.lineNumber = strToInt(line); + nameLoc.column = strToInt(col); + nameLoc.hash = strToInt(hash); fileInfo->classDefinitions.push_back(std::move(nameLoc)); } } diff --git a/lib/checkio.cpp b/lib/checkio.cpp index 2573d924622..a01ff0c302e 100644 --- a/lib/checkio.cpp +++ b/lib/checkio.cpp @@ -643,7 +643,7 @@ void CheckIO::checkFormatString(const Token * const tok, } else if (std::isdigit(*i)) { width += *i; } else if (*i == '$') { - parameterPosition = std::atoi(width.c_str()); + parameterPosition = strToInt(width); hasParameterPosition = true; width.clear(); } @@ -695,7 +695,7 @@ void CheckIO::checkFormatString(const Token * const tok, specifier += (*i == 's' || bracketBeg == formatString.end()) ? std::string{ 's' } : std::string{ bracketBeg, i + 1 }; if (argInfo.variableInfo && argInfo.isKnownType() && argInfo.variableInfo->isArray() && (argInfo.variableInfo->dimensions().size() == 1) && argInfo.variableInfo->dimensions()[0].known) { if (!width.empty()) { - const int numWidth = std::atoi(width.c_str()); + const int numWidth = strToInt(width); if (numWidth != (argInfo.variableInfo->dimension(0) - 1)) invalidScanfFormatWidthError(tok, numFormat, numWidth, argInfo.variableInfo, specifier); } @@ -718,7 +718,7 @@ void CheckIO::checkFormatString(const Token * const tok, case 'c': if (argInfo.variableInfo && argInfo.isKnownType() && argInfo.variableInfo->isArray() && (argInfo.variableInfo->dimensions().size() == 1) && argInfo.variableInfo->dimensions()[0].known) { if (!width.empty()) { - const int numWidth = std::atoi(width.c_str()); + const int numWidth = strToInt(width); if (numWidth > argInfo.variableInfo->dimension(0)) invalidScanfFormatWidthError(tok, numFormat, numWidth, argInfo.variableInfo, std::string(1, *i)); } diff --git a/lib/checkunusedfunctions.cpp b/lib/checkunusedfunctions.cpp index 7f621a91485..86e030c47eb 100644 --- a/lib/checkunusedfunctions.cpp +++ b/lib/checkunusedfunctions.cpp @@ -442,7 +442,7 @@ void CheckUnusedFunctions::analyseWholeProgram(const Settings &settings, ErrorLo } else if (std::strcmp(e2->Name(),"functiondecl") == 0) { const char* lineNumber = e2->Attribute("lineNumber"); if (lineNumber) - decls[functionName] = Location(sourcefile, std::atoi(lineNumber)); + decls[functionName] = Location(sourcefile, strToInt(lineNumber)); } } } diff --git a/lib/clangimport.cpp b/lib/clangimport.cpp index 4855fd50ad4..0d3e60fe2fa 100644 --- a/lib/clangimport.cpp +++ b/lib/clangimport.cpp @@ -501,11 +501,11 @@ void clangimport::AstNode::setLocations(TokenList *tokenList, int file, int line { for (const std::string &ext: mExtTokens) { if (ext.compare(0, 5, "(ext.substr(5)); else if (ext.compare(0, 6, "(ext.substr(6)); if (ext.find(", col:") != std::string::npos) - col = std::atoi(ext.c_str() + ext.find(", col:") + 6); + col = strToInt(ext.c_str() + ext.find(", col:") + 6); } else if (ext[0] == '<') { const std::string::size_type colon = ext.find(':'); if (colon != std::string::npos) { @@ -513,7 +513,7 @@ void clangimport::AstNode::setLocations(TokenList *tokenList, int file, int line const std::string::size_type sep1 = windowsPath ? ext.find(':', 4) : colon; const std::string::size_type sep2 = ext.find(':', sep1 + 1); file = tokenList->appendFileIfNew(ext.substr(1, sep1 - 1)); - line = MathLib::toLongNumber(ext.substr(sep1 + 1, sep2 - sep1 - 1)); + line = strToInt(ext.substr(sep1 + 1, sep2 - sep1 - 1)); } } } @@ -1551,7 +1551,7 @@ static void setValues(Tokenizer *tokenizer, SymbolDatabase *symbolDatabase) for (const Token *arrtok = tok->linkAt(1)->previous(); arrtok; arrtok = arrtok->previous()) { const std::string &a = arrtok->str(); if (a.size() > 2 && a[0] == '[' && a.back() == ']') - mul *= std::atoi(a.substr(1).c_str()); + mul *= strToInt(a.substr(1)); else break; } diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index c105fe5512b..a6d10b9cf42 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -452,8 +452,8 @@ static bool reportClangErrors(std::istream &is, const std::function(linenr); + loc.column = strToInt(colnr); ErrorMessage errmsg({std::move(loc)}, locFile, Severity::error, @@ -1730,8 +1730,8 @@ void CppCheck::analyseClangTidy(const ImportProject::FileSettings &fileSettings) const std::string errorString = line.substr(endErrorPos, line.length()); std::string fixedpath = Path::simplifyPath(line.substr(0, endNamePos)); - const int64_t lineNumber = std::atol(lineNumString.c_str()); - const int64_t column = std::atol(columnNumString.c_str()); + const int64_t lineNumber = strToInt(lineNumString); + const int64_t column = strToInt(columnNumString); fixedpath = Path::toNativeSeparators(fixedpath); ErrorMessage errmsg; diff --git a/lib/errorlogger.cpp b/lib/errorlogger.cpp index e1f15520efa..218d4a24e42 100644 --- a/lib/errorlogger.cpp +++ b/lib/errorlogger.cpp @@ -151,7 +151,7 @@ ErrorMessage::ErrorMessage(const tinyxml2::XMLElement * const errmsg) severity = attr ? Severity::fromString(attr) : Severity::none; attr = errmsg->Attribute("cwe"); - std::istringstream(attr ? attr : "0") >> cwe.id; + cwe.id = attr ? strToInt(attr) : 0; attr = errmsg->Attribute("inconclusive"); certainty = (attr && (std::strcmp(attr, "true") == 0)) ? Certainty::inconclusive : Certainty::normal; @@ -163,7 +163,7 @@ ErrorMessage::ErrorMessage(const tinyxml2::XMLElement * const errmsg) mVerboseMessage = attr ? attr : ""; attr = errmsg->Attribute("hash"); - std::istringstream(attr ? attr : "0") >> hash; + hash = attr ? strToInt(attr) : 0; for (const tinyxml2::XMLElement *e = errmsg->FirstChildElement(); e; e = e->NextSiblingElement()) { if (std::strcmp(e->Name(),"location")==0) { @@ -174,8 +174,8 @@ ErrorMessage::ErrorMessage(const tinyxml2::XMLElement * const errmsg) const char *file = strfile ? strfile : unknown; const char *info = strinfo ? strinfo : ""; - const int line = strline ? std::atoi(strline) : 0; - const int column = strcolumn ? std::atoi(strcolumn) : 0; + const int line = strline ? strToInt(strline) : 0; + const int column = strcolumn ? strToInt(strcolumn) : 0; callStack.emplace_front(file, info, line, column); } else if (std::strcmp(e->Name(),"symbol")==0) { mSymbolNames += e->GetText(); @@ -301,26 +301,17 @@ void ErrorMessage::deserialize(const std::string &data) id = std::move(results[0]); severity = Severity::fromString(results[1]); - unsigned long long tmp = 0; + cwe.id = 0; if (!results[2].empty()) { - try { - tmp = MathLib::toULongNumber(results[2]); - } - catch (const InternalError&) { - throw InternalError(nullptr, "Internal Error: Deserialization of error message failed - invalid CWE ID"); - } - if (tmp > std::numeric_limits::max()) - throw InternalError(nullptr, "Internal Error: Deserialization of error message failed - CWE ID is out of range"); + std::string err; + if (!strToInt(results[2], cwe.id, &err)) + throw InternalError(nullptr, "Internal Error: Deserialization of error message failed - invalid CWE ID - " + err); } - cwe.id = static_cast(tmp); hash = 0; if (!results[3].empty()) { - try { - hash = MathLib::toULongNumber(results[3]); - } - catch (const InternalError&) { - throw InternalError(nullptr, "Internal Error: Deserialization of error message failed - invalid hash"); - } + std::string err; + if (!strToInt(results[3], hash, &err)) + throw InternalError(nullptr, "Internal Error: Deserialization of error message failed - invalid hash - " + err); } file0 = std::move(results[4]); mShortMessage = std::move(results[5]); @@ -373,7 +364,7 @@ void ErrorMessage::deserialize(const std::string &data) // (*loc).line << '\t' << (*loc).column << '\t' << (*loc).getfile(false) << '\t' << loc->getOrigFile(false) << '\t' << loc->getinfo(); - ErrorMessage::FileLocation loc(substrings[3], MathLib::toLongNumber(substrings[0]), MathLib::toLongNumber(substrings[1])); + ErrorMessage::FileLocation loc(substrings[3], strToInt(substrings[0]), strToInt(substrings[1])); loc.setfile(std::move(substrings[2])); if (substrings.size() == 5) loc.setinfo(substrings[4]); diff --git a/lib/importproject.cpp b/lib/importproject.cpp index 714b7a5dc9a..21bc76105ee 100644 --- a/lib/importproject.cpp +++ b/lib/importproject.cpp @@ -1190,7 +1190,7 @@ bool ImportProject::importCppcheckGuiProject(std::istream &istr, Settings *setti s.fileName = joinRelativePath(path, s.fileName); s.lineNumber = child->IntAttribute("lineNumber", Suppressions::Suppression::NO_LINE); s.symbolName = readSafe(child->Attribute("symbolName"), ""); - std::istringstream(readSafe(child->Attribute("hash"), "0")) >> s.hash; + s.hash = strToInt(readSafe(child->Attribute("hash"), "0")); suppressions.push_back(std::move(s)); } } else if (strcmp(node->Name(), CppcheckXml::VSConfigurationElementName) == 0) @@ -1218,9 +1218,9 @@ bool ImportProject::importCppcheckGuiProject(std::istream &istr, Settings *setti else if (strcmp(node->Name(), CppcheckXml::CheckUnusedTemplatesElementName) == 0) temp.checkUnusedTemplates = (strcmp(readSafe(node->GetText(), ""), "true") == 0); else if (strcmp(node->Name(), CppcheckXml::MaxCtuDepthElementName) == 0) - temp.maxCtuDepth = std::atoi(readSafe(node->GetText(), "")); // TODO: get rid of atoi() + temp.maxCtuDepth = strToInt(readSafe(node->GetText(), "")); else if (strcmp(node->Name(), CppcheckXml::MaxTemplateRecursionElementName) == 0) - temp.maxTemplateRecursion = std::atoi(readSafe(node->GetText(), "")); // TODO: get rid of atoi() + temp.maxTemplateRecursion = strToInt(readSafe(node->GetText(), "")); else if (strcmp(node->Name(), CppcheckXml::CheckUnknownFunctionReturn) == 0) ; // TODO else if (strcmp(node->Name(), Settings::SafeChecks::XmlRootName) == 0) { diff --git a/lib/library.cpp b/lib/library.cpp index e5b08e463a6..7df47eb3b2d 100644 --- a/lib/library.cpp +++ b/lib/library.cpp @@ -327,7 +327,7 @@ Library::Error Library::load(const tinyxml2::XMLDocument &doc) if (!argString) return Error(ErrorCode::MISSING_ATTRIBUTE, "arg"); - mReflection[reflectionnode->GetText()] = atoi(argString); + mReflection[reflectionnode->GetText()] = strToInt(argString); } } @@ -402,7 +402,7 @@ Library::Error Library::load(const tinyxml2::XMLDocument &doc) mExecutableBlocks[extension].setEnd(end); const char * offset = blocknode->Attribute("offset"); if (offset) - mExecutableBlocks[extension].setOffset(atoi(offset)); + mExecutableBlocks[extension].setOffset(strToInt(offset)); } else @@ -490,7 +490,7 @@ Library::Error Library::load(const tinyxml2::XMLDocument &doc) if (containerNodeName == "size") { const char* const templateArg = containerNode->Attribute("templateParameter"); if (templateArg) - container.size_templateArgNo = atoi(templateArg); + container.size_templateArgNo = strToInt(templateArg); } else if (containerNodeName == "access") { const char* const indexArg = containerNode->Attribute("indexOperator"); if (indexArg) @@ -499,7 +499,7 @@ Library::Error Library::load(const tinyxml2::XMLDocument &doc) } else if (containerNodeName == "type") { const char* const templateArg = containerNode->Attribute("templateParameter"); if (templateArg) - container.type_templateArgNo = atoi(templateArg); + container.type_templateArgNo = strToInt(templateArg); const char* const string = containerNode->Attribute("string"); if (string) @@ -521,7 +521,7 @@ Library::Error Library::load(const tinyxml2::XMLDocument &doc) const char *memberTemplateParameter = memberNode->Attribute("templateParameter"); struct Container::RangeItemRecordTypeItem member; member.name = memberName ? memberName : ""; - member.templateParameter = memberTemplateParameter ? std::atoi(memberTemplateParameter) : -1; + member.templateParameter = memberTemplateParameter ? strToInt(memberTemplateParameter) : -1; container.rangeItemRecordType.emplace_back(std::move(member)); } } else @@ -584,7 +584,7 @@ Library::Error Library::load(const tinyxml2::XMLDocument &doc) } const char * const size = node->Attribute("size"); if (size) - podType.size = atoi(size); + podType.size = strToInt(size); const char * const sign = node->Attribute("sign"); if (sign) podType.sign = *sign; @@ -709,7 +709,7 @@ Library::Error Library::loadFunction(const tinyxml2::XMLElement * const node, co if (const char *type = functionnode->Attribute("type")) mReturnValueType[name] = type; if (const char *container = functionnode->Attribute("container")) - mReturnValueContainer[name] = std::atoi(container); + mReturnValueContainer[name] = strToInt(container); if (const char *unknownReturnValues = functionnode->Attribute("unknownValues")) { if (std::strcmp(unknownReturnValues, "all") == 0) { std::vector values{LLONG_MIN, LLONG_MAX}; @@ -722,7 +722,7 @@ Library::Error Library::loadFunction(const tinyxml2::XMLElement * const node, co return Error(ErrorCode::MISSING_ATTRIBUTE, "nr"); const bool bAnyArg = strcmp(argNrString, "any") == 0; const bool bVariadicArg = strcmp(argNrString, "variadic") == 0; - const int nr = (bAnyArg || bVariadicArg) ? -1 : std::atoi(argNrString); + const int nr = (bAnyArg || bVariadicArg) ? -1 : strToInt(argNrString); ArgumentChecks &ac = func.argumentChecks[nr]; ac.optional = functionnode->Attribute("default") != nullptr; ac.variadic = bVariadicArg; @@ -742,7 +742,7 @@ Library::Error Library::loadFunction(const tinyxml2::XMLElement * const node, co int indirect = 0; const char * const indirectStr = argnode->Attribute("indirect"); if (indirectStr) - indirect = atoi(indirectStr); + indirect = strToInt(indirectStr); if (argnodename == "not-bool") ac.notbool = true; else if (argnodename == "not-null") @@ -786,8 +786,8 @@ Library::Error Library::loadFunction(const tinyxml2::XMLElement * const node, co return Error(ErrorCode::MISSING_ATTRIBUTE, "value"); long long minsizevalue = 0; try { - minsizevalue = MathLib::toLongNumber(valueattr); - } catch (const InternalError&) { + minsizevalue = strToInt(valueattr); + } catch (const std::runtime_error&) { return Error(ErrorCode::BAD_ATTRIBUTE_VALUE, valueattr); } if (minsizevalue <= 0) @@ -1698,7 +1698,7 @@ std::shared_ptr createTokenFromExpression(const std::string& returnValue, for (Token* tok2 = tokenList->front(); tok2; tok2 = tok2->next()) { if (tok2->str().compare(0, 3, "arg") != 0) continue; - nonneg int const id = std::atoi(tok2->str().c_str() + 3); + nonneg int const id = strToInt(tok2->str().c_str() + 3); tok2->varId(id); if (lookupVarId) (*lookupVarId)[id] = tok2; diff --git a/lib/suppressions.cpp b/lib/suppressions.cpp index fcde8f5e7bd..37920e029c5 100644 --- a/lib/suppressions.cpp +++ b/lib/suppressions.cpp @@ -101,11 +101,11 @@ std::string Suppressions::parseXmlFile(const char *filename) else if (std::strcmp(e2->Name(), "fileName") == 0) s.fileName = text; else if (std::strcmp(e2->Name(), "lineNumber") == 0) - s.lineNumber = std::atoi(text); + s.lineNumber = strToInt(text); else if (std::strcmp(e2->Name(), "symbolName") == 0) s.symbolName = text; else if (*text && std::strcmp(e2->Name(), "hash") == 0) - std::istringstream(text) >> s.hash; + s.hash = strToInt(text); else return "Unknown suppression element \"" + std::string(e2->Name()) + "\", expected id/fileName/lineNumber/symbolName/hash"; } diff --git a/test/testerrorlogger.cpp b/test/testerrorlogger.cpp index c6ad97857fb..941c5201fe4 100644 --- a/test/testerrorlogger.cpp +++ b/test/testerrorlogger.cpp @@ -333,27 +333,27 @@ class TestErrorLogger : public TestFixture { // invalid CWE ID const char str[] = "7 errorId" "5 error" - "7 invalid" + "7 invalid" // cwe "1 0" "8 test.cpp" "17 Programming error" "17 Programming error" "0 "; ErrorMessage msg; - ASSERT_THROW_EQUALS(msg.deserialize(str), InternalError, "Internal Error: Deserialization of error message failed - invalid CWE ID"); + ASSERT_THROW_EQUALS(msg.deserialize(str), InternalError, "Internal Error: Deserialization of error message failed - invalid CWE ID - not an integer"); } { // invalid hash const char str[] = "7 errorId" "5 error" "1 0" - "7 invalid" + "7 invalid" // hash "8 test.cpp" "17 Programming error" "17 Programming error" "0 "; ErrorMessage msg; - ASSERT_THROW_EQUALS(msg.deserialize(str), InternalError, "Internal Error: Deserialization of error message failed - invalid hash"); + ASSERT_THROW_EQUALS(msg.deserialize(str), InternalError, "Internal Error: Deserialization of error message failed - invalid hash - not an integer"); } { // out-of-range CWE ID From 8e457a023198bf827296a946d398d9eeedc4e277 Mon Sep 17 00:00:00 2001 From: firewave Date: Tue, 21 Mar 2023 13:44:20 +0100 Subject: [PATCH 04/11] .clang-tidy: enabled `cert-err34-c` --- .clang-tidy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.clang-tidy b/.clang-tidy index 6dccb846e18..8542591b805 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,5 +1,5 @@ --- -Checks: '*,-abseil-*,-altera-*,-android-*,-boost-*,-cert-*,-cppcoreguidelines-*,-darwin-*,-fuchsia-*,-google-*,-hicpp-*,-linuxkernel-*,-llvm-*,-llvmlibc-*,-mpi-*,-objc-*,-openmp-*,-zircon-*,google-explicit-constructor,-readability-braces-around-statements,-readability-magic-numbers,-bugprone-macro-parentheses,-readability-isolate-declaration,-readability-function-size,-modernize-use-trailing-return-type,-readability-implicit-bool-conversion,-readability-uppercase-literal-suffix,-modernize-use-auto,-readability-else-after-return,-modernize-use-default-member-init,-readability-redundant-member-init,-modernize-avoid-c-arrays,-modernize-use-equals-default,-readability-container-size-empty,-bugprone-branch-clone,-bugprone-narrowing-conversions,-modernize-raw-string-literal,-readability-convert-member-functions-to-static,-modernize-loop-convert,-readability-const-return-type,-modernize-return-braced-init-list,-performance-inefficient-string-concatenation,-misc-throw-by-value-catch-by-reference,-readability-avoid-const-params-in-decls,-misc-non-private-member-variables-in-classes,-clang-analyzer-*,-bugprone-signed-char-misuse,-misc-no-recursion,-readability-use-anyofallof,-performance-no-automatic-move,-readability-function-cognitive-complexity,-readability-redundant-access-specifiers,-concurrency-mt-unsafe,-bugprone-easily-swappable-parameters,-readability-suspicious-call-argument,-readability-identifier-length,-readability-container-data-pointer,-bugprone-assignment-in-if-condition,-misc-const-correctness,-portability-std-allocator-const,-modernize-deprecated-ios-base-aliases,-bugprone-unchecked-optional-access,-modernize-replace-auto-ptr,-readability-identifier-naming,-portability-simd-intrinsics,-misc-use-anonymous-namespace' +Checks: '*,-abseil-*,-altera-*,-android-*,-boost-*,-cert-*,-cppcoreguidelines-*,-darwin-*,-fuchsia-*,-google-*,-hicpp-*,-linuxkernel-*,-llvm-*,-llvmlibc-*,-mpi-*,-objc-*,-openmp-*,-zircon-*,google-explicit-constructor,-readability-braces-around-statements,-readability-magic-numbers,-bugprone-macro-parentheses,-readability-isolate-declaration,-readability-function-size,-modernize-use-trailing-return-type,-readability-implicit-bool-conversion,-readability-uppercase-literal-suffix,-modernize-use-auto,-readability-else-after-return,-modernize-use-default-member-init,-readability-redundant-member-init,-modernize-avoid-c-arrays,-modernize-use-equals-default,-readability-container-size-empty,-bugprone-branch-clone,-bugprone-narrowing-conversions,-modernize-raw-string-literal,-readability-convert-member-functions-to-static,-modernize-loop-convert,-readability-const-return-type,-modernize-return-braced-init-list,-performance-inefficient-string-concatenation,-misc-throw-by-value-catch-by-reference,-readability-avoid-const-params-in-decls,-misc-non-private-member-variables-in-classes,-clang-analyzer-*,-bugprone-signed-char-misuse,-misc-no-recursion,-readability-use-anyofallof,-performance-no-automatic-move,-readability-function-cognitive-complexity,-readability-redundant-access-specifiers,-concurrency-mt-unsafe,-bugprone-easily-swappable-parameters,-readability-suspicious-call-argument,-readability-identifier-length,-readability-container-data-pointer,-bugprone-assignment-in-if-condition,-misc-const-correctness,-portability-std-allocator-const,-modernize-deprecated-ios-base-aliases,-bugprone-unchecked-optional-access,-modernize-replace-auto-ptr,-readability-identifier-naming,-portability-simd-intrinsics,-misc-use-anonymous-namespace,cert-err34-c' WarningsAsErrors: '*' HeaderFilterRegex: '(cli|gui|lib|oss-fuzz|test|triage)\/[a-z]+\.h' CheckOptions: From 9c857ba7db72057a92c184e6c7245bab7ffa2131 Mon Sep 17 00:00:00 2001 From: firewave Date: Tue, 21 Mar 2023 13:52:59 +0100 Subject: [PATCH 05/11] updates `releasenotes.txt` --- releasenotes.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/releasenotes.txt b/releasenotes.txt index a9800e10726..5f98590a39e 100644 --- a/releasenotes.txt +++ b/releasenotes.txt @@ -13,3 +13,4 @@ release notes for cppcheck-2.11 - `constVariableReference` - `constVariablePointer` - Limit valueflow analysis in function if the "if" count is high. By default max limit is 100. Limit can be tweaked with --performance-valueflow-max-if-count +- More command-line parameters will now check if the given integer argument is actually valid. Several other internal string-to-integer conversions will not be error checked. From aa202e541ef8ead14cd3fe4a3604b544c9b71cd2 Mon Sep 17 00:00:00 2001 From: firewave Date: Tue, 21 Mar 2023 14:40:57 +0100 Subject: [PATCH 06/11] clangimport.cpp: only try to convert the proper part of the location in `AstNode::setLocations()` --- lib/clangimport.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/clangimport.cpp b/lib/clangimport.cpp index 0d3e60fe2fa..1872d3cab33 100644 --- a/lib/clangimport.cpp +++ b/lib/clangimport.cpp @@ -501,11 +501,12 @@ void clangimport::AstNode::setLocations(TokenList *tokenList, int file, int line { for (const std::string &ext: mExtTokens) { if (ext.compare(0, 5, "(ext.substr(5)); + col = strToInt(ext.substr(5, ext.find_first_of(",>", 5) - 5)); else if (ext.compare(0, 6, "(ext.substr(6)); - if (ext.find(", col:") != std::string::npos) - col = strToInt(ext.c_str() + ext.find(", col:") + 6); + line = strToInt(ext.substr(6, ext.find_first_of(":,>", 6) - 6)); + const auto pos = ext.find(", col:"); + if (pos != std::string::npos) + col = strToInt(ext.substr(pos+6, ext.find_first_of(":,>", pos+6) - (pos+6))); } else if (ext[0] == '<') { const std::string::size_type colon = ext.find(':'); if (colon != std::string::npos) { From 1ce2a3ecb78375698df90f8911548b220eec86ad Mon Sep 17 00:00:00 2001 From: firewave Date: Tue, 21 Mar 2023 16:48:25 +0100 Subject: [PATCH 07/11] checkbufferoverrun.cpp: revert to `atoi()` (for now) --- lib/checkbufferoverrun.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 4887f6d065e..7b1a040ae7f 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -143,7 +143,8 @@ static int getMinFormatStringOutputLength(const std::vector ¶m outputStringSize++; if (handleNextParameter) { - int tempDigits = std::abs(strToInt(digits_string)); + // NOLINTNEXTLINE(cert-err34-c) - intentional use + int tempDigits = std::abs(std::atoi(digits_string.c_str())); if (i_d_x_f_found) tempDigits = std::max(tempDigits, 1); From e7f6c7e592e1d7714663bd83959780f54fc9b06a Mon Sep 17 00:00:00 2001 From: firewave Date: Tue, 21 Mar 2023 17:43:10 +0100 Subject: [PATCH 08/11] importproject.cpp: mitigated parsing of some empty fields in `importCppcheckGuiProject()` --- lib/importproject.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/importproject.cpp b/lib/importproject.cpp index 21bc76105ee..9e547535815 100644 --- a/lib/importproject.cpp +++ b/lib/importproject.cpp @@ -1147,6 +1147,7 @@ bool ImportProject::importCppcheckGuiProject(std::istream &istr, Settings *setti guiProject.analyzeAllVsConfigs.clear(); + // TODO: this should support all available command-line options for (const tinyxml2::XMLElement *node = rootnode->FirstChildElement(); node; node = node->NextSiblingElement()) { if (strcmp(node->Name(), CppcheckXml::RootPathName) == 0) { if (node->Attribute(CppcheckXml::RootPathNameAttrib)) { @@ -1218,9 +1219,9 @@ bool ImportProject::importCppcheckGuiProject(std::istream &istr, Settings *setti else if (strcmp(node->Name(), CppcheckXml::CheckUnusedTemplatesElementName) == 0) temp.checkUnusedTemplates = (strcmp(readSafe(node->GetText(), ""), "true") == 0); else if (strcmp(node->Name(), CppcheckXml::MaxCtuDepthElementName) == 0) - temp.maxCtuDepth = strToInt(readSafe(node->GetText(), "")); + temp.maxCtuDepth = strToInt(readSafe(node->GetText(), "2")); // TODO: bail out when missing? else if (strcmp(node->Name(), CppcheckXml::MaxTemplateRecursionElementName) == 0) - temp.maxTemplateRecursion = strToInt(readSafe(node->GetText(), "")); + temp.maxTemplateRecursion = strToInt(readSafe(node->GetText(), "100")); // TODO: bail out when missing? else if (strcmp(node->Name(), CppcheckXml::CheckUnknownFunctionReturn) == 0) ; // TODO else if (strcmp(node->Name(), Settings::SafeChecks::XmlRootName) == 0) { From 4be7df3832e200f73072ebff64625febf0c7d6b3 Mon Sep 17 00:00:00 2001 From: firewave Date: Tue, 21 Mar 2023 18:08:24 +0100 Subject: [PATCH 09/11] cmdlineparser.h: fixed `functionStatic` selfcheck warning --- cli/cmdlineparser.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/cmdlineparser.h b/cli/cmdlineparser.h index 6f92aaaf1c0..a5f3e4adb71 100644 --- a/cli/cmdlineparser.h +++ b/cli/cmdlineparser.h @@ -122,7 +122,7 @@ class CmdLineParser { // TODO: get rid of is_signed template - bool parseNumberArg(const char* const arg, std::size_t offset, T& num, bool is_signed = false) + static bool parseNumberArg(const char* const arg, std::size_t offset, T& num, bool is_signed = false) { T tmp; std::string err; From 147a0f9caf1b6d2285bcb5cb669f6d84c2e53a85 Mon Sep 17 00:00:00 2001 From: firewave Date: Tue, 21 Mar 2023 20:09:59 +0100 Subject: [PATCH 10/11] cmdlineparser.cpp: added TODO --- cli/cmdlineparser.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/cli/cmdlineparser.cpp b/cli/cmdlineparser.cpp index 2c606cf87db..553b218f2b6 100644 --- a/cli/cmdlineparser.cpp +++ b/cli/cmdlineparser.cpp @@ -287,6 +287,7 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[]) } else if (std::strncmp(argv[i], "--cppcheck-build-dir=", 21) == 0) { + // TODO: bail out when the folder does not exist? will silently do nothing mSettings.buildDir = Path::fromNativeSeparators(argv[i] + 21); if (endsWith(mSettings.buildDir, '/')) mSettings.buildDir.pop_back(); From e837878309cfbdf8bdb5ce1e97be8e648faaf82f Mon Sep 17 00:00:00 2001 From: firewave Date: Tue, 4 Apr 2023 13:23:17 +0200 Subject: [PATCH 11/11] disabled `templateInstantiation` in selfcheck for now --- .selfcheck_suppressions | 1 + 1 file changed, 1 insertion(+) diff --git a/.selfcheck_suppressions b/.selfcheck_suppressions index c61108cdc9c..b6fd4de668a 100644 --- a/.selfcheck_suppressions +++ b/.selfcheck_suppressions @@ -5,6 +5,7 @@ bitwiseOnBoolean # temporary suppressions - fix the warnings! simplifyUsing:lib/valueptr.h varid0:gui/projectfile.cpp +templateInstantiation # warnings in Qt generated code we cannot fix symbolDatabaseWarning:gui/temp/moc_*.cpp