Skip to content

Commit aa78c59

Browse files
committed
C#: Move handling of callables into shared control flow library
1 parent 62f15d0 commit aa78c59

2 files changed

Lines changed: 143 additions & 115 deletions

File tree

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

Lines changed: 66 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ private module Ast implements AstSig<Location> {
2525

2626
class AstNode = ControlFlowElementOrCallable;
2727

28-
private predicate skipControlFlow(AstNode e) {
28+
additional predicate skipControlFlow(AstNode e) {
2929
e instanceof TypeAccess and
3030
not e instanceof TypeAccessPatternExpr
3131
or
@@ -82,13 +82,8 @@ private module Ast implements AstSig<Location> {
8282

8383
AstNode callableGetBody(Callable c) {
8484
not skipControlFlow(result) and
85-
(
86-
result = c.getBody() or
87-
result = c.(Constructor).getObjectInitializerCall() or
88-
result = c.(Constructor).getInitializer() or
89-
c.(ObjectInitMethod).initializes(result) or
90-
Initializers::staticMemberInitializer(c, result)
91-
)
85+
result = c.getBody() and
86+
not c instanceof Constructor // handled in `callableGetBodyPart`
9287
}
9388

9489
class Stmt = CS::Stmt;
@@ -222,10 +217,21 @@ private module Ast implements AstSig<Location> {
222217
* Unlike the standard `Compilation` class, this class also supports buildless
223218
* extraction.
224219
*/
225-
private newtype CompilationExt =
220+
private newtype TCompilationExt =
226221
TCompilation(Compilation c) { not extractionIsStandalone() } or
227222
TBuildless() { extractionIsStandalone() }
228223

224+
private class CompilationExt extends TCompilationExt {
225+
string toString() {
226+
exists(Compilation c |
227+
this = TCompilation(c) and
228+
result = c.toString()
229+
)
230+
or
231+
this = TBuildless() and result = "buildless compilation"
232+
}
233+
}
234+
229235
/** Gets the compilation that source file `f` belongs to. */
230236
private CompilationExt getCompilation(File f) {
231237
exists(Compilation c |
@@ -274,12 +280,13 @@ private module Initializers {
274280
/**
275281
* Gets the `i`th static member initializer expression for static constructor `staticCtor`.
276282
*/
277-
Expr initializedStaticMemberOrder(Constructor staticCtor, int i) {
283+
Expr initializedStaticMemberOrder(Constructor staticCtor, CompilationExt comp, int i) {
278284
result =
279285
rank[i + 1](Expr init, Location l, string filepath, int startline, int startcolumn |
280286
staticMemberInitializer(staticCtor, init) and
281287
l = init.getLocation() and
282-
l.hasLocationInfo(filepath, startline, startcolumn, _, _)
288+
l.hasLocationInfo(filepath, startline, startcolumn, _, _) and
289+
getCompilation(l.getFile()) = comp
283290
|
284291
init order by startline, startcolumn, filepath
285292
)
@@ -301,17 +308,6 @@ private module Initializers {
301308
ae0 order by startline, startcolumn, filepath
302309
)
303310
}
304-
305-
/**
306-
* Gets the last member initializer expression for object initializer method `obinit`
307-
* in compilation `comp`.
308-
*/
309-
AssignExpr lastInitializer(ObjectInitMethod obinit, CompilationExt comp) {
310-
exists(int i |
311-
result = initializedInstanceMemberOrder(obinit, comp, i) and
312-
not exists(initializedInstanceMemberOrder(obinit, comp, i + 1))
313-
)
314-
}
315311
}
316312

317313
private module Exceptions {
@@ -424,6 +420,54 @@ private module Input implements InputSig1, InputSig2 {
424420
l = TLblGoto(n.(LabelStmt).getLabel())
425421
}
426422

423+
pragma[noinline]
424+
private MethodCall getObjectInitializerCall(Constructor ctor, CompilationExt comp) {
425+
result = ctor.getObjectInitializerCall() and
426+
comp = getCompilation(result.getFile())
427+
}
428+
429+
pragma[noinline]
430+
private ConstructorInitializer getInitializer(Constructor ctor, CompilationExt comp) {
431+
result = ctor.getInitializer() and
432+
comp = getCompilation(result.getFile())
433+
}
434+
435+
pragma[noinline]
436+
private Ast::AstNode getBody(Constructor ctor, CompilationExt comp) {
437+
result = ctor.getBody() and
438+
comp = getCompilation(result.getFile())
439+
}
440+
441+
class CallableBodyPartContext = CompilationExt;
442+
443+
Ast::AstNode callableGetBodyPart(Callable c, CallableBodyPartContext ctx, int index) {
444+
not Ast::skipControlFlow(result) and
445+
(
446+
result = Initializers::initializedInstanceMemberOrder(c, ctx, index)
447+
or
448+
result =
449+
rank[index + 1](Ast::AstNode child, int i, int j |
450+
i = 0 and
451+
child = Initializers::initializedStaticMemberOrder(c, ctx, j)
452+
or
453+
i = 1 and
454+
j = 0 and
455+
child = getObjectInitializerCall(c, ctx)
456+
or
457+
i = 2 and
458+
(
459+
j = 0 and
460+
child = getInitializer(c, ctx)
461+
or
462+
j = 1 and
463+
child = getBody(c, ctx)
464+
)
465+
|
466+
child order by i, j
467+
)
468+
)
469+
}
470+
427471
private Expr getQualifier(QualifiableExpr qe) {
428472
result = qe.getQualifier() or
429473
result = qe.(ExtensionMethodCall).getArgument(0)
@@ -474,80 +518,7 @@ private module Input implements InputSig1, InputSig2 {
474518
)
475519
}
476520

477-
pragma[noinline]
478-
private MethodCall getObjectInitializerCall(Constructor ctor, CompilationExt comp) {
479-
result = ctor.getObjectInitializerCall() and
480-
comp = getCompilation(result.getFile())
481-
}
482-
483-
pragma[noinline]
484-
private ConstructorInitializer getInitializer(Constructor ctor, CompilationExt comp) {
485-
result = ctor.getInitializer() and
486-
comp = getCompilation(result.getFile())
487-
}
488-
489-
pragma[noinline]
490-
private Ast::AstNode getBody(Constructor ctor, CompilationExt comp) {
491-
result = ctor.getBody() and
492-
comp = getCompilation(result.getFile())
493-
}
494-
495521
predicate step(PreControlFlowNode n1, PreControlFlowNode n2) {
496-
exists(Constructor ctor |
497-
n1.(EntryNodeImpl).getEnclosingCallable() = ctor and
498-
if Initializers::staticMemberInitializer(ctor, _)
499-
then n2.isBefore(Initializers::initializedStaticMemberOrder(ctor, 0))
500-
else
501-
if exists(ctor.getObjectInitializerCall())
502-
then n2.isBefore(ctor.getObjectInitializerCall())
503-
else
504-
if exists(ctor.getInitializer())
505-
then n2.isBefore(ctor.getInitializer())
506-
else n2.isBefore(ctor.getBody())
507-
or
508-
exists(int i | n1.isAfter(Initializers::initializedStaticMemberOrder(ctor, i)) |
509-
n2.isBefore(Initializers::initializedStaticMemberOrder(ctor, i + 1))
510-
or
511-
not exists(Initializers::initializedStaticMemberOrder(ctor, i + 1)) and
512-
n2.isBefore(ctor.getBody())
513-
)
514-
or
515-
exists(CompilationExt comp |
516-
n1.isAfter(getObjectInitializerCall(ctor, comp)) and
517-
if exists(getInitializer(ctor, comp))
518-
then n2.isBefore(getInitializer(ctor, comp))
519-
else
520-
// This is only relevant in the context of compilation errors, since
521-
// normally the existence of an object initializer call implies the
522-
// existence of an initializer.
523-
if exists(getBody(ctor, comp))
524-
then n2.isBefore(getBody(ctor, comp))
525-
else n2.(NormalExitNodeImpl).getEnclosingCallable() = ctor
526-
or
527-
n1.isAfter(getInitializer(ctor, comp)) and
528-
if exists(getBody(ctor, comp))
529-
then n2.isBefore(getBody(ctor, comp))
530-
else n2.(NormalExitNodeImpl).getEnclosingCallable() = ctor
531-
)
532-
or
533-
n1.isAfter(ctor.getBody()) and
534-
n2.(NormalExitNodeImpl).getEnclosingCallable() = ctor
535-
)
536-
or
537-
exists(ObjectInitMethod obinit |
538-
n1.(EntryNodeImpl).getEnclosingCallable() = obinit and
539-
n2.isBefore(Initializers::initializedInstanceMemberOrder(obinit, _, 0))
540-
or
541-
exists(CompilationExt comp, int i |
542-
// Flow from one member initializer to the next
543-
n1.isAfter(Initializers::initializedInstanceMemberOrder(obinit, comp, i)) and
544-
n2.isBefore(Initializers::initializedInstanceMemberOrder(obinit, comp, i + 1))
545-
)
546-
or
547-
n1.isAfter(Initializers::lastInitializer(obinit, _)) and
548-
n2.(NormalExitNodeImpl).getEnclosingCallable() = obinit
549-
)
550-
or
551522
exists(QualifiableExpr qe | qe.isConditional() |
552523
n1.isBefore(qe) and n2.isBefore(getQualifier(qe))
553524
or

shared/controlflow/codeql/controlflow/ControlFlowGraph.qll

Lines changed: 77 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -38,16 +38,16 @@ signature module AstSig<LocationSig Location> {
3838
Location getLocation();
3939
}
4040

41-
/** Gets the child of this AST node at the specified index. */
41+
/** Gets the child of AST node `n` at the specified index. */
4242
AstNode getChild(AstNode n, int index);
4343

44-
/** Gets the immediately enclosing callable that contains this node. */
44+
/** Gets the immediately enclosing callable that contains `node`. */
4545
Callable getEnclosingCallable(AstNode node);
4646

4747
/** A callable, for example a function, method, constructor, or top-level script. */
4848
class Callable extends AstNode;
4949

50-
/** Gets the body of this callable, if any. */
50+
/** Gets the body of callable `c`, if any. */
5151
AstNode callableGetBody(Callable c);
5252

5353
/** A statement. */
@@ -454,6 +454,26 @@ module Make0<LocationSig Location, AstSig<Location> Ast> {
454454
default predicate successorValueImplies(ConditionalSuccessor t1, ConditionalSuccessor t2) {
455455
none()
456456
}
457+
458+
/**
459+
* An additional context needed to identify the body parts of a callable.
460+
*
461+
* When not used, instantiate with the `Void` type.
462+
*/
463+
class CallableBodyPartContext {
464+
/** Gets a textual representation of this context. */
465+
string toString();
466+
}
467+
468+
/**
469+
* Gets the `index`th part of the body of `c` in context `ctx`.
470+
*
471+
* `callableGetBodyPart(c, _, _)` and `callableGetBody(c)` must never both hold
472+
* for a given callable `c`.
473+
*/
474+
default AstNode callableGetBodyPart(Callable c, CallableBodyPartContext ctx, int index) {
475+
none()
476+
}
457477
}
458478

459479
/**
@@ -661,6 +681,34 @@ module Make0<LocationSig Location, AstSig<Location> Ast> {
661681
* not step to it, since "after" represents normal termination).
662682
*/
663683

684+
private predicate isCallableBodyPart(Callable c, AstNode n) {
685+
n = callableGetBody(c) or n = Input1::callableGetBodyPart(c, _, _)
686+
}
687+
688+
private AstNode getRankedBodyPart(Callable c, Input1::CallableBodyPartContext ctx, int rnk) {
689+
result =
690+
rank[rnk](AstNode child, int ix |
691+
child = Input1::callableGetBodyPart(c, ctx, ix)
692+
|
693+
child order by ix
694+
)
695+
}
696+
697+
private AstNode getBodyEntry(Callable c) {
698+
result = callableGetBody(c)
699+
or
700+
result = getRankedBodyPart(c, _, 1)
701+
}
702+
703+
private AstNode getBodyExit(Callable c) {
704+
result = callableGetBody(c)
705+
or
706+
exists(Input1::CallableBodyPartContext ctx, int last |
707+
result = getRankedBodyPart(c, ctx, last) and
708+
not exists(getRankedBodyPart(c, ctx, last + 1))
709+
)
710+
}
711+
664712
cached
665713
private newtype TNode =
666714
TBeforeNode(AstNode n) { Input1::cfgCachedStageRef() and exists(getEnclosingCallable(n)) } or
@@ -677,9 +725,9 @@ module Make0<LocationSig Location, AstSig<Location> Ast> {
677725
TAdditionalNode(AstNode n, string tag) {
678726
additionalNode(n, tag, _) and exists(getEnclosingCallable(n))
679727
} or
680-
TEntryNode(Callable c) { exists(callableGetBody(c)) } or
681-
TAnnotatedExitNode(Callable c, Boolean normal) { exists(callableGetBody(c)) } or
682-
TExitNode(Callable c) { exists(callableGetBody(c)) }
728+
TEntryNode(Callable c) { isCallableBodyPart(c, _) } or
729+
TAnnotatedExitNode(Callable c, Boolean normal) { isCallableBodyPart(c, _) } or
730+
TExitNode(Callable c) { isCallableBodyPart(c, _) }
683731

684732
private class NodeImpl extends TNode {
685733
/**
@@ -895,7 +943,7 @@ module Make0<LocationSig Location, AstSig<Location> Ast> {
895943
}
896944

897945
/** The `PreControlFlowNode` at the entry point of a callable. */
898-
final class EntryNodeImpl extends NodeImpl, TEntryNode {
946+
final private class EntryNodeImpl extends NodeImpl, TEntryNode {
899947
private Callable c;
900948

901949
EntryNodeImpl() { this = TEntryNode(c) }
@@ -1097,7 +1145,7 @@ module Make0<LocationSig Location, AstSig<Location> Ast> {
10971145
private predicate endAbruptCompletion(AstNode ast, PreControlFlowNode n, AbruptCompletion c) {
10981146
Input2::endAbruptCompletion(ast, n, c)
10991147
or
1100-
exists(Callable callable | ast = callableGetBody(callable) |
1148+
exists(Callable callable | isCallableBodyPart(callable, ast) |
11011149
c.getSuccessorType() instanceof ReturnSuccessor and
11021150
n.(NormalExitNodeImpl).getEnclosingCallable() = callable
11031151
or
@@ -1255,22 +1303,15 @@ module Make0<LocationSig Location, AstSig<Location> Ast> {
12551303
)
12561304
}
12571305

1258-
private predicate hasSpecificCallableSteps(Callable c) {
1259-
exists(EntryNodeImpl entry | entry.getEnclosingCallable() = c and Input2::step(entry, _))
1260-
}
1261-
12621306
/** Holds if there is a local non-abrupt step from `n1` to `n2`. */
12631307
private predicate explicitStep(PreControlFlowNode n1, PreControlFlowNode n2) {
12641308
Input2::step(n1, n2)
12651309
or
12661310
exists(Callable c |
1267-
// Allow language-specific overrides for the default entry and exit edges.
1268-
not hasSpecificCallableSteps(c) and
12691311
n1.(EntryNodeImpl).getEnclosingCallable() = c and
1270-
n2.isBefore(callableGetBody(c))
1312+
n2.isBefore(getBodyEntry(c))
12711313
or
1272-
not hasSpecificCallableSteps(c) and
1273-
n1.isAfter(callableGetBody(c)) and
1314+
n1.isAfter(getBodyExit(c)) and
12741315
n2.(NormalExitNodeImpl).getEnclosingCallable() = c
12751316
or
12761317
n1.(AnnotatedExitNodeImpl).getEnclosingCallable() = c and
@@ -1650,9 +1691,17 @@ module Make0<LocationSig Location, AstSig<Location> Ast> {
16501691
n1.isBefore(ast) and
16511692
n2.isBefore(getRankedChild(ast, 1))
16521693
or
1653-
exists(int i |
1654-
n1.isAfter(getRankedChild(ast, i)) and
1655-
n2.isBefore(getRankedChild(ast, i + 1))
1694+
exists(int i, AstNode prev, AstNode next |
1695+
n1.isAfter(prev) and
1696+
n2.isBefore(next)
1697+
|
1698+
prev = getRankedChild(ast, i) and
1699+
next = getRankedChild(ast, i + 1)
1700+
or
1701+
exists(Input1::CallableBodyPartContext ctx |
1702+
prev = getRankedBodyPart(ast, ctx, i) and
1703+
next = getRankedBodyPart(ast, ctx, i + 1)
1704+
)
16561705
)
16571706
or
16581707
(
@@ -2128,6 +2177,14 @@ module Make0<LocationSig Location, AstSig<Location> Ast> {
21282177
query predicate selfLoop(ControlFlowNode node, SuccessorType t) {
21292178
node.getASuccessor(t) = node
21302179
}
2180+
2181+
/**
2182+
* Holds if `c` is included in both `callableGetBody` and `callableGetBodyPart`.
2183+
*/
2184+
query predicate bodyPartOverlap(Callable c) {
2185+
exists(callableGetBody(c)) and
2186+
exists(Input1::callableGetBodyPart(c, _, _))
2187+
}
21312188
}
21322189
}
21332190
}

0 commit comments

Comments
 (0)