Skip to content

Commit 36ec640

Browse files
Implement Rule 0-0-1, unreachable statements from DeadCode package
1 parent 6c50c4c commit 36ec640

File tree

13 files changed

+496
-17
lines changed

13 files changed

+496
-17
lines changed
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
- `M0-1-1`, `RULE-2-1` - `UnreachableCode.ql`:
2+
- Updated detection of compiler generated code to include "handler" blocks, part of EDG's IR.
3+
- "handler" blocks generated for `catch(...)` blocks are not excluded for technical reasons related to how the CFG is constructed.
4+
- `M15-3-6`, `ERR54-CPP` - `CatchBlockShadowingMisra.ql`, `CathcBlockShadowingCert.ql`:
5+
- Altered semantics to detect shadowing for a catch block involving type `T` preceding another catch block involving the same type `T`, such as `catch(T&)` shadowing `catch(T)` and vice versa. Previously, the involved types had to have a subtype relationship.
6+
- Refactored catch block shadowing into a shared library for use in `RULE-0-0-1`.
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import cpp
2+
3+
CatchBlock getEarlierCatchBlock(CatchBlock cb) {
4+
exists(TryStmt try, int i, int j |
5+
cb = try.getCatchClause(j) and
6+
i < j and
7+
result = try.getCatchClause(i)
8+
)
9+
}
10+
11+
/**
12+
* Get the body of a catch block, ie, the block of statements executed when the catch block is
13+
* entered.
14+
*
15+
* This is useful, for instance, for CFG traversal
16+
*/
17+
Stmt getCatchBody(CatchBlock cb) { result = cb.getChild(0) }

cpp/common/src/codingstandards/cpp/deadcode/UnreachableCode.qll

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,16 @@ predicate isCompilerGenerated(BasicBlock b) {
88
b.(Stmt).isCompilerGenerated()
99
or
1010
b.(Expr).isCompilerGenerated()
11+
or
12+
// Catch blocks come with a generated 'Handler' that owns the basic block of the catch body,
13+
// however, `isCompilerGenerated` does not hold these generated handlers.
14+
//
15+
// We must also handle the case of `CatchAnyBlock`s. Unlike other catch blocks, for these the
16+
// handler dominates the non-generated basic block, so in our CFG the `Handler` node is the entity
17+
// used in the CFG rather than the basic block itself. Therefore we must not exclude these
18+
// `BasicBlock`s.
19+
b instanceof Handler and
20+
not b.(Handler).getBlock() instanceof CatchAnyBlock
1121
}
1222

1323
/**
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import cpp
2+
import codingstandards.cpp.ast.Catch
3+
import codingstandards.cpp.exceptions.ExceptionFlow
4+
5+
/**
6+
* A catch block which is shadowed by an earlier catch block for a base type.
7+
*
8+
* For example:
9+
*
10+
* ```cpp
11+
* class Base { };
12+
* class Derived : public Base { };
13+
*
14+
* try {
15+
* // ...
16+
* } catch (const Base& b) { // This catch block shadows the next one
17+
* // Handle Base
18+
* } catch (const Derived& d) { // This catch block is shadowed
19+
* // Handle Derived
20+
* }
21+
* ```
22+
*/
23+
class ShadowedCatchBlock extends CatchBlock {
24+
CatchBlock cbBase;
25+
Class baseType;
26+
Class derivedType;
27+
28+
ShadowedCatchBlock() {
29+
cbBase = getEarlierCatchBlock(this) and
30+
baseType = simplifyHandlerType(cbBase.getParameter().getType()) and
31+
derivedType = simplifyHandlerType(this.getParameter().getType()) and
32+
baseType.getADerivedClass*() = derivedType
33+
}
34+
35+
/**
36+
* Get the earlier catch block which shadows this catch block.
37+
*/
38+
CatchBlock getShadowingBlock() { result = cbBase }
39+
40+
/**
41+
* Get the type of this catch block's derived class whose catch block is shadowed by an earlier
42+
* catch block.
43+
*/
44+
Class getShadowedType() { result = derivedType }
45+
46+
/**
47+
* Get the type of the base class whose catch block precedes, and thus shadows, this catch block.
48+
*/
49+
Class getShadowingType() { result = baseType }
50+
}
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/
2+
import cpp
3+
import RuleMetadata
4+
import codingstandards.cpp.exclusions.RuleMetadata
5+
6+
newtype DeadCode3Query = TUnreachableStatementQuery()
7+
8+
predicate isDeadCode3QueryMetadata(Query query, string queryId, string ruleId, string category) {
9+
query =
10+
// `Query` instance for the `unreachableStatement` query
11+
DeadCode3Package::unreachableStatementQuery() and
12+
queryId =
13+
// `@id` for the `unreachableStatement` query
14+
"cpp/misra/unreachable-statement" and
15+
ruleId = "RULE-0-0-1" and
16+
category = "required"
17+
}
18+
19+
module DeadCode3Package {
20+
Query unreachableStatementQuery() {
21+
//autogenerate `Query` type
22+
result =
23+
// `Query` type for `unreachableStatement` query
24+
TQueryCPP(TDeadCode3PackageQuery(TUnreachableStatementQuery()))
25+
}
26+
}

cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import Const
1616
import Conversions
1717
import Conversions2
1818
import DeadCode
19+
import DeadCode3
1920
import Declarations
2021
import ExceptionSafety
2122
import Exceptions1
@@ -78,6 +79,7 @@ newtype TCPPQuery =
7879
TConversionsPackageQuery(ConversionsQuery q) or
7980
TConversions2PackageQuery(Conversions2Query q) or
8081
TDeadCodePackageQuery(DeadCodeQuery q) or
82+
TDeadCode3PackageQuery(DeadCode3Query q) or
8183
TDeclarationsPackageQuery(DeclarationsQuery q) or
8284
TExceptionSafetyPackageQuery(ExceptionSafetyQuery q) or
8385
TExceptions1PackageQuery(Exceptions1Query q) or
@@ -140,6 +142,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId, string cat
140142
isConversionsQueryMetadata(query, queryId, ruleId, category) or
141143
isConversions2QueryMetadata(query, queryId, ruleId, category) or
142144
isDeadCodeQueryMetadata(query, queryId, ruleId, category) or
145+
isDeadCode3QueryMetadata(query, queryId, ruleId, category) or
143146
isDeclarationsQueryMetadata(query, queryId, ruleId, category) or
144147
isExceptionSafetyQueryMetadata(query, queryId, ruleId, category) or
145148
isExceptions1QueryMetadata(query, queryId, ruleId, category) or

cpp/common/src/codingstandards/cpp/rules/catchblockshadowing/CatchBlockShadowing.qll

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,27 +6,19 @@ import cpp
66
import codingstandards.cpp.Customizations
77
import codingstandards.cpp.Exclusions
88
import codingstandards.cpp.exceptions.ExceptionFlow
9+
import codingstandards.cpp.exceptions.Shadowing
910

1011
abstract class CatchBlockShadowingSharedQuery extends Query { }
1112

1213
Query getQuery() { result instanceof CatchBlockShadowingSharedQuery }
1314

