Skip to content

Commit 130c264

Browse files
lcarteylcartey
authored
Merge pull request #719 from github/michaelrfairhurst/implement-function-types-package
Implement function types package
2 parents 228b671 + c972403 commit 130c264

9 files changed

Lines changed: 209 additions & 2 deletions

File tree

‎c/misra/src/rules/RULE-17-12/FunctionAddressesShouldAddressOperator.ql‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/**
2+
* @id c/misra/function-addresses-should-address-operator
3+
* @name RULE-17-12: A function identifier should only be called with a parenthesized parameter list or used with a &
4+
* @description A function identifier should only be called with a parenthesized parameter list or
5+
* used with a & (address-of).
6+
* @kind problem
7+
* @precision very-high
8+
* @problem.severity error
9+
* @tags external/misra/id/rule-17-12
10+
* readability
11+
* external/misra/c/2012/amendment3
12+
* external/misra/obligation/advisory
13+
*/
14+
15+
import cpp
16+
import codingstandards.c.misra
17+
18+
predicate isImplicitlyAddressed(FunctionAccess access) {
19+
not access.getParent() instanceof AddressOfExpr and
20+
// Note: the following *seems* to only exist in c++ codebases, for instance,
21+
// when calling a member. In c, this syntax should always extract as a
22+
// [FunctionCall] rather than a [ExprCall] of a [FunctionAccess]. Still, this
23+
// is a good pattern to be defensive against.
24+
not exists(ExprCall call | call.getExpr() = access)
25+
}
26+
27+
from FunctionAccess funcAccess
28+
where
29+
not isExcluded(funcAccess, FunctionTypesPackage::functionAddressesShouldAddressOperatorQuery()) and
30+
isImplicitlyAddressed(funcAccess)
31+
select funcAccess,
32+
"The address of function " + funcAccess.getTarget().getName() +
33+
" is taken without the & operator."

‎c/misra/test/rules/RULE-17-12/FunctionAddressesShouldAddressOperator.expected‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
| test.c:14:25:14:29 | func2 | The address of function func2 is taken without the & operator. |
2+
| test.c:15:25:15:29 | func3 | The address of function func3 is taken without the & operator. |
3+
| test.c:21:12:21:16 | func1 | The address of function func1 is taken without the & operator. |
4+
| test.c:38:3:38:7 | func1 | The address of function func1 is taken without the & operator. |
5+
| test.c:39:3:39:7 | func2 | The address of function func2 is taken without the & operator. |
6+
| test.c:57:13:57:17 | func1 | The address of function func1 is taken without the & operator. |
7+
| test.c:58:21:58:25 | func2 | The address of function func2 is taken without the & operator. |
8+
| test.c:59:13:59:17 | func1 | The address of function func1 is taken without the & operator. |
9+
| test.c:59:20:59:24 | func2 | The address of function func2 is taken without the & operator. |
10+
| test.c:67:11:67:15 | func1 | The address of function func1 is taken without the & operator. |
11+
| test.c:68:12:68:16 | func1 | The address of function func1 is taken without the & operator. |
12+
| test.c:69:12:69:16 | func1 | The address of function func1 is taken without the & operator. |
13+
| test.c:71:18:71:22 | func1 | The address of function func1 is taken without the & operator. |

‎c/misra/test/rules/RULE-17-12/FunctionAddressesShouldAddressOperator.qlref‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
rules/RULE-17-12/FunctionAddressesShouldAddressOperator.ql

