Skip to content

Commit 09c2f90

Browse files
geoffw0geoffw0
authored
Merge pull request github#17525 from geoffw0/unreachable
Rust: Unreachable code query
2 parents 6a87eb0 + 719cef8 commit 09c2f90

File tree

8 files changed

+314
-2
lines changed

8 files changed

+314
-2
lines changed

‎rust/ql/src/queries/unusedentities/UnreachableCode.qhelp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>This rule finds code that is never reached. Unused code should be removed to increase readability and avoid confusion.</p>
8+
</overview>
9+
10+
<recommendation>
11+
<p>Remove any unreachable code.</p>
12+
</recommendation>
13+
14+
<example>
15+
<p>In the following example, the final <code>return</code> statement can never be reached:</p>
16+
<sample src="UnreachableCodeBad.rs" />
17+
<p>The problem can be fixed simply by removing the unreachable code:</p>
18+
<sample src="UnreachableCodeGood.rs" />
19+
</example>
20+
21+
<references>
22+
<li>Wikipedia: <a href="https://en.wikipedia.org/wiki/Unreachable_code">Unreachable code</a></li>
23+
</references>
24+
</qhelp>

‎rust/ql/src/queries/unusedentities/UnreachableCode.ql

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
/**
2+
* @name Unreachable code
3+
* @description Code that cannot be reached should be deleted.
4+
* @kind problem
5+
* @problem.severity recommendation
6+
* @precision medium
7+
* @id rust/dead-code
8+
* @tags maintainability
9+
*/
10+
11+
import rust
12+
import codeql.rust.controlflow.ControlFlowGraph
13+
import codeql.rust.controlflow.internal.ControlFlowGraphImpl as ControlFlowGraphImpl
14+
15+
/**
16+
* Holds if `n` is an AST node that's unreachable.
17+
*/
18+
private predicate unreachable(AstNode n) {
19+
not n = any(CfgNode cfn).getAstNode() and // reachable nodes
20+
exists(ControlFlowGraphImpl::ControlFlowTree cft |
21+
// nodes intended to be part of the CFG
22+
cft.succ(n, _, _)
23+
or
24+
cft.succ(_, n, _)
25+
)
26+
}
27+
28+
/**
29+
* Holds if `n` is an AST node that's unreachable, and is not the successor
30+
* of an unreachable node (which would be a duplicate result).
31+
*/
32+
private predicate firstUnreachable(AstNode n) {
33+
unreachable(n) and
34+
(
35+
// no predecessor -> we are the first unreachable node.
36+
not ControlFlowGraphImpl::succ(_, n, _)
37+
or
38+
// reachable predecessor -> we are the first unreachable node.
39+
exists(AstNode pred |
40+
ControlFlowGraphImpl::succ(pred, n, _) and
41+
not unreachable(pred)
42+
)
43+
)
44+
}
45+
46+
/**
47+
* Gets a node we'd prefer not to report as unreachable.
48+
*/
49+
predicate skipNode(AstNode n) {
50+
n instanceof ControlFlowGraphImpl::PostOrderTree or // location is counter-intuitive
51+
not n instanceof ControlFlowGraphImpl::ControlFlowTree // not expected to be reachable
52+
}
53+
54+
/**
55+
* Gets the `ControlFlowTree` successor of a node we'd prefer not to report.
56+
*/
57+
AstNode skipSuccessor(AstNode n) {
58+
skipNode(n) and
59+
ControlFlowGraphImpl::succ(n, result, _)
60+
}
61+
62+
/**
63+
* Gets the node `n`, skipping past any nodes we'd prefer not to report.
64+
*/
65+
AstNode skipSuccessors(AstNode n) {
66+
result = skipSuccessor*(n) and
67+
not skipNode(result)
68+
}
69+
70+
from AstNode first, AstNode report
71+
where
72+
firstUnreachable(first) and
73+
report = skipSuccessors(first) and
74+
exists(report.getFile().getRelativePath()) // in source
75+
select report, "This code is never reached."

‎rust/ql/src/queries/unusedentities/UnreachableCodeBad.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
fn fib(input: u32) -> u32 {
2+
if (input == 0) {
3+
return 0;
4+
} else if (input == 1) {
5+
return 1;
6+
} else {
7+
return fib(input - 1) + fib(input - 2);
8+
}
9+
10+
return input; // BAD: this code is never reached
11+
}

‎rust/ql/src/queries/unusedentities/UnreachableCodeGood.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
fn fib(input: u32) -> u32 {
2+
if (input == 0) {
3+
return 0;
4+
} else if (input == 1) {
5+
return 1;
6+
} else {
7+
return fib(input - 1) + fib(input - 2);
8+
}
9+
}

‎rust/ql/test/query-tests/unusedentities/UnreachableCode.expected

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
| unreachable.rs:12:3:12:17 | ExprStmt | This code is never reached. |
2+
| unreachable.rs:20:3:20:17 | ExprStmt | This code is never reached. |
3+
| unreachable.rs:32:3:32:17 | ExprStmt | This code is never reached. |
4+
| unreachable.rs:39:3:39:17 | ExprStmt | This code is never reached. |
5+
| unreachable.rs:60:2:60:16 | ExprStmt | This code is never reached. |
6+
| unreachable.rs:101:3:101:17 | ExprStmt | This code is never reached. |
7+
| unreachable.rs:109:3:109:17 | ExprStmt | This code is never reached. |
8+
| unreachable.rs:124:2:124:16 | ExprStmt | This code is never reached. |
9+
| unreachable.rs:134:2:134:16 | ExprStmt | This code is never reached. |
10+
| unreachable.rs:141:3:141:17 | ExprStmt | This code is never reached. |
11+
| unreachable.rs:150:4:150:18 | ExprStmt | This code is never reached. |
12+
| unreachable.rs:156:3:156:17 | ExprStmt | This code is never reached. |
13+
| unreachable.rs:162:4:162:18 | ExprStmt | This code is never reached. |
14+
| unreachable.rs:165:2:165:16 | ExprStmt | This code is never reached. |

