Skip to content

Commit 6714fe7

Browse files
knewbury01knewbury01
authored
Merge pull request #622 from knewbury01/knewbury01/fix-376
M0-1-2: improve template handling
2 parents 90abc2d + 7697c69 commit 6714fe7

File tree

4 files changed

+224
-18
lines changed

4 files changed

+224
-18
lines changed

‎change_notes/2024-06-11-fix-fp-376-M-0-1-2.md

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
- `M0-1-2` - `InfeasiblePath.ql`:
2+
- Fixes #376. For template functions we now only report when a path is infeasible regardless of instantiations present.

‎cpp/autosar/src/rules/M0-1-2/InfeasiblePath.ql

+184-9
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
import cpp
1818
import codingstandards.cpp.autosar
1919
import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
20+
import codingstandards.cpp.deadcode.UnreachableCode
21+
import semmle.code.cpp.controlflow.Guards
2022

2123
/**
2224
* A "conditional" node in the control flow graph, i.e. one that can potentially have a true and false path.
@@ -44,6 +46,7 @@ class BreakingLoop extends Loop {
4446
predicate hasCFGDeducedInfeasiblePath(
4547
ConditionalControlFlowNode cond, boolean infeasiblePath, string explanation
4648
) {
49+
not cond.isFromTemplateInstantiation(_) and
4750
// No true successor, so the true path has already been deduced as infeasible
4851
not exists(cond.getATrueSuccessor()) and
4952
infeasiblePath = true and
@@ -147,17 +150,189 @@ predicate isConstantRelationalOperation(
147150
/**
148151
* Holds if the `ConditionalNode` has an infeasible `path` for the reason given in `explanation`.
149152
*/
150-
predicate hasInfeasiblePath(
151-
ConditionalControlFlowNode node, boolean infeasiblePath, string explanation
152-
) {
153-
hasCFGDeducedInfeasiblePath(node, infeasiblePath, explanation) and
154-
not isConstantRelationalOperation(node, infeasiblePath, _)
153+
predicate hasInfeasiblePath(ConditionalControlFlowNode node, string message) {
154+
//deal with the infeasible in all uninstantiated templates separately
155+
node.isFromUninstantiatedTemplate(_) and
156+
node instanceof ConditionControllingUnreachable and
157+
message = "The path is unreachable in a template."
155158
or
156-
isConstantRelationalOperation(node, infeasiblePath, explanation)
159+
exists(boolean infeasiblePath, string explanation |
160+
(
161+
not node.isFromUninstantiatedTemplate(_) and
162+
not node.isFromTemplateInstantiation(_) and
163+
message = "The " + infeasiblePath + " path is infeasible because " + explanation + "."
164+
) and
165+
(
166+
hasCFGDeducedInfeasiblePath(node, infeasiblePath, explanation) and
167+
not isConstantRelationalOperation(node, infeasiblePath, _)
168+
or
169+
isConstantRelationalOperation(node, infeasiblePath, explanation)
170+
)
171+
)
172+
}
173+
174+
/**
175+
* A newtype representing "unreachable" blocks in the program. We use a newtype here to avoid
176+
* reporting the same block in multiple `Function` instances created from one function in a template.
177+
*/
178+
private newtype TUnreachableBasicBlock =
179+
TUnreachableNonTemplateBlock(BasicBlock bb) {
180+
bb.isUnreachable() and
181+
// Exclude anything template related from this case
182+
not bb.getEnclosingFunction().isFromTemplateInstantiation(_) and
183+
not bb.getEnclosingFunction().isFromUninstantiatedTemplate(_) and
184+
// Exclude compiler generated basic blocks
185+
not isCompilerGenerated(bb)
186+
} or
187+
/**
188+
* A `BasicBlock` that occurs in at least one `Function` instance for a template. `BasicBlock`s
189+
* are matched up across templates by location.
190+
*/
191+
TUnreachableTemplateBlock(
192+
string filepath, int startline, int startcolumn, int endline, int endcolumn,
193+
GuardCondition uninstantiatedGuardCondition
194+
) {
195+
exists(BasicBlock bb |
196+
// BasicBlock occurs in this location
197+
bb.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) and
198+
// And is contained in the `uninstantiatedFunction` only
199+
// not from anything constructed from it
200+
// because we want infeasible paths independent of parameters
201+
exists(Function enclosing | enclosing = bb.getEnclosingFunction() |
202+
//guard is in the template function
203+
(
204+
enclosing.getBlock().getAChild*() = uninstantiatedGuardCondition and
205+
//function template
206+
enclosing.isFromUninstantiatedTemplate(_) and
207+
uninstantiatedGuardCondition.isFromUninstantiatedTemplate(_) and
208+
//true condition is unreachable: basic block starts on same line as guard
209+
(
210+
not exists(uninstantiatedGuardCondition.getATrueSuccessor()) and
211+
bb.hasLocationInfo(filepath, uninstantiatedGuardCondition.getLocation().getStartLine(),
212+
startcolumn, endline, endcolumn)
213+
or
214+
//false condition is unreachable: false basic block starts on one line after its true basic block
215+
not exists(uninstantiatedGuardCondition.getAFalseSuccessor()) and
216+
bb.hasLocationInfo(filepath,
217+
uninstantiatedGuardCondition.getATrueSuccessor().getLocation().getEndLine() + 1,
218+
startcolumn, endline, endcolumn)
219+
)
220+
)
221+
) and
222+
// And is unreachable
223+
bb.isUnreachable() and
224+
// //Exclude compiler generated control flow nodes
225+
not isCompilerGenerated(bb) and
226+
//Exclude nodes affected by macros, because our find-the-same-basic-block-by-location doesn't
227+
//work in that case
228+
not bb.(ControlFlowNode).isAffectedByMacro()
229+
)
230+
}
231+
232+
/**
233+
* An unreachable basic block.
234+
*/
235+
class UnreachableBasicBlock extends TUnreachableBasicBlock {
236+
/** Gets a `BasicBlock` which is represented by this set of unreachable basic blocks. */
237+
BasicBlock getABasicBlock() { none() }
238+
239+
/** Gets a `GuardCondition` instance which we treat as the original GuardCondition. */
240+
GuardCondition getGuardCondition() { none() }
241+
242+
predicate hasLocationInfo(
243+
string filepath, int startline, int startcolumn, int endline, int endcolumn
244+
) {
245+
none()
246+
}
247+
248+
string toString() { result = "default" }
249+
}
250+
251+
/**
252+
* A non-templated unreachable basic block.
253+
*/
254+
class UnreachableNonTemplateBlock extends UnreachableBasicBlock, TUnreachableNonTemplateBlock {
255+
BasicBlock getBasicBlock() { this = TUnreachableNonTemplateBlock(result) }
256+
257+
override BasicBlock getABasicBlock() { result = getBasicBlock() }
258+
259+
override GuardCondition getGuardCondition() { result.controls(getBasicBlock(), true) }
260+
261+
override predicate hasLocationInfo(
262+
string filepath, int startline, int startcolumn, int endline, int endcolumn
263+
) {
264+
getBasicBlock().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
265+
}
266+
267+
override string toString() { result = getBasicBlock().toString() }
268+
}
269+
270+
/**
271+
* A templated unreachable basic block.
272+
*/
273+
class UnreachableTemplateBlock extends UnreachableBasicBlock, TUnreachableTemplateBlock {
274+
override BasicBlock getABasicBlock() {
275+
exists(
276+
string filepath, int startline, int startcolumn, int endline, int endcolumn,
277+
GuardCondition uninstantiatedGuardCondition
278+
|
279+
this =
280+
TUnreachableTemplateBlock(filepath, startline, startcolumn, endline, endcolumn,
281+
uninstantiatedGuardCondition) and
282+
result.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) and
283+
exists(Function enclosing |
284+
//guard is in the template function
285+
(
286+
enclosing.getBlock().getAChild*() = uninstantiatedGuardCondition and
287+
//function template
288+
enclosing.isFromUninstantiatedTemplate(_) and
289+
uninstantiatedGuardCondition.isFromUninstantiatedTemplate(_) and
290+
//true condition is unreachable: basic block starts on same line as guard
291+
(
292+
not exists(uninstantiatedGuardCondition.getATrueSuccessor()) and
293+
this.hasLocationInfo(filepath,
294+
uninstantiatedGuardCondition.getLocation().getStartLine(), startcolumn, endline,
295+
endcolumn)
296+
or
297+
//false condition is unreachable: false basic block starts on one line after its true basic block
298+
not exists(uninstantiatedGuardCondition.getAFalseSuccessor()) and
299+
this.hasLocationInfo(filepath,
300+
uninstantiatedGuardCondition.getATrueSuccessor().getLocation().getEndLine() + 1,
301+
startcolumn, endline, endcolumn)
302+
)
303+
)
304+
)
305+
|
306+
result.isUnreachable() and
307+
// Exclude compiler generated control flow nodes
308+
not isCompilerGenerated(result) and
309+
// Exclude nodes affected by macros, because our find-the-same-basic-block-by-location doesn't
310+
// work in that case
311+
not result.(ControlFlowNode).isAffectedByMacro()
312+
)
313+
}
314+
315+
override GuardCondition getGuardCondition() {
316+
this = TUnreachableTemplateBlock(_, _, _, _, _, result)
317+
}
318+
319+
override predicate hasLocationInfo(
320+
string filepath, int startline, int startcolumn, int endline, int endcolumn
321+
) {
322+
this = TUnreachableTemplateBlock(filepath, startline, startcolumn, endline, endcolumn, _)
323+
}
324+
325+
override string toString() { result = getABasicBlock().toString() }
326+
}
327+
328+
class ConditionControllingUnreachable extends GuardCondition {
329+
ConditionControllingUnreachable() {
330+
exists(UnreachableTemplateBlock b | this = b.getGuardCondition())
331+
}
157332
}
158333

