Rust: Unreachable code query

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

@@ -0,0 +1,24 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>This rule finds code that is never reached. Unused code should be removed to increase readability and avoid confusion.</p>
+</overview>
+
+<recommendation>
+<p>Remove any unreachable code.</p>
+</recommendation>
+
+<example>
+<p>In the following example, the final <code>return</code> statement can never be reached:</p>
+<sample src="UnreachableCodeBad.rs" />
+<p>The problem can be fixed simply by removing the unreachable code:</p>
+<sample src="UnreachableCodeGood.rs" />
+</example>
+
+<references>
+<li>Wikipedia: <a href="https://en.wikipedia.org/wiki/Unreachable_code">Unreachable code</a></li>
+</references>
+</qhelp>

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

@@ -0,0 +1,59 @@
+/**
+ * @name Unreachable code
+ * @description Code that cannot be reached should be deleted.
+ * @kind problem
+ * @problem.severity recommendation
+ * @precision medium
+ * @id rust/dead-code
+ * @tags maintainability
+ */
+
+import rust
+import codeql.rust.controlflow.ControlFlowGraph
+import codeql.rust.controlflow.internal.ControlFlowGraphImpl as ControlFlowGraphImpl
+
+/**
+ * Holds if `n` is an AST node that's unreachable, and is not the successor
+ * of an unreachable node (which would be a duplicate result).
+ */
+predicate firstUnreachable(AstNode n) {
+  // entry nodes are reachable
+  not exists(CfgScope s | s.scopeFirst(n)) and
+  // we never want a `ControlFlowTree` successor node:
+  //  - if the predecessor is reachable, so are we.
+  //  - if the predecessor is unreachable, we're not the *first* unreachable node.
+  not ControlFlowGraphImpl::succ(_, n, _)
+  // (note that an unreachable cycle of nodes could be missed by this logic, in
+  //  general it wouldn't be possible to pick one node to represent it)
+}
+
+/**
+ * Gets a node we'd prefer not to report as unreachable.
+ */
+predicate skipNode(AstNode n) {
+  n instanceof ControlFlowGraphImpl::PostOrderTree or // location is counter-intuitive
+  not n instanceof ControlFlowGraphImpl::ControlFlowTree // not expected to be reachable
+}
+
+/**
+ * Gets the `ControlFlowTree` successor of a node we'd prefer not to report.
+ */
+AstNode skipSuccessor(AstNode n) {
+  skipNode(n) and
+  ControlFlowGraphImpl::succ(n, result, _)
+}
+
+/**
+ * Gets the node `n`, skipping past any nodes we'd prefer not to report.
+ */
+AstNode skipSuccessors(AstNode n) {
+  result = skipSuccessor*(n) and
+  not skipNode(result)
+}
+
+from AstNode first, AstNode report
+where
+  firstUnreachable(first) and
+  report = skipSuccessors(first) and
+  exists(report.getFile().getRelativePath()) // in source
+select report, "This code is never reached."

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

@@ -0,0 +1,11 @@
+fn fib(input: u32) -> u32 {
+	if (input == 0) {
+		return 0;
+	} else if (input == 1) {
+		return 1;
+	} else {
+		return fib(input - 1) + fib(input - 2);
+	}
+
+	return input; // BAD: this code is never reached
+}

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

@@ -0,0 +1,9 @@
+fn fib(input: u32) -> u32 {
+	if (input == 0) {
+		return 0;
+	} else if (input == 1) {
+		return 1;
+	} else {
+		return fib(input - 1) + fib(input - 2);
+	}
+}

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

