diff --git a/java/ql/lib/change-notes/2023-09-28-case-rule-stmt-cfg-fix.md b/java/ql/lib/change-notes/2023-09-28-case-rule-stmt-cfg-fix.md new file mode 100644 index 00000000000..5e99335aba7 --- /dev/null +++ b/java/ql/lib/change-notes/2023-09-28-case-rule-stmt-cfg-fix.md @@ -0,0 +1,5 @@ +--- +category: minorAnalysis +--- +* Fixed a control-flow bug where case rule statements would incorrectly include a fall-through edge. +* Added support for default cases as proper guards in switch expressions to match switch statements. diff --git a/java/ql/lib/semmle/code/java/ControlFlowGraph.qll b/java/ql/lib/semmle/code/java/ControlFlowGraph.qll index 1ca3732ee93..6b3795df2b0 100644 --- a/java/ql/lib/semmle/code/java/ControlFlowGraph.qll +++ b/java/ql/lib/semmle/code/java/ControlFlowGraph.qll @@ -871,7 +871,13 @@ private module ControlFlowGraphImpl { ) or // the last node in a case rule is the last node in the right-hand side - last(n.(SwitchCase).getRuleStatement(), last, completion) + // if the rhs is a statement we wrap the completion as a break + exists(Completion caseCompletion | + last(n.(SwitchCase).getRuleStatement(), last, caseCompletion) and + if caseCompletion instanceof NormalOrBooleanCompletion + then completion = anonymousBreakCompletion() + else completion = caseCompletion + ) or // ...and if the rhs is an expression we wrap the completion as a yield exists(Completion caseCompletion | diff --git a/java/ql/lib/semmle/code/java/controlflow/internal/GuardsLogic.qll b/java/ql/lib/semmle/code/java/controlflow/internal/GuardsLogic.qll index e666143c8d3..bc49f1d3a77 100644 --- a/java/ql/lib/semmle/code/java/controlflow/internal/GuardsLogic.qll +++ b/java/ql/lib/semmle/code/java/controlflow/internal/GuardsLogic.qll @@ -57,6 +57,8 @@ predicate implies_v1(Guard g1, boolean b1, Guard g2, boolean b2) { or g1.(DefaultCase).getSwitch().getAConstCase() = g2 and b1 = true and b2 = false or + g1.(DefaultCase).getSwitchExpr().getAConstCase() = g2 and b1 = true and b2 = false + or exists(MethodAccess check, int argIndex | check = g1 | conditionCheckArgument(check, argIndex, _) and g2 = check.getArgument(argIndex) and diff --git a/java/ql/test/library-tests/guards12/PrintAst.expected b/java/ql/test/library-tests/guards12/PrintAst.expected index 1df3423239b..a8a96261ec0 100644 --- a/java/ql/test/library-tests/guards12/PrintAst.expected +++ b/java/ql/test/library-tests/guards12/PrintAst.expected @@ -19,5 +19,22 @@ Test.java: # 5| 1: [ConstCase] case ... # 5| -1: [IntegerLiteral] 2 # 5| 0: [StringLiteral] "c" -# 6| 2: [DefaultCase] default -# 6| -1: [IntegerLiteral] 3 +# 6| 2: [ConstCase] case ... +# 6| -1: [IntegerLiteral] 2 +# 6| 0: [StringLiteral] "d" +# 7| 3: [DefaultCase] default +# 7| -1: [IntegerLiteral] 3 +# 9| 1: [SwitchStmt] switch (...) +# 9| -1: [VarAccess] s +# 10| 0: [ConstCase] case ... +# 10| -1: [BlockStmt] { ... } +# 10| 0: [StringLiteral] "a" +# 10| 1: [StringLiteral] "b" +# 11| 1: [ConstCase] case ... +# 11| -1: [BlockStmt] { ... } +# 11| 0: [StringLiteral] "c" +# 12| 2: [ConstCase] case ... +# 12| -1: [BlockStmt] { ... } +# 12| 0: [StringLiteral] "d" +# 13| 3: [DefaultCase] default +# 13| -1: [BlockStmt] { ... } diff --git a/java/ql/test/library-tests/guards12/Test.java b/java/ql/test/library-tests/guards12/Test.java index 03ccc92d245..3ce8f6f4828 100644 --- a/java/ql/test/library-tests/guards12/Test.java +++ b/java/ql/test/library-tests/guards12/Test.java @@ -3,7 +3,14 @@ class Test { int x = switch(s) { case "a", "b" -> 1; case "c" -> 2; + case "d" -> 2; default -> 3; }; + switch (s) { + case "a", "b" -> { } + case "c" -> { } + case "d" -> { } + default -> { } + } } } diff --git a/java/ql/test/library-tests/guards12/guard.expected b/java/ql/test/library-tests/guards12/guard.expected index 4befc404376..71d1818f29c 100644 --- a/java/ql/test/library-tests/guards12/guard.expected +++ b/java/ql/test/library-tests/guards12/guard.expected @@ -1 +1,8 @@ +| Test.java:5:7:5:17 | case ... | Test.java:3:20:3:20 | s | Test.java:5:12:5:14 | "c" | true | false | Test.java:7:7:7:16 | default | | Test.java:5:7:5:17 | case ... | Test.java:3:20:3:20 | s | Test.java:5:12:5:14 | "c" | true | true | Test.java:5:7:5:17 | case ... | +| Test.java:6:7:6:17 | case ... | Test.java:3:20:3:20 | s | Test.java:6:12:6:14 | "d" | true | false | Test.java:7:7:7:16 | default | +| Test.java:6:7:6:17 | case ... | Test.java:3:20:3:20 | s | Test.java:6:12:6:14 | "d" | true | true | Test.java:6:7:6:17 | case ... | +| Test.java:11:7:11:17 | case ... | Test.java:9:13:9:13 | s | Test.java:11:12:11:14 | "c" | true | false | Test.java:13:7:13:16 | default | +| Test.java:11:7:11:17 | case ... | Test.java:9:13:9:13 | s | Test.java:11:12:11:14 | "c" | true | true | Test.java:11:7:11:17 | case ... | +| Test.java:12:7:12:17 | case ... | Test.java:9:13:9:13 | s | Test.java:12:12:12:14 | "d" | true | false | Test.java:13:7:13:16 | default | +| Test.java:12:7:12:17 | case ... | Test.java:9:13:9:13 | s | Test.java:12:12:12:14 | "d" | true | true | Test.java:12:7:12:17 | case ... |