159-
from ConditionalControlFlowNode cond, boolean infeasiblePath, string explanation
334+
from ConditionalControlFlowNode cond, string explanation
160335
where
161336
not isExcluded(cond, DeadCodePackage::infeasiblePathQuery()) and
162-
hasInfeasiblePath(cond, infeasiblePath, explanation)
163-
select cond, "The " + infeasiblePath + " path is infeasible because " + explanation + "."
337+
hasInfeasiblePath(cond, explanation)
338+
select cond, explanation

‎cpp/autosar/test/rules/M0-1-2/InfeasiblePath.expected

+3-7
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,9 @@
22
| test.cpp:7:7:7:22 | ... <= ... | The false path is infeasible because a (max value: 4294967295) is always less than or equal to 4294967295 (minimum value: 4294967295). |
33
| test.cpp:15:7:15:13 | ... < ... | The false path is infeasible because l1 (max value: 2) is always less than l2 (minimum value: 10). |
44
| test.cpp:19:9:19:14 | ... < ... | The false path is infeasible because a (max value: 1) is always less than l2 (minimum value: 10). |
5-
| test.cpp:33:7:33:7 | 0 | The true path is infeasible because this expression consists of constants which evaluate to false. |
6-
| test.cpp:33:7:33:7 | 0 | The true path is infeasible because this expression consists of constants which evaluate to false. |
7-
| test.cpp:33:7:33:7 | 0 | The true path is infeasible because this expression consists of constants which evaluate to false. |
8-
| test.cpp:36:7:36:14 | call to isVal | The false path is infeasible because this expression consists of constants which evaluate to true. |
9-
| test.cpp:36:7:36:14 | call to isVal | The false path is infeasible because this expression consists of constants which evaluate to true. |
10-
| test.cpp:43:7:43:15 | call to isVal2 | The false path is infeasible because this expression consists of constants which evaluate to true. |
11-
| test.cpp:43:7:43:15 | call to isVal2 | The true path is infeasible because this expression consists of constants which evaluate to false. |
5+
| test.cpp:33:7:33:7 | 0 | The path is unreachable in a template. |
126
| test.cpp:77:9:77:14 | ... < ... | The true path is infeasible because 0 (max value: 0) is always less than or equal to a (minimum value: 0). |
137
| test.cpp:80:9:80:15 | ... >= ... | The false path is infeasible because 0 (max value: 0) is always less than or equal to a (minimum value: 0). |
148
| test.cpp:86:9:86:14 | ... < ... | The true path is infeasible because 0 (max value: 0) is always less than or equal to a (minimum value: 0). |
9+
| test.cpp:117:7:117:7 | 0 | The path is unreachable in a template. |
10+
| test.cpp:123:7:123:8 | ! ... | The path is unreachable in a template. |

