Skip to content

Commit 65fba17

Browse files
knewbury01knewbury01
authored
Merge pull request #534 from rvermeulen/rvermeulen/fix-388
Fix FP reported in #388
2 parents ec4247b + ed673a9 commit 65fba17

File tree

5 files changed

+133
-26
lines changed

5 files changed

+133
-26
lines changed

‎change_notes/2024-02-14-fix-fp-m0-1-4.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
- `M0-1-4` - `SingleUseMemberPODVariable.ql`:
2+
- Address FP reported in #388. Include aggregrate initialization as a use of a member.
3+
- Include indirect initialization of members. For example, casting a pointer to a buffer to a struct pointer.
4+
- Reformat the alert message to adhere to the style-guide.

‎cpp/autosar/src/rules/M0-1-4/SingleUseMemberPODVariable.ql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,5 +24,5 @@ where
2424
not isExcluded(v, DeadCodePackage::singleUseMemberPODVariableQuery()) and
2525
isSingleUseNonVolatilePODVariable(v)
2626
select v,
27-
"Member POD variable " + v.getName() + " in " + v.getDeclaringType().getName() + " is only $@.",
28-
getSingleUse(v), "used once"
27+
"Member POD variable '" + v.getName() + "' in '" + v.getDeclaringType().getName() +
28+
"' is only $@.", getSingleUse(v), "used once"

‎cpp/autosar/src/rules/M0-1-4/SingleUsePODVariable.qll

Lines changed: 60 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,23 +10,68 @@ private string getConstExprValue(Variable v) {
1010
v.isConstexpr()
1111
}
1212

13+
/**
14+
* Gets the number of uses of variable `v` in an opaque assignment, where an opaqua assignment for example a cast from one type to the other and `v` is assumed to be a member of the resulting type.
15+
* e.g.,
16+
* struct foo {
17+
* int bar;
18+
* }
19+
*
20+
* struct foo * v = (struct foo*)buffer;
21+
*/
22+
Expr getIndirectSubObjectAssignedValue(MemberVariable subobject) {
23+
// struct foo * ptr = (struct foo*)buffer;
24+
exists(Struct someStruct, Variable instanceOfSomeStruct | someStruct.getAMember() = subobject |
25+
instanceOfSomeStruct.getType().(PointerType).getBaseType() = someStruct and
26+
exists(Cast assignedValue |
27+
// Exclude cases like struct foo * v = nullptr;
28+
not assignedValue.isImplicit() and
29+
// `v` is a subobject of another type that reinterprets another object. We count that as a use of `v`.
30+
assignedValue.getExpr() = instanceOfSomeStruct.getAnAssignedValue() and
31+
result = assignedValue
32+
)
33+
or
34+
// struct foo; read(..., (char *)&foo);
35+
instanceOfSomeStruct.getType() = someStruct and
36+
exists(Call externalInitializerCall, Cast castToCharPointer, int n |
37+
externalInitializerCall.getArgument(n).(AddressOfExpr).getOperand() =
38+
instanceOfSomeStruct.getAnAccess() and
39+
externalInitializerCall.getArgument(n) = castToCharPointer.getExpr() and
40+
castToCharPointer.getType().(PointerType).getBaseType().getUnspecifiedType() instanceof
41+
CharType and
42+
result = externalInitializerCall
43+
)
44+
or
45+
// the object this subject is part of is initialized and we assumes this initializes the subobject.
46+
instanceOfSomeStruct.getType() = someStruct and
47+
result = instanceOfSomeStruct.getInitializer().getExpr()
48+
)
49+
}
50+
1351
/** Gets a "use" count according to rule M0-1-4. */
1452
int getUseCount(Variable v) {
15-
exists(int initializers |
16-
// We enforce that it's a POD type variable, so if it has an initializer it is explicit
17-
(if v.hasInitializer() then initializers = 1 else initializers = 0) and
18-
result =
19-
initializers +
20-
count(VariableAccess access | access = v.getAnAccess() and not access.isCompilerGenerated())
21-
+ count(UserProvidedConstructorFieldInit cfi | cfi.getTarget() = v) +
22-
// For constexpr variables used as template arguments, we don't see accesses (just the
23-
// appropriate literals). We therefore take a conservative approach and count the number of
24-
// template instantiations that use the given constant, and consider each one to be a use
25-
// of the variable
26-
count(ClassTemplateInstantiation cti |
27-
cti.getTemplateArgument(_).(Expr).getValue() = getConstExprValue(v)
28-
)
29-
)
53+
// We enforce that it's a POD type variable, so if it has an initializer it is explicit
54+
result =
55+
count(getAUserInitializedValue(v)) +
56+
count(VariableAccess access | access = v.getAnAccess() and not access.isCompilerGenerated()) +
57+
// For constexpr variables used as template arguments, we don't see accesses (just the
58+
// appropriate literals). We therefore take a conservative approach and count the number of
59+
// template instantiations that use the given constant, and consider each one to be a use
60+
// of the variable
61+
count(ClassTemplateInstantiation cti |
62+
cti.getTemplateArgument(_).(Expr).getValue() = getConstExprValue(v)
63+
) + count(getIndirectSubObjectAssignedValue(v))
64+
}
65+
66+
Expr getAUserInitializedValue(Variable v) {
67+
(
68+
result = v.getInitializer().getExpr()
69+
or
70+
exists(UserProvidedConstructorFieldInit cfi | cfi.getTarget() = v and result = cfi.getExpr())
71+
or
72+
exists(ClassAggregateLiteral l | not l.isCompilerGenerated() | result = l.getAFieldExpr(v))
73+
) and
74+
not result.isCompilerGenerated()
3075
}
3176

