Skip to content

Commit 86ead25

Browse files
owen-mcCopilot
andcommitted
Generalize shared CFG jump targets
Represent labeled and resolved jumps with a common targeted abrupt completion, and let language libraries define resolved jump destinations and additional goto catch boundaries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent fba894a commit 86ead25

7 files changed

Lines changed: 155 additions & 98 deletions

File tree

‎csharp/ql/lib/semmle/code/csharp/controlflow/ControlFlowGraph.qll‎

Lines changed: 29 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import csharp
66
private import internal.ControlFlowGraph
77
private import codeql.controlflow.SuccessorType
8+
private import codeql.util.Void
89
private import semmle.code.csharp.commons.Compilation
910
private import semmle.code.csharp.controlflow.internal.NonReturning as NonReturning
1011

@@ -137,29 +138,37 @@ private module Input implements InputSig1, InputSig2 {
137138

138139
predicate matchAll(Ast::Case c) { c instanceof DefaultCase or c.(SwitchCaseExpr).matchesAll() }
139140

140-
private newtype TLabel =
141-
TLblGoto(string label) { any(GotoLabelStmt goto).getLabel() = label } or
142-
TLblSwitchCase(string value) { any(GotoCaseStmt goto).getLabel() = value } or
143-
TLblSwitchDefault()
141+
class Label = Void;
144142

145-
class Label extends TLabel {
146-
string toString() {
147-
this = TLblGoto(result)
148-
or
149-
this = TLblSwitchCase(result)
150-
or
151-
this = TLblSwitchDefault() and result = "default"
152-
}
143+
class JumpTarget = LabeledStmt;
144+
145+
private SwitchStmt getEnclosingSwitch(GotoStmt jump) {
146+
result.getAChild*() = jump and
147+
not exists(SwitchStmt inner |
148+
inner != result and
149+
result.getAChild*() = inner and
150+
inner.getAChild*() = jump
151+
)
153152
}
154153

155-
predicate hasLabel(Ast::AstNode n, Label l) {
156-
l = TLblGoto(n.(GotoLabelStmt).getLabel())
157-
or
158-
l = TLblSwitchCase(n.(GotoCaseStmt).getLabel())
154+
JumpTarget getAGotoTarget(Ast::GotoStmt jump) {
155+
result = jump.(GotoLabelStmt).getTarget()
159156
or
160-
l = TLblSwitchDefault() and n instanceof GotoDefaultStmt
161-
or
162-
l = TLblGoto(n.(LabelStmt).getLabel())
157+
exists(SwitchStmt switch | switch = getEnclosingSwitch(jump) |
158+
result = switch.getAConstCase() and result.getLabel() = jump.(GotoCaseStmt).getLabel()
159+
or
160+
result = switch.getDefaultCase() and jump instanceof GotoDefaultStmt
161+
)
162+
}
163+
164+
Ast::AstNode getJumpTargetDestination(JumpTarget target) { result = target }
165+
166+
predicate additionalGotoCatchBoundary(Ast::AstNode completedChild, JumpTarget target) {
167+
exists(SwitchStmt switch |
168+
completedChild.(Stmt).getParent() = switch and
169+
target.getParent() = switch and
170+
target instanceof CaseStmt
171+
)
163172
}
164173

165174
class CallableContext = CompilationExt;
@@ -230,21 +239,7 @@ private module Input implements InputSig1, InputSig2 {
230239
}
231240

232241
predicate endAbruptCompletion(Ast::AstNode ast, PreControlFlowNode n, AbruptCompletion c) {
233-
exists(SwitchStmt switch, Label l, Ast::Case case |
234-
ast.(Stmt).getParent() = switch and
235-
c.getSuccessorType() instanceof GotoSuccessor and
236-
c.hasLabel(l) and
237-
n.isAfterValue(case, any(MatchingSuccessor t | t.getValue() = true))
238-
|
239-
exists(string value, ConstCase cc |
240-
l = TLblSwitchCase(value) and
241-
switch.getAConstCase() = cc and
242-
cc.getLabel() = value and
243-
cc = case
244-
)
245-
or
246-
l = TLblSwitchDefault() and switch.getDefaultCase() = case
247-
)
242+
none()
248243
}
249244

250245
predicate step(PreControlFlowNode n1, PreControlFlowNode n2) {

‎go/ql/lib/semmle/go/controlflow/ControlFlowGraphImpl.qll‎

Lines changed: 20 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -433,28 +433,34 @@ module CfgImpl {
433433
string toString() { result = this }
434434
}
435435

436+
class JumpTarget = Go::LabeledStmt;
437+
438+
JumpTarget getAGotoTarget(Ast::GotoStmt jump) {
439+
result.getEnclosingFunction() = jump.getEnclosingFunction() and
440+
result.getLabel() = jump.getLabel()
441+
}
442+
443+
Ast::AstNode getJumpTargetDestination(JumpTarget target) { result = target }
444+
445+
predicate additionalGotoCatchBoundary(Ast::AstNode completedChild, JumpTarget target) {
446+
exists(Go::FuncDef fd |
447+
completedChild = fd.getBody() and
448+
target = fd.getBody().getAStmt()
449+
)
450+
or
451+
completedChild = target.getStmt()
452+
}
453+
436454
predicate hasLabel(Ast::AstNode n, Label l) {
437-
// A statement carries the label of every `LabeledStmt` that wraps it.
438-
// This is recursive because Go allows stacked labels (`L1: L2: stmt`),
439-
// which the extractor represents as nested `LabeledStmt`s, so a single
440-
// statement may have several labels.
455+
// Loops and selects carry the label of every `LabeledStmt` that wraps
456+
// them, for matching labeled break and continue statements.
441457
exists(Go::LabeledStmt ls | n = ls.getStmt() | l = ls.getLabel() or hasLabel(ls, l))
442458
or
443-
// The `LabeledStmt` wrapper itself also carries its label. Blocks contain
444-
// the wrapper (not the inner statement) as a direct child, so the shared
445-
// library's block-level `goto` target resolution -- which looks for a
446-
// labelled statement that is a direct child of a block -- matches on the
447-
// wrapper.
448459
l = n.(Go::LabeledStmt).getLabel()
449460
or
450461
l = n.(Go::BreakStmt).getLabel()
451462
or
452463
l = n.(Go::ContinueStmt).getLabel()
453-
or
454-
// A `goto` statement carries its target label, so that the shared
455-
// library's `beginAbruptCompletion` produces a *labelled* goto completion
456-
// (matching the target label) rather than an unlabelled one.
457-
l = n.(Go::GotoStmt).getLabel()
458464
}
459465

460466
predicate preOrderExpr(Ast::Expr e) {
@@ -835,28 +841,6 @@ module CfgImpl {
835841
exists(fd.getResultVar(0)) and
836842
n.isAdditional(fd.getBody(), "result-read:0")
837843
)
838-
or
839-
// Handle goto targets that the shared block logic cannot see: top-level
840-
// statements of function bodies and labels enclosing the current node.
841-
exists(Go::Stmt target, Label l |
842-
(
843-
exists(Go::FuncDef fd |
844-
ast = fd.getBody() and
845-
target = fd.getBody().getAStmt() and
846-
hasLabel(target, l)
847-
)
848-
or
849-
exists(Go::LabeledStmt lbl |
850-
ast = lbl.getStmt() and
851-
target = lbl and
852-
l = lbl.getLabel()
853-
)
854-
) and
855-
not target instanceof Go::GotoStmt and
856-
n.isBefore(target) and
857-
c.getSuccessorType() instanceof GotoSuccessor and
858-
c.hasLabel(l)
859-
)
860844
}
861845

862846
/** Holds if `ast` or one of its CFG children may panic. */

‎java/ql/lib/semmle/code/java/ControlFlowGraph.qll‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,8 @@ private module Input implements InputSig1, InputSig2 {
543543
}
544544
}
545545

546+
class JumpTarget = Void;
547+
546548
private Label getLabelOfLoop(Stmt s) {
547549
exists(LabeledStmt l | s = l.getStmt() |
548550
result = TJavaLabel(l.getLabel()) or

‎python/ql/lib/semmle/python/controlflow/internal/AstNodeImpl.qll‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1618,6 +1618,8 @@ private module Input implements InputSig1, InputSig2 {
16181618
string toString() { result = "label" }
16191619
}
16201620

1621+
class JumpTarget = Void;
1622+
16211623
class CallableContext = Void;
16221624

16231625
predicate inConditionalContext(Ast::AstNode n, ConditionKind kind) {

‎ruby/ql/lib/codeql/ruby/controlflow/ControlFlowGraph.qll‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -465,6 +465,8 @@ private module Input implements InputSig1, InputSig2 {
465465

466466
class Label = Void;
467467

468+
class JumpTarget = Void;
469+
468470
predicate preOrderExpr(Ast::Expr e) {
469471
e instanceof R::Ast::StmtSequence or
470472
e instanceof R::Ast::RescueClause or

0 commit comments

Comments
 (0)