diff --git a/go/ql/src/RedundantCode/UnreachableStatement.ql b/go/ql/src/RedundantCode/UnreachableStatement.ql index 68df01215919..edd2b17ab674 100644 --- a/go/ql/src/RedundantCode/UnreachableStatement.ql +++ b/go/ql/src/RedundantCode/UnreachableStatement.ql @@ -37,7 +37,6 @@ Stmt getPreviousStmt(Stmt s) { */ predicate firstUnreachableStmt(Stmt s) { not isReachable(s) and - not s instanceof EmptyStmt and ( // a statement whose preceding statement in the same list is reachable isReachable(getPreviousStmt(s)) @@ -47,6 +46,16 @@ predicate firstUnreachableStmt(Stmt s) { ) } +/** Holds if `s` is in a run of unreachable statements following a constant condition. */ +predicate isInUnreachableRunAfterConstantCondition(Stmt s) { + not isReachable(s) and + ( + exists(getPreviousStmt(s).(IfStmt).getCondition().getBoolValue()) + or + isInUnreachableRunAfterConstantCondition(getPreviousStmt(s)) + ) +} + /** * Matches if `retval` is a constant or a struct composed wholly of constants. */ @@ -78,6 +87,8 @@ predicate isAllowedReturnValue(Expr retval) { * Matches if `s` is an allowed unreachable statement. */ predicate allowlist(Stmt s) { + s instanceof EmptyStmt + or // `panic("unreachable")` and similar exists(CallExpr ce | ce = s.(ExprStmt).getExpr() or ce = s.(ReturnStmt).getExpr() | ce.getTarget().mustPanic() or ce.getCalleeName().toLowerCase() = "error" @@ -87,14 +98,28 @@ predicate allowlist(Stmt s) { exists(ReturnStmt ret | ret = s | forall(Expr retval | retval = ret.getAnExpr() | isAllowedReturnValue(retval)) ) - or - // statements deliberately made unreachable by a constant condition, such as the code - // following `if true { return }` - exists(getPreviousStmt(s).(IfStmt).getCondition().getBoolValue()) +} + +Stmt firstNonAllowlisted(Stmt s) { + not isReachable(s) and + ( + not allowlist(s) and result = s + or + allowlist(s) and + exists(Stmt next | getPreviousStmt(next) = s | result = firstNonAllowlisted(next)) + ) +} + +/** Holds if `s` is the first non-allowlisted statement in a run of unreachable statements. */ +predicate firstNonAllowlistedUnreachableStmt(Stmt s) { + exists(Stmt unreachable | + firstUnreachableStmt(unreachable) and + s = firstNonAllowlisted(unreachable) + ) } from Stmt s where - firstUnreachableStmt(s) and - not allowlist(s) + firstNonAllowlistedUnreachableStmt(s) and + not isInUnreachableRunAfterConstantCondition(s) select s, "This statement is unreachable." diff --git a/go/ql/test/query-tests/RedundantCode/UnreachableStatement/CONSISTENCY/CfgConsistency.expected b/go/ql/test/query-tests/RedundantCode/UnreachableStatement/CONSISTENCY/CfgConsistency.expected index e38f26b8c1ce..887fef4a49c7 100644 --- a/go/ql/test/query-tests/RedundantCode/UnreachableStatement/CONSISTENCY/CfgConsistency.expected +++ b/go/ql/test/query-tests/RedundantCode/UnreachableStatement/CONSISTENCY/CfgConsistency.expected @@ -1,5 +1,5 @@ consistencyOverview -| deadEnd | 10 | +| deadEnd | 11 | deadEnd | main.go:17:2:17:10 | select statement | | main.go:109:2:109:10 | select statement | @@ -11,3 +11,4 @@ deadEnd | main.go:151:2:151:10 | select statement | | main.go:157:2:157:10 | select statement | | main.go:164:2:164:10 | select statement | +| main.go:168:2:168:10 | select statement | diff --git a/go/ql/test/query-tests/RedundantCode/UnreachableStatement/UnreachableStatement.expected b/go/ql/test/query-tests/RedundantCode/UnreachableStatement/UnreachableStatement.expected index 9c70b439d5b3..240cfcaf1e5e 100644 --- a/go/ql/test/query-tests/RedundantCode/UnreachableStatement/UnreachableStatement.expected +++ b/go/ql/test/query-tests/RedundantCode/UnreachableStatement/UnreachableStatement.expected @@ -10,3 +10,4 @@ | main.go:147:2:147:17 | return statement | This statement is unreachable. | | main.go:153:2:153:22 | return statement | This statement is unreachable. | | main.go:159:2:159:43 | return statement | This statement is unreachable. | +| main.go:171:2:171:14 | expression statement | This statement is unreachable. | diff --git a/go/ql/test/query-tests/RedundantCode/UnreachableStatement/main.go b/go/ql/test/query-tests/RedundantCode/UnreachableStatement/main.go index 85d5fc80a938..0dacf0c44984 100644 --- a/go/ql/test/query-tests/RedundantCode/UnreachableStatement/main.go +++ b/go/ql/test/query-tests/RedundantCode/UnreachableStatement/main.go @@ -164,4 +164,26 @@ func test20() { select {} // OK: reachable after starting the goroutine } +func test21() { + select {} + panic("unreachable") // OK: allowlisted statement + // OK: empty statement + unreachable() // $ Alert +} + +func test22() { + goto reachableLabel + panic("unreachable") // OK: allowlisted statement +reachableLabel: + reachable() // OK: reachable through the goto +} + +func test23() { + if true { + return + } + unreachable() // OK: deliberately unreachable + unreachable() // OK: deliberately unreachable +} + func main() {}