From edf88b34da6c7bb1671804fb54661e5c38735c5b Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Tue, 10 Mar 2026 11:02:58 +0100 Subject: [PATCH] Cfg: Move Case.getBodyElement to shared code. --- .../lib/semmle/code/java/ControlFlowGraph.qll | 41 ++++-------- .../codeql/controlflow/ControlFlowGraph.qll | 66 +++++++++++++++---- 2 files changed, 65 insertions(+), 42 deletions(-) diff --git a/java/ql/lib/semmle/code/java/ControlFlowGraph.qll b/java/ql/lib/semmle/code/java/ControlFlowGraph.qll index 26672a22ed5..a2881e62e70 100644 --- a/java/ql/lib/semmle/code/java/ControlFlowGraph.qll +++ b/java/ql/lib/semmle/code/java/ControlFlowGraph.qll @@ -132,18 +132,11 @@ private module Ast implements AstSig { result = this.(SwitchStmt).getCase(index) or result = this.(SwitchExpr).getCase(index) } - } - private Stmt getSwitchStmt(Switch s, int i) { - result = s.(SwitchStmt).getStmt(i) or - result = s.(SwitchExpr).getStmt(i) - } - - private int numberOfStmts(Switch s) { result = strictcount(getSwitchStmt(s, _)) } - - private predicate caseIndex(Switch s, Case c, int caseIdx, int caseStmtPos) { - c = s.getCase(caseIdx) and - c = getSwitchStmt(s, caseStmtPos) + Stmt getStmt(int index) { + result = this.(SwitchStmt).getStmt(index) or + result = this.(SwitchExpr).getStmt(index) + } } class Case extends AstNode instanceof J::SwitchCase { @@ -157,28 +150,16 @@ private module Ast implements AstSig { Expr getGuard() { result = this.(PatternCase).getGuard() } /** - * Gets the body element of this case at the specified (zero-based) `index`. + * Gets the body of this case, if any. * - * This is either unique when the case has a single right-hand side, or it - * is the sequence of statements between this case and the next case. + * A case can either have a body as a single child AST node given by this + * predicate, or it can have an implicit body given by the sequence of + * statements between this case and the next case. */ - AstNode getBodyElement(int index) { - result = this.(J::SwitchCase).getRuleExpression() and index = 0 + AstNode getBody() { + result = this.(J::SwitchCase).getRuleExpression() or - result = this.(J::SwitchCase).getRuleStatement() and index = 0 - or - not this.(J::SwitchCase).isRule() and - exists(Switch s, int caseIdx, int caseStmtPos, int nextCaseStmtPos | - caseIndex(pragma[only_bind_into](s), this, caseIdx, caseStmtPos) and - ( - caseIndex(pragma[only_bind_into](s), _, caseIdx + 1, nextCaseStmtPos) - or - not exists(s.getCase(caseIdx + 1)) and - nextCaseStmtPos = numberOfStmts(s) - ) and - index = [0 .. nextCaseStmtPos - caseStmtPos - 2] and - result = getSwitchStmt(pragma[only_bind_into](s), caseStmtPos + 1 + index) - ) + result = this.(J::SwitchCase).getRuleStatement() } } diff --git a/shared/controlflow/codeql/controlflow/ControlFlowGraph.qll b/shared/controlflow/codeql/controlflow/ControlFlowGraph.qll index 1d070564f1c..f8a4b903579 100644 --- a/shared/controlflow/codeql/controlflow/ControlFlowGraph.qll +++ b/shared/controlflow/codeql/controlflow/ControlFlowGraph.qll @@ -224,6 +224,17 @@ signature module AstSig { /** Gets the case at the specified (zero-based) `index`. */ Case getCase(int index); + + /** + * Gets the statement at the specified (zero-based) `index`, if any. + * + * Depending on the language, switches may have their case bodies nested + * under the case nodes, which may or may not be statements themselves, or + * the switches may have a flat structure where the cases are just labels + * and the case bodies are sequences of statements between case statements. + * This predicate accomodates the latter. + */ + Stmt getStmt(int index); } /** A case in a switch. */ @@ -235,12 +246,13 @@ signature module AstSig { Expr getGuard(); /** - * Gets the body element of this case at the specified (zero-based) `index`. + * Gets the body of this case, if any. * - * This is either unique when the case has a single right-hand side, or it - * is the sequence of statements between this case and the next case. + * A case can either have a body as a single child AST node given by this + * predicate, or it can have an implicit body given by the sequence of + * statements between this case and the next case. */ - AstNode getBodyElement(int index); + AstNode getBody(); } class DefaultCase extends Case; @@ -1075,7 +1087,7 @@ module Make0 Ast> { ) or exists(Switch switch | - ast = switch.getCase(_).getBodyElement(_) and + ast = getCaseBodyElement(switch.getCase(_), _) and n.isAfter(switch) and c.getSuccessorType() instanceof BreakSuccessor | @@ -1107,10 +1119,40 @@ module Make0 Ast> { result = rank[rnk](Case c, int i | getCaseControlFlowOrder(s, c) = i | c order by i) } - private AstNode getFirstCaseBodyElement(Case case) { - result = case.getBodyElement(0) + private int numberOfStmts(Switch s) { result = strictcount(s.getStmt(_)) } + + private predicate caseIndex(Switch s, Case c, int caseIdx, int caseStmtPos) { + c = s.getCase(caseIdx) and + c = s.getStmt(caseStmtPos) + } + + /** + * Gets the body element of `case` at the specified (zero-based) `index`. + * + * This is either unique when the case has a single right-hand side, or it + * is the sequence of statements between this case and the next case. + */ + private AstNode getCaseBodyElement(Case case, int index) { + result = case.getBody() and index = 0 or - not exists(case.getBodyElement(0)) and + not exists(case.getBody()) and + exists(Switch s, int caseIdx, int caseStmtPos, int nextCaseStmtPos | + caseIndex(pragma[only_bind_into](s), case, caseIdx, caseStmtPos) and + ( + caseIndex(pragma[only_bind_into](s), _, caseIdx + 1, nextCaseStmtPos) + or + not exists(s.getCase(caseIdx + 1)) and + nextCaseStmtPos = numberOfStmts(s) + ) and + index = [0 .. nextCaseStmtPos - caseStmtPos - 2] and + result = pragma[only_bind_into](s).getStmt(caseStmtPos + 1 + index) + ) + } + + private AstNode getFirstCaseBodyElement(Case case) { + result = getCaseBodyElement(case, 0) + or + not exists(getCaseBodyElement(case, 0)) and exists(Switch s, int i | fallsThrough(case) and // fall-through follows AST order, not case control flow order: @@ -1120,10 +1162,10 @@ module Make0 Ast> { } private AstNode getNextCaseBodyElement(AstNode bodyElement) { - exists(Case case, int i | case.getBodyElement(i) = bodyElement | - result = case.getBodyElement(i + 1) + exists(Case case, int i | getCaseBodyElement(case, i) = bodyElement | + result = getCaseBodyElement(case, i + 1) or - not exists(case.getBodyElement(i + 1)) and + not exists(getCaseBodyElement(case, i + 1)) and exists(Switch s, int j | fallsThrough(case) and // fall-through follows AST order, not case control flow order: @@ -1474,7 +1516,7 @@ module Make0 Ast> { or n1.isAfter(caseBodyElement) and not exists(getNextCaseBodyElement(caseBodyElement)) and - n2.isAfter(any(Switch s | s.getCase(_).getBodyElement(_) = caseBodyElement)) + n2.isAfter(any(Switch s | getCaseBodyElement(s.getCase(_), _) = caseBodyElement)) ) }