‎c/misra/test/rules/RULE-17-12/test.c‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
void func1() {}
2+
void func2(int x, char *y) {}
3+
4+
typedef struct {
5+
} s;
6+
7+
int func3() { return 0; }
8+
9+
typedef void (*func_ptr_t1)();
10+
typedef void (*func_ptr_t2)(int x, char *y);
11+
typedef s (*func_ptr_t3)();
12+
13+
func_ptr_t1 func_ptr1 = &func1; // COMPLIANT
14+
func_ptr_t2 func_ptr2 = func2; // NON-COMPLIANT
15+
func_ptr_t3 func_ptr3 = func3 + 0; // NON-COMPLIANT
16+
17+
void take_func(func_ptr_t1 f1, func_ptr_t2 f2);
18+
19+
func_ptr_t1 returns_func(int x) {
20+
if (x == 0) {
21+
return func1; // NON-COMPLIANT
22+
} else if (x == 1) {
23+
return &func1; // COMPLIANT
24+
}
25+
26+
return returns_func(0); // COMPLIANT
27+
}
28+
29+
#define MACRO_IDENTITY(f) (f)
30+
#define MACRO_INVOKE_RISKY(f) (f())
31+
#define MACRO_INVOKE_IMPROVED(f) ((f)())
32+
#define MACRO_INVOKE_AND_USE_AS_TOKEN(f) f(0, #f)
33+
34+
void test() {
35+
func1(); // COMPLIANT
36+
func2(1, "hello"); // COMPLIANT
37+
38+
func1; // NON-COMPLIANT
39+
func2; // NON-COMPLIANT
40+
41+
&func1; // COMPLIANT
42+
&func2; // COMPLIANT
43+
44+
(func1)(); // COMPLIANT
45+
(func2)(1, "hello"); // COMPLIANT
46+
47+
&(func1); // COMPLIANT
48+
&(func2); // COMPLIANT
49+
50+
(&func1)(); // COMPLIANT
51+
(&func2)(1, "hello"); // COMPLIANT
52+
53+
(func1()); // COMPLIANT
54+
(func2(1, "hello")); // COMPLIANT
55+
56+
take_func(&func1, &func2); // COMPLIANT
57+
take_func(func1, &func2); // NON-COMPLIANT
58+
take_func(&func1, func2); // NON-COMPLIANT
59+
take_func(func1, func2); // NON-COMPLIANT
60+
61+
returns_func(0); // COMPLIANT
62+
returns_func(0)(); // COMPLIANT
63+
(returns_func(0))(); // COMPLIANT
64+
65+
(void *)&func1; // COMPLIANT
66+
(void *)(&func1); // COMPLIANT
67+
(void *)func1; // NON-COMPLIANT
68+
(void *)(func1); // NON-COMPLIANT
69+
((void *)func1); // NON-COMPLIANT
70+
71+
MACRO_IDENTITY(func1); // NON-COMPLIANT
72+
MACRO_IDENTITY(func1)(); // NON-COMPLIANT[FALSE NEGATIVE]
73+
MACRO_IDENTITY(&func1); // COMPLIANT
74+
MACRO_IDENTITY (&func1)(); // COMPLIANT
75+
76+
MACRO_INVOKE_RISKY(func3); // NON-COMPLIANT[FALSE NEGATIVE]
77+
MACRO_INVOKE_IMPROVED(func3); // NON-COMPLIANT[FALSE NEGATIVE]
78+
MACRO_INVOKE_IMPROVED(&func3); // COMPLIANT
79+
80+
MACRO_INVOKE_AND_USE_AS_TOKEN(func1); // COMPLIANT
81+
82+
// Function pointers are exempt from this rule.
83+
func_ptr1(); // COMPLIANT
84+
func_ptr2(1, "hello"); // COMPLIANT
85+
func_ptr1; // COMPLIANT
86+
func_ptr2; // COMPLIANT
87+
&func_ptr1; // COMPLIANT
88+
&func_ptr2; // COMPLIANT
89+
(func_ptr1)(); // COMPLIANT
90+
(func_ptr2)(1, "hello"); // COMPLIANT
91+
(*func_ptr1)(); // COMPLIANT
92+
(*func_ptr2)(1, "hello"); // COMPLIANT
93+
take_func(func_ptr1, func_ptr2); // COMPLIANT
94+
(void *)func_ptr1; // COMPLIANT
95+
(void *)&func_ptr1; // COMPLIANT
96+
(void *)(&func_ptr1); // COMPLIANT
97+
(void *)func_ptr1; // COMPLIANT
98+
(void *)(func_ptr1); // COMPLIANT
99+
((void *)func_ptr1); // COMPLIANT
100+
MACRO_IDENTITY(func_ptr1); // COMPLIANT
101+
MACRO_IDENTITY(func_ptr1)(); // COMPLIANT
102+
MACRO_IDENTITY(&func_ptr1); // COMPLIANT
103+
(*MACRO_IDENTITY(&func_ptr1))(); // COMPLIANT
104+
MACRO_INVOKE_RISKY(func_ptr3); // COMPLIANT
105+
MACRO_INVOKE_IMPROVED(func_ptr3); // COMPLIANT
106+
MACRO_INVOKE_IMPROVED(*&func_ptr3); // COMPLIANT
107+
}

‎cpp/common/src/codingstandards/cpp/exclusions/c/FunctionTypes.qll‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/
2+
import cpp
3+
import RuleMetadata
4+
import codingstandards.cpp.exclusions.RuleMetadata
5+
6+
newtype FunctionTypesQuery = TFunctionAddressesShouldAddressOperatorQuery()
7+
8+
predicate isFunctionTypesQueryMetadata(Query query, string queryId, string ruleId, string category) {
9+
query =
10+
// `Query` instance for the `functionAddressesShouldAddressOperator` query
11+
FunctionTypesPackage::functionAddressesShouldAddressOperatorQuery() and
12+
queryId =
13+
// `@id` for the `functionAddressesShouldAddressOperator` query
14+
"c/misra/function-addresses-should-address-operator" and
15+
ruleId = "RULE-17-12" and
16+
category = "advisory"
17+
}
18+
19+
module FunctionTypesPackage {
20+
Query functionAddressesShouldAddressOperatorQuery() {
21+
//autogenerate `Query` type
22+
result =
23+
// `Query` type for `functionAddressesShouldAddressOperator` query
24+
TQueryC(TFunctionTypesPackageQuery(TFunctionAddressesShouldAddressOperatorQuery()))
25+
}
26+
}

