Skip to content

Commit 5ce9b25

Browse files
committed
C#: Improve CFG for assignments
Write accesses in assignments, such as the access to `x` in `x = 0` are not evaluated, so they should not have entries in the control flow graph. However, qualifiers (and indexer arguments) should still be evaluated, for example in ``` x.Foo.Bar = 0; ``` the CFG should be `x --> x.Foo --> 0 --> x.Foo.Bar = 0` (as opposed to `x --> x.Foo --> x.Foo.Bar --> 0 --> x.Foo.Bar = 0`, prior to this change). A special case is assignments via acessors (properties, indexers, and event adders), where we do want to include the access in the control flow graph, as it represents the accessor call: ``` x.Prop = 0; ``` But instead of `x --> x.set_Prop --> 0 --> x.Prop = 0` the CFG should be `x --> 0 --> x.set_Prop --> x.Prop = 0`, as the setter is called *after* the assigned value has been evaluated. An even more special case is tuple assignments via accessors: ``` (x.Prop1, y.Prop2) = (0, 1); ``` Here the CFG should be `x --> y --> 0 --> 1 --> x.set_Prop1 --> y.set_Prop2 --> (x.Prop1, y.Prop2) = (0, 1)`.
1 parent 096757d commit 5ce9b25

File tree

18 files changed

+927
-1318
lines changed

18 files changed

+927
-1318
lines changed

csharp/ql/src/semmle/code/csharp/Assignable.qll

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -601,20 +601,19 @@ module AssignableDefinitions {
601601
/** Gets the underlying call. */
602602
Call getCall() { result.getAnArgument() = aa }
603603

604+
private int getPosition() { aa = this.getCall().getArgument(result) }
605+
604606
/**
605607
* Gets the index of this definition among the other definitions in the
606608
* `out`/`ref` assignment. For example, in `M(out x, ref y)` the index of
607609
* the definitions of `x` and `y` are 0 and 1, respectively.
608610
*/
609611
int getIndex() {
610-
exists(ControlFlow::BasicBlock bb, int i | bb.getNode(i).getElement() = aa |
611-
i = rank[result + 1](int j, OutRefDefinition def |
612-
bb.getNode(j).getElement() = def.getTargetAccess() and
613-
this.getCall() = def.getCall()
614-
|
615-
j
616-
)
617-
)
612+
this = rank[result + 1](OutRefDefinition def |
613+
def.getCall() = this.getCall()
614+
|
615+
def order by def.getPosition()
616+
)
618617
}
619618

620619
override predicate isCertain() { not isUncertainRefCall(this.getTargetAccess()) }

0 commit comments

Comments
 (0)