1415
query predicate problems(
15-
CatchBlock cbDerived, string message, CatchBlock cbBase, string cbBaseDescription
16+
ShadowedCatchBlock cbDerived, string message, CatchBlock cbBase, string cbBaseDescription
1617
) {
17-
exists(TryStmt try, Class baseType, Class derivedType |
18-
not isExcluded(cbDerived, getQuery()) and
19-
exists(int i, int j |
20-
cbBase = try.getCatchClause(i) and
21-
cbDerived = try.getCatchClause(j) and
22-
i < j
23-
) and
24-
baseType = simplifyHandlerType(cbBase.getParameter().getType()) and
25-
derivedType = simplifyHandlerType(cbDerived.getParameter().getType()) and
26-
baseType.getADerivedClass+() = derivedType and
27-
message =
28-
"Catch block for derived type " + derivedType +
29-
" is shadowed by $@ earlier catch block for base type " + baseType + "." and
30-
cbBaseDescription = "this"
31-
)
18+
not isExcluded(cbDerived, getQuery()) and
19+
cbBase = cbDerived.getShadowingBlock() and
20+
message =
21+
"Catch block for derived type " + cbDerived.getShadowedType() +
22+
" is shadowed by $@ earlier catch block for base type " + cbDerived.getShadowingType() + "." and
23+
cbBaseDescription = "this"
3224
}
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
/**
2+
* @id cpp/misra/unreachable-statement
3+
* @name RULE-0-0-1: A function shall not contain unreachable statements
4+
* @description Dead code can indicate a logic error, potentially introduced during code edits, or
5+
* it may be unnecessary code that can be deleted.
6+
* @kind problem
7+
* @precision very-high
8+
* @problem.severity error
9+
* @tags external/misra/id/rule-0-0-1
10+
* maintainability
11+
* scope/single-translation-unit
12+
* external/misra/enforcement/decidable
13+
* external/misra/obligation/required
14+
*/
15+
16+
import cpp
17+
import codingstandards.cpp.misra
18+
import codingstandards.cpp.deadcode.UnreachableCode
19+
import codingstandards.cpp.exceptions.ExceptionFlow
20+
import codingstandards.cpp.exceptions.ExceptionSpecifications
21+
import codingstandards.cpp.exceptions.Shadowing
22+
23+
/**
24+
* MISRA C++ defines its own notion of unreachable statements, which is similar to, but distinct
25+
* from, the general concept of unreachable code.
26+
*
27+
* This is not a superset of `BasicBlock.isReachable()`, because that includes all catch blocks.
28+
* However, it is a superset of the transitive closure of blocks reachable from function entry via
29+
* `getASuccessor`.
30+
*
31+
* The superset relationship can be read below, with extra reachable cases added for `&&`, `||`,
32+
* `?:`, and `constexpr if`, and catch blocks that aren't shadowed by prior catch blocks.
33+
*/
34+
predicate isReachable(BasicBlock bb) {
35+
bb = any(Function f).getEntryPoint()
36+
or
37+
isReachable(bb.getAPredecessor())
38+
or
39+
exists(BinaryLogicalOperation op |
40+
isReachable(op.getBasicBlock()) and
41+
bb = op.getAnOperand().getBasicBlock()
42+
)
43+
or
44+
exists(ConditionalExpr cond |
45+
isReachable(cond.getBasicBlock()) and
46+
bb = [cond.getThen(), cond.getElse()].getBasicBlock()
47+
)
48+
or
49+
exists(FunctionCall call, TryStmt try, CatchBlock cb |
50+
isReachable(call.getBasicBlock()) and
51+
not isNoExceptTrue(call.getTarget()) and
52+
try = getNearestTry(call.getEnclosingStmt()) and
53+
cb = try.getACatchClause() and
54+
not cb instanceof ShadowedCatchBlock and
55+
bb = cb.getBasicBlock()
56+
)
57+
or
58+
exists(ConstexprIfStmt ifStmt |
59+
isReachable(ifStmt.getBasicBlock()) and
60+
bb = [ifStmt.getThen(), ifStmt.getElse()].getBasicBlock()
61+
)
62+
}
63+
64+
from BasicBlock bb
65+
where
66+
not isExcluded(bb, DeadCode3Package::unreachableStatementQuery()) and
67+
not isReachable(bb) and
68+
not isCompilerGenerated(bb) and
69+
not affectedByMacro(bb)
70+
select bb, "Unreachable statement in function '$@'.", bb.getEnclosingFunction(),
71+
bb.getEnclosingFunction().getName()
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
| test.cpp:7:3:7:12 | declaration | Unreachable statement in function '$@'. | test.cpp:5:6:5:7 | f2 | f2 |
2+
| test.cpp:18:14:19:14 | { ... } | Unreachable statement in function '$@'. | test.cpp:10:6:10:7 | f3 | f3 |
3+
| test.cpp:26:10:27:14 | { ... } | Unreachable statement in function '$@'. | test.cpp:10:6:10:7 | f3 | f3 |
4+
| test.cpp:43:5:43:14 | declaration | Unreachable statement in function '$@'. | test.cpp:40:6:40:7 | f4 | f4 |
5+
| test.cpp:52:5:52:14 | declaration | Unreachable statement in function '$@'. | test.cpp:40:6:40:7 | f4 | f4 |
6+
| test.cpp:63:3:63:12 | declaration | Unreachable statement in function '$@'. | test.cpp:40:6:40:7 | f4 | f4 |
7+
| test.cpp:70:10:71:14 | { ... } | Unreachable statement in function '$@'. | test.cpp:66:6:66:7 | f5 | f5 |
8+
| test.cpp:75:11:76:14 | { ... } | Unreachable statement in function '$@'. | test.cpp:66:6:66:7 | f5 | f5 |
9+
| test.cpp:89:10:90:14 | { ... } | Unreachable statement in function '$@'. | test.cpp:88:24:88:24 | f6 | f6 |
10+
| test.cpp:92:10:93:14 | { ... } | Unreachable statement in function '$@'. | test.cpp:88:24:88:24 | f6 | f6 |
11+
| test.cpp:114:5:114:14 | declaration | Unreachable statement in function '$@'. | test.cpp:109:6:109:7 | f8 | f8 |
12+
| test.cpp:119:5:119:14 | declaration | Unreachable statement in function '$@'. | test.cpp:109:6:109:7 | f8 | f8 |
13+
| test.cpp:131:3:131:12 | declaration | Unreachable statement in function '$@'. | test.cpp:109:6:109:7 | f8 | f8 |
14+
| test.cpp:135:18:136:14 | { ... } | Unreachable statement in function '$@'. | test.cpp:134:6:134:7 | f9 | f9 |
15+
| test.cpp:146:5:146:14 | declaration | Unreachable statement in function '$@'. | test.cpp:134:6:134:7 | f9 | f9 |
16+
| test.cpp:158:3:134:7 | declaration | Unreachable statement in function '$@'. | test.cpp:134:6:134:7 | f9 | f9 |
17+
| test.cpp:162:17:163:14 | { ... } | Unreachable statement in function '$@'. | test.cpp:161:6:161:8 | f10 | f10 |
18+
| test.cpp:173:5:173:14 | declaration | Unreachable statement in function '$@'. | test.cpp:161:6:161:8 | f10 | f10 |
19+
| test.cpp:185:3:161:8 | declaration | Unreachable statement in function '$@'. | test.cpp:161:6:161:8 | f10 | f10 |
20+
| test.cpp:192:3:190:8 | declaration | Unreachable statement in function '$@'. | test.cpp:190:6:190:8 | f11 | f11 |
21+
| test.cpp:213:30:214:14 | { ... } | Unreachable statement in function '$@'. | test.cpp:200:6:200:8 | f12 | f12 |
22+
| test.cpp:215:27:216:14 | { ... } | Unreachable statement in function '$@'. | test.cpp:200:6:200:8 | f12 | f12 |
23+
| test.cpp:217:17:218:14 | <handler> | Unreachable statement in function '$@'. | test.cpp:200:6:200:8 | f12 | f12 |
24+
| test.cpp:225:30:226:14 | { ... } | Unreachable statement in function '$@'. | test.cpp:200:6:200:8 | f12 | f12 |
25+
| test.cpp:233:30:234:14 | { ... } | Unreachable statement in function '$@'. | test.cpp:200:6:200:8 | f12 | f12 |
26+
| test.cpp:235:24:236:14 | { ... } | Unreachable statement in function '$@'. | test.cpp:200:6:200:8 | f12 | f12 |
27+
| test.cpp:237:23:238:14 | { ... } | Unreachable statement in function '$@'. | test.cpp:200:6:200:8 | f12 | f12 |
28+
| test.cpp:246:19:247:16 | <handler> | Unreachable statement in function '$@'. | test.cpp:200:6:200:8 | f12 | f12 |
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
rules/RULE-0-0-1/UnreachableStatement.ql

0 commit comments

Comments
 (0)