From b1e92fc399a1bc58fe77e4f432a2254e461a65d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Martins?= Date: Sun, 27 Feb 2022 18:17:48 +0000 Subject: [PATCH] Misra false positive fixes for rules 8.7 and 5.9 (#3844) * Fix misra 8.7 false positives on single function usage When there is a single usage of a function, we should first check if the file it is used in, is the same one it was defined in. When this is not the case, there is no violatior to be reported. * Fix misra rule 5.9 false positives for exception The exception for rule 5.9 described in the guidelines allows for multiple definitions of internal linkage obejcts when these regard a static inlined function defined in the same header file. * Fix neglecting of inline keyword flag upon simplifications When the inline keyword is being "simplified" and the inline flag is degated to the next token. However, this information might be lost if the next token itself is simplified/deleted in a futher pass. Therefore, we must propagated the flag to all the next named tokens, so we can make sure the function name token itself is tagged with this property. * add tests for misra addon rules 8.7 and 5.9 --- addons/misra.py | 37 ++++++++++++++++++---------- addons/test/misra/misra-ctu-1-test.c | 1 + addons/test/misra/misra-ctu-2-test.c | 5 +++- addons/test/misra/misra-ctu-test.h | 4 +++ lib/tokenize.cpp | 9 +++++-- 5 files changed, 40 insertions(+), 16 deletions(-) diff --git a/addons/misra.py b/addons/misra.py index 47724476e..89d4351ec 100755 --- a/addons/misra.py +++ b/addons/misra.py @@ -1395,7 +1395,9 @@ class MisraChecker: local_identifiers.append(identifier(var.nameToken)) elif var.isStatic: names.append(var.nameToken.str) - internal_identifiers.append(identifier(var.nameToken)) + i = identifier(var.nameToken) + i['inlinefunc'] = False + internal_identifiers.append(i) else: names.append(var.nameToken.str) i = identifier(var.nameToken) @@ -1406,9 +1408,14 @@ class MisraChecker: if func.tokenDef is None: continue if func.isStatic: - internal_identifiers.append(identifier(func.tokenDef)) - else: i = identifier(func.tokenDef) + i['inlinefunc'] = func.isInlineKeyword + internal_identifiers.append(i) + else: + if func.token is None: + i = identifier(func.tokenDef) + else: + i = identifier(func.token) i['decl'] = func.token is None external_identifiers.append(i) @@ -1427,12 +1434,12 @@ class MisraChecker: continue if token.function and token.scope.isExecutable: if (not token.function.isStatic) and (token.str not in names): - names.append(token.str) + names.append({'name': token.str, 'file': token.file}) elif token.variable: if token == token.variable.nameToken: continue if token.variable.access == 'Global' and (not token.variable.isStatic) and (token.str not in names): - names.append(token.str) + names.append({'name': token.str, 'file': token.file}) if len(names) > 0: cppcheckdata.reportSummary(dumpfile, 'MisraUsage', names) @@ -4397,7 +4404,7 @@ class MisraChecker: all_external_identifiers_def = {} all_internal_identifiers = {} all_local_identifiers = {} - all_usage_count = {} + all_usage_files = {} from cppcheckdata import Location @@ -4475,8 +4482,9 @@ class MisraChecker: if summary_type == 'MisraInternalIdentifiers': for s in summary_data: if s['name'] in all_internal_identifiers: - self.reportError(Location(s), 5, 9) - self.reportError(Location(all_internal_identifiers[s['name']]), 5, 9) + if not s['inlinefunc'] or s['file'] != all_internal_identifiers[s['name']]['file']: + self.reportError(Location(s), 5, 9) + self.reportError(Location(all_internal_identifiers[s['name']]), 5, 9) all_internal_identifiers[s['name']] = s if summary_type == 'MisraLocalIdentifiers': @@ -4485,10 +4493,10 @@ class MisraChecker: if summary_type == 'MisraUsage': for s in summary_data: - if s in all_usage_count: - all_usage_count[s] += 1 + if s['name'] in all_usage_files: + all_usage_files[s['name']].append(s['file']) else: - all_usage_count[s] = 1 + all_usage_files[s['name']] = [s['file']] for ti in all_typedef_info: if not ti['used']: @@ -4515,9 +4523,12 @@ class MisraChecker: self.reportError(Location(local_identifier), 5, 8) self.reportError(Location(external_identifier), 5, 8) - for name, count in all_usage_count.items(): + for name, files in all_usage_files.items(): #print('%s:%i' % (name, count)) - if count != 1: + count = len(files) + if count != 1 or name not in all_external_identifiers_def: + continue + if files[0] != Location(all_external_identifiers_def[name]).file: continue if name in all_external_identifiers: self.reportError(Location(all_external_identifiers[name]), 8, 7) diff --git a/addons/test/misra/misra-ctu-1-test.c b/addons/test/misra/misra-ctu-1-test.c index 03fdc9d0b..6c5f5344d 100644 --- a/addons/test/misra/misra-ctu-1-test.c +++ b/addons/test/misra/misra-ctu-1-test.c @@ -39,3 +39,4 @@ extern int misra_8_5; // cppcheck-suppress misra-c2012-8.6 int32_t misra_8_6 = 1; +void misra_8_7_external(void) {} diff --git a/addons/test/misra/misra-ctu-2-test.c b/addons/test/misra/misra-ctu-2-test.c index b28bf856e..85e66f1eb 100644 --- a/addons/test/misra/misra-ctu-2-test.c +++ b/addons/test/misra/misra-ctu-2-test.c @@ -41,5 +41,8 @@ int32_t misra_8_6 = 2; // cppcheck-suppress misra-c2012-8.4 // cppcheck-suppress misra-c2012-8.7 void misra_8_7(void) {} -static void misra_8_7_caller(void) { misra_8_7(); } +static void misra_8_7_caller(void) { + misra_8_7(); + misra_8_7_external(); +} diff --git a/addons/test/misra/misra-ctu-test.h b/addons/test/misra/misra-ctu-test.h index b971cd8a9..c9d2a7ba8 100644 --- a/addons/test/misra/misra-ctu-test.h +++ b/addons/test/misra/misra-ctu-test.h @@ -9,6 +9,10 @@ struct misra_2_4_violation_t { int x; }; +static inline void misra_5_9_exception(void) {} + +void misra_8_7_external(void); + #define MISRA_2_5_OK_1 1 #define MISRA_2_5_OK_2 2 // cppcheck-suppress misra-c2012-2.5 diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index cb5f43b8e..2fa35c0a7 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -11237,8 +11237,13 @@ void Tokenizer::simplifyKeyword() if (keywords.find(tok->str()) != keywords.end()) { // Don't remove struct members if (!Token::simpleMatch(tok->previous(), ".")) { - if (tok->str().find("inline") != std::string::npos && Token::Match(tok->next(), "%name%")) - tok->next()->isInline(true); + if (tok->str().find("inline") != std::string::npos) { + Token *temp = tok->next(); + while (temp != nullptr && Token::Match(temp, "%name%")) { + temp->isInline(true); + temp = temp->next(); + } + } tok->deleteThis(); // Simplify.. } }