@@ -0,0 +1,15 @@
+| unreachable.rs:32:3:32:17 | ExprStmt | This code is never reached. |
+| unreachable.rs:39:3:39:17 | ExprStmt | This code is never reached. |
+| unreachable.rs:48:2:48:16 | ExprStmt | This code is never reached. |
+| unreachable.rs:89:3:91:3 | MatchArm | This code is never reached. |
+| unreachable.rs:92:3:94:3 | MatchArm | This code is never reached. |
+| unreachable.rs:96:2:96:16 | ExprStmt | This code is never reached. |
+| unreachable.rs:99:3:101:3 | MatchArm | This code is never reached. |
+| unreachable.rs:102:3:104:3 | MatchArm | This code is never reached. |
+| unreachable.rs:106:2:106:16 | ExprStmt | This code is never reached. |
+| unreachable.rs:113:3:113:17 | ExprStmt | This code is never reached. |
+| unreachable.rs:118:4:118:18 | ExprStmt | This code is never reached. |
+| unreachable.rs:122:4:122:18 | ExprStmt | This code is never reached. |
+| unreachable.rs:126:4:126:18 | ExprStmt | This code is never reached. |
+| unreachable.rs:134:4:134:18 | ExprStmt | This code is never reached. |
+| unreachable.rs:137:2:137:16 | ExprStmt | This code is never reached. |

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

@@ -0,0 +1 @@
+queries/unusedentities/UnreachableCode.ql

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

@@ -0,0 +1,140 @@
+
+//fn cond() -> bool;
+//fn get_a_number() -> i32;
+
+// --- unreachable code --
+
+fn do_something() {
+}
+
+fn unreachable_if() {
+	if false {
+		do_something(); // BAD: unreachable code [NOT DETECTED]
+	} else {
+		do_something();
+	}
+
+	if true {
+		do_something();
+	} else {
+		do_something(); // BAD: unreachable code [NOT DETECTED]
+	}
+
+	let v = get_a_number();
+	if v == 1 {
+		if v != 1 {
+			do_something(); // BAD: unreachable code [NOT DETECTED]
+		}
+	}
+
+	if cond() {
+		return;
+		do_something(); // BAD: unreachable code
+	}
+
+	if cond() {
+		do_something();
+	} else {
+		return;
+		do_something(); // BAD: unreachable code
+	}
+	do_something();
+
+	if cond() {
+		return;
+	} else {
+		return;
+	}
+	do_something(); // BAD: unreachable code
+}
+
+fn unreachable_panic() {
+	if cond() {
+		do_something();
+		panic!("Oh no!!!");
+		do_something(); // BAD: unreachable code [NOT DETECTED]
+	}
+
+	if cond() {
+		do_something();
+		unimplemented!();
+		do_something(); // BAD: unreachable code [NOT DETECTED]
+	}
+
+	if cond() {
+		do_something();
+		todo!();
+		do_something(); // BAD: unreachable code [NOT DETECTED]
+	}
+
+	if cond() {
+		let mut maybe;
+
+		maybe = Some("Thing");
+		_ = maybe.unwrap(); // (safe)
+		do_something();
+
+		maybe = if cond() { Some("Other") } else { None };
+		_ = maybe.unwrap(); // (might panic)
+		do_something();
+
+		maybe = None;
+		_ = maybe.unwrap(); // (always panics)
+		do_something(); // BAD: unreachable code [NOT DETECTED]
+	}
+}
+
+fn unreachable_match() {
+	match get_a_number() {
+		1=>{ // [unreachable FALSE POSITIVE]
+			return;
+		}
+		_=>{ // [unreachable FALSE POSITIVE]
+			do_something();
+		}
+	}
+	do_something(); // [unreachable FALSE POSITIVE]
+
+	match get_a_number() {
+		1=>{ // [unreachable FALSE POSITIVE]
+			return;
+		}
+		_=>{ // [unreachable FALSE POSITIVE]
+			return;
+		}
+	}
+	do_something(); // BAD: unreachable code
+}
+
+fn unreachable_loop() {
+	loop {
+		do_something();
+		break;
+		do_something(); // BAD: unreachable code
+	}
+
+	if cond() {
+		while cond() {
+			do_something();{ // [unreachable FALSE POSITIVE]
+		}
+
+		while false {
+			do_something(); // BAD: unreachable code
+		}
+
+		while true {
+			do_something(); // [unreachable FALSE POSITIVE]
+		}
+		do_something(); // BAD: unreachable code [NOT DETECTED]
+	}
+
+	loop {
+		if cond() {
+			return;
+			do_something(); // BAD: unreachable code
+		}
+	}
+	do_something(); // BAD: unreachable code
+	do_something();
+	do_something();
+}