‎rust/ql/test/query-tests/unusedentities/UnreachableCode.qlref

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
queries/unusedentities/UnreachableCode.ql

‎rust/ql/test/query-tests/unusedentities/main.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ fn arrays() {
114114
for k // SPURIOUS: unused variable [macros not yet supported]
115115
in ks
116116
{
117-
println!("lets use {}", k);
117+
println!("lets use {}", k); // [unreachable FALSE POSITIVE]
118118
}
119119
}
120120

@@ -322,15 +322,21 @@ fn shadowing() -> i32 {
322322
}
323323
}
324324

325+
// --- main ---
325326
fn main() {
326327
locals_1();
327328
locals_2();
328329
structs();
329330
arrays();
330331
statics();
332+
println!("lets use result {}", parameters(1, 2, 3));
331333
loops();
332334
if_lets_matches();
333335
shadowing();
334336

335-
println!("lets use result {}", parameters(1, 2, 3));
337+
unreachable_if();
338+
unreachable_panic();
339+
unreachable_match();
340+
unreachable_loop();
341+
unreachable_paren();
336342
}

‎rust/ql/test/query-tests/unusedentities/unreachable.rs

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
2+
//fn cond() -> bool;
3+
//fn get_a_number() -> i32;
4+
5+
// --- unreachable code --
6+
7+
fn do_something() {
8+
}
9+
10+
fn unreachable_if() {
11+
if false {
12+
do_something(); // BAD: unreachable code
13+
} else {
14+
do_something();
15+
}
16+
17+
if true {
18+
do_something();
19+
} else {
20+
do_something(); // BAD: unreachable code
21+
}
22+
23+
let v = get_a_number();
24+
if v == 1 {
25+
if v != 1 {
26+
do_something(); // BAD: unreachable code [NOT DETECTED]
27+
}
28+
}
29+
30+
if cond() {
31+
return;
32+
do_something(); // BAD: unreachable code
33+
}
34+
35+
if cond() {
36+
do_something();
37+
} else {
38+
return;
39+
do_something(); // BAD: unreachable code
40+
}
41+
do_something();
42+
43+
if cond() {
44+
let x = cond();
45+
46+
if (x) {
47+
do_something();
48+
if (!x) {
49+
do_something(); // BAD: unreachable code [NOT DETECTED]
50+
}
51+
do_something();
52+
}
53+
}
54+
55+
if cond() {
56+
return;
57+
} else {
58+
return;
59+
}
60+
do_something(); // BAD: unreachable code
61+
}
62+
63+
fn unreachable_panic() {
64+
if cond() {
65+
do_something();
66+
panic!("Oh no!!!");
67+
do_something(); // BAD: unreachable code [NOT DETECTED]
68+
}
69+
70+
if cond() {
71+
do_something();
72+
unimplemented!();
73+
do_something(); // BAD: unreachable code [NOT DETECTED]
74+
}
75+
76+
if cond() {
77+
do_something();
78+
todo!();
79+
do_something(); // BAD: unreachable code [NOT DETECTED]
80+
}
81+
82+
if cond() {
83+
let mut maybe;
84+
85+
maybe = Some("Thing");
86+
_ = maybe.unwrap(); // (safe)
87+
do_something();
88+
89+
maybe = if cond() { Some("Other") } else { None };
90+
_ = maybe.unwrap(); // (might panic)
91+
do_something();
92+
93+
maybe = None;
94+
_ = maybe.unwrap(); // (always panics)
95+
do_something(); // BAD: unreachable code [NOT DETECTED]
96+
}
97+
98+
if cond() {
99+
do_something();
100+
_ = false && panic!(); // does not panic due to short-circuiting
101+
do_something(); // SPURIOUS: unreachable
102+
_ = false || panic!();
103+
do_something(); // BAD: unreachable code [NOT DETECTED]
104+
}
105+
106+
if cond() {
107+
do_something();
108+
_ = true || panic!(); // does not panic due to short-circuiting
109+
do_something(); // SPURIOUS: unreachable
110+
_ = true && panic!();
111+
do_something(); // BAD: unreachable code [NOT DETECTED]
112+
}
113+
}
114+
115+
fn unreachable_match() {
116+
match get_a_number() {
117+
1=>{
118+
return;
119+
}
120+
_=>{
121+
do_something();
122+
}
123+
}
124+
do_something(); // [unreachable FALSE POSITIVE]
125+
126+
match get_a_number() {
127+
1=>{
128+
return;
129+
}
130+
_=>{
131+
return;
132+
}
133+
}
134+
do_something(); // BAD: unreachable code
135+
}
136+
137+
fn unreachable_loop() {
138+
loop {
139+
do_something();
140+
break;
141+
do_something(); // BAD: unreachable code
142+
}
143+
144+
if cond() {
145+
while cond() {
146+
do_something();
147+
}
148+
149+
while false {
150+
do_something(); // BAD: unreachable code
151+
}
152+
153+
while true {
154+
do_something();
155+
}
156+
do_something(); // BAD: unreachable code
157+
}
158+
159+
loop {
160+
if cond() {
161+
return;
162+
do_something(); // BAD: unreachable code
163+
}
164+
}
165+
do_something(); // BAD: unreachable code
166+
do_something();
167+
do_something();
168+
}
169+
170+
fn unreachable_paren() {
171+
let _ = (((1)));
172+
}

0 commit comments

Comments
 (0)