‎cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import Declarations8
2929
import EssentialTypes
3030
import Expressions
3131
import FloatingTypes
32+
import FunctionTypes
3233
import IO1
3334
import IO2
3435
import IO3
@@ -102,6 +103,7 @@ newtype TCQuery =
102103
TEssentialTypesPackageQuery(EssentialTypesQuery q) or
103104
TExpressionsPackageQuery(ExpressionsQuery q) or
104105
TFloatingTypesPackageQuery(FloatingTypesQuery q) or
106+
TFunctionTypesPackageQuery(FunctionTypesQuery q) or
105107
TIO1PackageQuery(IO1Query q) or
106108
TIO2PackageQuery(IO2Query q) or
107109
TIO3PackageQuery(IO3Query q) or
@@ -175,6 +177,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId, string cat
175177
isEssentialTypesQueryMetadata(query, queryId, ruleId, category) or
176178
isExpressionsQueryMetadata(query, queryId, ruleId, category) or
177179
isFloatingTypesQueryMetadata(query, queryId, ruleId, category) or
180+
isFunctionTypesQueryMetadata(query, queryId, ruleId, category) or
178181
isIO1QueryMetadata(query, queryId, ruleId, category) or
179182
isIO2QueryMetadata(query, queryId, ruleId, category) or
180183
isIO3QueryMetadata(query, queryId, ruleId, category) or

‎docs/user_manual.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ The datasheet _"CodeQL Coding Standards: supported rules"_, provided with each r
7878

7979
[^1]: AUTOSAR C++ versions R22-11, R21-11, R20-11, R19-11 and R19-03 are all identical as indicated in the document change history.
8080
[^2]: The unimplemented supportable AUTOSAR rules are `A7-1-8` and `A8-2-1`. These rules require additional support in the CodeQL CLI to ensure the required information is available in the CodeQL database to identify violations of these rules.
81-
[^3]: The unimplemented supportable MISRA C 2012 rules are `Rule 9.5` and `Dir 4.14`. `Rule 9.5` requires additional support in the CodeQL CLI to ensure the required information is available in the CodeQL database to identify violations of these rules. `Dir 4.14` is covered by the default CodeQL queries, which identify potential security vulnerabilities caused by not validating external input.
81+
[^3]: The unimplemented supportable MISRA C 2012 rules are `Rule 9.5`, `Rule 17.13`, and `Dir 4.14`. `Rule 9.5` and `Rule 17.13` require additional support in the CodeQL CLI to ensure the required information is available in the CodeQL database to identify violations of these rules. `Dir 4.14` is covered by the default CodeQL queries, which identify potential security vulnerabilities caused by not validating external input.
8282
[^4]: The rules 5.13.7, 19.0.1 and 19.1.2 are not planned to be implemented by CodeQL as they are compiler checked in all supported compilers.
8383

8484
## Supported environment

‎rule_packages/c/FunctionTypes.json‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
{
2+
"MISRA-C-2012": {
3+
"RULE-17-12": {
4+
"properties": {
5+
"obligation": "advisory"
6+
},
7+
"queries": [
8+
{
9+
"description": "A function identifier should only be called with a parenthesized parameter list or used with a & (address-of).",
10+
"kind": "problem",
11+
"name": "A function identifier should only be called with a parenthesized parameter list or used with a &",
12+
"precision": "very-high",
13+
"severity": "error",
14+
"short_name": "FunctionAddressesShouldAddressOperator",
15+
"tags": [
16+
"readability",
17+
"external/misra/c/2012/amendment3"
18+
]
19+
}
20+
],
21+
"title": "A function identifier should only be called with a parenthesized parameter list or used with a & (address-of)"
22+
}
23+
}
24+
}

‎rules.csv‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -739,7 +739,7 @@ c,MISRA-C-2012,RULE-17-9,Yes,Mandatory,,,Verify that a function declared with _N
739739
c,MISRA-C-2012,RULE-17-10,Yes,Required,,,A function declared with _noreturn shall have a return type of void,,NoReturn,Easy,
740740
c,MISRA-C-2012,RULE-17-11,Yes,Advisory,,,A function without a branch that returns shall be declared with _Noreturn,,NoReturn,Easy,
741741
c,MISRA-C-2012,RULE-17-12,Yes,Advisory,,,A function identifier should only be called with a parenthesized parameter list or used with a & (address-of),,FunctionTypes,Easy,
742-
c,MISRA-C-2012,RULE-17-13,Yes,Required,,,"A function type shall not include any type qualifiers (const, volatile, restrict, or _Atomic)",,FunctionTypes,Easy,
742+
c,MISRA-C-2012,RULE-17-13,No,Required,,,"A function type shall not include any type qualifiers (const, volatile, restrict, or _Atomic)",,,Easy,
743743
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,
744744
c,MISRA-C-2012,RULE-18-2,Yes,Required,,,Subtraction between pointers shall only be applied to pointers that address elements of the same array,M5-0-17,Pointers1,Import,
745745
c,MISRA-C-2012,RULE-18-3,Yes,Required,,,"The relational operators >, >=, < and <= shall not be applied to objects of pointer type except where they point into the same object",M5-0-18,Pointers1,Import,

0 commit comments

Comments
 (0)