Skip to content

Commit 9000deb

Browse files
authored
Merge branch 'main' into michaelrfairhurst/misra-c-rule-6-3-bit-fields-in-union
2 parents ba844d1 + a51fff7 commit 9000deb

14 files changed

+242
-38
lines changed

c/misra/src/rules/RULE-11-4/ConversionBetweenPointerToObjectAndIntegerType.ql

Lines changed: 64 additions & 5 deletions
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

c/misra/src/rules/RULE-11-6/CastBetweenPointerToVoidAndArithmeticType.ql

Lines changed: 1 addition & 1 deletion
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."

c/misra/src/rules/RULE-11-9/MacroNullNotUsedAsIntegerNullPointerConstant.ql

Lines changed: 14 additions & 4 deletions
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

c/misra/test/rules/RULE-11-1/ConversionBetweenFunctionPointerAndOtherType.expected

Lines changed: 0 additions & 1 deletion
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. |

c/misra/test/rules/RULE-11-1/test.c

Lines changed: 1 addition & 1 deletion
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;
Lines changed: 6 additions & 6 deletions
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 |

c/misra/test/rules/RULE-11-4/test.c

Lines changed: 14 additions & 2 deletions
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
}
Lines changed: 3 additions & 2 deletions
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 |

c/misra/test/rules/RULE-11-9/test.c

Lines changed: 12 additions & 5 deletions
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
}
Lines changed: 12 additions & 0 deletions
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.
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
- `M3-4-1` - `UnnecessaryExposedIdentifierDeclarationShared.qll`:
2+
- Fixes #665. Exclude variables that are constexpr and coming from template instantiations.

cpp/common/src/codingstandards/cpp/Pointers.qll

Lines changed: 3 additions & 6 deletions
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
}

cpp/common/src/codingstandards/cpp/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.qll

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,10 @@ private predicate isTypeUse(Type t1, Type t2) {
120120
}
121121

122122
newtype TDeclarationAccess =
123-
ObjectAccess(Variable v, VariableAccess va) { va = v.getAnAccess() } or
123+
ObjectAccess(Variable v, VariableAccess va) {
124+
va = v.getAnAccess() or
125+
v.(TemplateVariable).getAnInstantiation().getAnAccess() = va
126+
} or
124127
/* Type access can be done in a declaration or an expression (e.g., static member function call) */
125128
TypeAccess(Type t, Element access) {
126129
isTypeUse(access.(Variable).getUnspecifiedType(), t)
@@ -205,9 +208,13 @@ class DeclarationAccess extends TDeclarationAccess {
205208

206209
class CandidateDeclaration extends Declaration {
207210
CandidateDeclaration() {
208-
this instanceof LocalVariable
211+
this instanceof LocalVariable and
212+
not this.(LocalVariable).isConstexpr() and
213+
not this.isFromTemplateInstantiation(_)
209214
or
210-
this instanceof GlobalOrNamespaceVariable
215+
this instanceof GlobalOrNamespaceVariable and
216+
not this.isFromTemplateInstantiation(_) and
217+
not this.(GlobalOrNamespaceVariable).isConstexpr()
211218
or
212219
this instanceof Type and
213220
not this instanceof ClassTemplateInstantiation and
@@ -229,7 +236,13 @@ Scope possibleScopesForDeclaration(CandidateDeclaration d) {
229236
result = scope.getStrictParent*()
230237
) and
231238
// Limit the best scope to block statements and namespaces or control structures
232-
(result instanceof BlockStmt or result instanceof Namespace)
239+
(
240+
result instanceof BlockStmt and
241+
// Template variables cannot be in block scope
242+
not d instanceof TemplateVariable
243+
or
244+
result instanceof Namespace
245+
)
233246
}
234247

235248
/* Gets the smallest scope that includes all the declaration accesses of declaration `d`. */

cpp/common/test/rules/unnecessaryexposedidentifierdeclarationshared/test.cpp

Lines changed: 93 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,4 +136,96 @@ void f17() {
136136
ptr = &i;
137137
}
138138
*ptr = 1;
139-
}
139+
}
140+
141+
namespace a_namespace {
142+
143+
constexpr static unsigned int a_constexpr_var{
144+
10U}; // COMPLIANT; used in
145+
// a_namespace and
146+
// another_namespace_function
147+
static unsigned int
148+
a_namespace_var[a_constexpr_var]{}; // COMPLIANT; used in
149+
// a_namespace_function and
150+
// another_namespace_function
151+
152+
constexpr static unsigned int a_namespace_function(void) noexcept {
153+
unsigned int a_return_value{0U};
154+
155+
for (auto loop_var : a_namespace_var) { // usage of a_namespace_var
156+
a_return_value += loop_var;
157+
}
158+
return a_return_value;
159+
}
160+
161+
constexpr static unsigned int another_namespace_function(void) noexcept {
162+
unsigned int a_return_value{0U};
163+
164+
for (unsigned int i{0U}; i < a_constexpr_var;
165+
i++) { // usage of a_constexpr_var
166+
a_return_value += a_namespace_var[i]; // usage of a_namespace_var
167+
}
168+
return a_return_value;
169+
}
170+
} // namespace a_namespace
171+
172+
namespace parent_namespace {
173+
namespace child_namespace {
174+
template <typename From> class a_class_in_child_namespace {
175+
public:
176+
template <typename To> constexpr auto &&operator()(To &&val) const noexcept {
177+
return static_cast<To>(val);
178+
}
179+
}; // a_class_in_child_namespace end
180+
181+
template <typename From>
182+
extern constexpr a_class_in_child_namespace<From>
183+
a_class_in_child_namespace_impl{};
184+
185+
} // namespace child_namespace
186+
187+
template <typename From>
188+
static constexpr auto const &a_parent_namespace_variable =
189+
child_namespace::a_class_in_child_namespace_impl<
190+
From>; // COMPLIANT; used in child_namespace2::a_class::bar() and
191+
// parent_namespace::another_class::foo()
192+
193+
namespace child_namespace2 {
194+
class a_class {
195+
public:
196+
int func(...) { return 0; }
197+
void foo(int x) { x++; }
198+
template <typename F> constexpr auto bar(F(*func), int b) {
199+
foo(func(a_parent_namespace_variable<F>(
200+
b))); // usage of a_parent_namespace_variable
201+
}
202+
}; // a_class
203+
} // namespace child_namespace2
204+
205+
class another_class {
206+
int a;
207+
int b;
208+
void bar(int param) { param++; }
209+
210+
bool has_value() { return a == b; }
211+
212+
public:
213+
template <typename F> int foo(F(*func), int b) {
214+
if (has_value()) {
215+
bar(func(a_parent_namespace_variable<F>(
216+
b))); // usage of a_parent_namespace_variable
217+
}
218+
return 0;
219+
}
220+
}; // another_class
221+
} // namespace parent_namespace
222+
223+
template <typename T> T a_func(T v) { return v++; }
224+
225+
int main() {
226+
parent_namespace::child_namespace2::a_class a_class_obj;
227+
a_class_obj.bar(a_func<int>, 10);
228+
parent_namespace::another_class another_class_obj;
229+
another_class_obj.foo(a_func<int>, 10);
230+
return 0;
231+
}

0 commit comments

Comments
 (0)