From d5cfc676291403d89049165273e59e3ae163a0fa Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Tue, 22 Nov 2022 10:11:14 -0500 Subject: [PATCH 1/7] Declarations5: add RULE-8-5 --- .vscode/tasks.json | 1 + ...nalObjectOrFunctionNotDeclaredInOneFile.ql | 43 +++++++++++++++++++ ...ectOrFunctionNotDeclaredInOneFile.expected | 2 + ...ObjectOrFunctionNotDeclaredInOneFile.qlref | 1 + c/misra/test/rules/RULE-8-5/test.c | 8 ++++ c/misra/test/rules/RULE-8-5/test.h | 3 ++ c/misra/test/rules/RULE-8-5/test1.c | 1 + c/misra/test/rules/RULE-8-5/test1.h | 3 ++ .../cpp/exclusions/c/Declarations5.qll | 25 +++++++++++ .../cpp/exclusions/c/RuleMetadata.qll | 3 ++ rule_packages/c/Declarations5.json | 23 ++++++++++ rules.csv | 2 +- 12 files changed, 114 insertions(+), 1 deletion(-) create mode 100644 c/misra/src/rules/RULE-8-5/ExternalObjectOrFunctionNotDeclaredInOneFile.ql create mode 100644 c/misra/test/rules/RULE-8-5/ExternalObjectOrFunctionNotDeclaredInOneFile.expected create mode 100644 c/misra/test/rules/RULE-8-5/ExternalObjectOrFunctionNotDeclaredInOneFile.qlref create mode 100644 c/misra/test/rules/RULE-8-5/test.c create mode 100644 c/misra/test/rules/RULE-8-5/test.h create mode 100644 c/misra/test/rules/RULE-8-5/test1.c create mode 100644 c/misra/test/rules/RULE-8-5/test1.h create mode 100644 cpp/common/src/codingstandards/cpp/exclusions/c/Declarations5.qll create mode 100644 rule_packages/c/Declarations5.json diff --git a/.vscode/tasks.json b/.vscode/tasks.json index 5915064e54..823c754920 100644 --- a/.vscode/tasks.json +++ b/.vscode/tasks.json @@ -206,6 +206,7 @@ "Declarations", "Declarations1", "Declarations2", + "Declarations5", "Exceptions1", "Exceptions2", "Expressions", diff --git a/c/misra/src/rules/RULE-8-5/ExternalObjectOrFunctionNotDeclaredInOneFile.ql b/c/misra/src/rules/RULE-8-5/ExternalObjectOrFunctionNotDeclaredInOneFile.ql new file mode 100644 index 0000000000..0c9cfe9cb2 --- /dev/null +++ b/c/misra/src/rules/RULE-8-5/ExternalObjectOrFunctionNotDeclaredInOneFile.ql @@ -0,0 +1,43 @@ +/** + * @id c/misra/external-object-or-function-not-declared-in-one-file + * @name RULE-8-5: An external object or function shall be declared once in one and only one file + * @description Declarations in multiple files can lead to unexpected program behaviour. + * @kind problem + * @precision very-high + * @problem.severity warning + * @tags external/misra/id/rule-8-5 + * correctness + * external/misra/obligation/required + */ + +import cpp +import codingstandards.c.misra + +from DeclarationEntry de, DeclarationEntry otherDeclaration, string kind +where + not isExcluded(de, Declarations5Package::externalObjectOrFunctionNotDeclaredInOneFileQuery()) and + //this rule applies to non-defining declarations only + not de.isDefinition() and + not otherDeclaration.isDefinition() and + exists(Declaration d | + de.getDeclaration() = d and + otherDeclaration.getDeclaration() = d and + de.getFile() != otherDeclaration.getFile() + ) and + ( + de.getDeclaration() instanceof Function and kind = "function" + or + de.getDeclaration() instanceof Variable and + not de.getDeclaration() instanceof Parameter and + kind = "variable" + ) and + // Apply an ordering based on location to enforce that (de1, de2) = (de2, de1) and we only report (de1, de2). + ( + de.getFile().getAbsolutePath() < otherDeclaration.getFile().getAbsolutePath() + or + de.getFile().getAbsolutePath() = otherDeclaration.getFile().getAbsolutePath() and + de.getLocation().getStartLine() < otherDeclaration.getLocation().getStartLine() + ) +select de, + "The " + kind + " declaration " + de.getName() + + " is declared in multiple files and has an additional $@.", otherDeclaration, "declaration" diff --git a/c/misra/test/rules/RULE-8-5/ExternalObjectOrFunctionNotDeclaredInOneFile.expected b/c/misra/test/rules/RULE-8-5/ExternalObjectOrFunctionNotDeclaredInOneFile.expected new file mode 100644 index 0000000000..63276c3831 --- /dev/null +++ b/c/misra/test/rules/RULE-8-5/ExternalObjectOrFunctionNotDeclaredInOneFile.expected @@ -0,0 +1,2 @@ +| test.c:8:12:8:13 | declaration of g3 | The variable declaration g3 is declared in multiple files and has an additional $@. | test1.c:1:12:1:13 | declaration of g3 | declaration | +| test.h:1:12:1:12 | declaration of g | The variable declaration g is declared in multiple files and has an additional $@. | test1.h:1:12:1:12 | declaration of g | declaration | diff --git a/c/misra/test/rules/RULE-8-5/ExternalObjectOrFunctionNotDeclaredInOneFile.qlref b/c/misra/test/rules/RULE-8-5/ExternalObjectOrFunctionNotDeclaredInOneFile.qlref new file mode 100644 index 0000000000..5359406e92 --- /dev/null +++ b/c/misra/test/rules/RULE-8-5/ExternalObjectOrFunctionNotDeclaredInOneFile.qlref @@ -0,0 +1 @@ +rules/RULE-8-5/ExternalObjectOrFunctionNotDeclaredInOneFile.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-8-5/test.c b/c/misra/test/rules/RULE-8-5/test.c new file mode 100644 index 0000000000..89f0a0972d --- /dev/null +++ b/c/misra/test/rules/RULE-8-5/test.c @@ -0,0 +1,8 @@ +#include "test.h" +#include "test1.h" + +int g = 1; // COMPLIANT + +extern int g1; // COMPLIANT + +extern int g3; // NON_COMPLIANT \ No newline at end of file diff --git a/c/misra/test/rules/RULE-8-5/test.h b/c/misra/test/rules/RULE-8-5/test.h new file mode 100644 index 0000000000..fe5e1e5e2e --- /dev/null +++ b/c/misra/test/rules/RULE-8-5/test.h @@ -0,0 +1,3 @@ +extern int g; // NON_COMPLIANT + +int g2; // COMPLIANT \ No newline at end of file diff --git a/c/misra/test/rules/RULE-8-5/test1.c b/c/misra/test/rules/RULE-8-5/test1.c new file mode 100644 index 0000000000..f00c54998e --- /dev/null +++ b/c/misra/test/rules/RULE-8-5/test1.c @@ -0,0 +1 @@ +extern int g3; // NON_COMPLIANT \ No newline at end of file diff --git a/c/misra/test/rules/RULE-8-5/test1.h b/c/misra/test/rules/RULE-8-5/test1.h new file mode 100644 index 0000000000..fe5e1e5e2e --- /dev/null +++ b/c/misra/test/rules/RULE-8-5/test1.h @@ -0,0 +1,3 @@ +extern int g; // NON_COMPLIANT + +int g2; // COMPLIANT \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/exclusions/c/Declarations5.qll b/cpp/common/src/codingstandards/cpp/exclusions/c/Declarations5.qll new file mode 100644 index 0000000000..db3fa5e08e --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/exclusions/c/Declarations5.qll @@ -0,0 +1,25 @@ +//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/ +import cpp +import RuleMetadata +import codingstandards.cpp.exclusions.RuleMetadata + +newtype Declarations5Query = TExternalObjectOrFunctionNotDeclaredInOneFileQuery() + +predicate isDeclarations5QueryMetadata(Query query, string queryId, string ruleId) { + query = + // `Query` instance for the `externalObjectOrFunctionNotDeclaredInOneFile` query + Declarations5Package::externalObjectOrFunctionNotDeclaredInOneFileQuery() and + queryId = + // `@id` for the `externalObjectOrFunctionNotDeclaredInOneFile` query + "c/misra/external-object-or-function-not-declared-in-one-file" and + ruleId = "RULE-8-5" +} + +module Declarations5Package { + Query externalObjectOrFunctionNotDeclaredInOneFileQuery() { + //autogenerate `Query` type + result = + // `Query` type for `externalObjectOrFunctionNotDeclaredInOneFile` query + TQueryC(TDeclarations5PackageQuery(TExternalObjectOrFunctionNotDeclaredInOneFileQuery())) + } +} diff --git a/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll b/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll index 4dc785d482..699cdc30ca 100644 --- a/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll +++ b/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll @@ -15,6 +15,7 @@ import Contracts4 import Declarations1 import Declarations2 import Declarations3 +import Declarations5 import Expressions import IO1 import IO2 @@ -52,6 +53,7 @@ newtype TCQuery = TDeclarations1PackageQuery(Declarations1Query q) or TDeclarations2PackageQuery(Declarations2Query q) or TDeclarations3PackageQuery(Declarations3Query q) or + TDeclarations5PackageQuery(Declarations5Query q) or TExpressionsPackageQuery(ExpressionsQuery q) or TIO1PackageQuery(IO1Query q) or TIO2PackageQuery(IO2Query q) or @@ -89,6 +91,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId) { isDeclarations1QueryMetadata(query, queryId, ruleId) or isDeclarations2QueryMetadata(query, queryId, ruleId) or isDeclarations3QueryMetadata(query, queryId, ruleId) or + isDeclarations5QueryMetadata(query, queryId, ruleId) or isExpressionsQueryMetadata(query, queryId, ruleId) or isIO1QueryMetadata(query, queryId, ruleId) or isIO2QueryMetadata(query, queryId, ruleId) or diff --git a/rule_packages/c/Declarations5.json b/rule_packages/c/Declarations5.json new file mode 100644 index 0000000000..1680d3e3af --- /dev/null +++ b/rule_packages/c/Declarations5.json @@ -0,0 +1,23 @@ +{ + "MISRA-C-2012": { + "RULE-8-5": { + "properties": { + "obligation": "required" + }, + "queries": [ + { + "description": "Declarations in multiple files can lead to unexpected program behaviour.", + "kind": "problem", + "name": "An external object or function shall be declared once in one and only one file", + "precision": "very-high", + "severity": "warning", + "short_name": "ExternalObjectOrFunctionNotDeclaredInOneFile", + "tags": [ + "correctness" + ] + } + ], + "title": "An external object or function shall be declared once in one and only one file" + } + } +} \ No newline at end of file diff --git a/rules.csv b/rules.csv index 7b283db8b0..de928dd040 100644 --- a/rules.csv +++ b/rules.csv @@ -650,7 +650,7 @@ c,MISRA-C-2012,RULE-8-1,Yes,Required,,,Types shall be explicitly specified,,Decl c,MISRA-C-2012,RULE-8-2,Yes,Required,,,Function types shall be in prototype form with named parameters,,Declarations,Medium, c,MISRA-C-2012,RULE-8-3,Yes,Required,,,All declarations of an object or function shall use the same names and type qualifiers,M3-2-1,Declarations,Medium, c,MISRA-C-2012,RULE-8-4,Yes,Required,,,A compatible declaration shall be visible when an object or function with external linkage is defined,,Declarations,Medium, -c,MISRA-C-2012,RULE-8-5,Yes,Required,,,An external object or function shall be declared once in one and only one file,,Declarations,Medium, +c,MISRA-C-2012,RULE-8-5,Yes,Required,,,An external object or function shall be declared once in one and only one file,,Declarations5,Medium, c,MISRA-C-2012,RULE-8-6,Yes,Required,,,An identifier with external linkage shall have exactly one external definition,M3-2-4,Declarations,Import, c,MISRA-C-2012,RULE-8-7,Yes,Advisory,,,Functions and objects should not be defined with external linkage if they are referenced in only one translation unit,,Declarations,Medium, c,MISRA-C-2012,RULE-8-8,Yes,Required,,,The static storage class specifier shall be used in all declarations of objects and functions that have internal linkage,M3-3-2,Declarations,Medium, From 0c2f3d54d2c7e1e9db44b5619aed099d2d30c3b2 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Tue, 22 Nov 2022 16:30:34 -0500 Subject: [PATCH 2/7] Declarations5: add RULE-5-2 --- ...ifiersDeclaredInTheSameScopeNotDistinct.ql | 38 +++++++++++++++++++ ...DeclaredInTheSameScopeNotDistinct.expected | 1 + ...ersDeclaredInTheSameScopeNotDistinct.qlref | 1 + c/misra/test/rules/RULE-5-2/test.c | 26 +++++++++++++ .../cpp/exclusions/c/Declarations5.qll | 19 +++++++++- rule_packages/c/Declarations5.json | 25 ++++++++++++ rules.csv | 2 +- 7 files changed, 110 insertions(+), 2 deletions(-) create mode 100644 c/misra/src/rules/RULE-5-2/IdentifiersDeclaredInTheSameScopeNotDistinct.ql create mode 100644 c/misra/test/rules/RULE-5-2/IdentifiersDeclaredInTheSameScopeNotDistinct.expected create mode 100644 c/misra/test/rules/RULE-5-2/IdentifiersDeclaredInTheSameScopeNotDistinct.qlref create mode 100644 c/misra/test/rules/RULE-5-2/test.c diff --git a/c/misra/src/rules/RULE-5-2/IdentifiersDeclaredInTheSameScopeNotDistinct.ql b/c/misra/src/rules/RULE-5-2/IdentifiersDeclaredInTheSameScopeNotDistinct.ql new file mode 100644 index 0000000000..682d7538c5 --- /dev/null +++ b/c/misra/src/rules/RULE-5-2/IdentifiersDeclaredInTheSameScopeNotDistinct.ql @@ -0,0 +1,38 @@ +/** + * @id c/misra/identifiers-declared-in-the-same-scope-not-distinct + * @name RULE-5-2: Identifiers declared in the same scope and name space shall be distinct + * @description Using nondistinct identifiers results in undefined behaviour. + * @kind problem + * @precision very-high + * @problem.severity warning + * @tags external/misra/id/rule-5-2 + * correctness + * maintainability + * readability + * external/misra/obligation/required + */ + +import cpp +import codingstandards.c.misra +import codingstandards.cpp.Identifiers + +from InterestingIdentifiers d, InterestingIdentifiers d2 +where + not isExcluded(d, Declarations5Package::identifiersDeclaredInTheSameScopeNotDistinctQuery()) and + not isExcluded(d2, Declarations5Package::identifiersDeclaredInTheSameScopeNotDistinctQuery()) and + //this rule does not apply if both are external identifiers + //that is covered by RULE-5-3 + not ( + d instanceof ExternalIdentifiers and + d2 instanceof ExternalIdentifiers + ) and + d.getNamespace() = d2.getNamespace() and + d.getParentScope() = d2.getParentScope() and + not d = d2 and + d.getLocation().getStartLine() >= d2.getLocation().getStartLine() and + //first 63 chars in the name as per C99 + d.getSignificantNameComparedToMacro() = d2.getSignificantNameComparedToMacro() and + not d.getName() = d2.getName() +select d, + "Identifer " + d.getName() + " is nondistinct in characters at or over 63 limit, compared to $@", + d2, d2.getName() diff --git a/c/misra/test/rules/RULE-5-2/IdentifiersDeclaredInTheSameScopeNotDistinct.expected b/c/misra/test/rules/RULE-5-2/IdentifiersDeclaredInTheSameScopeNotDistinct.expected new file mode 100644 index 0000000000..b7d33ba120 --- /dev/null +++ b/c/misra/test/rules/RULE-5-2/IdentifiersDeclaredInTheSameScopeNotDistinct.expected @@ -0,0 +1 @@ +| test.c:8:5:8:68 | iltiqzxgfqsgigwfuyntzghvzltueatcxqnqofnnvjyszmcsylyohvqaosjbqyyB | Identifer iltiqzxgfqsgigwfuyntzghvzltueatcxqnqofnnvjyszmcsylyohvqaosjbqyyB is nondistinct in characters at or over 63 limit, compared to $@ | test.c:2:5:2:68 | iltiqzxgfqsgigwfuyntzghvzltueatcxqnqofnnvjyszmcsylyohvqaosjbqyyA | iltiqzxgfqsgigwfuyntzghvzltueatcxqnqofnnvjyszmcsylyohvqaosjbqyyA | diff --git a/c/misra/test/rules/RULE-5-2/IdentifiersDeclaredInTheSameScopeNotDistinct.qlref b/c/misra/test/rules/RULE-5-2/IdentifiersDeclaredInTheSameScopeNotDistinct.qlref new file mode 100644 index 0000000000..59fc518cf7 --- /dev/null +++ b/c/misra/test/rules/RULE-5-2/IdentifiersDeclaredInTheSameScopeNotDistinct.qlref @@ -0,0 +1 @@ +rules/RULE-5-2/IdentifiersDeclaredInTheSameScopeNotDistinct.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-5-2/test.c b/c/misra/test/rules/RULE-5-2/test.c new file mode 100644 index 0000000000..3f3f8f1e00 --- /dev/null +++ b/c/misra/test/rules/RULE-5-2/test.c @@ -0,0 +1,26 @@ +extern int + iltiqzxgfqsgigwfuyntzghvzltueatcxqnqofnnvjyszmcsylyohvqaosjbqyyA; // NON_COMPLIANT + // - + // length + // 64 + +static int + iltiqzxgfqsgigwfuyntzghvzltueatcxqnqofnnvjyszmcsylyohvqaosjbqyyB; // NON_COMPLIANT + // - + // length + // 64 + +void f() { + int iltiqzxgfqsgigwfuyntzghvzltueatcxqnqofnnvjyszmcsylyohvqaosjbqyyC; // COMPLIANT + // - + // length + // 64 + // but + // diff + // scope +} + +static int + iltiqzxgfqsgigwfuyntzghvzltueatcxqnqofnnvjy_C; // COMPLIANT length <63 +static int + iltiqzxgfqsgigwfuyntzghvzltueatcxqnqofnnvjy_D; // COMPLIANT length <63 \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/exclusions/c/Declarations5.qll b/cpp/common/src/codingstandards/cpp/exclusions/c/Declarations5.qll index db3fa5e08e..55e998b4d4 100644 --- a/cpp/common/src/codingstandards/cpp/exclusions/c/Declarations5.qll +++ b/cpp/common/src/codingstandards/cpp/exclusions/c/Declarations5.qll @@ -3,9 +3,19 @@ import cpp import RuleMetadata import codingstandards.cpp.exclusions.RuleMetadata -newtype Declarations5Query = TExternalObjectOrFunctionNotDeclaredInOneFileQuery() +newtype Declarations5Query = + TIdentifiersDeclaredInTheSameScopeNotDistinctQuery() or + TExternalObjectOrFunctionNotDeclaredInOneFileQuery() predicate isDeclarations5QueryMetadata(Query query, string queryId, string ruleId) { + query = + // `Query` instance for the `identifiersDeclaredInTheSameScopeNotDistinct` query + Declarations5Package::identifiersDeclaredInTheSameScopeNotDistinctQuery() and + queryId = + // `@id` for the `identifiersDeclaredInTheSameScopeNotDistinct` query + "c/misra/identifiers-declared-in-the-same-scope-not-distinct" and + ruleId = "RULE-5-2" + or query = // `Query` instance for the `externalObjectOrFunctionNotDeclaredInOneFile` query Declarations5Package::externalObjectOrFunctionNotDeclaredInOneFileQuery() and @@ -16,6 +26,13 @@ predicate isDeclarations5QueryMetadata(Query query, string queryId, string ruleI } module Declarations5Package { + Query identifiersDeclaredInTheSameScopeNotDistinctQuery() { + //autogenerate `Query` type + result = + // `Query` type for `identifiersDeclaredInTheSameScopeNotDistinct` query + TQueryC(TDeclarations5PackageQuery(TIdentifiersDeclaredInTheSameScopeNotDistinctQuery())) + } + Query externalObjectOrFunctionNotDeclaredInOneFileQuery() { //autogenerate `Query` type result = diff --git a/rule_packages/c/Declarations5.json b/rule_packages/c/Declarations5.json index 1680d3e3af..da656fa500 100644 --- a/rule_packages/c/Declarations5.json +++ b/rule_packages/c/Declarations5.json @@ -1,5 +1,30 @@ { "MISRA-C-2012": { + "RULE-5-2": { + "properties": { + "obligation": "required" + }, + "queries": [ + { + "description": "Using nondistinct identifiers results in undefined behaviour.", + "kind": "problem", + "name": "Identifiers declared in the same scope and name space shall be distinct", + "precision": "very-high", + "severity": "warning", + "short_name": "IdentifiersDeclaredInTheSameScopeNotDistinct", + "tags": [ + "correctness", + "maintainability", + "readability" + ], + "implementation_scope": { + "description": "This query considers the first 63 characters of identifiers as significant, as per C99 for nonexternal identifiers and reports the case when names are longer than 63 characters and differ in those characters past the 63 first only. This query does not consider universal or extended source characters.", + "items": [] + } + } + ], + "title": "Identifiers declared in the same scope and name space shall be distinct" + }, "RULE-8-5": { "properties": { "obligation": "required" diff --git a/rules.csv b/rules.csv index de928dd040..83fd1753b9 100644 --- a/rules.csv +++ b/rules.csv @@ -632,7 +632,7 @@ c,MISRA-C-2012,RULE-3-2,Yes,Required,,,Line-splicing shall not be used in // com c,MISRA-C-2012,RULE-4-1,Yes,Required,,,Octal and hexadecimal escape sequences shall be terminated,A2-13-1 M2-13-2,Syntax,Medium, c,MISRA-C-2012,RULE-4-2,No,Advisory,,,Trigraphs should not be used,A2-5-1,,Import, c,MISRA-C-2012,RULE-5-1,Yes,Required,,,External identifiers shall be distinct,,Declarations1,Medium, -c,MISRA-C-2012,RULE-5-2,Yes,Required,,,Identifiers declared in the same scope and name space shall be distinct,,Declarations,Medium, +c,MISRA-C-2012,RULE-5-2,Yes,Required,,,Identifiers declared in the same scope and name space shall be distinct,,Declarations5,Medium, c,MISRA-C-2012,RULE-5-3,Yes,Required,,,An identifier declared in an inner scope shall not hide an identifier declared in an outer scope,A2-10-1,Declarations3,Import, c,MISRA-C-2012,RULE-5-4,Yes,Required,,,Macro identifiers shall be distinct,,Declarations1,Easy, c,MISRA-C-2012,RULE-5-5,Yes,Required,,,Identifiers shall be distinct from macro names,,Declarations3,Easy, From f866dd52b20b4547cdca948f2c23fb155c576e90 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Wed, 23 Nov 2022 12:26:32 -0500 Subject: [PATCH 3/7] Declarations5: add RULE-8-8 --- ...ngStaticSpecifierFunctionRedeclarationC.ql | 22 ++++++++++++ ...singStaticSpecifierObjectRedeclarationC.ql | 27 +++++++++++++++ ...ticSpecifierFunctionRedeclarationC.testref | 1 + ...aticSpecifierObjectRedeclarationC.expected | 1 + ...gStaticSpecifierObjectRedeclarationC.qlref | 1 + c/misra/test/rules/RULE-8-8/test.c | 8 +++++ ...aticSpecifierOnFunctionRedeclaration.qlref | 1 - ...icSpecifierOnFunctionRedeclaration.testref | 1 + .../cpp/exclusions/c/Declarations5.qll | 34 ++++++++++++++++++- ...icSpecifierFunctionRedeclarationShared.qll | 25 ++++++++++++++ ...ifierFunctionRedeclarationShared.expected} | 0 ...ticSpecifierFunctionRedeclarationShared.ql | 2 ++ .../test.cpp | 0 rule_packages/c/Declarations5.json | 31 +++++++++++++++++ rule_packages/cpp/Scope.json | 1 + rules.csv | 2 +- 16 files changed, 154 insertions(+), 3 deletions(-) create mode 100644 c/misra/src/rules/RULE-8-8/MissingStaticSpecifierFunctionRedeclarationC.ql create mode 100644 c/misra/src/rules/RULE-8-8/MissingStaticSpecifierObjectRedeclarationC.ql create mode 100644 c/misra/test/rules/RULE-8-8/MissingStaticSpecifierFunctionRedeclarationC.testref create mode 100644 c/misra/test/rules/RULE-8-8/MissingStaticSpecifierObjectRedeclarationC.expected create mode 100644 c/misra/test/rules/RULE-8-8/MissingStaticSpecifierObjectRedeclarationC.qlref create mode 100644 c/misra/test/rules/RULE-8-8/test.c delete mode 100644 cpp/autosar/test/rules/M3-3-2/MissingStaticSpecifierOnFunctionRedeclaration.qlref create mode 100644 cpp/autosar/test/rules/M3-3-2/MissingStaticSpecifierOnFunctionRedeclaration.testref create mode 100644 cpp/common/src/codingstandards/cpp/rules/missingstaticspecifierfunctionredeclarationshared/MissingStaticSpecifierFunctionRedeclarationShared.qll rename cpp/{autosar/test/rules/M3-3-2/MissingStaticSpecifierOnFunctionRedeclaration.expected => common/test/rules/missingstaticspecifierfunctionredeclarationshared/MissingStaticSpecifierFunctionRedeclarationShared.expected} (100%) create mode 100644 cpp/common/test/rules/missingstaticspecifierfunctionredeclarationshared/MissingStaticSpecifierFunctionRedeclarationShared.ql rename cpp/{autosar/test/rules/M3-3-2 => common/test/rules/missingstaticspecifierfunctionredeclarationshared}/test.cpp (100%) diff --git a/c/misra/src/rules/RULE-8-8/MissingStaticSpecifierFunctionRedeclarationC.ql b/c/misra/src/rules/RULE-8-8/MissingStaticSpecifierFunctionRedeclarationC.ql new file mode 100644 index 0000000000..a56d4ca426 --- /dev/null +++ b/c/misra/src/rules/RULE-8-8/MissingStaticSpecifierFunctionRedeclarationC.ql @@ -0,0 +1,22 @@ +/** + * @id c/misra/missing-static-specifier-function-redeclaration-c + * @name RULE-8-8: If a function has internal linkage then all re-declarations shall include the static storage class + * @description If a function has internal linkage then all re-declarations shall include the static + * storage class specifier to make the internal linkage explicit. + * @kind problem + * @precision very-high + * @problem.severity warning + * @tags external/misra/id/rule-8-8 + * readability + * external/misra/obligation/required + */ + +import cpp +import codingstandards.c.misra +import codingstandards.cpp.rules.missingstaticspecifierfunctionredeclarationshared.MissingStaticSpecifierFunctionRedeclarationShared + +class MissingStaticSpecifierFunctionRedeclarationCQuery extends MissingStaticSpecifierFunctionRedeclarationSharedSharedQuery { + MissingStaticSpecifierFunctionRedeclarationCQuery() { + this = Declarations5Package::missingStaticSpecifierFunctionRedeclarationCQuery() + } +} diff --git a/c/misra/src/rules/RULE-8-8/MissingStaticSpecifierObjectRedeclarationC.ql b/c/misra/src/rules/RULE-8-8/MissingStaticSpecifierObjectRedeclarationC.ql new file mode 100644 index 0000000000..9d583623de --- /dev/null +++ b/c/misra/src/rules/RULE-8-8/MissingStaticSpecifierObjectRedeclarationC.ql @@ -0,0 +1,27 @@ +/** + * @id c/misra/missing-static-specifier-object-redeclaration-c + * @name RULE-8-8: If an object has internal linkage then all re-declarations shall include the static storage class + * @description If an object has internal linkage then all re-declarations shall include the static + * storage class specifier to make the internal linkage explicit. + * @kind problem + * @precision very-high + * @problem.severity warning + * @tags external/misra/id/rule-8-8 + * readability + * external/misra/obligation/required + */ + +import cpp +import codingstandards.c.misra + +from VariableDeclarationEntry redeclaration, VariableDeclarationEntry de +where + not isExcluded(redeclaration, + Declarations5Package::missingStaticSpecifierObjectRedeclarationCQuery()) and + de.hasSpecifier("static") and + de.getDeclaration().isTopLevel() and + redeclaration.getDeclaration() = de.getDeclaration() and + not redeclaration.hasSpecifier("static") and + de != redeclaration +select redeclaration, "The redeclaration of $@ with internal linkage misses the static specifier.", + de, de.getName() diff --git a/c/misra/test/rules/RULE-8-8/MissingStaticSpecifierFunctionRedeclarationC.testref b/c/misra/test/rules/RULE-8-8/MissingStaticSpecifierFunctionRedeclarationC.testref new file mode 100644 index 0000000000..5b93ea365a --- /dev/null +++ b/c/misra/test/rules/RULE-8-8/MissingStaticSpecifierFunctionRedeclarationC.testref @@ -0,0 +1 @@ +cpp/common/test/rules/missingstaticspecifierfunctionredeclarationshared/MissingStaticSpecifierFunctionRedeclarationShared.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-8-8/MissingStaticSpecifierObjectRedeclarationC.expected b/c/misra/test/rules/RULE-8-8/MissingStaticSpecifierObjectRedeclarationC.expected new file mode 100644 index 0000000000..34a7723bcd --- /dev/null +++ b/c/misra/test/rules/RULE-8-8/MissingStaticSpecifierObjectRedeclarationC.expected @@ -0,0 +1 @@ +| test.c:2:12:2:12 | declaration of g | The redeclaration of $@ with internal linkage misses the static specifier. | test.c:1:12:1:12 | definition of g | g | diff --git a/c/misra/test/rules/RULE-8-8/MissingStaticSpecifierObjectRedeclarationC.qlref b/c/misra/test/rules/RULE-8-8/MissingStaticSpecifierObjectRedeclarationC.qlref new file mode 100644 index 0000000000..70b6073e14 --- /dev/null +++ b/c/misra/test/rules/RULE-8-8/MissingStaticSpecifierObjectRedeclarationC.qlref @@ -0,0 +1 @@ +rules/RULE-8-8/MissingStaticSpecifierObjectRedeclarationC.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-8-8/test.c b/c/misra/test/rules/RULE-8-8/test.c new file mode 100644 index 0000000000..d98d71c6f0 --- /dev/null +++ b/c/misra/test/rules/RULE-8-8/test.c @@ -0,0 +1,8 @@ +static int g = 0; +extern int g; // NON_COMPLIANT + +static int g1; +static int g1 = 0; // COMPLIANT + +int g2; +int g2 = 0; // COMPLIANT diff --git a/cpp/autosar/test/rules/M3-3-2/MissingStaticSpecifierOnFunctionRedeclaration.qlref b/cpp/autosar/test/rules/M3-3-2/MissingStaticSpecifierOnFunctionRedeclaration.qlref deleted file mode 100644 index 052000073f..0000000000 --- a/cpp/autosar/test/rules/M3-3-2/MissingStaticSpecifierOnFunctionRedeclaration.qlref +++ /dev/null @@ -1 +0,0 @@ -rules/M3-3-2/MissingStaticSpecifierOnFunctionRedeclaration.ql \ No newline at end of file diff --git a/cpp/autosar/test/rules/M3-3-2/MissingStaticSpecifierOnFunctionRedeclaration.testref b/cpp/autosar/test/rules/M3-3-2/MissingStaticSpecifierOnFunctionRedeclaration.testref new file mode 100644 index 0000000000..5b93ea365a --- /dev/null +++ b/cpp/autosar/test/rules/M3-3-2/MissingStaticSpecifierOnFunctionRedeclaration.testref @@ -0,0 +1 @@ +cpp/common/test/rules/missingstaticspecifierfunctionredeclarationshared/MissingStaticSpecifierFunctionRedeclarationShared.ql \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/exclusions/c/Declarations5.qll b/cpp/common/src/codingstandards/cpp/exclusions/c/Declarations5.qll index 55e998b4d4..884038c7c5 100644 --- a/cpp/common/src/codingstandards/cpp/exclusions/c/Declarations5.qll +++ b/cpp/common/src/codingstandards/cpp/exclusions/c/Declarations5.qll @@ -5,7 +5,9 @@ import codingstandards.cpp.exclusions.RuleMetadata newtype Declarations5Query = TIdentifiersDeclaredInTheSameScopeNotDistinctQuery() or - TExternalObjectOrFunctionNotDeclaredInOneFileQuery() + TExternalObjectOrFunctionNotDeclaredInOneFileQuery() or + TMissingStaticSpecifierFunctionRedeclarationCQuery() or + TMissingStaticSpecifierObjectRedeclarationCQuery() predicate isDeclarations5QueryMetadata(Query query, string queryId, string ruleId) { query = @@ -23,6 +25,22 @@ predicate isDeclarations5QueryMetadata(Query query, string queryId, string ruleI // `@id` for the `externalObjectOrFunctionNotDeclaredInOneFile` query "c/misra/external-object-or-function-not-declared-in-one-file" and ruleId = "RULE-8-5" + or + query = + // `Query` instance for the `missingStaticSpecifierFunctionRedeclarationC` query + Declarations5Package::missingStaticSpecifierFunctionRedeclarationCQuery() and + queryId = + // `@id` for the `missingStaticSpecifierFunctionRedeclarationC` query + "c/misra/missing-static-specifier-function-redeclaration-c" and + ruleId = "RULE-8-8" + or + query = + // `Query` instance for the `missingStaticSpecifierObjectRedeclarationC` query + Declarations5Package::missingStaticSpecifierObjectRedeclarationCQuery() and + queryId = + // `@id` for the `missingStaticSpecifierObjectRedeclarationC` query + "c/misra/missing-static-specifier-object-redeclaration-c" and + ruleId = "RULE-8-8" } module Declarations5Package { @@ -39,4 +57,18 @@ module Declarations5Package { // `Query` type for `externalObjectOrFunctionNotDeclaredInOneFile` query TQueryC(TDeclarations5PackageQuery(TExternalObjectOrFunctionNotDeclaredInOneFileQuery())) } + + Query missingStaticSpecifierFunctionRedeclarationCQuery() { + //autogenerate `Query` type + result = + // `Query` type for `missingStaticSpecifierFunctionRedeclarationC` query + TQueryC(TDeclarations5PackageQuery(TMissingStaticSpecifierFunctionRedeclarationCQuery())) + } + + Query missingStaticSpecifierObjectRedeclarationCQuery() { + //autogenerate `Query` type + result = + // `Query` type for `missingStaticSpecifierObjectRedeclarationC` query + TQueryC(TDeclarations5PackageQuery(TMissingStaticSpecifierObjectRedeclarationCQuery())) + } } diff --git a/cpp/common/src/codingstandards/cpp/rules/missingstaticspecifierfunctionredeclarationshared/MissingStaticSpecifierFunctionRedeclarationShared.qll b/cpp/common/src/codingstandards/cpp/rules/missingstaticspecifierfunctionredeclarationshared/MissingStaticSpecifierFunctionRedeclarationShared.qll new file mode 100644 index 0000000000..70d498827f --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/rules/missingstaticspecifierfunctionredeclarationshared/MissingStaticSpecifierFunctionRedeclarationShared.qll @@ -0,0 +1,25 @@ +/** + * Provides a library which includes a `problems` predicate for reporting.... + */ + +import cpp +import codingstandards.cpp.Customizations +import codingstandards.cpp.Exclusions + +abstract class MissingStaticSpecifierFunctionRedeclarationSharedSharedQuery extends Query { } + +Query getQuery() { result instanceof MissingStaticSpecifierFunctionRedeclarationSharedSharedQuery } + +query predicate problems( + FunctionDeclarationEntry redeclaration, string message, FunctionDeclarationEntry fde, + string msgpiece +) { + not isExcluded(redeclaration, getQuery()) and + fde.hasSpecifier("static") and + fde.getDeclaration().isTopLevel() and + redeclaration.getDeclaration() = fde.getDeclaration() and + not redeclaration.hasSpecifier("static") and + fde != redeclaration and + message = "The redeclaration of $@ with internal linkage misses the static specifier." and + msgpiece = "function" +} diff --git a/cpp/autosar/test/rules/M3-3-2/MissingStaticSpecifierOnFunctionRedeclaration.expected b/cpp/common/test/rules/missingstaticspecifierfunctionredeclarationshared/MissingStaticSpecifierFunctionRedeclarationShared.expected similarity index 100% rename from cpp/autosar/test/rules/M3-3-2/MissingStaticSpecifierOnFunctionRedeclaration.expected rename to cpp/common/test/rules/missingstaticspecifierfunctionredeclarationshared/MissingStaticSpecifierFunctionRedeclarationShared.expected diff --git a/cpp/common/test/rules/missingstaticspecifierfunctionredeclarationshared/MissingStaticSpecifierFunctionRedeclarationShared.ql b/cpp/common/test/rules/missingstaticspecifierfunctionredeclarationshared/MissingStaticSpecifierFunctionRedeclarationShared.ql new file mode 100644 index 0000000000..50954b88bf --- /dev/null +++ b/cpp/common/test/rules/missingstaticspecifierfunctionredeclarationshared/MissingStaticSpecifierFunctionRedeclarationShared.ql @@ -0,0 +1,2 @@ +// GENERATED FILE - DO NOT MODIFY +import codingstandards.cpp.rules.missingstaticspecifierfunctionredeclarationshared.MissingStaticSpecifierFunctionRedeclarationShared diff --git a/cpp/autosar/test/rules/M3-3-2/test.cpp b/cpp/common/test/rules/missingstaticspecifierfunctionredeclarationshared/test.cpp similarity index 100% rename from cpp/autosar/test/rules/M3-3-2/test.cpp rename to cpp/common/test/rules/missingstaticspecifierfunctionredeclarationshared/test.cpp diff --git a/rule_packages/c/Declarations5.json b/rule_packages/c/Declarations5.json index da656fa500..2d6eeee5c2 100644 --- a/rule_packages/c/Declarations5.json +++ b/rule_packages/c/Declarations5.json @@ -43,6 +43,37 @@ } ], "title": "An external object or function shall be declared once in one and only one file" + }, + "RULE-8-8": { + "properties": { + "obligation": "required" + }, + "queries": [ + { + "description": "If a function has internal linkage then all re-declarations shall include the static storage class specifier to make the internal linkage explicit.", + "kind": "problem", + "name": "If a function has internal linkage then all re-declarations shall include the static storage class", + "precision": "very-high", + "severity": "warning", + "short_name": "MissingStaticSpecifierFunctionRedeclarationC", + "shared_implementation_short_name": "MissingStaticSpecifierFunctionRedeclarationShared", + "tags": [ + "readability" + ] + }, + { + "description": "If an object has internal linkage then all re-declarations shall include the static storage class specifier to make the internal linkage explicit.", + "kind": "problem", + "name": "If an object has internal linkage then all re-declarations shall include the static storage class", + "precision": "very-high", + "severity": "warning", + "short_name": "MissingStaticSpecifierObjectRedeclarationC", + "tags": [ + "readability" + ] + } + ], + "title": "The static storage class specifier shall be used in all declarations of objects and functions that have internal linkage" } } } \ No newline at end of file diff --git a/rule_packages/cpp/Scope.json b/rule_packages/cpp/Scope.json index c169badcbe..0d3abcbc0a 100644 --- a/rule_packages/cpp/Scope.json +++ b/rule_packages/cpp/Scope.json @@ -179,6 +179,7 @@ "precision": "very-high", "severity": "warning", "short_name": "MissingStaticSpecifierOnFunctionRedeclaration", + "shared_implementation_short_name": "MissingStaticSpecifierFunctionRedeclarationShared", "tags": [ "readability" ] diff --git a/rules.csv b/rules.csv index 83fd1753b9..a39e8610e2 100644 --- a/rules.csv +++ b/rules.csv @@ -653,7 +653,7 @@ c,MISRA-C-2012,RULE-8-4,Yes,Required,,,A compatible declaration shall be visible c,MISRA-C-2012,RULE-8-5,Yes,Required,,,An external object or function shall be declared once in one and only one file,,Declarations5,Medium, c,MISRA-C-2012,RULE-8-6,Yes,Required,,,An identifier with external linkage shall have exactly one external definition,M3-2-4,Declarations,Import, c,MISRA-C-2012,RULE-8-7,Yes,Advisory,,,Functions and objects should not be defined with external linkage if they are referenced in only one translation unit,,Declarations,Medium, -c,MISRA-C-2012,RULE-8-8,Yes,Required,,,The static storage class specifier shall be used in all declarations of objects and functions that have internal linkage,M3-3-2,Declarations,Medium, +c,MISRA-C-2012,RULE-8-8,Yes,Required,,,The static storage class specifier shall be used in all declarations of objects and functions that have internal linkage,M3-3-2,Declarations5,Medium, c,MISRA-C-2012,RULE-8-9,Yes,Advisory,,,An object should be defined at block scope if its identifier only appears in a single function,M3-4-1,Declarations,Medium, c,MISRA-C-2012,RULE-8-10,Yes,Required,,,An inline function shall be declared with the static storage class,,Declarations,Medium, c,MISRA-C-2012,RULE-8-11,Yes,Advisory,,,"When an array with external linkage is declared, its size should be explicitly specified",,Declarations,Medium, From 654f9fa720496d19d704698699167a2114f9bdf6 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Tue, 29 Nov 2022 15:40:32 -0500 Subject: [PATCH 4/7] Declarations5: add RULE-8-9 --- ...xposedIdentifierDeclarationShared.expected | 4 + ...ssaryExposedIdentifierDeclarationShared.ql | 2 + .../test.c | 110 +++++++ ...nnecessaryExposedIdentifierDeclarationC.ql | 22 ++ ...ssaryExposedIdentifierDeclarationC.testref | 1 + ...UnnecessaryExposedIdentifierDeclaration.ql | 275 +---------------- ...ecessaryExposedIdentifierDeclaration.qlref | 1 - ...essaryExposedIdentifierDeclaration.testref | 1 + .../cpp/exclusions/c/Declarations5.qll | 18 +- ...saryExposedIdentifierDeclarationShared.qll | 289 ++++++++++++++++++ ...posedIdentifierDeclarationShared.expected} | 0 ...ssaryExposedIdentifierDeclarationShared.ql | 2 + .../test.cpp | 0 rule_packages/c/Declarations5.json | 20 ++ rule_packages/cpp/Scope.json | 1 + rules.csv | 2 +- 16 files changed, 474 insertions(+), 274 deletions(-) create mode 100644 c/common/test/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.expected create mode 100644 c/common/test/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.ql create mode 100644 c/common/test/rules/unnecessaryexposedidentifierdeclarationshared/test.c create mode 100644 c/misra/src/rules/RULE-8-9/UnnecessaryExposedIdentifierDeclarationC.ql create mode 100644 c/misra/test/rules/RULE-8-9/UnnecessaryExposedIdentifierDeclarationC.testref delete mode 100644 cpp/autosar/test/rules/M3-4-1/UnnecessaryExposedIdentifierDeclaration.qlref create mode 100644 cpp/autosar/test/rules/M3-4-1/UnnecessaryExposedIdentifierDeclaration.testref create mode 100644 cpp/common/src/codingstandards/cpp/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.qll rename cpp/{autosar/test/rules/M3-4-1/UnnecessaryExposedIdentifierDeclaration.expected => common/test/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.expected} (100%) create mode 100644 cpp/common/test/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.ql rename cpp/{autosar/test/rules/M3-4-1 => common/test/rules/unnecessaryexposedidentifierdeclarationshared}/test.cpp (100%) diff --git a/c/common/test/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.expected b/c/common/test/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.expected new file mode 100644 index 0000000000..e9d863a111 --- /dev/null +++ b/c/common/test/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.expected @@ -0,0 +1,4 @@ +| test.c:4:12:4:13 | g2 | The declaration g2 should be moved from the global namespace scope$@ into the $@ too minimize its visibility. | file://:0:0:0:0 | (global namespace) | scope | test.c:59:11:59:25 | { ... } | scope | +| test.c:7:7:7:7 | j | The declaration j should be moved from $@ into the $@ too minimize its visibility. | test.c:6:11:13:1 | { ... } | scope | test.c:8:13:12:3 | { ... } | scope | +| test.c:62:7:62:7 | i | The declaration i should be moved from $@ into the $@ too minimize its visibility. | test.c:61:11:71:1 | { ... } | scope | test.c:64:13:70:3 | { ... } | scope | +| test.c:73:8:73:9 | S1 | The declaration S1 should be moved from the global namespace scope$@ into the $@ too minimize its visibility. | file://:0:0:0:0 | (global namespace) | scope | test.c:77:12:77:28 | { ... } | scope | diff --git a/c/common/test/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.ql b/c/common/test/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.ql new file mode 100644 index 0000000000..9914ad0b1e --- /dev/null +++ b/c/common/test/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.ql @@ -0,0 +1,2 @@ +// GENERATED FILE - DO NOT MODIFY +import codingstandards.cpp.rules.unnecessaryexposedidentifierdeclarationshared.UnnecessaryExposedIdentifierDeclarationShared diff --git a/c/common/test/rules/unnecessaryexposedidentifierdeclarationshared/test.c b/c/common/test/rules/unnecessaryexposedidentifierdeclarationshared/test.c new file mode 100644 index 0000000000..0ef89369fc --- /dev/null +++ b/c/common/test/rules/unnecessaryexposedidentifierdeclarationshared/test.c @@ -0,0 +1,110 @@ +#include +extern void f1(int i); +extern int g1; // COMPLIANT +extern int g2; // NON_COMPLIANT; single use of a global variable +bool f2() { return g1 == 1; } +void f3() { + int j = g1; // NON_COMPLIANT + if (f2()) { + int k; // COMPLIANT + f1(j); + f1(k); + } +} + +void f4() { + int j = g1; // COMPLIANT; value of g1 changed between + // definition and use + g1 = 1; + if (f2()) { + f1(j); + } +} + +void f5() { + int j = g1; // COMPLIANT; shouldn't be moved inside loop + while (true) { + int i = g1++; + while (f2()) { + i += j; + } + + if (i % 2) + break; + } +} + +void f6() { + int j = g1; // COMPLIANT; can't moved into smaller scope +#ifdef FOO + if (g1) { + g1 = j + 1; + } +#else + if (g1) { + g1 = j + 2; + } +#endif +} + +void f7() { + int j = g1; // COMPLIANT; potentially stores previous value of + // g1 so moving this would be incorrect. + f1(1); // f1 may change the value of g1 + if (f2()) { + f1(j); + } +} + +void f8() { int i = g2; } + +void f9() { + int i; // NON_COMPLIANT + + if (f2()) { + if (f2()) { + i++; + } else { + i--; + } + } +} + +struct S1 { // NON_COMPLIANT + int i; +}; + +void f10() { struct S1 l1; } + +void f11() { + struct S2 { // COMPLIANT + int i; + } l1; +} + +struct S3 { + int i; +}; + +struct S4 { // NON_COMPLIANT; single use in function f13 + int i; +}; + +void f15() { + int i; // COMPLIANT + + if (i == 0) { + i++; + } +} + +void f17() { + int i; // COMPLIANT + int *ptr; + { + // Moving the declaration of i into the reduced scope will result in a + // dangling pointer + ptr = &i; + } + *ptr = 1; +} \ No newline at end of file diff --git a/c/misra/src/rules/RULE-8-9/UnnecessaryExposedIdentifierDeclarationC.ql b/c/misra/src/rules/RULE-8-9/UnnecessaryExposedIdentifierDeclarationC.ql new file mode 100644 index 0000000000..09cad2f08d --- /dev/null +++ b/c/misra/src/rules/RULE-8-9/UnnecessaryExposedIdentifierDeclarationC.ql @@ -0,0 +1,22 @@ +/** + * @id c/misra/unnecessary-exposed-identifier-declaration-c + * @name RULE-8-9: An object should be defined at block scope if its identifier only appears in a single function + * @description An identifier declared to be an object or type shall be defined in a block that + * minimizes its visibility to prevent any accidental use of the identifier. + * @kind problem + * @precision high + * @problem.severity warning + * @tags external/misra/id/rule-8-9 + * correctness + * external/misra/obligation/advisory + */ + +import cpp +import codingstandards.c.misra +import codingstandards.cpp.rules.unnecessaryexposedidentifierdeclarationshared.UnnecessaryExposedIdentifierDeclarationShared + +class UnnecessaryExposedIdentifierDeclarationCQuery extends UnnecessaryExposedIdentifierDeclarationSharedSharedQuery { + UnnecessaryExposedIdentifierDeclarationCQuery() { + this = Declarations5Package::unnecessaryExposedIdentifierDeclarationCQuery() + } +} diff --git a/c/misra/test/rules/RULE-8-9/UnnecessaryExposedIdentifierDeclarationC.testref b/c/misra/test/rules/RULE-8-9/UnnecessaryExposedIdentifierDeclarationC.testref new file mode 100644 index 0000000000..35c4abc5d4 --- /dev/null +++ b/c/misra/test/rules/RULE-8-9/UnnecessaryExposedIdentifierDeclarationC.testref @@ -0,0 +1 @@ +c/common/test/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.ql \ No newline at end of file diff --git a/cpp/autosar/src/rules/M3-4-1/UnnecessaryExposedIdentifierDeclaration.ql b/cpp/autosar/src/rules/M3-4-1/UnnecessaryExposedIdentifierDeclaration.ql index 6a99cb820a..1d84a385e5 100644 --- a/cpp/autosar/src/rules/M3-4-1/UnnecessaryExposedIdentifierDeclaration.ql +++ b/cpp/autosar/src/rules/M3-4-1/UnnecessaryExposedIdentifierDeclaration.ql @@ -15,277 +15,10 @@ import cpp import codingstandards.cpp.autosar -import codingstandards.cpp.Scope -import codingstandards.cpp.SideEffect -import codingstandards.cpp.sideeffect.Customizations +import codingstandards.cpp.rules.unnecessaryexposedidentifierdeclarationshared.UnnecessaryExposedIdentifierDeclarationShared -class ExternalCall extends Call { - ExternalCall() { - exists(Function f | this.getTarget() = f | - not f.hasDefinition() and not f.isCompilerGenerated() - ) +class UnnecessaryExposedIdentifierDeclarationQuery extends UnnecessaryExposedIdentifierDeclarationSharedSharedQuery { + UnnecessaryExposedIdentifierDeclarationQuery() { + this = ScopePackage::unnecessaryExposedIdentifierDeclarationQuery() } } - -class LoopOrSwitchBody extends BlockStmt { - LoopOrSwitchBody() { - exists(Loop l | l.getStmt() = this.getParentScope*()) - or - exists(SwitchStmt ss | ss.getStmt() = this) - } -} - -/* Gets a scope for `b` that is an ancestor of `b`, but is not a loop or switch scope. */ -Scope getCandidateScope(Scope b) { - if b instanceof LoopOrSwitchBody or b instanceof ControlStructure - then result = getCandidateScope(b.getStrictParent()) - else - if b.isGenerated() - then result = b.getStrictParent() - else result = b -} - -private predicate getLocationInfo( - CandidateDeclaration d, PreprocessorBranchDirective pbd, int startline, int endline, string path1, - string path2 -) { - d.getLocation().getEndLine() = endline and - pbd.getLocation().getStartLine() = startline and - d.getFile().getAbsolutePath() = path1 and - pbd.getFile().getAbsolutePath() = path2 -} - -predicate isStrictlyBefore(CandidateDeclaration d, PreprocessorBranchDirective branch) { - exists(string path, int startLine, int endLine | - getLocationInfo(d, branch, startLine, endLine, path, path) and - endLine < startLine - ) -} - -Variable getADependentVariable(Variable v) { - exists(VariableAccess va | - va.getTarget() = result and v.getInitializer().getExpr().getAChild*() = va - ) -} - -/** - * Holds if it is assigned a value that is modified in between the declaration of `v` and a use of `v`. - */ -predicate isTempVariable(LocalVariable v) { - exists( - DeclStmt ds, VariableDeclarationEntry vde, Variable dependentVariable, Expr sideEffect, - VariableAccess va - | - v.getAnAccess() = va and - dependentVariable = getADependentVariable(v) and - exists( - BasicBlock declarationStmtBb, BasicBlock sideEffectBb, BasicBlock variableAccessBb, - int declarationStmtPos, int sideEffectPos, int variableAccessPos - | - declarationStmtBb.getNode(declarationStmtPos) = ds and - variableAccessBb.getNode(variableAccessPos) = va - | - ( - ( - sideEffect.(VariableEffect).getTarget() = dependentVariable and - if not sideEffect.getEnclosingFunction() = va.getEnclosingFunction() - then - exists(FunctionCall call | - call.getEnclosingFunction() = va.getEnclosingFunction() and - call.getTarget().calls(sideEffect.getEnclosingFunction()) and - sideEffectBb.getNode(sideEffectPos) = call - ) - else sideEffectBb.getNode(sideEffectPos) = sideEffect - ) - or - dependentVariable instanceof GlobalVariable and - sideEffect instanceof ExternalCall and - ds.getEnclosingFunction() = sideEffect.getEnclosingFunction() and - sideEffectBb.getNode(sideEffectPos) = sideEffect - ) and - ( - declarationStmtBb.getASuccessor+() = sideEffectBb - or - declarationStmtBb = sideEffectBb and declarationStmtPos < sideEffectPos - ) and - ( - sideEffectBb.getASuccessor+() = variableAccessBb - or - sideEffectBb = variableAccessBb and sideEffectPos < variableAccessPos - ) - ) and - vde.getDeclaration() = v and - ds.getDeclarationEntry(_) = vde - ) -} - -private predicate isTypeUse(Type t1, Type t2) { - t1.getUnspecifiedType() = t2 - or - t1.(PointerType).getBaseType().getUnspecifiedType() = t2 - or - t1.(ReferenceType).getBaseType().getUnspecifiedType() = t2 - or - t1.(ArrayType).getBaseType().getUnspecifiedType() = t2 -} - -newtype TDeclarationAccess = - ObjectAccess(Variable v, VariableAccess va) { va = v.getAnAccess() } or - /* Type access can be done in a declaration or an expression (e.g., static member function call) */ - TypeAccess(Type t, Element access) { - isTypeUse(access.(Variable).getUnspecifiedType(), t) - or - exists(ClassTemplateInstantiation cti | - isTypeUse(cti.getATemplateArgument(), t) and - access.(Variable).getUnspecifiedType() = cti - ) - or - exists(FunctionTemplateInstantiation fti | - isTypeUse(fti.getATemplateArgument(), t) and - fti = access - ) - or - exists(FunctionCall call, MemberFunction mf | - call = access and call.getTarget() = mf and mf.isStatic() and mf.getDeclaringType() = t - ) - or - exists(Function f | - isTypeUse(f.getType(), t) and - f = access - ) - } - -class DeclarationAccess extends TDeclarationAccess { - Location getLocation() { - exists(VariableAccess va, Variable v | this = ObjectAccess(v, va) and result = va.getLocation()) - or - exists(Element access | - this = TypeAccess(_, access) and - result = access.getLocation() - ) - } - - string toString() { - exists(Variable v | this = ObjectAccess(v, _) and result = "Object access for " + v.getName()) - or - exists(Type t | - this = TypeAccess(t, _) and - result = "Type access for " + t.getName() - ) - } - - /* Gets the declaration that is being accessed. */ - Declaration getDeclaration() { - this = ObjectAccess(result, _) - or - this = TypeAccess(result, _) - } - - /* Gets the declaration or expression that uses the type being accessed. */ - Element getUnderlyingTypeAccess() { this = TypeAccess(_, result) } - - VariableAccess getUnderlyingObjectAccess() { this = ObjectAccess(_, result) } - - /* Gets the scope of the access. */ - Scope getScope() { - exists(VariableAccess va | - va = getUnderlyingObjectAccess() and - result.getAnExpr() = va - ) - or - exists(Element e | e = getUnderlyingTypeAccess() and result = e.getParentScope()) - } - - /* Holds if a type access is generated from the template instantiation `instantionion` */ - predicate isFromTemplateInstantiation(Element instantiation) { - exists(Element access | - this = TypeAccess(_, access) and access.isFromTemplateInstantiation(instantiation) - ) - } - - predicate isCompilerGenerated() { - exists(VariableAccess va | va = getUnderlyingObjectAccess() and va.isCompilerGenerated()) - or - exists(Element e | - e = getUnderlyingTypeAccess() and - (compgenerated(underlyingElement(e)) or compgenerated(underlyingElement(e.getParentScope()))) - ) - } -} - -class CandidateDeclaration extends Declaration { - CandidateDeclaration() { - this instanceof LocalVariable - or - this instanceof GlobalOrNamespaceVariable - or - this instanceof Type and - not this instanceof ClassTemplateInstantiation and - not this instanceof TemplateParameter - } -} - -/* Gets the scopes that include all the declaration accesses for declaration `d`. */ -Scope possibleScopesForDeclaration(CandidateDeclaration d) { - forex(Scope scope, DeclarationAccess da | - da.getDeclaration() = d and - // Exclude declaration accesses that are compiler generated so we can minimize the visibility of types. - // Otherwise, for example, we cannot reduce the scope of classes with compiler generated member functions based on - // declaration accesses. - not da.isCompilerGenerated() and - not da.isFromTemplateInstantiation(_) and - scope = da.getScope() - | - result = scope.getStrictParent*() - ) and - // Limit the best scope to block statements and namespaces or control structures - (result instanceof BlockStmt or result instanceof Namespace) -} - -/* Gets the smallest scope that includes all the declaration accesses of declaration `d`. */ -Scope bestScopeForDeclarationEntry(CandidateDeclaration d, Scope currentScope) { - result = possibleScopesForDeclaration(d) and - not exists(Scope other | other = possibleScopesForDeclaration(d) | result = other.getAnAncestor()) and - currentScope.getADeclaration() = d and - result.getAnAncestor() = currentScope and - not result instanceof LoopOrSwitchBody and - not result.isGenerated() -} - -/** - * Gets a string suitable for printing a scope in an alert message, that includes an `$@` - * formatting string. - * - * This is necessary because some scopes (e.g. `Namespace`) do not have meaningful - * locations in the database and the alert message will not render the name if that is the case. - */ -string getScopeDescription(Scope s) { - if s instanceof GlobalNamespace then result = "the global namespace scope$@" else result = "$@" -} - -from CandidateDeclaration d, Scope candidateScope, Scope currentScope -where - not isExcluded(d, ScopePackage::unnecessaryExposedIdentifierDeclarationQuery()) and - candidateScope = bestScopeForDeclarationEntry(d, currentScope) and - // We can't reduce the scope if the value stored in the declaration is changed before the declared - // variable is used, because this would change the semantics of the use. - (d instanceof Variable implies not isTempVariable(d)) and - not exists(AddressOfExpr e | e.getAddressable() = d) and - // We can't reduce the scope of the declaration if its minimal scope resides inside a preprocessor - // branch directive while the current scope isn't. This can result in an incorrect program - // where a variable is used but not declared. - not exists(PreprocessorBranchDirective branch | - isStrictlyBefore(d, branch) and - branch = candidateScope.getAnEnclosingPreprocessorBranchDirective() - ) and - // We can't promote a class to a local class if it has static data members (See [class.local] paragraph 4 N3797.) - ( - (d instanceof Class and candidateScope.getStrictParent() instanceof Function) - implies - not exists(Variable member | d.(Class).getAMember() = member and member.isStatic()) - ) and - not candidateScope.isAffectedByMacro() -select d, - "The declaration " + d.getName() + " should be moved from " + getScopeDescription(currentScope) + - " into the " + getScopeDescription(candidateScope) + " too minimize its visibility.", - currentScope, "scope", candidateScope, "scope" diff --git a/cpp/autosar/test/rules/M3-4-1/UnnecessaryExposedIdentifierDeclaration.qlref b/cpp/autosar/test/rules/M3-4-1/UnnecessaryExposedIdentifierDeclaration.qlref deleted file mode 100644 index 6f6edc783a..0000000000 --- a/cpp/autosar/test/rules/M3-4-1/UnnecessaryExposedIdentifierDeclaration.qlref +++ /dev/null @@ -1 +0,0 @@ -rules/M3-4-1/UnnecessaryExposedIdentifierDeclaration.ql \ No newline at end of file diff --git a/cpp/autosar/test/rules/M3-4-1/UnnecessaryExposedIdentifierDeclaration.testref b/cpp/autosar/test/rules/M3-4-1/UnnecessaryExposedIdentifierDeclaration.testref new file mode 100644 index 0000000000..f66784283e --- /dev/null +++ b/cpp/autosar/test/rules/M3-4-1/UnnecessaryExposedIdentifierDeclaration.testref @@ -0,0 +1 @@ +cpp/common/test/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.ql \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/exclusions/c/Declarations5.qll b/cpp/common/src/codingstandards/cpp/exclusions/c/Declarations5.qll index 884038c7c5..a5ec7f311d 100644 --- a/cpp/common/src/codingstandards/cpp/exclusions/c/Declarations5.qll +++ b/cpp/common/src/codingstandards/cpp/exclusions/c/Declarations5.qll @@ -7,7 +7,8 @@ newtype Declarations5Query = TIdentifiersDeclaredInTheSameScopeNotDistinctQuery() or TExternalObjectOrFunctionNotDeclaredInOneFileQuery() or TMissingStaticSpecifierFunctionRedeclarationCQuery() or - TMissingStaticSpecifierObjectRedeclarationCQuery() + TMissingStaticSpecifierObjectRedeclarationCQuery() or + TUnnecessaryExposedIdentifierDeclarationCQuery() predicate isDeclarations5QueryMetadata(Query query, string queryId, string ruleId) { query = @@ -41,6 +42,14 @@ predicate isDeclarations5QueryMetadata(Query query, string queryId, string ruleI // `@id` for the `missingStaticSpecifierObjectRedeclarationC` query "c/misra/missing-static-specifier-object-redeclaration-c" and ruleId = "RULE-8-8" + or + query = + // `Query` instance for the `unnecessaryExposedIdentifierDeclarationC` query + Declarations5Package::unnecessaryExposedIdentifierDeclarationCQuery() and + queryId = + // `@id` for the `unnecessaryExposedIdentifierDeclarationC` query + "c/misra/unnecessary-exposed-identifier-declaration-c" and + ruleId = "RULE-8-9" } module Declarations5Package { @@ -71,4 +80,11 @@ module Declarations5Package { // `Query` type for `missingStaticSpecifierObjectRedeclarationC` query TQueryC(TDeclarations5PackageQuery(TMissingStaticSpecifierObjectRedeclarationCQuery())) } + + Query unnecessaryExposedIdentifierDeclarationCQuery() { + //autogenerate `Query` type + result = + // `Query` type for `unnecessaryExposedIdentifierDeclarationC` query + TQueryC(TDeclarations5PackageQuery(TUnnecessaryExposedIdentifierDeclarationCQuery())) + } } diff --git a/cpp/common/src/codingstandards/cpp/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.qll b/cpp/common/src/codingstandards/cpp/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.qll new file mode 100644 index 0000000000..7006ef7b38 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.qll @@ -0,0 +1,289 @@ +/** + * Provides a library which includes a `problems` predicate for reporting.... + */ + +import cpp +import codingstandards.cpp.Customizations +import codingstandards.cpp.Exclusions +import codingstandards.cpp.Scope +import codingstandards.cpp.SideEffect +import codingstandards.cpp.sideeffect.Customizations + +class ExternalCall extends Call { + ExternalCall() { + exists(Function f | this.getTarget() = f | + not f.hasDefinition() and not f.isCompilerGenerated() + ) + } +} + +class LoopOrSwitchBody extends BlockStmt { + LoopOrSwitchBody() { + exists(Loop l | l.getStmt() = this.getParentScope*()) + or + exists(SwitchStmt ss | ss.getStmt() = this) + } +} + +/* Gets a scope for `b` that is an ancestor of `b`, but is not a loop or switch scope. */ +Scope getCandidateScope(Scope b) { + if b instanceof LoopOrSwitchBody or b instanceof ControlStructure + then result = getCandidateScope(b.getStrictParent()) + else + if b.isGenerated() + then result = b.getStrictParent() + else result = b +} + +private predicate getLocationInfo( + CandidateDeclaration d, PreprocessorBranchDirective pbd, int startline, int endline, string path1, + string path2 +) { + d.getLocation().getEndLine() = endline and + pbd.getLocation().getStartLine() = startline and + d.getFile().getAbsolutePath() = path1 and + pbd.getFile().getAbsolutePath() = path2 +} + +predicate isStrictlyBefore(CandidateDeclaration d, PreprocessorBranchDirective branch) { + exists(string path, int startLine, int endLine | + getLocationInfo(d, branch, startLine, endLine, path, path) and + endLine < startLine + ) +} + +Variable getADependentVariable(Variable v) { + exists(VariableAccess va | + va.getTarget() = result and v.getInitializer().getExpr().getAChild*() = va + ) +} + +/** + * Holds if it is assigned a value that is modified in between the declaration of `v` and a use of `v`. + */ +predicate isTempVariable(LocalVariable v) { + exists( + DeclStmt ds, VariableDeclarationEntry vde, Variable dependentVariable, Expr sideEffect, + VariableAccess va + | + v.getAnAccess() = va and + dependentVariable = getADependentVariable(v) and + exists( + BasicBlock declarationStmtBb, BasicBlock sideEffectBb, BasicBlock variableAccessBb, + int declarationStmtPos, int sideEffectPos, int variableAccessPos + | + declarationStmtBb.getNode(declarationStmtPos) = ds and + variableAccessBb.getNode(variableAccessPos) = va + | + ( + ( + sideEffect.(VariableEffect).getTarget() = dependentVariable and + if not sideEffect.getEnclosingFunction() = va.getEnclosingFunction() + then + exists(FunctionCall call | + call.getEnclosingFunction() = va.getEnclosingFunction() and + call.getTarget().calls(sideEffect.getEnclosingFunction()) and + sideEffectBb.getNode(sideEffectPos) = call + ) + else sideEffectBb.getNode(sideEffectPos) = sideEffect + ) + or + dependentVariable instanceof GlobalVariable and + sideEffect instanceof ExternalCall and + ds.getEnclosingFunction() = sideEffect.getEnclosingFunction() and + sideEffectBb.getNode(sideEffectPos) = sideEffect + ) and + ( + declarationStmtBb.getASuccessor+() = sideEffectBb + or + declarationStmtBb = sideEffectBb and declarationStmtPos < sideEffectPos + ) and + ( + sideEffectBb.getASuccessor+() = variableAccessBb + or + sideEffectBb = variableAccessBb and sideEffectPos < variableAccessPos + ) + ) and + vde.getDeclaration() = v and + ds.getDeclarationEntry(_) = vde + ) +} + +private predicate isTypeUse(Type t1, Type t2) { + t1.getUnspecifiedType() = t2 + or + t1.(PointerType).getBaseType().getUnspecifiedType() = t2 + or + t1.(ReferenceType).getBaseType().getUnspecifiedType() = t2 + or + t1.(ArrayType).getBaseType().getUnspecifiedType() = t2 +} + +newtype TDeclarationAccess = + ObjectAccess(Variable v, VariableAccess va) { va = v.getAnAccess() } or + /* Type access can be done in a declaration or an expression (e.g., static member function call) */ + TypeAccess(Type t, Element access) { + isTypeUse(access.(Variable).getUnspecifiedType(), t) + or + exists(ClassTemplateInstantiation cti | + isTypeUse(cti.getATemplateArgument(), t) and + access.(Variable).getUnspecifiedType() = cti + ) + or + exists(FunctionTemplateInstantiation fti | + isTypeUse(fti.getATemplateArgument(), t) and + fti = access + ) + or + exists(FunctionCall call, MemberFunction mf | + call = access and call.getTarget() = mf and mf.isStatic() and mf.getDeclaringType() = t + ) + or + exists(Function f | + isTypeUse(f.getType(), t) and + f = access + ) + } + +class DeclarationAccess extends TDeclarationAccess { + Location getLocation() { + exists(VariableAccess va, Variable v | this = ObjectAccess(v, va) and result = va.getLocation()) + or + exists(Element access | + this = TypeAccess(_, access) and + result = access.getLocation() + ) + } + + string toString() { + exists(Variable v | this = ObjectAccess(v, _) and result = "Object access for " + v.getName()) + or + exists(Type t | + this = TypeAccess(t, _) and + result = "Type access for " + t.getName() + ) + } + + /* Gets the declaration that is being accessed. */ + Declaration getDeclaration() { + this = ObjectAccess(result, _) + or + this = TypeAccess(result, _) + } + + /* Gets the declaration or expression that uses the type being accessed. */ + Element getUnderlyingTypeAccess() { this = TypeAccess(_, result) } + + VariableAccess getUnderlyingObjectAccess() { this = ObjectAccess(_, result) } + + /* Gets the scope of the access. */ + Scope getScope() { + exists(VariableAccess va | + va = getUnderlyingObjectAccess() and + result.getAnExpr() = va + ) + or + exists(Element e | e = getUnderlyingTypeAccess() and result = e.getParentScope()) + } + + /* Holds if a type access is generated from the template instantiation `instantionion` */ + predicate isFromTemplateInstantiation(Element instantiation) { + exists(Element access | + this = TypeAccess(_, access) and access.isFromTemplateInstantiation(instantiation) + ) + } + + predicate isCompilerGenerated() { + exists(VariableAccess va | va = getUnderlyingObjectAccess() and va.isCompilerGenerated()) + or + exists(Element e | + e = getUnderlyingTypeAccess() and + (compgenerated(underlyingElement(e)) or compgenerated(underlyingElement(e.getParentScope()))) + ) + } +} + +class CandidateDeclaration extends Declaration { + CandidateDeclaration() { + this instanceof LocalVariable + or + this instanceof GlobalOrNamespaceVariable + or + this instanceof Type and + not this instanceof ClassTemplateInstantiation and + not this instanceof TemplateParameter + } +} + +/* Gets the scopes that include all the declaration accesses for declaration `d`. */ +Scope possibleScopesForDeclaration(CandidateDeclaration d) { + forex(Scope scope, DeclarationAccess da | + da.getDeclaration() = d and + // Exclude declaration accesses that are compiler generated so we can minimize the visibility of types. + // Otherwise, for example, we cannot reduce the scope of classes with compiler generated member functions based on + // declaration accesses. + not da.isCompilerGenerated() and + not da.isFromTemplateInstantiation(_) and + scope = da.getScope() + | + result = scope.getStrictParent*() + ) and + // Limit the best scope to block statements and namespaces or control structures + (result instanceof BlockStmt or result instanceof Namespace) +} + +/* Gets the smallest scope that includes all the declaration accesses of declaration `d`. */ +Scope bestScopeForDeclarationEntry(CandidateDeclaration d, Scope currentScope) { + result = possibleScopesForDeclaration(d) and + not exists(Scope other | other = possibleScopesForDeclaration(d) | result = other.getAnAncestor()) and + currentScope.getADeclaration() = d and + result.getAnAncestor() = currentScope and + not result instanceof LoopOrSwitchBody and + not result.isGenerated() +} + +/** + * Gets a string suitable for printing a scope in an alert message, that includes an `$@` + * formatting string. + * + * This is necessary because some scopes (e.g. `Namespace`) do not have meaningful + * locations in the database and the alert message will not render the name if that is the case. + */ +string getScopeDescription(Scope s) { + if s instanceof GlobalNamespace then result = "the global namespace scope$@" else result = "$@" +} + +abstract class UnnecessaryExposedIdentifierDeclarationSharedSharedQuery extends Query { } + +Query getQuery() { result instanceof UnnecessaryExposedIdentifierDeclarationSharedSharedQuery } + +query predicate problems( + CandidateDeclaration d, string message, Scope currentScope, string msgP1, Scope candidateScope, + string msgP2 +) { + not isExcluded(d, getQuery()) and + candidateScope = bestScopeForDeclarationEntry(d, currentScope) and + // We can't reduce the scope if the value stored in the declaration is changed before the declared + // variable is used, because this would change the semantics of the use. + (d instanceof Variable implies not isTempVariable(d)) and + not exists(AddressOfExpr e | e.getAddressable() = d) and + // We can't reduce the scope of the declaration if its minimal scope resides inside a preprocessor + // branch directive while the current scope isn't. This can result in an incorrect program + // where a variable is used but not declared. + not exists(PreprocessorBranchDirective branch | + isStrictlyBefore(d, branch) and + branch = candidateScope.getAnEnclosingPreprocessorBranchDirective() + ) and + // We can't promote a class to a local class if it has static data members (See [class.local] paragraph 4 N3797.) + ( + (d instanceof Class and candidateScope.getStrictParent() instanceof Function) + implies + not exists(Variable member | d.(Class).getAMember() = member and member.isStatic()) + ) and + not candidateScope.isAffectedByMacro() and + msgP1 = "scope" and + msgP2 = "scope" and + message = + "The declaration " + d.getName() + " should be moved from " + getScopeDescription(currentScope) + + " into the " + getScopeDescription(candidateScope) + " too minimize its visibility." +} diff --git a/cpp/autosar/test/rules/M3-4-1/UnnecessaryExposedIdentifierDeclaration.expected b/cpp/common/test/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.expected similarity index 100% rename from cpp/autosar/test/rules/M3-4-1/UnnecessaryExposedIdentifierDeclaration.expected rename to cpp/common/test/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.expected diff --git a/cpp/common/test/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.ql b/cpp/common/test/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.ql new file mode 100644 index 0000000000..9914ad0b1e --- /dev/null +++ b/cpp/common/test/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.ql @@ -0,0 +1,2 @@ +// GENERATED FILE - DO NOT MODIFY +import codingstandards.cpp.rules.unnecessaryexposedidentifierdeclarationshared.UnnecessaryExposedIdentifierDeclarationShared diff --git a/cpp/autosar/test/rules/M3-4-1/test.cpp b/cpp/common/test/rules/unnecessaryexposedidentifierdeclarationshared/test.cpp similarity index 100% rename from cpp/autosar/test/rules/M3-4-1/test.cpp rename to cpp/common/test/rules/unnecessaryexposedidentifierdeclarationshared/test.cpp diff --git a/rule_packages/c/Declarations5.json b/rule_packages/c/Declarations5.json index 2d6eeee5c2..705f72791c 100644 --- a/rule_packages/c/Declarations5.json +++ b/rule_packages/c/Declarations5.json @@ -74,6 +74,26 @@ } ], "title": "The static storage class specifier shall be used in all declarations of objects and functions that have internal linkage" + }, + "RULE-8-9": { + "properties": { + "obligation": "advisory" + }, + "queries": [ + { + "description": "An identifier declared to be an object or type shall be defined in a block that minimizes its visibility to prevent any accidental use of the identifier.", + "kind": "problem", + "name": "An object should be defined at block scope if its identifier only appears in a single function", + "precision": "high", + "severity": "warning", + "short_name": "UnnecessaryExposedIdentifierDeclarationC", + "shared_implementation_short_name": "UnnecessaryExposedIdentifierDeclarationShared", + "tags": [ + "correctness" + ] + } + ], + "title": "An object should be defined at block scope if its identifier only appears in a single function" } } } \ No newline at end of file diff --git a/rule_packages/cpp/Scope.json b/rule_packages/cpp/Scope.json index 0d3abcbc0a..3b3c28067b 100644 --- a/rule_packages/cpp/Scope.json +++ b/rule_packages/cpp/Scope.json @@ -203,6 +203,7 @@ "precision": "high", "severity": "warning", "short_name": "UnnecessaryExposedIdentifierDeclaration", + "shared_implementation_short_name": "UnnecessaryExposedIdentifierDeclarationShared", "tags": [ "correctness" ] diff --git a/rules.csv b/rules.csv index a39e8610e2..d4ef15040c 100644 --- a/rules.csv +++ b/rules.csv @@ -654,7 +654,7 @@ c,MISRA-C-2012,RULE-8-5,Yes,Required,,,An external object or function shall be d c,MISRA-C-2012,RULE-8-6,Yes,Required,,,An identifier with external linkage shall have exactly one external definition,M3-2-4,Declarations,Import, c,MISRA-C-2012,RULE-8-7,Yes,Advisory,,,Functions and objects should not be defined with external linkage if they are referenced in only one translation unit,,Declarations,Medium, c,MISRA-C-2012,RULE-8-8,Yes,Required,,,The static storage class specifier shall be used in all declarations of objects and functions that have internal linkage,M3-3-2,Declarations5,Medium, -c,MISRA-C-2012,RULE-8-9,Yes,Advisory,,,An object should be defined at block scope if its identifier only appears in a single function,M3-4-1,Declarations,Medium, +c,MISRA-C-2012,RULE-8-9,Yes,Advisory,,,An object should be defined at block scope if its identifier only appears in a single function,M3-4-1,Declarations5,Medium, c,MISRA-C-2012,RULE-8-10,Yes,Required,,,An inline function shall be declared with the static storage class,,Declarations,Medium, c,MISRA-C-2012,RULE-8-11,Yes,Advisory,,,"When an array with external linkage is declared, its size should be explicitly specified",,Declarations,Medium, c,MISRA-C-2012,RULE-8-12,Yes,Required,,,"Within an enumerator list, the value of an implicitly-specified enumeration constant shall be unique",,Declarations,Medium, From 872ea183e492f63e625fb49d04eee5249ec8625e Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Fri, 16 Dec 2022 10:30:01 -0500 Subject: [PATCH 5/7] Declarations5: address review comments --- ...nalObjectOrFunctionNotDeclaredInOneFile.ql | 7 +----- ...singStaticSpecifierObjectRedeclarationC.ql | 6 ++--- c/misra/test/rules/RULE-5-2/test.c | 23 ++++++++++++++++++- ...icSpecifierFunctionRedeclarationShared.qll | 2 +- ...saryExposedIdentifierDeclarationShared.qll | 2 +- 5 files changed, 28 insertions(+), 12 deletions(-) diff --git a/c/misra/src/rules/RULE-8-5/ExternalObjectOrFunctionNotDeclaredInOneFile.ql b/c/misra/src/rules/RULE-8-5/ExternalObjectOrFunctionNotDeclaredInOneFile.ql index 0c9cfe9cb2..56e1d742a6 100644 --- a/c/misra/src/rules/RULE-8-5/ExternalObjectOrFunctionNotDeclaredInOneFile.ql +++ b/c/misra/src/rules/RULE-8-5/ExternalObjectOrFunctionNotDeclaredInOneFile.ql @@ -32,12 +32,7 @@ where kind = "variable" ) and // Apply an ordering based on location to enforce that (de1, de2) = (de2, de1) and we only report (de1, de2). - ( - de.getFile().getAbsolutePath() < otherDeclaration.getFile().getAbsolutePath() - or - de.getFile().getAbsolutePath() = otherDeclaration.getFile().getAbsolutePath() and - de.getLocation().getStartLine() < otherDeclaration.getLocation().getStartLine() - ) + de.getFile().getAbsolutePath() < otherDeclaration.getFile().getAbsolutePath() select de, "The " + kind + " declaration " + de.getName() + " is declared in multiple files and has an additional $@.", otherDeclaration, "declaration" diff --git a/c/misra/src/rules/RULE-8-8/MissingStaticSpecifierObjectRedeclarationC.ql b/c/misra/src/rules/RULE-8-8/MissingStaticSpecifierObjectRedeclarationC.ql index 9d583623de..2cb65c4fda 100644 --- a/c/misra/src/rules/RULE-8-8/MissingStaticSpecifierObjectRedeclarationC.ql +++ b/c/misra/src/rules/RULE-8-8/MissingStaticSpecifierObjectRedeclarationC.ql @@ -18,10 +18,10 @@ from VariableDeclarationEntry redeclaration, VariableDeclarationEntry de where not isExcluded(redeclaration, Declarations5Package::missingStaticSpecifierObjectRedeclarationCQuery()) and + //following implies de != redeclaration de.hasSpecifier("static") and - de.getDeclaration().isTopLevel() and - redeclaration.getDeclaration() = de.getDeclaration() and not redeclaration.hasSpecifier("static") and - de != redeclaration + de.getDeclaration().isTopLevel() and + redeclaration.getDeclaration() = de.getDeclaration() select redeclaration, "The redeclaration of $@ with internal linkage misses the static specifier.", de, de.getName() diff --git a/c/misra/test/rules/RULE-5-2/test.c b/c/misra/test/rules/RULE-5-2/test.c index 3f3f8f1e00..e299e514bc 100644 --- a/c/misra/test/rules/RULE-5-2/test.c +++ b/c/misra/test/rules/RULE-5-2/test.c @@ -23,4 +23,25 @@ void f() { static int iltiqzxgfqsgigwfuyntzghvzltueatcxqnqofnnvjy_C; // COMPLIANT length <63 static int - iltiqzxgfqsgigwfuyntzghvzltueatcxqnqofnnvjy_D; // COMPLIANT length <63 \ No newline at end of file + iltiqzxgfqsgigwfuyntzghvzltueatcxqnqofnnvjy_D; // COMPLIANT length <63 + +#define iltiqzxgfqsgigwfuyntzghvzltueatcxqnqofnnvjyszmcsylyohvqaosjbqyyA // COMPLIANT + // - + // this + // rule + // does + // not + // consider + // macros +extern int + iltiqzxgfqsgigwfuyntzghvzltueatcxqnqofnnvjyszmcsylyohvqaosjbqyyA; // COMPLIANT + // - this + // rule + // does + // not + // consider + // when + // both + // identifiers + // are + // external \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/rules/missingstaticspecifierfunctionredeclarationshared/MissingStaticSpecifierFunctionRedeclarationShared.qll b/cpp/common/src/codingstandards/cpp/rules/missingstaticspecifierfunctionredeclarationshared/MissingStaticSpecifierFunctionRedeclarationShared.qll index 70d498827f..43c1821e2e 100644 --- a/cpp/common/src/codingstandards/cpp/rules/missingstaticspecifierfunctionredeclarationshared/MissingStaticSpecifierFunctionRedeclarationShared.qll +++ b/cpp/common/src/codingstandards/cpp/rules/missingstaticspecifierfunctionredeclarationshared/MissingStaticSpecifierFunctionRedeclarationShared.qll @@ -1,5 +1,5 @@ /** - * Provides a library which includes a `problems` predicate for reporting.... + * Provides a library which includes a `problems` predicate for reporting missing static specifiers for redeclarations of functions with internal linkage. */ import cpp diff --git a/cpp/common/src/codingstandards/cpp/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.qll b/cpp/common/src/codingstandards/cpp/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.qll index 7006ef7b38..a18ab593bb 100644 --- a/cpp/common/src/codingstandards/cpp/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.qll +++ b/cpp/common/src/codingstandards/cpp/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.qll @@ -1,5 +1,5 @@ /** - * Provides a library which includes a `problems` predicate for reporting.... + * Provides a library which includes a `problems` predicate for reporting unnecessarily exposed identifiers due to too broad of a scope. */ import cpp From a08270e525fda638ffe0263fcba1645e1ac099e0 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Fri, 16 Dec 2022 11:49:12 -0500 Subject: [PATCH 6/7] Declarations5: finish adding shared rule for RULE-8-8 had previously missed commiting full shared --- ...singStaticSpecifierOnFunctionRedeclaration.ql | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/cpp/autosar/src/rules/M3-3-2/MissingStaticSpecifierOnFunctionRedeclaration.ql b/cpp/autosar/src/rules/M3-3-2/MissingStaticSpecifierOnFunctionRedeclaration.ql index 86823160bd..3904e267b6 100644 --- a/cpp/autosar/src/rules/M3-3-2/MissingStaticSpecifierOnFunctionRedeclaration.ql +++ b/cpp/autosar/src/rules/M3-3-2/MissingStaticSpecifierOnFunctionRedeclaration.ql @@ -15,14 +15,10 @@ import cpp import codingstandards.cpp.autosar +import codingstandards.cpp.rules.missingstaticspecifierfunctionredeclarationshared.MissingStaticSpecifierFunctionRedeclarationShared -from FunctionDeclarationEntry fde, FunctionDeclarationEntry redeclaration -where - not isExcluded(redeclaration) and - fde.hasSpecifier("static") and - fde.getDeclaration().isTopLevel() and - redeclaration.getDeclaration() = fde.getDeclaration() and - not redeclaration.hasSpecifier("static") and - fde != redeclaration -select redeclaration, "The redeclaration of $@ with internal linkage misses the static specifier.", - fde, "function" +class MissingStaticSpecifierOnFunctionRedeclarationQuery extends MissingStaticSpecifierFunctionRedeclarationSharedSharedQuery { + MissingStaticSpecifierOnFunctionRedeclarationQuery() { + this = ScopePackage::missingStaticSpecifierOnFunctionRedeclarationQuery() + } +} From 3b9c51d5bedbd4f15faf668539997d1967d3190b Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Tue, 17 Jan 2023 15:05:10 -0500 Subject: [PATCH 7/7] Declarations5: fix shared query structure now always duplicating testcases even if same --- ...issingStaticSpecifierFunctionRedeclarationShared.expected | 1 + .../MissingStaticSpecifierFunctionRedeclarationShared.ql | 2 ++ .../missingstaticspecifierfunctionredeclarationshared/test.c | 5 +++++ .../MissingStaticSpecifierFunctionRedeclarationC.testref | 2 +- 4 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 c/common/test/rules/missingstaticspecifierfunctionredeclarationshared/MissingStaticSpecifierFunctionRedeclarationShared.expected create mode 100644 c/common/test/rules/missingstaticspecifierfunctionredeclarationshared/MissingStaticSpecifierFunctionRedeclarationShared.ql create mode 100644 c/common/test/rules/missingstaticspecifierfunctionredeclarationshared/test.c diff --git a/c/common/test/rules/missingstaticspecifierfunctionredeclarationshared/MissingStaticSpecifierFunctionRedeclarationShared.expected b/c/common/test/rules/missingstaticspecifierfunctionredeclarationshared/MissingStaticSpecifierFunctionRedeclarationShared.expected new file mode 100644 index 0000000000..f6cde5d73b --- /dev/null +++ b/c/common/test/rules/missingstaticspecifierfunctionredeclarationshared/MissingStaticSpecifierFunctionRedeclarationShared.expected @@ -0,0 +1 @@ +| test.c:2:6:2:7 | definition of f1 | The redeclaration of $@ with internal linkage misses the static specifier. | test.c:1:13:1:14 | declaration of f1 | function | diff --git a/c/common/test/rules/missingstaticspecifierfunctionredeclarationshared/MissingStaticSpecifierFunctionRedeclarationShared.ql b/c/common/test/rules/missingstaticspecifierfunctionredeclarationshared/MissingStaticSpecifierFunctionRedeclarationShared.ql new file mode 100644 index 0000000000..50954b88bf --- /dev/null +++ b/c/common/test/rules/missingstaticspecifierfunctionredeclarationshared/MissingStaticSpecifierFunctionRedeclarationShared.ql @@ -0,0 +1,2 @@ +// GENERATED FILE - DO NOT MODIFY +import codingstandards.cpp.rules.missingstaticspecifierfunctionredeclarationshared.MissingStaticSpecifierFunctionRedeclarationShared diff --git a/c/common/test/rules/missingstaticspecifierfunctionredeclarationshared/test.c b/c/common/test/rules/missingstaticspecifierfunctionredeclarationshared/test.c new file mode 100644 index 0000000000..85e1aa467d --- /dev/null +++ b/c/common/test/rules/missingstaticspecifierfunctionredeclarationshared/test.c @@ -0,0 +1,5 @@ +static void f1(); +void f1() {} // NON_COMPLIANT + +static void f2(); +static void f2() {} // COMPLIANT \ No newline at end of file diff --git a/c/misra/test/rules/RULE-8-8/MissingStaticSpecifierFunctionRedeclarationC.testref b/c/misra/test/rules/RULE-8-8/MissingStaticSpecifierFunctionRedeclarationC.testref index 5b93ea365a..7d9f2ebc04 100644 --- a/c/misra/test/rules/RULE-8-8/MissingStaticSpecifierFunctionRedeclarationC.testref +++ b/c/misra/test/rules/RULE-8-8/MissingStaticSpecifierFunctionRedeclarationC.testref @@ -1 +1 @@ -cpp/common/test/rules/missingstaticspecifierfunctionredeclarationshared/MissingStaticSpecifierFunctionRedeclarationShared.ql \ No newline at end of file +c/common/test/rules/missingstaticspecifierfunctionredeclarationshared/MissingStaticSpecifierFunctionRedeclarationShared.ql \ No newline at end of file