Skip to content

Commit a51fff7

Browse files
knewbury01knewbury01
authored
Merge pull request #332 from github/lcartey/rule-11-4-improvements
Rule 11.4 improvements
2 parents 9e73400 + 2c6baca commit a51fff7

11 files changed

+130
-33
lines changed

Diff for: ‎c/misra/src/rules/RULE-11-4/ConversionBetweenPointerToObjectAndIntegerType.ql

+64-5
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,73 @@
1313

1414
import cpp
1515
import codingstandards.c.misra
16+
import codingstandards.cpp.Macro
1617
import codingstandards.cpp.Pointers
1718

18-
from CStyleCast cast, Type typeFrom, Type typeTo
19+
MacroInvocation getAMacroInvocation(CStyleCast cast) { result.getAnExpandedElement() = cast }
20+
21+
Macro getPrimaryMacro(CStyleCast cast) {
22+
exists(MacroInvocation mi |
23+
mi = getAMacroInvocation(cast) and
24+
not exists(MacroInvocation otherMi |
25+
otherMi = getAMacroInvocation(cast) and otherMi.getParentInvocation() = mi
26+
) and
27+
result = mi.getMacro()
28+
)
29+
}
30+
31+
Macro getNonFunctionPrimaryMacro(CStyleCast cast) {
32+
result = getPrimaryMacro(cast) and
33+
not result instanceof FunctionLikeMacro
34+
}
35+
36+
from
37+
Locatable primaryLocation, CStyleCast cast, Type typeFrom, Type typeTo, string message,
38+
string extraMessage, Locatable optionalPlaceholderLocation, string optionalPlaceholderMessage
1939
where
2040
not isExcluded(cast, Pointers1Package::conversionBetweenPointerToObjectAndIntegerTypeQuery()) and
2141
typeFrom = cast.getExpr().getUnderlyingType() and
2242
typeTo = cast.getUnderlyingType() and
23-
[typeFrom, typeTo] instanceof IntegralType and
24-
[typeFrom, typeTo] instanceof PointerToObjectType and
25-
not isNullPointerConstant(cast.getExpr())
26-
select cast, "Cast performed between a pointer to object type and a pointer to an integer type."
43+
(
44+
typeFrom instanceof PointerToObjectType and
45+
typeTo instanceof IntegralType and
46+
message =
47+
"Cast from pointer to object type '" + typeFrom + "' to integer type '" + typeTo + "'" +
48+
extraMessage + "."
49+
or
50+
typeFrom instanceof IntegralType and
51+
typeTo instanceof PointerToObjectType and
52+
message =
53+
"Cast from integer type '" + typeFrom + "' to pointer to object type '" + typeTo + "'" +
54+
extraMessage + "."
55+
) and
56+
not isNullPointerConstant(cast.getExpr()) and
57+
// If this alert is arising through a non-function-like macro expansion, flag the macro instead, to
58+
// help make the alerts more manageable. We only do this for non-function-like macros because they
59+
// cannot be context specific.
60+
if exists(getNonFunctionPrimaryMacro(cast))
61+
then
62+
primaryLocation = getNonFunctionPrimaryMacro(cast) and
63+
extraMessage = "" and
64+
optionalPlaceholderLocation = primaryLocation and
65+
optionalPlaceholderMessage = ""
66+
else (
67+
primaryLocation = cast and
68+
// If the cast is in a macro expansion which is context specific, we still report the original
69+
// location, but also add a link to the most specific macro that contains the cast, to aid
70+
// validation.
71+
if exists(getPrimaryMacro(cast))
72+
then
73+
extraMessage = " from expansion of macro $@" and
74+
exists(Macro m |
75+
m = getPrimaryMacro(cast) and
76+
optionalPlaceholderLocation = m and
77+
optionalPlaceholderMessage = m.getName()
78+
)
79+
else (
80+
extraMessage = "" and
81+
optionalPlaceholderLocation = cast and
82+
optionalPlaceholderMessage = ""
83+
)
84+
)
85+
select primaryLocation, message, optionalPlaceholderLocation, optionalPlaceholderMessage

