Skip to content

Commit eb97d7b

Browse files
hvitvedhvitved
committed
Revert "C#: Generalize CFG entry/exit nodes to include field/property initializers"
This reverts commit b7e732f.
1 parent 7ab9c8b commit eb97d7b

File tree

12 files changed

+60
-119
lines changed

12 files changed

+60
-119
lines changed

‎csharp/ql/src/semmle/code/csharp/Callable.qll‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,7 @@ private import semmle.code.csharp.metrics.Complexity
1919
* an anonymous function (`AnonymousFunctionExpr`), or a local function
2020
* (`LocalFunction`).
2121
*/
22-
class Callable extends DotNet::Callable, Parameterizable, ExprOrStmtParent, ControlFlowEntryElement,
23-
@callable {
22+
class Callable extends DotNet::Callable, Parameterizable, ExprOrStmtParent, @callable {
2423
override Type getReturnType() { none() }
2524

2625
/** Gets the annotated return type of this callable. */

‎csharp/ql/src/semmle/code/csharp/Enclosing.qll‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -54,30 +54,30 @@ module Internal {
5454
getAChildExpr+(s) = e
5555
}
5656

57-
private predicate childExprOfEntryElement(ControlFlowEntryElement parent, Expr child) {
57+
private predicate childExprOfCallable(Callable parent, Expr child) {
5858
child = getAChildExpr(parent)
5959
or
60-
exists(Expr mid | childExprOfEntryElement(parent, mid) |
61-
not mid instanceof ControlFlowEntryElement and
60+
exists(Expr mid | childExprOfCallable(parent, mid) |
61+
not mid instanceof Callable and
6262
child = getAChildExpr(mid)
6363
)
6464
}
6565

6666
/**
6767
* INTERNAL: Do not use.
6868
*
69-
* Holds if `entry` is the enclosing entry element of expression `e`.
69+
* Holds if `c` is the enclosing callable of expression `e`.
7070
*/
7171
cached
72-
predicate exprEnclosingEntryElement(Expr e, ControlFlowEntryElement entry) {
72+
predicate exprEnclosingCallable(Expr e, Callable c) {
7373
// Compute the enclosing callable of an expression. Note that expressions in
7474
// lambda functions should have the lambdas as enclosing callables, and their
7575
// enclosing statement may be the same as the enclosing statement of the
7676
// lambda; thus, it is *not* safe to go up to the enclosing statement and
7777
// take its own enclosing callable.
78-
childExprOfEntryElement(entry, e)
78+
childExprOfCallable(c, e)
7979
or
80-
not childExprOfEntryElement(_, e) and
81-
exists(Stmt s | enclosingStmt(e, s) | enclosingCallable(s, entry))
80+
not childExprOfCallable(_, e) and
81+
exists(Stmt s | enclosingStmt(e, s) | enclosingCallable(s, c))
8282
}
8383
}

‎csharp/ql/src/semmle/code/csharp/ExprOrStmtParent.qll‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import csharp
1212
* The `expr_parent_top_level()` relation extended to include a relation
1313
* between getters and expression bodies in properties such as `int P => 0`.
1414
*/
15-
predicate expr_parent_top_level_adjusted(Expr child, int i, ControlFlowEntryElement parent) {
15+
predicate expr_parent_top_level_adjusted(Expr child, int i, @top_level_exprorstmt_parent parent) {
1616
expr_parent_top_level(child, i, parent)
1717
or
1818
parent = any(Getter g | expr_parent_top_level(child, i, g.getDeclaration())) and
@@ -108,7 +108,7 @@ class ExprOrStmtParent extends Element, @exprorstmt_parent {
108108
*
109109
* An element that can have a child top-level expression.
110110
*/
111-
class TopLevelExprParent extends ControlFlowEntryElement, @top_level_expr_parent {
111+
class TopLevelExprParent extends Element, @top_level_expr_parent {
112112
final override Expr getChild(int i) { result = this.getChildExpr(i) }
113113

114114
/** Gets the `i`th child expression of this element (zero-based). */

‎csharp/ql/src/semmle/code/csharp/controlflow/BasicBlocks.qll‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -74,13 +74,8 @@ class BasicBlock extends TBasicBlockStart {
7474
/** Gets the last control flow node in this basic block. */
7575
ControlFlow::Node getLastNode() { result = getNode(length() - 1) }
7676

77-
/** Gets the callable that this basic block belongs to, if any. */
78-
final Callable getCallable() { result = this.getEntryElement() }
79-
80-
/** Gets the entry element that this basic block belongs to. */
81-
final ControlFlowEntryElement getEntryElement() {
82-
result = this.getFirstNode().getEnclosingElement()
83-
}
77+
/** Gets the callable that this basic block belongs to. */
78+
Callable getCallable() { result = this.getAPredecessor().getCallable() }
8479

8580
/** Gets the length of this basic block. */
8681
int length() { result = strictcount(getANode()) }
@@ -347,10 +342,12 @@ private import Internal
347342

348343
/**
349344
* An entry basic block, that is, a basic block whose first node is
350-
* an entry node (`ControlFlow::Nodes::EntryNode`).
345+
* the entry node of a callable.
351346
*/
352347
class EntryBasicBlock extends BasicBlock {
353348
EntryBasicBlock() { entryBB(this) }
349+
350+
override Callable getCallable() { result.getEntryPoint() = this.getFirstNode() }
354351
}
355352

356353
/** Holds if `bb` is an entry basic block. */
@@ -360,7 +357,7 @@ private predicate entryBB(BasicBlock bb) {
360357

361358
/**
362359
* An exit basic block, that is, a basic block whose last node is
363-
* an exit node (`ControlFlow::Nodes::ExitNode`).
360+
* the exit node of a callable.
364361
*/
365362
class ExitBasicBlock extends BasicBlock {
366363
ExitBasicBlock() { exitBB(this) }

‎csharp/ql/src/semmle/code/csharp/controlflow/ControlFlowElement.qll‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,6 @@ private import ControlFlow::BasicBlocks
77
private import SuccessorTypes
88
private import semmle.code.csharp.Caching
99

10-
/**
11-
* An element that contains a top-level statement or expression. Either a callable
12-
* (`Callable`), an attribute (`Attribute`), a field (`Field`), a property
13-
* (`Property`), an indexer (`Indexer`), or a parameter (`Parameter`).
14-
*/
15-
class ControlFlowEntryElement extends Element, @top_level_exprorstmt_parent { }
16-
1710
/**
1811
* A program element that can possess control flow. That is, either a statement or
1912
* an expression.

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

Lines changed: 31 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -239,55 +239,38 @@ module ControlFlow {
239239
/** Holds if this node has more than one successor. */
240240
predicate isBranch() { strictcount(this.getASuccessor()) > 1 }
241241

242-
/** Gets the enclosing callable of this control flow node, if any. */
242+
/** Gets the enclosing callable of this control flow node. */
243243
Callable getEnclosingCallable() { none() }
244-
245-
/** Gets the enclosing entry element of this control flow node. */
246-
ControlFlowEntryElement getEnclosingElement() { none() }
247244
}
248245

249246
/** Provides different types of control flow nodes. */
250247
module Nodes {
251-
private import semmle.code.csharp.Enclosing::Internal
252-
253-
/** An entry node, for example a callable or a field initializer. */
248+
/** A node for a callable entry point. */
254249
class EntryNode extends Node, TEntryNode {
255-
private ControlFlowEntryElement e;
256-
257-
EntryNode() { this = TEntryNode(e) }
258-
259-
/** Gets the callable that this entry applies to, if any. */
260-
Callable getCallable() { result = e }
250+
/** Gets the callable that this entry applies to. */
251+
Callable getCallable() { this = TEntryNode(result) }
261252

262253
override BasicBlocks::EntryBlock getBasicBlock() { result = Node.super.getBasicBlock() }
263254

264255
override Callable getEnclosingCallable() { result = this.getCallable() }
265256

266-
override ControlFlowEntryElement getEnclosingElement() { result = e }
257+
override Location getLocation() { result = getCallable().getLocation() }
267258

268-
override Location getLocation() { result = this.getEnclosingElement().getLocation() }
269-
270-
override string toString() { result = "enter " + this.getEnclosingElement().toString() }
259+
override string toString() { result = "enter " + getCallable().toString() }
271260
}
272261

273-
/** An exit node, for example a callable or a field initializer. */
262+
/** A node for a callable exit point. */
274263
class ExitNode extends Node, TExitNode {
275-
private ControlFlowEntryElement e;
276-
277-
ExitNode() { this = TExitNode(e) }
278-
279-
/** Gets the callable that this exit applies to, if any. */
280-
Callable getCallable() { result = e }
264+
/** Gets the callable that this exit applies to. */
265+
Callable getCallable() { this = TExitNode(result) }
281266

282267
override BasicBlocks::ExitBlock getBasicBlock() { result = Node.super.getBasicBlock() }
283268

284269
override Callable getEnclosingCallable() { result = this.getCallable() }
285270

286-
override ControlFlowEntryElement getEnclosingElement() { result = e }
271+
override Location getLocation() { result = getCallable().getLocation() }
287272

288-
override Location getLocation() { result = this.getEnclosingElement().getLocation() }
289-
290-
override string toString() { result = "exit " + this.getEnclosingElement().toString() }
273+
override string toString() { result = "exit " + getCallable().toString() }
291274
}
292275

293276
/**
@@ -306,10 +289,6 @@ module ControlFlow {
306289

307290
override Callable getEnclosingCallable() { result = cfe.getEnclosingCallable() }
308291

309-
override ControlFlowEntryElement getEnclosingElement() {
310-
exprEnclosingEntryElement(cfe, result)
311-
}
312-
313292
override ControlFlowElement getElement() { result = cfe }
314293

315294
override string toString() {
@@ -1845,32 +1824,27 @@ module ControlFlow {
18451824
}
18461825

18471826
/**
1848-
* Gets the control flow element that is first executed when entering `e`.
1827+
* Gets the control flow element that is first executed when entering
1828+
* callable `c`.
18491829
*/
1850-
ControlFlowElement succEntry(ControlFlowEntryElement e) {
1851-
e = any(Callable c |
1830+
ControlFlowElement succEntry(@top_level_exprorstmt_parent p) {
1831+
p = any(Callable c |
18521832
if exists(c.(Constructor).getInitializer())
18531833
then result = first(c.(Constructor).getInitializer())
18541834
else result = first(c.getBody())
18551835
)
18561836
or
1857-
expr_parent_top_level_adjusted(any(Expr e0 | result = first(e0)), _, e) and
1858-
not e instanceof Callable
1837+
expr_parent_top_level_adjusted(any(Expr e | result = first(e)), _, p) and
1838+
not p instanceof Callable
18591839
}
18601840

18611841
/**
1862-
* Gets the element that is exited when `cfe` finishes with completion `c`,
1842+
* Gets the callable that is exited when `cfe` finishes with completion `c`,
18631843
* if any.
18641844
*/
1865-
ControlFlowEntryElement succExit(ControlFlowElement cfe, Completion c) {
1866-
cfe = last(result.(Callable).getBody(), c) and
1845+
Callable succExit(ControlFlowElement cfe, Completion c) {
1846+
cfe = last(result.getBody(), c) and
18671847
not c instanceof GotoCompletion
1868-
or
1869-
exists(Expr e |
1870-
expr_parent_top_level_adjusted(e, _, result) and
1871-
cfe = last(e, c) and
1872-
not result instanceof Callable
1873-
)
18741848
}
18751849
}
18761850
import Successor
@@ -1885,13 +1859,13 @@ module ControlFlow {
18851859
*/
18861860
cached
18871861
newtype TNode =
1888-
TEntryNode(ControlFlowEntryElement e) {
1862+
TEntryNode(Callable c) {
18891863
Stages::ControlFlowStage::forceCachingInSameStage() and
1890-
succEntrySplits(e, _, _, _)
1864+
succEntrySplits(c, _, _, _)
18911865
} or
1892-
TExitNode(ControlFlowEntryElement e) {
1866+
TExitNode(Callable c) {
18931867
exists(Reachability::SameSplitsBlock b | b.isReachable(_) |
1894-
succExitSplits(b.getAnElement(), _, e, _)
1868+
succExitSplits(b.getAnElement(), _, c, _)
18951869
)
18961870
} or
18971871
TElementNode(ControlFlowElement cfe, Splits splits) {
@@ -1901,20 +1875,18 @@ module ControlFlow {
19011875
/** Gets a successor node of a given flow type, if any. */
19021876
cached
19031877
Node getASuccessorByType(Node pred, SuccessorType t) {
1904-
// Entry node -> body
1905-
exists(ControlFlowElement succElement, Splits succSplits, ControlFlowEntryElement e |
1906-
result = TElementNode(succElement, succSplits) and
1907-
pred = TEntryNode(e) and
1908-
succEntrySplits(e, succElement, succSplits, t)
1878+
// Callable entry node -> callable body
1879+
exists(ControlFlowElement succElement, Splits succSplits |
1880+
result = TElementNode(succElement, succSplits)
1881+
|
1882+
succEntrySplits(pred.(Nodes::EntryNode).getCallable(), succElement, succSplits, t)
19091883
)
19101884
or
19111885
exists(ControlFlowElement predElement, Splits predSplits |
19121886
pred = TElementNode(predElement, predSplits)
19131887
|
1914-
// Element node -> exit node
1915-
exists(ControlFlowEntryElement e | result = TExitNode(e) |
1916-
succExitSplits(predElement, predSplits, e, t)
1917-
)
1888+
// Element node -> callable exit
1889+
succExitSplits(predElement, predSplits, result.(Nodes::ExitNode).getCallable(), t)
19181890
or
19191891
// Element node -> element node
19201892
exists(ControlFlowElement succElement, Splits succSplits, Completion c |

‎csharp/ql/src/semmle/code/csharp/controlflow/internal/Splitting.qll‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -836,7 +836,7 @@ class Splits extends TSplits {
836836
*/
837837
pragma[noinline]
838838
predicate succEntrySplits(
839-
ControlFlowEntryElement pred, ControlFlowElement succ, Splits succSplits, SuccessorType t
839+
@top_level_exprorstmt_parent pred, ControlFlowElement succ, Splits succSplits, SuccessorType t
840840
) {
841841
succ = succEntry(pred) and
842842
t instanceof NormalSuccessor and
@@ -847,9 +847,7 @@ predicate succEntrySplits(
847847
* Holds if `pred` with splits `predSplits` can exit the enclosing callable
848848
* `succ` with type `t`.
849849
*/
850-
predicate succExitSplits(
851-
ControlFlowElement pred, Splits predSplits, ControlFlowEntryElement succ, SuccessorType t
852-
) {
850+
predicate succExitSplits(ControlFlowElement pred, Splits predSplits, Callable succ, SuccessorType t) {
853851
exists(Reachability::SameSplitsBlock b, Completion c | pred = b.getAnElement() |
854852
b.isReachable(predSplits) and
855853
t.matchesCompletion(c) and

‎csharp/ql/src/semmle/code/csharp/exprs/Expr.qll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ class Expr extends DotNet::Expr, ControlFlowElement, @expr {
5757
final Stmt getEnclosingStmt() { enclosingStmt(this, result) }
5858

5959
/** Gets the enclosing callable of this expression, if any. */
60-
override Callable getEnclosingCallable() { exprEnclosingEntryElement(this, result) }
60+
override Callable getEnclosingCallable() { exprEnclosingCallable(this, result) }
6161

6262
/**
6363
* Holds if this expression is generated by the compiler and does not appear

‎csharp/ql/test/library-tests/controlflow/graph/BasicBlock.expected‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -277,11 +277,11 @@
277277
| Foreach.cs:36:10:36:11 | exit M6 | Foreach.cs:36:10:36:11 | exit M6 | 1 |
278278
| Foreach.cs:38:9:39:11 | foreach (... ... in ...) ... | Foreach.cs:38:9:39:11 | foreach (... ... in ...) ... | 1 |
279279
| Foreach.cs:38:26:38:26 | String x | Foreach.cs:39:11:39:11 | ; | 4 |
280-
| Initializers.cs:3:9:3:9 | enter F | Initializers.cs:3:9:3:9 | exit F | 7 |
281-
| Initializers.cs:4:9:4:9 | enter G | Initializers.cs:4:9:4:9 | exit G | 8 |
280+
| Initializers.cs:3:9:3:9 | this access | Initializers.cs:3:9:3:17 | ... = ... | 5 |
281+
| Initializers.cs:4:9:4:9 | this access | Initializers.cs:4:25:4:31 | ... = ... | 6 |
282282
| Initializers.cs:6:5:6:16 | enter Initializers | Initializers.cs:6:5:6:16 | exit Initializers | 3 |
283283
| Initializers.cs:8:10:8:10 | enter M | Initializers.cs:8:10:8:10 | exit M | 20 |
284-
| Initializers.cs:14:16:14:16 | enter H | Initializers.cs:14:16:14:16 | exit H | 4 |
284+
| Initializers.cs:14:20:14:20 | 1 | Initializers.cs:14:16:14:20 | ... = ... | 2 |
285285
| NullCoalescing.cs:3:9:3:10 | enter M1 | NullCoalescing.cs:3:23:3:23 | access to parameter i | 3 |
286286
| NullCoalescing.cs:3:9:3:10 | exit M1 | NullCoalescing.cs:3:9:3:10 | exit M1 | 1 |
287287
| NullCoalescing.cs:3:28:3:28 | 0 | NullCoalescing.cs:3:28:3:28 | 0 | 1 |

‎csharp/ql/test/library-tests/controlflow/graph/BasicBlockDominance.expected‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -564,11 +564,11 @@
564564
| post | Foreach.cs:38:9:39:11 | foreach (... ... in ...) ... | Foreach.cs:38:9:39:11 | foreach (... ... in ...) ... |
565565
| post | Foreach.cs:38:9:39:11 | foreach (... ... in ...) ... | Foreach.cs:38:26:38:26 | String x |
566566
| post | Foreach.cs:38:26:38:26 | String x | Foreach.cs:38:26:38:26 | String x |
567-
| post | Initializers.cs:3:9:3:9 | enter F | Initializers.cs:3:9:3:9 | enter F |
568-
| post | Initializers.cs:4:9:4:9 | enter G | Initializers.cs:4:9:4:9 | enter G |
567+
| post | Initializers.cs:3:9:3:9 | this access | Initializers.cs:3:9:3:9 | this access |
568+
| post | Initializers.cs:4:9:4:9 | this access | Initializers.cs:4:9:4:9 | this access |
569569
| post | Initializers.cs:6:5:6:16 | enter Initializers | Initializers.cs:6:5:6:16 | enter Initializers |
570570
| post | Initializers.cs:8:10:8:10 | enter M | Initializers.cs:8:10:8:10 | enter M |
571-
| post | Initializers.cs:14:16:14:16 | enter H | Initializers.cs:14:16:14:16 | enter H |
571+
| post | Initializers.cs:14:20:14:20 | 1 | Initializers.cs:14:20:14:20 | 1 |
572572
| post | NullCoalescing.cs:3:9:3:10 | enter M1 | NullCoalescing.cs:3:9:3:10 | enter M1 |
573573
| post | NullCoalescing.cs:3:9:3:10 | exit M1 | NullCoalescing.cs:3:9:3:10 | enter M1 |
574574
| post | NullCoalescing.cs:3:9:3:10 | exit M1 | NullCoalescing.cs:3:9:3:10 | exit M1 |
@@ -2111,11 +2111,11 @@
21112111
| pre | Foreach.cs:38:9:39:11 | foreach (... ... in ...) ... | Foreach.cs:38:9:39:11 | foreach (... ... in ...) ... |
21122112
| pre | Foreach.cs:38:9:39:11 | foreach (... ... in ...) ... | Foreach.cs:38:26:38:26 | String x |
21132113
| pre | Foreach.cs:38:26:38:26 | String x | Foreach.cs:38:26:38:26 | String x |
2114-
| pre | Initializers.cs:3:9:3:9 | enter F | Initializers.cs:3:9:3:9 | enter F |
2115-
| pre | Initializers.cs:4:9:4:9 | enter G | Initializers.cs:4:9:4:9 | enter G |
2114+
| pre | Initializers.cs:3:9:3:9 | this access | Initializers.cs:3:9:3:9 | this access |
2115+
| pre | Initializers.cs:4:9:4:9 | this access | Initializers.cs:4:9:4:9 | this access |
21162116
| pre | Initializers.cs:6:5:6:16 | enter Initializers | Initializers.cs:6:5:6:16 | enter Initializers |
21172117
| pre | Initializers.cs:8:10:8:10 | enter M | Initializers.cs:8:10:8:10 | enter M |
2118-
| pre | Initializers.cs:14:16:14:16 | enter H | Initializers.cs:14:16:14:16 | enter H |
2118+
| pre | Initializers.cs:14:20:14:20 | 1 | Initializers.cs:14:20:14:20 | 1 |
21192119
| pre | NullCoalescing.cs:3:9:3:10 | enter M1 | NullCoalescing.cs:3:9:3:10 | enter M1 |
21202120
| pre | NullCoalescing.cs:3:9:3:10 | enter M1 | NullCoalescing.cs:3:9:3:10 | exit M1 |
21212121
| pre | NullCoalescing.cs:3:9:3:10 | enter M1 | NullCoalescing.cs:3:28:3:28 | 0 |

0 commit comments

Comments
 (0)