From 8294b04047671c66b1d77eacc21a328cfb12083f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Mon, 14 Sep 2026 11:34:49 +0200 Subject: [PATCH 1/2] Fix #15028 (release: check premium ids) --- createrelease | 14 ++-- lib/checkclass.cpp | 24 +++--- lib/checkcondition.cpp | 4 +- lib/checkother.cpp | 14 ++-- lib/checkother.h | 2 +- lib/checkuninitvar.cpp | 7 +- lib/preprocessor.cpp | 1 + tools/release-check-premium-ids.py | 114 +++++++++++++++++++++++++++++ 8 files changed, 147 insertions(+), 33 deletions(-) create mode 100755 tools/release-check-premium-ids.py diff --git a/createrelease b/createrelease index 6a611a5a469..880b7f6880f 100755 --- a/createrelease +++ b/createrelease @@ -23,14 +23,12 @@ # cd ~/cppchecksolutions/addon/coverage # CPPCHECK_REPO=~/cppchecksolutions/cppcheck python3 coverage.py --code # -# Check every isPremiumEnabled call: TODO write helper script -# - every id should be in --errorlist -# git grep 'isPremiumEnabled[(]"' | sed 's/.*isPremiumEnabled[(]"//' | sed 's/".*//' | sort | uniq > ids1.txt -# ./cppcheck --errorlist | grep ' id="' | sed 's/.* id="//' | sed 's/".*//' | sort | uniq > ids2.txt -# diff -y ids1.txt ids2.txt -# - premiumaddon: check coverage.py -# python3 coverage.py --id ; sort ids-*.txt | uniq > ~/cppcheck/ids3.txt -# diff -y ids2.txt ids3.txt +# Check that every premium check id is a real, known Cppcheck error id: every +# id passed to isPremiumEnabled(...) and every id referenced by the premium +# addon's MISRA/CERT/AUTOSAR/CWE coverage mapping (coverage.py) must show up +# in `cppcheck --errorlist`, otherwise the id was renamed/removed and the +# premium check or compliance mapping is silently broken. +# tools/release-check-premium-ids.py # # Windows installer: # - ensure latest build was successful diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 372adf839ff..f9cf521f0ea 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -3841,6 +3841,18 @@ const Check::FileInfo * CheckClass::loadFileInfoFromXml(const tinyxml2::XMLEleme return fileInfo; } +static ErrorMessage oneDefinitionRuleViolationErrorMessage(std::list locationList, const std::string &file0, const std::string &symbolName) +{ + return ErrorMessage(std::move(locationList), + file0, + Severity::error, + "$symbol:" + symbolName + + "\nThe one definition rule is violated, different classes/structs have the same name '$symbol'", + "ctuOneDefinitionRuleViolation", + CWE_ONE_DEFINITION_RULE, + Certainty::normal); +} + bool CheckClass::analyseWholeProgram(const CTU::FileInfo &ctu, const std::list &fileInfo, const Settings& settings, ErrorLogger &errorLogger) { (void)ctu; @@ -3879,15 +3891,7 @@ bool CheckClass::analyseWholeProgram(const CTU::FileInfo &ctu, const std::listsecond.fileName, it->second.lineNumber, it->second.column); - const ErrorMessage errmsg(std::move(locationList), - fi->file0, - Severity::error, - "$symbol:" + nameLoc.className + - "\nThe one definition rule is violated, different classes/structs have the same name '$symbol'", - "ctuOneDefinitionRuleViolation", - CWE_ONE_DEFINITION_RULE, - Certainty::normal); - errorLogger.reportErr(errmsg); + errorLogger.reportErr(oneDefinitionRuleViolationErrorMessage(std::move(locationList), fi->file0, nameLoc.className)); foundErrors = true; } @@ -3968,5 +3972,5 @@ void CheckClass::getErrorMessages(ErrorLogger& errorLogger, const Settings &sett c.virtualFunctionCallInConstructorError(nullptr, std::list(), "f"); c.thisUseAfterFree(nullptr, nullptr, nullptr); c.unsafeClassRefMemberError(nullptr, "UnsafeClass::var"); - // TODO: ctuOneDefinitionRuleViolation + errorLogger.reportErr(oneDefinitionRuleViolationErrorMessage({}, "", "classname")); } diff --git a/lib/checkcondition.cpp b/lib/checkcondition.cpp index 4c2909e251a..e47e3961258 100644 --- a/lib/checkcondition.cpp +++ b/lib/checkcondition.cpp @@ -1516,9 +1516,7 @@ void CheckConditionImpl::clarifyConditionError(const Token *tok, bool assign, bo void CheckConditionImpl::alwaysTrueFalse() { - const bool pedantic = mSettings.isPremiumEnabled("alwaysTrue") || - mSettings.isPremiumEnabled("alwaysFalse") || - mSettings.isPremiumEnabled("knownConditionTrueFalse"); + const bool pedantic = mSettings.isPremiumEnabled("knownConditionTrueFalse"); if (!pedantic && !mSettings.severity.isEnabled(Severity::style)) return; diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 0aa9dd18a84..78351a229c0 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -4513,18 +4513,16 @@ void CheckOtherImpl::checkComparePointers() if (const Token* parent1 = getParentLifetime(v1.tokvalue, mSettings.library)) if (var2 == parent1->variable()) continue; - comparePointersError(tok, &v1, &v2); + comparePointersError(tok, &v1, &v2, Token::simpleMatch(tok, "-")); } } } -void CheckOtherImpl::comparePointersError(const Token *tok, const ValueFlow::Value *v1, const ValueFlow::Value *v2) +void CheckOtherImpl::comparePointersError(const Token *tok, const ValueFlow::Value *v1, const ValueFlow::Value *v2, bool subtract) { ErrorPath errorPath; - std::string verb = "Comparing"; - if (Token::simpleMatch(tok, "-")) - verb = "Subtracting"; - const char * const id = (verb[0] == 'C') ? "comparePointers" : "subtractPointers"; + const std::string verb = subtract ? "Subtracting" : "Comparing"; + const char * const id = subtract ? "subtractPointers" : "comparePointers"; if (v1) { errorPath.emplace_back(v1->tokvalue->variable()->nameToken(), "Variable declared here."); errorPath.insert(errorPath.end(), v1->errorPath.cbegin(), v1->errorPath.cend()); @@ -4993,8 +4991,8 @@ void CheckOther::getErrorMessages(ErrorLogger& errorLogger, const Settings &sett c.shadowError(nullptr, "local variable", nullptr, "member"); c.knownArgumentError(nullptr, nullptr, nullptr, "x", false); c.knownPointerToBoolError(nullptr, nullptr); - c.comparePointersError(nullptr, nullptr, nullptr); - // TODO: subtractPointers + c.comparePointersError(nullptr, nullptr, nullptr, false); + c.comparePointersError(nullptr, nullptr, nullptr, true); c.redundantAssignmentError(nullptr, nullptr, "var", false); c.redundantInitializationError(nullptr, nullptr, "var", false); c.redundantContinueError(nullptr); diff --git a/lib/checkother.h b/lib/checkother.h index c9f7a1f9530..823a5783fb3 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -325,7 +325,7 @@ class CPPCHECKLIB CheckOtherImpl : public CheckImpl { void shadowError(const Token *shadows, const std::string &shadowsType, const Token *shadowed, const std::string &shadowedType); void knownArgumentError(const Token *tok, const Token *ftok, const ValueFlow::Value *value, const std::string &varexpr, bool isVariableExpressionHidden); void knownPointerToBoolError(const Token* tok, const ValueFlow::Value* value); - void comparePointersError(const Token *tok, const ValueFlow::Value *v1, const ValueFlow::Value *v2); + void comparePointersError(const Token *tok, const ValueFlow::Value *v1, const ValueFlow::Value *v2, bool subtract); void checkModuloOfOneError(const Token *tok); void unionZeroInitError(const Token *tok, const UnionMember& largestMember); diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index d05ea3fbd28..3a5ddcb1ac0 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -1557,7 +1557,7 @@ void CheckUninitVarImpl::uninitdataError(const Token *tok, const std::string &va void CheckUninitVarImpl::uninitvarError(const Token *tok, const std::string &varname, ErrorPath errorPath) { - if (diag(tok)) + if (tok && diag(tok)) return; errorPath.emplace_back(tok, ""); reportError(std::move(errorPath), @@ -1572,7 +1572,7 @@ void CheckUninitVarImpl::uninitvarError(const Token* tok, const ValueFlow::Value { if (!mSettings.isEnabled(&v)) return; - if (diag(tok)) + if (tok && diag(tok)) return; const Token* ltok = tok; if (tok && Token::simpleMatch(tok->astParent(), ".") && astIsRHS(tok)) @@ -1810,7 +1810,8 @@ void CheckUninitVar::getErrorMessages(ErrorLogger& errorLogger, const Settings& ValueFlow::Value v{}; - c.uninitvarError(nullptr, v); // TODO: does not produce any output + c.uninitvarError(nullptr, v); + c.uninitvarError(nullptr, "varname", ErrorPath{}); c.uninitdataError(nullptr, "varname"); c.uninitStructMemberError(nullptr, "a.b"); } diff --git a/lib/preprocessor.cpp b/lib/preprocessor.cpp index d18a1fae4f2..0a0acecab1a 100644 --- a/lib/preprocessor.cpp +++ b/lib/preprocessor.cpp @@ -1067,6 +1067,7 @@ void Preprocessor::getErrorMessages(ErrorLogger &errorLogger, const Settings &se preprocessor.missingInclude(loc, "", SystemHeader); preprocessor.error(loc, "message", simplecpp::Output::ERROR); preprocessor.error(loc, "message", simplecpp::Output::SYNTAX_ERROR); + preprocessor.error(loc, "message", simplecpp::Output::DIRECTIVE_AS_MACRO_PARAMETER); preprocessor.error(loc, "message", simplecpp::Output::UNHANDLED_CHAR_ERROR); preprocessor.error(loc, "message", simplecpp::Output::INCLUDE_NESTED_TOO_DEEPLY); preprocessor.error(loc, "message", simplecpp::Output::FILE_NOT_FOUND); diff --git a/tools/release-check-premium-ids.py b/tools/release-check-premium-ids.py new file mode 100755 index 00000000000..2b28da9928b --- /dev/null +++ b/tools/release-check-premium-ids.py @@ -0,0 +1,114 @@ +#!/usr/bin/env python3 +# +# Automates the release checklist item in `createrelease`: +# +# # Check every isPremiumEnabled call: TODO write helper script +# # - every id should be in --errorlist +# # - premiumaddon: check coverage.py +# +# Two checks are performed: +# +# 1. Every id passed to isPremiumEnabled("...") in lib/ must be a real, +# known Cppcheck error id (i.e. it must show up in `cppcheck +# --errorlist`). Otherwise the premium check can never actually be +# turned on/off since its id does not exist. +# +# 2. Every Cppcheck id referenced by the premium addon's compliance +# mapping tables (MISRA/CERT/AUTOSAR/CWE -> Cppcheck id, generated by +# `coverage.py --id`) must also show up in `cppcheck --errorlist`. +# Otherwise the mapping refers to an id that has been renamed or +# removed. +# +# A warning is printed for every id that fails a check; exit status is 1 +# if any warning was printed, 0 otherwise. +# +# Usage: +# tools/release-check-premium-ids.py [path-to-addon/coverage] +# +# The addon/coverage directory defaults to ../addon/coverage (relative to +# this repo, i.e. a checkout of the premium addon repo next to this one). +# If it can't be found, check 2 is skipped. + +import argparse +import glob +import os +import re +import subprocess +import sys + +REPO_DIR = os.path.dirname(os.path.dirname(os.path.realpath(__file__))) +CPPCHECK_BIN = os.path.join(REPO_DIR, 'cppcheck') + +ID_RE = re.compile(r' id="([^"]*)"') +IS_PREMIUM_ENABLED_RE = re.compile(r'isPremiumEnabled\s*\(\s*"([^"]*)"') + +warning_count = 0 + + +def warn(message): + global warning_count + warning_count += 1 + print('warning: ' + message, file=sys.stderr) + + +def get_errorlist_ids(): + out = subprocess.check_output([CPPCHECK_BIN, '--errorlist'], universal_newlines=True) + return set(ID_RE.findall(out)) + + +def get_is_premium_enabled_ids(): + ids = set() + for filename in glob.glob(os.path.join(REPO_DIR, 'lib', '*.cpp')) + glob.glob(os.path.join(REPO_DIR, 'lib', '*.h')): + with open(filename, 'rt', encoding='utf-8') as f: + ids.update(IS_PREMIUM_ENABLED_RE.findall(f.read())) + return ids + + +def get_addon_coverage_ids(addon_coverage_dir): + coverage_py = os.path.join(addon_coverage_dir, 'coverage.py') + if not os.path.isfile(coverage_py): + print(f"(skipped premium addon coverage.py check: '{coverage_py}' not found)", file=sys.stderr) + return None + + ids_files = glob.glob(os.path.join(addon_coverage_dir, 'ids-*.txt')) + for f in ids_files: + os.remove(f) + + env = dict(os.environ, CPPCHECK_REPO=REPO_DIR) + subprocess.check_call(['python3', 'coverage.py', '--id'], cwd=addon_coverage_dir, env=env, stdout=subprocess.DEVNULL) + + ids = set() + ids_files = glob.glob(os.path.join(addon_coverage_dir, 'ids-*.txt')) + for filename in ids_files: + with open(filename, 'rt', encoding='utf-8') as f: + ids.update(line.strip() for line in f if line.strip()) + os.remove(filename) + return ids + + +def main(): + parser = argparse.ArgumentParser(description='Check that isPremiumEnabled() ids and premium addon coverage ids are known Cppcheck error ids.') + parser.add_argument('addon_coverage_dir', nargs='?', default=os.path.join(REPO_DIR, '..', 'addon', 'coverage'), + help='path to the premium addon coverage/ directory (default: ../addon/coverage)') + args = parser.parse_args() + + if not os.access(CPPCHECK_BIN, os.X_OK): + print(f"error: '{CPPCHECK_BIN}' not found or not executable, build it first (make -j$(nproc))", file=sys.stderr) + return 1 + + errorlist_ids = get_errorlist_ids() + + for id_ in sorted(get_is_premium_enabled_ids() - errorlist_ids): + warn(f'isPremiumEnabled("{id_}") but there is no such id in --errorlist') + + addon_coverage_dir = os.path.realpath(args.addon_coverage_dir) + addon_ids = get_addon_coverage_ids(addon_coverage_dir) + if addon_ids is not None: + for id_ in sorted(addon_ids - errorlist_ids): + warn(f'premium addon coverage.py references id "{id_}" but there is no such id in --errorlist') + + return 1 if warning_count else 0 + + +if __name__ == '__main__': + sys.exit(main()) From 19c5459b16705b7163a3dbd2cb481bbe0033020f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Mon, 14 Sep 2026 11:52:44 +0200 Subject: [PATCH 2/2] format --- lib/checkclass.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index f9cf521f0ea..8d7b035f7da 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -3844,13 +3844,13 @@ const Check::FileInfo * CheckClass::loadFileInfoFromXml(const tinyxml2::XMLEleme static ErrorMessage oneDefinitionRuleViolationErrorMessage(std::list locationList, const std::string &file0, const std::string &symbolName) { return ErrorMessage(std::move(locationList), - file0, - Severity::error, - "$symbol:" + symbolName + - "\nThe one definition rule is violated, different classes/structs have the same name '$symbol'", - "ctuOneDefinitionRuleViolation", - CWE_ONE_DEFINITION_RULE, - Certainty::normal); + file0, + Severity::error, + "$symbol:" + symbolName + + "\nThe one definition rule is violated, different classes/structs have the same name '$symbol'", + "ctuOneDefinitionRuleViolation", + CWE_ONE_DEFINITION_RULE, + Certainty::normal); } bool CheckClass::analyseWholeProgram(const CTU::FileInfo &ctu, const std::list &fileInfo, const Settings& settings, ErrorLogger &errorLogger)