Skip to content

Commit 06ba26d

Browse files
MichaelRFairhurstMichaelRFairhurst
committed
Address feedback
1 parent ab4a7bf commit 06ba26d

File tree

5 files changed

+92
-108
lines changed

5 files changed

+92
-108
lines changed

‎c/misra/src/rules/RULE-18-10/PointersToVariablyModifiedArrayTypesUsed.ql

Lines changed: 3 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -18,84 +18,11 @@ import cpp
1818
import codingstandards.c.misra
1919
import codingstandards.cpp.VariablyModifiedTypes
2020

21-
/**
22-
* Check that the declaration entry, which may be a parameter or a variable
23-
* etc., seems to subsume the location of `inner`, including the declaration
24-
* type text.
25-
*
26-
* The location of the `DeclarationEntry` itself points to the _identifier_
27-
* that is declared. This range will not include the type of the declaration.
28-
*
29-
* For parameters, the `before` and `end` `Location` objects will be
30-
* constrained to the closest earlier element (parameter or function body),
31-
* these values can therefore be captured and inspected for debugging.
32-
*
33-
* For declarations which occur in statements, the `before` and `end`
34-
* `Location` objects will be both constrained to be equal, and equal to,
35-
* the `Location` of the containing `DeclStmt`.
36-
*/
37-
predicate declarationSubsumes(
38-
DeclarationEntry entry, Location inner, Location before, Location after
39-
) {
40-
inner.getFile() = entry.getLocation().getFile() and
41-
(
42-
exists(ParameterDeclarationEntry param, FunctionDeclarationEntry func, int i |
43-
param = entry and
44-
func = param.getFunctionDeclarationEntry() and
45-
func.getParameterDeclarationEntry(i) = param and
46-
before = entry.getLocation() and
47-
(
48-
after = func.getParameterDeclarationEntry(i + 1).getLocation()
49-
or
50-
not exists(ParameterDeclarationEntry afterParam |
51-
afterParam = func.getParameterDeclarationEntry(i + 1)
52-
) and
53-
after = func.getBlock().getLocation()
54-
)
55-
) and
56-
before.isBefore(inner, _) and
57-
inner.isBefore(after, _)
58-
or
59-
exists(DeclStmt s |
60-
s.getADeclaration() = entry.getDeclaration() and
61-
before = s.getLocation() and
62-
after = before and
63-
before.subsumes(inner)
64-
)
65-
)
66-
}
67-
68-
/**
69-
* A declaration involving a pointer to a variably-modified type.
70-
*/
71-
class InvalidDeclaration extends DeclarationEntry {
72-
Expr sizeExpr;
73-
CandidateVlaType vlaType;
74-
// `before` and `after` are captured for debugging, see doc comment for
75-
// `declarationSubsumes`.
76-
Location before;
77-
Location after;
78-
79-
InvalidDeclaration() {
80-
sizeExpr = any(VlaDimensionStmt vla).getDimensionExpr() and
81-
declarationSubsumes(this, sizeExpr.getLocation(), before, after) and
82-
(
83-
if this instanceof ParameterDeclarationEntry
84-
then vlaType = this.getType().(VariablyModifiedTypeIfAdjusted).getInnerVlaType()
85-
else vlaType = this.getType().(VariablyModifiedTypeIfUnadjusted).getInnerVlaType()
86-
) and
87-
// Capture only pointers to VLA types, not raw VLA types.
88-
not vlaType = this.getType()
89-
}
90-
91-
Expr getSizeExpr() { result = sizeExpr }
92-
93-
CandidateVlaType getVlaType() { result = vlaType }
94-
}
95-
96-
from InvalidDeclaration v, string declstr, string adjuststr, string relationstr
21+
from VmtDeclarationEntry v, string declstr, string adjuststr, string relationstr
9722
where
9823
not isExcluded(v, InvalidMemory3Package::pointersToVariablyModifiedArrayTypesUsedQuery()) and
24+
// Capture only pointers to VLA types, not raw VLA types.
25+
not v.getVlaType() = v.getType() and
9926
(
10027
if v instanceof ParameterDeclarationEntry
10128
then declstr = "Parameter "

‎c/misra/src/rules/RULE-18-8/VariableLengthArrayTypesUsed.ql

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,11 @@ where
2020
not isExcluded(v, Declarations7Package::variableLengthArrayTypesUsedQuery()) and
2121
size = v.getVlaDimensionStmt(0).getDimensionExpr() and
2222
(
23-
arrayType = v.getVariable().getType()
23+
// Holds is if v is a variable declaration:
24+
arrayType = v.getVariable().getType().stripTopLevelSpecifiers()
2425
or
25-
arrayType = v.getType().getUnspecifiedType()
26+
// Holds is if v is a typedef declaration:
27+
arrayType = v.getType().stripTopLevelSpecifiers()
2628
) and
2729
typeStr = arrayType.getBaseType().toString()
2830
select v, "Variable length array of element type '" + typeStr + "' with non-constant size $@.",

‎c/misra/src/rules/RULE-18-9/ArrayToPointerConversionOfTemporaryObject.ql

Lines changed: 6 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -18,32 +18,20 @@ import codingstandards.c.misra
1818
import codingstandards.cpp.lifetimes.CLifetimes
1919

2020
/**
21-
* Get the expression(s) whose value is "used" by this expression.
21+
* Holds if the value of an expression is used or stored.
2222
*
2323
* For instance, `(x)` does not use any values, but `x + y` uses `x` and `y`.
2424
*
2525
* A pointer-to-array conversion does not need to be flagged if the result of
2626
* that conversion is not used or stored.
2727
*/
28-
Expr usedValuesOf(Expr expr) {
29-
result = expr.(BinaryOperation).getLeftOperand()
28+
predicate isUsedOrStored(Expr e) {
29+
e = any(Operation o).getAnOperand()
3030
or
31-
result = expr.(BinaryOperation).getRightOperand()
31+
e = any(ConditionalExpr c).getCondition()
3232
or
33-
result = expr.(UnaryOperation).getOperand()
33+
e = any(Call c).getAnArgument()
3434
or
35-
result = expr.(ConditionalExpr).getCondition()
36-
or
37-
result = expr.(Call).getAnArgument()
38-
}
39-
40-
/**
41-
* Get the expression(s) whose value is stored by this declaration.
42-
*
43-
* A pointer-to-array conversion does not need to be flagged if the result of
44-
* that conversion is not used or stored.
45-
*/
46-
predicate isStored(Expr e) {
4735
e = any(VariableDeclarationEntry d).getDeclaration().getInitializer().getExpr()
4836
or
4937
e = any(ClassAggregateLiteral l).getAFieldExpr(_)
@@ -77,10 +65,6 @@ where
7765
not isExcluded(conversion, InvalidMemory3Package::arrayToPointerConversionOfTemporaryObjectQuery()) and
7866
fa.getTemporary() = temporary and
7967
conversion.getExpr() = fa and
80-
(
81-
temporaryObjectFlowStep*(conversion.getExpr()) = usedValuesOf(any(Expr e))
82-
or
83-
isStored(temporaryObjectFlowStep*(conversion.getExpr()))
84-
)
68+
isUsedOrStored(temporaryObjectFlowStep*(conversion.getExpr()))
8569
select conversion, "Array to pointer conversion of array $@ from temporary object $@",
8670
fa.getTarget(), fa.getTarget().getName(), temporary, temporary.toString()

‎cpp/common/src/codingstandards/cpp/VariablyModifiedTypes.qll

Lines changed: 78 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,82 @@
11
import cpp
22

3+
/**
4+
* A declaration involving a variably-modified type.
5+
*/
6+
class VmtDeclarationEntry extends DeclarationEntry {
7+
Expr sizeExpr;
8+
CandidateVlaType vlaType;
9+
// `before` and `after` are captured for debugging, see doc comment for
10+
// `declarationSubsumes`.
11+
Location before;
12+
Location after;
13+
14+
VmtDeclarationEntry() {
15+
// Most of this library looks for candidate VLA types, by looking for arrays
16+
// without a size. These may or may not be VLA types. To confirm an a
17+
// candidate type is really a VLA type, we check that the location of the
18+
// declaration subsumes a `VlaDimensionStmt` which indicates a real VLA.
19+
sizeExpr = any(VlaDimensionStmt vla).getDimensionExpr() and
20+
declarationSubsumes(this, sizeExpr.getLocation(), before, after) and
21+
(
22+
if this instanceof ParameterDeclarationEntry
23+
then vlaType = this.getType().(VariablyModifiedTypeIfAdjusted).getInnerVlaType()
24+
else vlaType = this.getType().(VariablyModifiedTypeIfUnadjusted).getInnerVlaType()
25+
)
26+
}
27+
28+
Expr getSizeExpr() { result = sizeExpr }
29+
30+
CandidateVlaType getVlaType() { result = vlaType }
31+
}
32+
33+
/**
34+
* Check that the declaration entry, which may be a parameter or a variable
35+
* etc., seems to subsume the location of `inner`, including the declaration
36+
* type text.
37+
*
38+
* The location of the `DeclarationEntry` itself points to the _identifier_
39+
* that is declared. This range will not include the type of the declaration.
40+
*
41+
* For parameters, the `before` and `end` `Location` objects will be
42+
* constrained to the closest earlier element (parameter or function body),
43+
* these values can therefore be captured and inspected for debugging.
44+
*
45+
* For declarations which occur in statements, the `before` and `end`
46+
* `Location` objects will be both constrained to be equal, and equal to,
47+
* the `Location` of the containing `DeclStmt`.
48+
*/
49+
private predicate declarationSubsumes(
50+
DeclarationEntry entry, Location inner, Location before, Location after
51+
) {
52+
inner.getFile() = entry.getLocation().getFile() and
53+
(
54+
exists(ParameterDeclarationEntry param, FunctionDeclarationEntry func, int i |
55+
param = entry and
56+
func = param.getFunctionDeclarationEntry() and
57+
func.getParameterDeclarationEntry(i) = param and
58+
before = entry.getLocation() and
59+
(
60+
after = func.getParameterDeclarationEntry(i + 1).getLocation()
61+
or
62+
not exists(ParameterDeclarationEntry afterParam |
63+
afterParam = func.getParameterDeclarationEntry(i + 1)
64+
) and
65+
after = func.getBlock().getLocation()
66+
)
67+
) and
68+
before.isBefore(inner, _) and
69+
inner.isBefore(after, _)
70+
or
71+
exists(DeclStmt s |
72+
s.getADeclaration() = entry.getDeclaration() and
73+
before = s.getLocation() and
74+
after = before and
75+
before.subsumes(inner)
76+
)
77+
)
78+
}
79+
380
/**
481
* A candidate to be a variably length array type (VLA).
582
*
@@ -90,19 +167,13 @@ class NoAdjustmentVariablyModifiedType extends Type {
90167
NoAdjustmentVariablyModifiedType() {
91168
exists(Type innerType |
92169
(
93-
innerType = this.(PointerType).getBaseType()
94-
or
95-
innerType = this.(ArrayType).getBaseType()
170+
innerType = this.(DerivedType).getBaseType()
96171
or
97172
innerType = this.(RoutineType).getReturnType()
98173
or
99-
innerType = this.(RoutineType).getAParameterType()
100-
or
101174
innerType = this.(FunctionPointerType).getReturnType()
102175
or
103176
innerType = this.(TypedefType).getBaseType()
104-
or
105-
innerType = this.(SpecifiedType).getBaseType()
106177
) and
107178
vlaType = innerType.(VariablyModifiedTypeIfUnadjusted).getInnerVlaType()
108179
)

‎cpp/common/src/codingstandards/cpp/lifetimes/CLifetimes.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ class TemporaryLifetimeExpr extends Expr {
2525
getUnconverted().getUnspecifiedType() instanceof StructOrUnionTypeWithArrayField and
2626
not isCLValue(this)
2727
or
28-
this.(ArrayExpr).getArrayBase() instanceof TemporaryLifetimeArrayAccess
28+
this.getUnconverted().(ArrayExpr).getArrayBase() instanceof TemporaryLifetimeArrayAccess
2929
}
3030
}
3131

0 commit comments

Comments
 (0)