diff --git a/.vscode/tasks.json b/.vscode/tasks.json index 97bc7c4800..3393c5da8b 100644 --- a/.vscode/tasks.json +++ b/.vscode/tasks.json @@ -208,6 +208,7 @@ "Declarations2", "Declarations3", "Declarations4", + "Declarations5", "Exceptions1", "Exceptions2", "Expressions", 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/cpp/autosar/test/rules/M3-3-2/test.cpp b/c/common/test/rules/missingstaticspecifierfunctionredeclarationshared/test.c similarity index 100% rename from cpp/autosar/test/rules/M3-3-2/test.cpp rename to c/common/test/rules/missingstaticspecifierfunctionredeclarationshared/test.c 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-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/src/rules/RULE-8-5/ExternalObjectOrFunctionNotDeclaredInOneFile.ql b/c/misra/src/rules/RULE-8-5/ExternalObjectOrFunctionNotDeclaredInOneFile.ql new file mode 100644 index 0000000000..56e1d742a6 --- /dev/null +++ b/c/misra/src/rules/RULE-8-5/ExternalObjectOrFunctionNotDeclaredInOneFile.ql @@ -0,0 +1,38 @@ +/** + * @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() +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/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..2cb65c4fda --- /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 + //following implies de != redeclaration + de.hasSpecifier("static") and + not redeclaration.hasSpecifier("static") and + 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/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-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..e299e514bc --- /dev/null +++ b/c/misra/test/rules/RULE-5-2/test.c @@ -0,0 +1,47 @@ +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 + +#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/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/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..7d9f2ebc04 --- /dev/null +++ b/c/misra/test/rules/RULE-8-8/MissingStaticSpecifierFunctionRedeclarationC.testref @@ -0,0 +1 @@ +c/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/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-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() + } +} 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-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/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 new file mode 100644 index 0000000000..b1fe8eacb6 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/exclusions/c/Declarations5.qll @@ -0,0 +1,95 @@ +//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/ +import cpp +import RuleMetadata +import codingstandards.cpp.exclusions.RuleMetadata + +newtype Declarations5Query = + TIdentifiersDeclaredInTheSameScopeNotDistinctQuery() or + TExternalObjectOrFunctionNotDeclaredInOneFileQuery() or + TMissingStaticSpecifierFunctionRedeclarationCQuery() or + TMissingStaticSpecifierObjectRedeclarationCQuery() or + TUnnecessaryExposedIdentifierDeclarationCQuery() + +predicate isDeclarations5QueryMetadata(Query query, string queryId, string ruleId, string category) { + 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" and + category = "required" + or + 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" and + category = "required" + 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" and + category = "required" + 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" and + category = "required" + 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" and + category = "advisory" +} + +module Declarations5Package { + Query identifiersDeclaredInTheSameScopeNotDistinctQuery() { + //autogenerate `Query` type + result = + // `Query` type for `identifiersDeclaredInTheSameScopeNotDistinct` query + TQueryC(TDeclarations5PackageQuery(TIdentifiersDeclaredInTheSameScopeNotDistinctQuery())) + } + + Query externalObjectOrFunctionNotDeclaredInOneFileQuery() { + //autogenerate `Query` type + result = + // `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())) + } + + Query unnecessaryExposedIdentifierDeclarationCQuery() { + //autogenerate `Query` type + result = + // `Query` type for `unnecessaryExposedIdentifierDeclarationC` query + TQueryC(TDeclarations5PackageQuery(TUnnecessaryExposedIdentifierDeclarationCQuery())) + } +} diff --git a/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll b/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll index 8e51523c74..df2c692358 100644 --- a/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll +++ b/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll @@ -18,6 +18,7 @@ import Declarations1 import Declarations2 import Declarations3 import Declarations4 +import Declarations5 import Expressions import IO1 import IO2 @@ -59,6 +60,7 @@ newtype TCQuery = TDeclarations2PackageQuery(Declarations2Query q) or TDeclarations3PackageQuery(Declarations3Query q) or TDeclarations4PackageQuery(Declarations4Query q) or + TDeclarations5PackageQuery(Declarations5Query q) or TExpressionsPackageQuery(ExpressionsQuery q) or TIO1PackageQuery(IO1Query q) or TIO2PackageQuery(IO2Query q) or @@ -100,6 +102,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId, string cat isDeclarations2QueryMetadata(query, queryId, ruleId, category) or isDeclarations3QueryMetadata(query, queryId, ruleId, category) or isDeclarations4QueryMetadata(query, queryId, ruleId, category) or + isDeclarations5QueryMetadata(query, queryId, ruleId, category) or isExpressionsQueryMetadata(query, queryId, ruleId, category) or isIO1QueryMetadata(query, queryId, ruleId, category) or isIO2QueryMetadata(query, queryId, ruleId, category) or 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..43c1821e2e --- /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 missing static specifiers for redeclarations of functions with internal linkage. + */ + +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/common/src/codingstandards/cpp/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.qll b/cpp/common/src/codingstandards/cpp/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.qll new file mode 100644 index 0000000000..a18ab593bb --- /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 unnecessarily exposed identifiers due to too broad of a scope. + */ + +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-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/common/test/rules/missingstaticspecifierfunctionredeclarationshared/test.cpp b/cpp/common/test/rules/missingstaticspecifierfunctionredeclarationshared/test.cpp new file mode 100644 index 0000000000..85e1aa467d --- /dev/null +++ b/cpp/common/test/rules/missingstaticspecifierfunctionredeclarationshared/test.cpp @@ -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/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 new file mode 100644 index 0000000000..705f72791c --- /dev/null +++ b/rule_packages/c/Declarations5.json @@ -0,0 +1,99 @@ +{ + "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" + }, + "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" + }, + "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" + }, + "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 6db01eba25..30b92a0675 100644 --- a/rule_packages/cpp/Scope.json +++ b/rule_packages/cpp/Scope.json @@ -180,6 +180,7 @@ "precision": "very-high", "severity": "warning", "short_name": "MissingStaticSpecifierOnFunctionRedeclaration", + "shared_implementation_short_name": "MissingStaticSpecifierFunctionRedeclarationShared", "tags": [ "readability" ] @@ -203,6 +204,7 @@ "precision": "high", "severity": "warning", "short_name": "UnnecessaryExposedIdentifierDeclaration", + "shared_implementation_short_name": "UnnecessaryExposedIdentifierDeclarationShared", "tags": [ "correctness" ] diff --git a/rules.csv b/rules.csv index 551e8f6fb5..6d2f11ba57 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, @@ -650,11 +650,11 @@ 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,,Declarations4,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,Declarations4,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,,Declarations4,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,Declarations4,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-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-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,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,