Diff for: ‎c/misra/src/rules/RULE-11-6/CastBetweenPointerToVoidAndArithmeticType.ql

+1-1
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,5 @@ where
2222
typeTo = cast.getUnderlyingType() and
2323
[typeFrom, typeTo] instanceof ArithmeticType and
2424
[typeFrom, typeTo] instanceof VoidPointerType and
25-
not isNullPointerConstant(cast.getExpr())
25+
not cast.getExpr() instanceof Zero
2626
select cast, "Cast performed between a pointer to void type and an arithmetic type."

Diff for: ‎c/misra/src/rules/RULE-11-9/MacroNullNotUsedAsIntegerNullPointerConstant.ql

+14-4
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,25 @@ import codingstandards.cpp.Type
1818
from Zero zero, Expr e, string type
1919
where
2020
not isExcluded(zero, Pointers1Package::macroNullNotUsedAsIntegerNullPointerConstantQuery()) and
21-
// exclude the base-case (NULL macros and void pointer casts)
22-
not isNullPointerConstant(zero) and
21+
// Exclude the base-case (NULL macros and void pointer casts)
22+
// Note: we cannot use the isNullPointerConstant predicate here because it permits
23+
// the use of `0` without casting, which is prohibited here.
24+
not (
25+
zero.findRootCause() instanceof NullMacro
26+
or
27+
// integer constant `0` explicitly cast to void pointer
28+
exists(Conversion c | c = zero.getConversion() |
29+
not c.isImplicit() and
30+
c.getUnderlyingType() instanceof VoidPointerType
31+
)
32+
) and
2333
(
2434
// ?: operator
2535
exists(ConditionalExpr parent |
2636
(
27-
parent.getThen().getAChild*() = zero and parent.getElse().getType() instanceof PointerType
37+
parent.getThen() = zero and parent.getElse().getType() instanceof PointerType
2838
or
29-
parent.getElse().getAChild*() = zero and parent.getThen().getType() instanceof PointerType
39+
parent.getElse() = zero and parent.getThen().getType() instanceof PointerType
3040
) and
3141
// exclude a common conditional pattern used in macros such as 'assert'
3242
not parent.isInMacroExpansion() and

Diff for: ‎c/misra/test/rules/RULE-11-1/ConversionBetweenFunctionPointerAndOtherType.expected

-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
| test.c:11:8:11:16 | (fp1 *)... | Cast performed between a function pointer and another type. |
22
| test.c:11:8:11:16 | (fp1)... | Cast performed between a function pointer and another type. |
33
| test.c:12:14:12:23 | (void *)... | Cast performed between a function pointer and another type. |
4-
| test.c:14:8:14:15 | (fp2)... | Cast performed between a function pointer and another type. |
54
| test.c:15:8:15:15 | (fp2)... | Cast performed between a function pointer and another type. |
65
| test.c:22:12:22:13 | (fp1)... | Cast performed between a function pointer and another type. |
76
| test.c:25:8:25:9 | (fp1)... | Cast performed between a function pointer and another type. |

Diff for: ‎c/misra/test/rules/RULE-11-1/test.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ void f1(void) {
1111
v1 = (fp1 *)v2; // NON_COMPLIANT
1212
void *v3 = (void *)v1; // NON_COMPLIANT
1313

14-
v2 = (fp2 *)0; // NON_COMPLIANT
14+
v2 = (fp2 *)0; // COMPLIANT - null pointer constant
1515
v2 = (fp2 *)1; // NON_COMPLIANT
1616

1717
pfp2 v4;

Diff for: ‎c/misra/test/rules/RULE-11-4/ConversionBetweenPointerToObjectAndIntegerType.expected

+6-6
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
| test.c:5:21:5:42 | (unsigned int)... | Cast performed between a pointer to object type and a pointer to an integer type. |
2-
| test.c:5:35:5:42 | (int *)... | Cast performed between a pointer to object type and a pointer to an integer type. |
3-
| test.c:6:21:6:37 | (unsigned int)... | Cast performed between a pointer to object type and a pointer to an integer type. |
4-
| test.c:8:8:8:24 | (unsigned int)... | Cast performed between a pointer to object type and a pointer to an integer type. |
5-
| test.c:10:22:10:22 | (unsigned int *)... | Cast performed between a pointer to object type and a pointer to an integer type. |
6-
| test.c:12:22:12:39 | (unsigned int *)... | Cast performed between a pointer to object type and a pointer to an integer type. |
1+
| test.c:6:21:6:37 | (unsigned int)... | Cast from pointer to object type 'unsigned int *' to integer type 'unsigned int'. | test.c:6:21:6:37 | (unsigned int)... | |
2+
| test.c:8:8:8:24 | (unsigned int)... | Cast from pointer to object type 'unsigned int *' to integer type 'unsigned int'. | test.c:8:8:8:24 | (unsigned int)... | |
3+
| test.c:12:22:12:39 | (unsigned int *)... | Cast from integer type 'unsigned int' to pointer to object type 'unsigned int *'. | test.c:12:22:12:39 | (unsigned int *)... | |
4+
| test.c:15:1:15:24 | #define FOO (int *)0x200 | Cast from integer type 'int' to pointer to object type 'int *'. | test.c:15:1:15:24 | #define FOO (int *)0x200 | |
5+
| test.c:23:3:23:22 | (int *)... | Cast from integer type 'int' to pointer to object type 'int *' from expansion of macro $@. | test.c:17:1:17:34 | #define FOO_FUNCTIONAL(x) (int *)x | FOO_FUNCTIONAL |
6+
| test.c:24:14:24:25 | (int *)... | Cast from integer type 'int' to pointer to object type 'int *' from expansion of macro $@. | test.c:18:1:18:23 | #define FOO_INSERT(x) x | FOO_INSERT |

Diff for: ‎c/misra/test/rules/RULE-11-4/test.c

+14-2
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,24 @@
22

33
void f1(void) {
44
unsigned int v1 = (unsigned int)(void *)0; // COMPLIANT
5-
unsigned int v2 = (unsigned int)(int *)0; // NON_COMPLIANT
5+
unsigned int v2 = (unsigned int)(int *)0; // COMPLIANT
66
unsigned int v3 = (unsigned int)&v2; // NON_COMPLIANT
77
v3 = v2; // COMPLIANT
88
v3 = (unsigned int)&v2; // NON_COMPLIANT
99
v3 = NULL; // COMPLIANT
10-
unsigned int *v4 = 0; // NON_COMPLIANT
10+
unsigned int *v4 = 0; // COMPLIANT
1111
unsigned int *v5 = NULL; // COMPLIANT
1212
unsigned int *v6 = (unsigned int *)v2; // NON_COMPLIANT
13+
}
14+
15+
#define FOO (int *)0x200 // NON_COMPLIANT
16+
#define FOO_WRAPPER FOO;
17+
#define FOO_FUNCTIONAL(x) (int *)x
18+
#define FOO_INSERT(x) x
19+
20+
void test_macros() {
21+
FOO; // Issue is reported at the macro
22+
FOO_WRAPPER; // Issue is reported at the macro
23+
FOO_FUNCTIONAL(0x200); // NON_COMPLIANT
24+
FOO_INSERT((int *)0x200); // NON_COMPLIANT
1325
}

Diff for: ‎c/misra/test/rules/RULE-11-9/MacroNullNotUsedAsIntegerNullPointerConstant.expected

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
| test.c:15:13:15:13 | 0 | $@ uses zero-value integer constant expression as null pointer constant. | test.c:15:7:15:13 | ... == ... | Equality operator |
22
| test.c:17:8:17:8 | 0 | $@ uses zero-value integer constant expression as null pointer constant. | test.c:17:3:17:8 | ... = ... | Assignment to pointer |
3-
| test.c:25:20:25:20 | 0 | $@ uses zero-value integer constant expression as null pointer constant. | test.c:25:3:25:35 | ... ? ... : ... | Ternary operator |
4-
| test.c:25:20:25:20 | 0 | $@ uses zero-value integer constant expression as null pointer constant. | test.c:25:15:25:20 | ... = ... | Assignment to pointer |
3+
| test.c:23:13:23:13 | 0 | $@ uses zero-value integer constant expression as null pointer constant. | test.c:23:3:23:13 | ... ? ... : ... | Ternary operator |
4+
| test.c:24:8:24:8 | 0 | $@ uses zero-value integer constant expression as null pointer constant. | test.c:24:3:24:13 | ... ? ... : ... | Ternary operator |
5+
| test.c:31:14:31:14 | 0 | $@ uses zero-value integer constant expression as null pointer constant. | test.c:31:9:31:14 | ... = ... | Assignment to pointer |

Diff for: ‎c/misra/test/rules/RULE-11-9/test.c

+12-5
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,16 @@ void *f1(void *p1, int p2) {
1919
p1 = NULL; // COMPLIANT
2020
if (p2 == 0) { // COMPLIANT
2121
return NULL;
22-
} // COMPLIANT
23-
(p1) ? (p1 = NULL) : (p1 = NULL); // COMPLIANT
24-
(p2 > 0) ? (p1 = NULL) : (p1 = NULL); // COMPLIANT
25-
(p2 > 0) ? (p1 = 0) : (p1 = NULL); // NON_COMPLIANT
26-
return 0; // COMPLIANT
22+
}
23+
p2 ? p1 : 0; // NON_COMPLIANT
24+
p2 ? 0 : p1; // NON_COMPLIANT
25+
p2 ? (void *)0 : p1; // COMPLIANT
26+
p2 ? p1 : (void *)0; // COMPLIANT
27+
p2 ? p2 : 0; // COMPLIANT - p2 is not a pointer type
28+
p2 ? 0 : p2; // COMPLIANT - p2 is not a pointer type
29+
int x;
30+
int *y;
31+
p2 ? (p1 = 0) : p1; // NON_COMPLIANT - p1 is a pointer type
32+
p2 ? (p2 = 0) : p1; // COMPLIANT - p2 is not a pointer type
33+
return 0; // COMPLIANT
2734
}

Diff for: ‎change_notes/2023-07-28-rule-11-4-improvements.md

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
- `RULE-11-1` - `ConversionBetweenFunctionPointerAndOtherType.ql`:
2+
- Fixed issue #331 - consider `0` a null pointer constant.
3+
- `RULE-11-4` - `ConversionBetweenPointerToObjectAndIntegerType.ql`:
4+
- Fixed issue #331 - consider `0` a null pointer constant.
5+
- Improve reporting of the order of the cast and the actual types involved.
6+
- Improve reporting where the result is expanded from a macro by either reporting the macro itself (if it is not dependent on the context) or by including a link to the macro in the alert message.
7+
- `RULE-11-5` - `ConversionFromPointerToVoidIntoPointerToObject.ql`:
8+
- Fixed issue #331 - consider `0` a null pointer constant.
9+
- `RULE-11-6` - `CastBetweenPointerToVoidAndArithmeticType.ql`:
10+
- Fixed issue #331 - accept integer constant expressions with value `0` instead of null pointer constants.
11+
- `RULE-11-9` - `MacroNullNotUsedAsIntegerNullPointerConstant.ql`:
12+
- Remove false positives in branches of ternary expressions, where `0` was used correctly.

Diff for: ‎cpp/common/src/codingstandards/cpp/Pointers.qll

+3-6
Original file line numberDiff line numberDiff line change
@@ -62,12 +62,9 @@ class ArrayPointerArithmeticExpr extends PointerArithmeticExpr, ArrayExpr {
6262
predicate isNullPointerConstant(Expr e) {
6363
e.findRootCause() instanceof NullMacro
6464
or
65-
exists(CStyleCast c |
66-
not c.isImplicit() and
67-
c.getExpr() = e and
68-
e instanceof Zero and
69-
c.getType() instanceof VoidPointerType
70-
)
65+
// 8.11 Pointer type conversions states:
66+
// A null pointer constant, i.e. the value 0, optionally cast to void *.
67+
e instanceof Zero
7168
or
7269
isNullPointerConstant(e.(Conversion).getExpr())
7370
}

0 commit comments

Comments
 (0)