3277
/** Gets a single use of `v`, if `isSingleUseNonVolatilePODVariable` holds. */

‎cpp/autosar/test/rules/M0-1-4/SingleUseMemberPODVariable.expected

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
| test_global_or_namespace.cpp:16:7:16:7 | x | Member POD variable x in GA is only $@. | test_global_or_namespace.cpp:38:6:38:6 | x | used once |
2-
| test_global_or_namespace.cpp:54:7:54:7 | x | Member POD variable x in N1A is only $@. | test_global_or_namespace.cpp:76:6:76:6 | x | used once |
3-
| test_member.cpp:5:7:5:8 | m2 | Member POD variable m2 in A is only $@. | test_member.cpp:9:21:9:25 | constructor init of field m2 | used once |
4-
| test_member.cpp:6:7:6:8 | m3 | Member POD variable m3 in A is only $@. | test_member.cpp:10:23:10:24 | m3 | used once |
5-
| test_member.cpp:7:7:7:8 | m4 | Member POD variable m4 in A is only $@. | test_member.cpp:14:23:14:24 | m4 | used once |
6-
| test_member.cpp:18:9:18:11 | sm1 | Member POD variable sm1 in s1 is only $@. | test_member.cpp:23:6:23:8 | sm1 | used once |
7-
| test_member.cpp:36:7:36:8 | m1 | Member POD variable m1 in C is only $@. | test_member.cpp:39:21:39:22 | m1 | used once |
8-
| test_member.cpp:37:7:37:8 | m2 | Member POD variable m2 in C is only $@. | test_member.cpp:46:5:46:6 | m2 | used once |
9-
| test_member.cpp:55:5:55:6 | m3 | Member POD variable m3 in E<B> is only $@. | test_member.cpp:56:27:56:32 | constructor init of field m3 | used once |
1+
| test_global_or_namespace.cpp:16:7:16:7 | x | Member POD variable 'x' in 'GA' is only $@. | test_global_or_namespace.cpp:38:6:38:6 | x | used once |
2+
| test_global_or_namespace.cpp:54:7:54:7 | x | Member POD variable 'x' in 'N1A' is only $@. | test_global_or_namespace.cpp:76:6:76:6 | x | used once |
3+
| test_member.cpp:5:7:5:8 | m2 | Member POD variable 'm2' in 'A' is only $@. | test_member.cpp:9:21:9:25 | constructor init of field m2 | used once |
4+
| test_member.cpp:6:7:6:8 | m3 | Member POD variable 'm3' in 'A' is only $@. | test_member.cpp:10:23:10:24 | m3 | used once |
5+
| test_member.cpp:7:7:7:8 | m4 | Member POD variable 'm4' in 'A' is only $@. | test_member.cpp:14:23:14:24 | m4 | used once |
6+
| test_member.cpp:18:9:18:11 | sm1 | Member POD variable 'sm1' in 's1' is only $@. | test_member.cpp:23:6:23:8 | sm1 | used once |
7+
| test_member.cpp:36:7:36:8 | m1 | Member POD variable 'm1' in 'C' is only $@. | test_member.cpp:39:21:39:22 | m1 | used once |
8+
| test_member.cpp:37:7:37:8 | m2 | Member POD variable 'm2' in 'C' is only $@. | test_member.cpp:46:5:46:6 | m2 | used once |
9+
| test_member.cpp:55:5:55:6 | m3 | Member POD variable 'm3' in 'E<B>' is only $@. | test_member.cpp:56:27:56:32 | constructor init of field m3 | used once |

‎cpp/autosar/test/rules/M0-1-4/test_member.cpp

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,4 +72,62 @@ void test_e() { // Ensure that the template E is fully instantiated
7272
e2.getT();
7373
}
7474

75+
void test_fp_reported_in_388() {
76+
struct s1 {
77+
int m1; // COMPLIANT
78+
};
79+
80+
s1 l1 = {1}; // m1 is used here
81+
l1.m1;
82+
}
83+
84+
void test_array_initialized_members() {
85+
struct s1 {
86+
int m1; // COMPLIANT
87+
};
88+
89+
struct s1 l1[] = {
90+
{.m1 = 1},
91+
{.m1 = 2},
92+
};
93+
94+
l1[0].m1;
95+
}
96+
97+
void test_indirect_assigned_members(void *opaque) {
98+
struct s1 {
99+
int m1; // COMPLIANT
100+
};
101+
102+
struct s1 *p = (struct s1 *)opaque;
103+
p->m1;
104+
105+
struct s2 {
106+
int m1; // COMPLIANT
107+
};
108+
109+
char buffer[sizeof(struct s2) + 8] = {0};
110+
struct s2 *l2 = (struct s2 *)&buffer[8];
111+
l2->m1;
112+
}
113+
114+
void test_external_assigned_members(void (*fp)(unsigned char *)) {
115+
116+
struct s1 {
117+
int m1; // COMPLIANT
118+
};
119+
120+
struct s1 l1;
121+
fp((unsigned char *)&l1);
122+
l1.m1;
123+
124+
struct s2 {
125+
int m1; // COMPLIANT
126+
};
127+
128+
struct s2 (*copy_init)();
129+
struct s2 l2 = copy_init();
130+
l2.m1;
131+
}
132+
75133
} // namespace test

0 commit comments

Comments
 (0)