From 11bc7ce50bf6a93c8d5928d4c299f56293969f2e Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Wed, 29 Mar 2023 23:47:17 +0100 Subject: [PATCH 1/2] Implement Rule 17.6 Adds a query to identify parameter array types which use the static keyword. Note: there is a CodeQL bug which means the static keyword is associated with the array size, not the specific parameter. --- .../src/rules/RULE-17-6/UseOfArrayStatic.ql | 21 +++++++++++++++ .../rules/RULE-17-6/UseOfArrayStatic.expected | 3 +++ .../rules/RULE-17-6/UseOfArrayStatic.qlref | 1 + c/misra/test/rules/RULE-17-6/test.c | 8 ++++++ .../cpp/exclusions/c/RuleMetadata.qll | 3 +++ .../cpp/exclusions/c/Static.qll | 26 +++++++++++++++++++ rule_packages/c/Static.json | 23 ++++++++++++++++ rules.csv | 2 +- 8 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 c/misra/src/rules/RULE-17-6/UseOfArrayStatic.ql create mode 100644 c/misra/test/rules/RULE-17-6/UseOfArrayStatic.expected create mode 100644 c/misra/test/rules/RULE-17-6/UseOfArrayStatic.qlref create mode 100644 c/misra/test/rules/RULE-17-6/test.c create mode 100644 cpp/common/src/codingstandards/cpp/exclusions/c/Static.qll create mode 100644 rule_packages/c/Static.json diff --git a/c/misra/src/rules/RULE-17-6/UseOfArrayStatic.ql b/c/misra/src/rules/RULE-17-6/UseOfArrayStatic.ql new file mode 100644 index 0000000000..876321c455 --- /dev/null +++ b/c/misra/src/rules/RULE-17-6/UseOfArrayStatic.ql @@ -0,0 +1,21 @@ +/** + * @id c/misra/use-of-array-static + * @name RULE-17-6: The declaration of an array parameter shall not contain the static keyword between the [ ] + * @description Using the static keyword in an array type is error prone, and relies on the + * programmer to adhere to the guarantees to avoid undefined behavior. + * @kind problem + * @precision very-high + * @problem.severity error + * @tags external/misra/id/rule-17-6 + * correctness + * external/misra/obligation/mandatory + */ + +import cpp +import codingstandards.c.misra + +from Parameter p +where + not isExcluded(p, StaticPackage::useOfArrayStaticQuery()) and + p.getType().(ArrayType).hasSpecifier("static") +select p, "Parameter " + p + " is declared as an array type using the static keyword." diff --git a/c/misra/test/rules/RULE-17-6/UseOfArrayStatic.expected b/c/misra/test/rules/RULE-17-6/UseOfArrayStatic.expected new file mode 100644 index 0000000000..ddf892a15c --- /dev/null +++ b/c/misra/test/rules/RULE-17-6/UseOfArrayStatic.expected @@ -0,0 +1,3 @@ +| test.c:2:33:2:36 | arr2 | Parameter arr2 is declared as an array type using the static keyword. | +| test.c:3:39:3:42 | arr3 | Parameter arr3 is declared as an array type using the static keyword. | +| test.c:5:9:5:12 | arr4 | Parameter arr4 is declared as an array type using the static keyword. | diff --git a/c/misra/test/rules/RULE-17-6/UseOfArrayStatic.qlref b/c/misra/test/rules/RULE-17-6/UseOfArrayStatic.qlref new file mode 100644 index 0000000000..ecb67b2dfb --- /dev/null +++ b/c/misra/test/rules/RULE-17-6/UseOfArrayStatic.qlref @@ -0,0 +1 @@ +rules/RULE-17-6/UseOfArrayStatic.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-17-6/test.c b/c/misra/test/rules/RULE-17-6/test.c new file mode 100644 index 0000000000..14f04b5a9f --- /dev/null +++ b/c/misra/test/rules/RULE-17-6/test.c @@ -0,0 +1,8 @@ +void test_array(int arr1[10]) {} // COMPLIANT +void test_array_uses_static(int arr2[static 11]) {} // NON_COMPLIANT +void test_array_uses_static_multi(int arr3[static 12][5]) {} // NON_COMPLIANT +void test_array_uses_static_again( + int arr4[11]) { // COMPLIANT[FALSE_POSITIVE] - apparently a CodeQL + // bug where the static is associated with the fixed + // size +} \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll b/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll index 58fd7b84cf..21cf4fcc9d 100644 --- a/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll +++ b/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll @@ -61,6 +61,7 @@ import Statements3 import Statements4 import Statements5 import Statements6 +import Static import Strings1 import Strings2 import Strings3 @@ -128,6 +129,7 @@ newtype TCQuery = TStatements4PackageQuery(Statements4Query q) or TStatements5PackageQuery(Statements5Query q) or TStatements6PackageQuery(Statements6Query q) or + TStaticPackageQuery(StaticQuery q) or TStrings1PackageQuery(Strings1Query q) or TStrings2PackageQuery(Strings2Query q) or TStrings3PackageQuery(Strings3Query q) or @@ -195,6 +197,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId, string cat isStatements4QueryMetadata(query, queryId, ruleId, category) or isStatements5QueryMetadata(query, queryId, ruleId, category) or isStatements6QueryMetadata(query, queryId, ruleId, category) or + isStaticQueryMetadata(query, queryId, ruleId, category) or isStrings1QueryMetadata(query, queryId, ruleId, category) or isStrings2QueryMetadata(query, queryId, ruleId, category) or isStrings3QueryMetadata(query, queryId, ruleId, category) or diff --git a/cpp/common/src/codingstandards/cpp/exclusions/c/Static.qll b/cpp/common/src/codingstandards/cpp/exclusions/c/Static.qll new file mode 100644 index 0000000000..92b07dd448 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/exclusions/c/Static.qll @@ -0,0 +1,26 @@ +//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/ +import cpp +import RuleMetadata +import codingstandards.cpp.exclusions.RuleMetadata + +newtype StaticQuery = TUseOfArrayStaticQuery() + +predicate isStaticQueryMetadata(Query query, string queryId, string ruleId, string category) { + query = + // `Query` instance for the `useOfArrayStatic` query + StaticPackage::useOfArrayStaticQuery() and + queryId = + // `@id` for the `useOfArrayStatic` query + "c/misra/use-of-array-static" and + ruleId = "RULE-17-6" and + category = "mandatory" +} + +module StaticPackage { + Query useOfArrayStaticQuery() { + //autogenerate `Query` type + result = + // `Query` type for `useOfArrayStatic` query + TQueryC(TStaticPackageQuery(TUseOfArrayStaticQuery())) + } +} diff --git a/rule_packages/c/Static.json b/rule_packages/c/Static.json new file mode 100644 index 0000000000..07f9240fa8 --- /dev/null +++ b/rule_packages/c/Static.json @@ -0,0 +1,23 @@ +{ + "MISRA-C-2012": { + "RULE-17-6": { + "properties": { + "obligation": "mandatory" + }, + "queries": [ + { + "description": "Using the static keyword in an array type is error prone, and relies on the programmer to adhere to the guarantees to avoid undefined behavior.", + "kind": "problem", + "name": "The declaration of an array parameter shall not contain the static keyword between the [ ]", + "precision": "very-high", + "severity": "error", + "short_name": "UseOfArrayStatic", + "tags": [ + "correctness" + ] + } + ], + "title": "The declaration of an array parameter shall not contain the static keyword between the [ ]" + } + } +} \ No newline at end of file diff --git a/rules.csv b/rules.csv index 7032f05284..e1ce57d6d5 100644 --- a/rules.csv +++ b/rules.csv @@ -716,7 +716,7 @@ c,MISRA-C-2012,RULE-17-2,Yes,Required,,,"Functions shall not call themselves, ei c,MISRA-C-2012,RULE-17-3,Yes,Mandatory,,,A function shall not be declared implicitly,,Declarations6,Medium, c,MISRA-C-2012,RULE-17-4,Yes,Mandatory,,,All exit paths from a function with non-void return type shall have an explicit return statement with an expression,MSC52-CPP,Statements5,Medium, c,MISRA-C-2012,RULE-17-5,Yes,Advisory,,,The function argument corresponding to a parameter declared to have an array type shall have an appropriate number of elements,,Contracts6,Hard, -c,MISRA-C-2012,RULE-17-6,No,Mandatory,,,The declaration of an array parameter shall not contain the static keyword between the [ ],,,, +c,MISRA-C-2012,RULE-17-6,Yes,Mandatory,,,The declaration of an array parameter shall not contain the static keyword between the [ ],,Static,Easy, c,MISRA-C-2012,RULE-17-7,Yes,Required,,,The value returned by a function having non-void return type shall be used,A0-1-2,Contracts6,Easy, c,MISRA-C-2012,RULE-17-8,Yes,Advisory,,,A function parameter should not be modified,,SideEffects2,Medium, c,MISRA-C-2012,RULE-18-1,Yes,Required,,,A pointer resulting from arithmetic on a pointer operand shall address an element of the same array as that pointer operand,M5-0-16,Pointers1,Import, From bd58c293352b553ce576accbd46714eeccb4cb1f Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Thu, 30 Mar 2023 00:04:37 +0100 Subject: [PATCH 2/2] Add implementation scope. --- rule_packages/c/Static.json | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/rule_packages/c/Static.json b/rule_packages/c/Static.json index 07f9240fa8..7edf903703 100644 --- a/rule_packages/c/Static.json +++ b/rule_packages/c/Static.json @@ -14,7 +14,10 @@ "short_name": "UseOfArrayStatic", "tags": [ "correctness" - ] + ], + "implementation_scope": { + "description": "The static keyword is associated with particular array types in our model. This means we can get false positives when two parameter use the same array type and size, but only one of which uses the `static` keyword." + } } ], "title": "The declaration of an array parameter shall not contain the static keyword between the [ ]"