From 81bf2f041f67cad7b6c2fde61c8b69a8cdd35d5e Mon Sep 17 00:00:00 2001 From: firewave Date: Fri, 20 Oct 2023 11:49:43 +0200 Subject: [PATCH] fixed #13937/#12089 - disallowed unknown char type signedness in platforms / deprecated `unspecified` platform [skip ci] --- lib/platform.cpp | 40 +++++++++++++----- lib/platform.h | 4 +- lib/symboldatabase.cpp | 6 +-- releasenotes.txt | 3 ++ test/cli/other_test.py | 85 ++++++++++++++++++++++++++++++++++++++ test/testcmdlineparser.cpp | 10 ----- test/testplatform.cpp | 47 +++++++++++++++------ 7 files changed, 155 insertions(+), 40 deletions(-) diff --git a/lib/platform.cpp b/lib/platform.cpp index 48567c86d05..168b7e1bd39 100644 --- a/lib/platform.cpp +++ b/lib/platform.cpp @@ -52,11 +52,7 @@ bool Platform::set(Type t) sizeof_wchar_t = sizeof(wchar_t); sizeof_size_t = sizeof(std::size_t); sizeof_pointer = sizeof(void *); - if (type == Type::Unspecified) { - defaultSign = '\0'; - } else { - defaultSign = std::numeric_limits::is_signed ? 's' : 'u'; - } + defaultSign = std::numeric_limits::is_signed ? 's' : 'u'; char_bit = 8; calculateBitMembers(); return true; @@ -157,7 +153,10 @@ bool Platform::set(const std::string& platformstr, std::string& errstr, const st else if (platformstr == "native") set(Type::Native); else if (platformstr == "unspecified") + { + std::cout << "Platform 'unspecified' is deprecated and will be removed in a future version. It is also now identical to 'native' (i.e. char type signedness based on compiler instead of unknown)." << std::endl; set(Type::Unspecified); + } else if (paths.empty()) { errstr = "unrecognized platform: '" + platformstr + "' (no lookup)."; return false; @@ -234,7 +233,8 @@ bool Platform::loadFromFile(const std::vector& paths, const std::st if (err != tinyxml2::XML_SUCCESS) return false; - return loadFromXmlDocument(&doc); + std::string errmsg; // TODO + return loadFromXmlDocument(&doc, errmsg); } static const char* xmlText(const tinyxml2::XMLElement* node, bool& error) @@ -261,20 +261,33 @@ static unsigned int xmlTextAsBool(const tinyxml2::XMLElement* node, bool& error) return retval; } -bool Platform::loadFromXmlDocument(const tinyxml2::XMLDocument *doc) +bool Platform::loadFromXmlDocument(const tinyxml2::XMLDocument *doc, std::string& errmsg) { const tinyxml2::XMLElement * const rootnode = doc->FirstChildElement(); - if (!rootnode || std::strcmp(rootnode->Name(), "platform") != 0) + if (!rootnode) + { + errmsg = "no root node found"; return false; + } - bool error = false; + if (std::strcmp(rootnode->Name(), "platform") != 0) + { + errmsg = "invalid root node"; + return false; + } + + // TODO: improve error reporting + bool res = true; for (const tinyxml2::XMLElement *node = rootnode->FirstChildElement(); node; node = node->NextSiblingElement()) { + bool error = false; const char* name = node->Name(); if (std::strcmp(name, "default-sign") == 0) { const char * const str = xmlText(node, error); - if (!error) + if (!error && (*str == 'u' || *str == 's')) defaultSign = *str; + else + error = true; } else if (std::strcmp(name, "char_bit") == 0) char_bit = xmlTextAsUInt(node, error); else if (std::strcmp(name, "sizeof") == 0) { @@ -307,10 +320,15 @@ bool Platform::loadFromXmlDocument(const tinyxml2::XMLDocument *doc) else if (std::strcmp(node->Name(), "windows") == 0) { windows = xmlTextAsBool(node, error); } + if (error) + { + res = false; + errmsg = std::string("'") + name + "' failed"; + } } calculateBitMembers(); type = Type::File; - return !error; + return res; } std::string Platform::getLimitsDefines(bool c99) const diff --git a/lib/platform.h b/lib/platform.h index 2ab33461ddb..319b2511b70 100644 --- a/lib/platform.h +++ b/lib/platform.h @@ -81,7 +81,7 @@ class CPPCHECKLIB Platform { protected: /** load platform from xml document, primarily for testing */ - bool loadFromXmlDocument(const tinyxml2::XMLDocument *doc); + bool loadFromXmlDocument(const tinyxml2::XMLDocument *doc, std::string& errmsg); public: Platform(); @@ -130,7 +130,7 @@ class CPPCHECKLIB Platform { std::size_t sizeof_size_t; std::size_t sizeof_pointer; - char defaultSign; // unsigned:'u', signed:'s', unknown:'\0' + char defaultSign; // unsigned:'u', signed:'s' bool windows{false}; // indicates if the platform is Windows diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index f5ffa4e1604..fa1c0e5c946 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -64,12 +64,10 @@ SymbolDatabase::SymbolDatabase(Tokenizer& tokenizer) if (!mTokenizer.tokens()) return; - if (mSettings.platform.defaultSign == 's' || mSettings.platform.defaultSign == 'S') - mDefaultSignedness = ValueType::SIGNED; - else if (mSettings.platform.defaultSign == 'u' || mSettings.platform.defaultSign == 'U') + if (mSettings.platform.defaultSign == 'u') mDefaultSignedness = ValueType::UNSIGNED; else - mDefaultSignedness = ValueType::UNKNOWN_SIGN; + mDefaultSignedness = ValueType::SIGNED; createSymbolDatabaseFindAllScopes(); createSymbolDatabaseClassInfo(); diff --git a/releasenotes.txt b/releasenotes.txt index e288b784a2e..fdbd4fdd4b7 100644 --- a/releasenotes.txt +++ b/releasenotes.txt @@ -15,11 +15,14 @@ GUI: Changed interface: - removed CMake option "DISABLE_CRTDBG_MAP_ALLOC" +- The "unspecified" platform has been deprecated and will be removed in a future version. It is also now identical to 'native' (i.e. char type signedness based on compiler instead of unknown). +- - Infrastructure & dependencies: - Other: +- The possibility of an unknown signedness of the char type in a platform definition has been removed. - The built-in "win*" and "unix*" platforms will now default to signed char type instead of unknown signedness. If you require unsigned chars please specify "--funsigned-char" - diff --git a/test/cli/other_test.py b/test/cli/other_test.py index b8433bceef3..a67a3619ffb 100644 --- a/test/cli/other_test.py +++ b/test/cli/other_test.py @@ -4181,3 +4181,88 @@ def run_and_assert_cppcheck(stdout_exp): # TODO: # - invalid error # - internalError + + +def test_platform_custom(tmp_path): + test_cfg = tmp_path / 'test.cfg' + with open(test_cfg, 'wt') as f: + f.write(""" + + + 8 + unsigned + + 1 + 2 + 2 + 4 + 8 + 4 + 4 + 4 + 2 + 2 + 2 + + + """) + + # TODO: use a sample to make sure the file is actually used + test_file = tmp_path / 'test.c' + with open(test_file, 'wt') as f: + f.write("""""") + + args = [ + '--platform={}'.format(test_cfg), + str(test_file) + ] + + exitcode, stdout, stderr = cppcheck(args) + assert exitcode == 0, stdout + lines = stdout.splitlines() + assert lines == [ + 'Checking {} ...'.format(test_file) + ] + assert stderr == '' + + +def test_platform_invalid_defaultsign(tmp_path): + test_cfg = tmp_path / 'test.cfg' + with open(test_cfg, 'wt') as f: + f.write(""" + + + 8 + notsigned + + 1 + 2 + 2 + 4 + 8 + 4 + 4 + 4 + 2 + 2 + 2 + + + """) + + test_file = tmp_path / 'test.c' + with open(test_file, 'wt') as f: + f.write("""""") + + args = [ + '--platform={}'.format(test_cfg), + str(test_file) + ] + + exitcode, stdout, stderr = cppcheck(args) + assert exitcode == 0, stdout + lines = stdout.splitlines() + assert lines == [ + 'Checking {} ...'.format(test_file) + ] + assert stderr == '' diff --git a/test/testcmdlineparser.cpp b/test/testcmdlineparser.cpp index b6690f8d5c7..01902224540 100644 --- a/test/testcmdlineparser.cpp +++ b/test/testcmdlineparser.cpp @@ -1800,7 +1800,6 @@ class TestCmdlineParser : public TestFixture { void platformWin64() { REDIRECT; const char * const argv[] = {"cppcheck", "--platform=win64", "file.cpp"}; - ASSERT(settings->platform.set(Platform::Type::Unspecified)); ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parseFromArgs(argv)); ASSERT_EQUALS(Platform::Type::Win64, settings->platform.type); } @@ -1808,7 +1807,6 @@ class TestCmdlineParser : public TestFixture { void platformWin32A() { REDIRECT; const char * const argv[] = {"cppcheck", "--platform=win32A", "file.cpp"}; - ASSERT(settings->platform.set(Platform::Type::Unspecified)); ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parseFromArgs(argv)); ASSERT_EQUALS(Platform::Type::Win32A, settings->platform.type); } @@ -1816,7 +1814,6 @@ class TestCmdlineParser : public TestFixture { void platformWin32W() { REDIRECT; const char * const argv[] = {"cppcheck", "--platform=win32W", "file.cpp"}; - ASSERT(settings->platform.set(Platform::Type::Unspecified)); ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parseFromArgs(argv)); ASSERT_EQUALS(Platform::Type::Win32W, settings->platform.type); } @@ -1824,7 +1821,6 @@ class TestCmdlineParser : public TestFixture { void platformUnix32() { REDIRECT; const char * const argv[] = {"cppcheck", "--platform=unix32", "file.cpp"}; - ASSERT(settings->platform.set(Platform::Type::Unspecified)); ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parseFromArgs(argv)); ASSERT_EQUALS(Platform::Type::Unix32, settings->platform.type); } @@ -1832,7 +1828,6 @@ class TestCmdlineParser : public TestFixture { void platformUnix32Unsigned() { REDIRECT; const char * const argv[] = {"cppcheck", "--platform=unix32-unsigned", "file.cpp"}; - ASSERT(settings->platform.set(Platform::Type::Unspecified)); ASSERT_EQUALS_ENUM(CmdLineParser::Result::Fail, parseFromArgs(argv)); ASSERT_EQUALS("cppcheck: error: unrecognized platform: 'unix32-unsigned'.\n", logger->str()); } @@ -1840,7 +1835,6 @@ class TestCmdlineParser : public TestFixture { void platformUnix64() { REDIRECT; const char * const argv[] = {"cppcheck", "--platform=unix64", "file.cpp"}; - ASSERT(settings->platform.set(Platform::Type::Unspecified)); ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parseFromArgs(argv)); ASSERT_EQUALS(Platform::Type::Unix64, settings->platform.type); } @@ -1848,7 +1842,6 @@ class TestCmdlineParser : public TestFixture { void platformUnix64Unsigned() { REDIRECT; const char * const argv[] = {"cppcheck", "--platform=unix64-unsigned", "file.cpp"}; - ASSERT(settings->platform.set(Platform::Type::Unspecified)); ASSERT_EQUALS_ENUM(CmdLineParser::Result::Fail, parseFromArgs(argv)); ASSERT_EQUALS("cppcheck: error: unrecognized platform: 'unix64-unsigned'.\n", logger->str()); } @@ -1856,7 +1849,6 @@ class TestCmdlineParser : public TestFixture { void platformNative() { REDIRECT; const char * const argv[] = {"cppcheck", "--platform=native", "file.cpp"}; - ASSERT(settings->platform.set(Platform::Type::Unspecified)); ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parseFromArgs(argv)); ASSERT_EQUALS(Platform::Type::Native, settings->platform.type); } @@ -1864,7 +1856,6 @@ class TestCmdlineParser : public TestFixture { void platformUnspecified() { REDIRECT; const char * const argv[] = {"cppcheck", "--platform=unspecified", "file.cpp"}; - ASSERT(settings->platform.set(Platform::Type::Native)); ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parseFromArgs(argv)); ASSERT_EQUALS(Platform::Type::Unspecified, settings->platform.type); } @@ -1872,7 +1863,6 @@ class TestCmdlineParser : public TestFixture { void platformPlatformFile() { REDIRECT; const char * const argv[] = {"cppcheck", "--platform=avr8", "file.cpp"}; - ASSERT(settings->platform.set(Platform::Type::Unspecified)); ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parseFromArgs(argv)); ASSERT_EQUALS(Platform::Type::File, settings->platform.type); } diff --git a/test/testplatform.cpp b/test/testplatform.cpp index f82c46640e9..b63b4eee666 100644 --- a/test/testplatform.cpp +++ b/test/testplatform.cpp @@ -56,9 +56,9 @@ class TestPlatform : public TestFixture { friend class TestPlatform; }; - static bool readPlatform(PlatformTest& platform, const char* xmldata) { + static bool readPlatform(PlatformTest& platform, const char* xmldata, std::string& errmsg) { tinyxml2::XMLDocument doc; - return (doc.Parse(xmldata) == tinyxml2::XML_SUCCESS) && platform.loadFromXmlDocument(&doc); + return (doc.Parse(xmldata) == tinyxml2::XML_SUCCESS) && platform.loadFromXmlDocument(&doc, errmsg); } void empty() const { @@ -66,7 +66,9 @@ class TestPlatform : public TestFixture { constexpr char xmldata[] = "\n"; PlatformTest platform; // TODO: this should fail - platform files need to be complete - TODO_ASSERT(!readPlatform(platform, xmldata)); + std::string errmsg; + TODO_ASSERT(!readPlatform(platform, xmldata, errmsg)); + ASSERT_EQUALS("", errmsg); } void valid_config_win32a() const { @@ -229,7 +231,10 @@ class TestPlatform : public TestFixture { " \n" " "; PlatformTest platform; - ASSERT(readPlatform(platform, xmldata)); + std::string errmsg; + ASSERT(readPlatform(platform, xmldata, errmsg)); + ASSERT_EQUALS("", errmsg); + ASSERT_EQUALS("", errmsg); ASSERT_EQUALS(Platform::Type::File, platform.type); ASSERT(!platform.isWindows()); ASSERT_EQUALS(8, platform.char_bit); @@ -274,7 +279,9 @@ class TestPlatform : public TestFixture { " \n" " "; PlatformTest platform; - ASSERT(readPlatform(platform, xmldata)); + std::string errmsg; + ASSERT(readPlatform(platform, xmldata, errmsg)); + ASSERT_EQUALS("", errmsg); ASSERT_EQUALS(Platform::Type::File, platform.type); ASSERT(platform.isWindows()); ASSERT_EQUALS(20, platform.char_bit); @@ -320,7 +327,9 @@ class TestPlatform : public TestFixture { " "; PlatformTest platform; // TODO: needs to fail - files need to be complete - TODO_ASSERT(!readPlatform(platform, xmldata)); + std::string errmsg; + TODO_ASSERT(!readPlatform(platform, xmldata, errmsg)); + ASSERT_EQUALS("", errmsg); } void valid_config_file_4() const { @@ -330,7 +339,7 @@ class TestPlatform : public TestFixture { "\n" " true\n" " 0\n" - " z\n" + " s\n" " \n" " 0\n" " 0\n" @@ -346,11 +355,13 @@ class TestPlatform : public TestFixture { " \n" " "; PlatformTest platform; - ASSERT(readPlatform(platform, xmldata)); + std::string errmsg; + ASSERT(readPlatform(platform, xmldata, errmsg)); + ASSERT_EQUALS("", errmsg); ASSERT_EQUALS(Platform::Type::File, platform.type); ASSERT(platform.isWindows()); ASSERT_EQUALS(0, platform.char_bit); - ASSERT_EQUALS('z', platform.defaultSign); + ASSERT_EQUALS('s', platform.defaultSign); ASSERT_EQUALS(0, platform.sizeof_bool); ASSERT_EQUALS(0, platform.sizeof_short); ASSERT_EQUALS(0, platform.sizeof_int); @@ -390,7 +401,9 @@ class TestPlatform : public TestFixture { " \n" " "; PlatformTest platform; - ASSERT(!readPlatform(platform, xmldata)); + std::string errmsg; + ASSERT(!readPlatform(platform, xmldata, errmsg)); + ASSERT_EQUALS("", errmsg); } void empty_elements() const { @@ -416,7 +429,9 @@ class TestPlatform : public TestFixture { " \n" " "; PlatformTest platform; - ASSERT(!readPlatform(platform, xmldata)); + std::string errmsg; + ASSERT(!readPlatform(platform, xmldata, errmsg)); + ASSERT_EQUALS("'sizeof' failed", errmsg); // TODO: improve message } void default_platform() const { @@ -447,15 +462,21 @@ class TestPlatform : public TestFixture { void no_root_node() const { constexpr char xmldata[] = ""; PlatformTest platform; - ASSERT(!readPlatform(platform, xmldata)); + std::string errmsg; + ASSERT(!readPlatform(platform, xmldata, errmsg)); + ASSERT_EQUALS("no root node found", errmsg); } void wrong_root_node() const { constexpr char xmldata[] = "\n" ""; PlatformTest platform; - ASSERT(!readPlatform(platform, xmldata)); + std::string errmsg; + ASSERT(!readPlatform(platform, xmldata, errmsg)); + ASSERT_EQUALS("invalid root node", errmsg); } + + // TODO: test unspecified }; REGISTER_TEST(TestPlatform)