‎cpp/autosar/test/rules/M0-1-2/test.cpp

+35-2
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,14 @@ template <class T> int f() {
3333
if (0) { // NON_COMPLIANT - true path is infeasible in all circumstances
3434
return 3;
3535
}
36-
if (T::isVal()) { // COMPLIANT[FALSE_POSITIVE] - `isVal` is `true` for all
36+
if (T::isVal()) { // COMPLIANT - `isVal` is `true` for all
3737
// visible instantiations, but in the uninstantiated
3838
// template both paths are feasible. This represents that
3939
// this is template dependent, so we consider it compliant
4040
return 2;
4141
}
4242

43-
if (T::isVal2()) { // COMPLIANT[FALSE_POSITIVE] - `isVal2` is either true or
43+
if (T::isVal2()) { // COMPLIANT - `isVal2` is either true or
4444
// false
4545
return 2;
4646
}
@@ -99,3 +99,36 @@ void test_loop(int a) {
9999
a++;
100100
}
101101
}
102+
103+
template <bool x> int foo() {
104+
if (x) { // COMPLIANT - block is reachable in the one of the instantiated
105+
// template
106+
return 1;
107+
}
108+
return 0; // COMPLIANT - block is reachable in the uninstantiated template
109+
}
110+
111+
void test() {
112+
foo<true>();
113+
foo<false>();
114+
}
115+
116+
template <class T> int template_infeasible_true_path() {
117+
if (0) { // NON_COMPLIANT - true path is infeasible in all circumstances
118+
return 3;
119+
}
120+
}
121+
122+
template <class T> int template_infeasible_false_path() {
123+
if (!0) {
124+
return 3;
125+
}
126+
return 1; // NON_COMPLIANT - false path is infeasible in all circumstances
127+
}
128+
129+
void test_infeasible_instantiates() {
130+
template_infeasible_true_path<A>();
131+
template_infeasible_true_path<B>();
132+
template_infeasible_false_path<A>();
133+
template_infeasible_false_path<B>();
134+
}

0 commit comments

Comments
 (0)