Skip to content

Commit 972fb91

Browse files
committed
C#: Add CFG support for null conditional assignments and include eg. field access in the CFG.
1 parent 8257475 commit 972fb91

File tree

1 file changed

+52
-17
lines changed

1 file changed

+52
-17
lines changed

csharp/ql/lib/semmle/code/csharp/controlflow/internal/ControlFlowGraphImpl.qll

Lines changed: 52 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,8 @@ module Expressions {
429429
not this instanceof ObjectCreation and
430430
not this instanceof ArrayCreation and
431431
not this instanceof QualifiedWriteAccess and
432-
not this instanceof AccessorWrite and
432+
not this instanceof QualifiedAssignments and
433+
not this instanceof QualifiedAccessorWrite and
433434
not this instanceof NoNodeExpr and
434435
not this instanceof SwitchExpr and
435436
not this instanceof SwitchCaseExpr and
@@ -481,11 +482,36 @@ module Expressions {
481482
}
482483
}
483484

484-
private class StatOrDynAccessorCall_ =
485-
@dynamic_member_access_expr or @dynamic_element_access_expr or @call_access_expr;
485+
private class QualifiedAssignments extends PostOrderTree instanceof Assignment {
486+
QualifiedWriteAccess left;
487+
Expr right;
486488

487-
/** A normal or a (potential) dynamic call to an accessor. */
488-
private class StatOrDynAccessorCall extends Expr, StatOrDynAccessorCall_ { }
489+
QualifiedAssignments() {
490+
left = this.getLValue() and
491+
right = this.getRValue() and
492+
not this instanceof AssignOperationWithExpandedAssignment
493+
}
494+
495+
final override predicate propagatesAbnormal(AstNode child) { child = [left, right] }
496+
497+
final override predicate first(AstNode first) { first(left, first) }
498+
499+
final override predicate last(AstNode last, Completion c) {
500+
PostOrderTree.super.last(last, c)
501+
or
502+
last(left, last, c) and
503+
left.(QualifiableExpr).isConditional() and
504+
c.(NullnessCompletion).isNull()
505+
}
506+
507+
final override predicate succ(AstNode pred, AstNode succ, Completion c) {
508+
// Standard left-to-right evaluation
509+
last(left, pred, c) and
510+
c instanceof NormalCompletion and
511+
not c.(NullnessCompletion).isNull() and
512+
first(right, succ)
513+
}
514+
}
489515

490516
/**
491517
* An expression that writes via an accessor call, for example `x.Prop = 0`,
@@ -499,23 +525,23 @@ module Expressions {
499525
* x -> 0 -> set_Prop -> x.Prop = 0
500526
* ```
501527
*/
502-
class AccessorWrite extends PostOrderTree instanceof Expr {
528+
private class QualifiedAccessorWrite extends PostOrderTree instanceof Expr {
503529
AssignableDefinition def;
504530

505-
AccessorWrite() {
531+
QualifiedAccessorWrite() {
506532
def.getExpr() = this and
507-
def.getTargetAccess().(WriteAccess) instanceof StatOrDynAccessorCall and
533+
def.getTargetAccess() instanceof QualifiedWriteAccess and
508534
not this instanceof AssignOperationWithExpandedAssignment
509535
}
510536

511537
/**
512538
* Gets the `i`th accessor being called in this write. More than one call
513539
* can happen in tuple assignments.
514540
*/
515-
StatOrDynAccessorCall getCall(int i) {
541+
WriteAccess getAccess(int i) {
516542
result =
517543
rank[i + 1](AssignableDefinitions::TupleAssignmentDefinition tdef |
518-
tdef.getExpr() = this and tdef.getTargetAccess() instanceof StatOrDynAccessorCall
544+
tdef.getExpr() = this
519545
|
520546
tdef order by tdef.getEvaluationOrder()
521547
).getTargetAccess()
@@ -528,7 +554,13 @@ module Expressions {
528554
final override predicate propagatesAbnormal(AstNode child) {
529555
child = getExprChild(this, _)
530556
or
531-
child = this.getCall(_)
557+
child = this.getAccess(_)
558+
}
559+
560+
final override predicate last(AstNode last, Completion c) {
561+
PostOrderTree.super.last(last, c)
562+
or
563+
last(getExprChild(this, 0), last, c) and c.(NullnessCompletion).isNull()
532564
}
533565

534566
final override predicate first(AstNode first) { first(getExprChild(this, 0), first) }
@@ -538,24 +570,25 @@ module Expressions {
538570
exists(int i |
539571
last(getExprChild(this, i), pred, c) and
540572
c instanceof NormalCompletion and
573+
(i != 0 or not c.(NullnessCompletion).isNull()) and
541574
first(getExprChild(this, i + 1), succ)
542575
)
543576
or
544577
// Flow from last element of last child to first accessor call
545578
last(getLastExprChild(this), pred, c) and
546-
succ = this.getCall(0) and
579+
succ = this.getAccess(0) and
547580
c instanceof NormalCompletion
548581
or
549582
// Flow from one call to the next
550-
exists(int i | pred = this.getCall(i) |
551-
succ = this.getCall(i + 1) and
583+
exists(int i | pred = this.getAccess(i) |
584+
succ = this.getAccess(i + 1) and
552585
c.isValidFor(pred) and
553586
c instanceof NormalCompletion
554587
)
555588
or
556589
// Post-order: flow from last call to element itself
557-
exists(int last | last = max(int i | exists(this.getCall(i))) |
558-
pred = this.getCall(last) and
590+
exists(int last | last = max(int i | exists(this.getAccess(i))) |
591+
pred = this.getAccess(last) and
559592
succ = this and
560593
c.isValidFor(pred) and
561594
c instanceof NormalCompletion
@@ -704,7 +737,9 @@ module Expressions {
704737
private class ConditionallyQualifiedExpr extends PostOrderTree instanceof QualifiableExpr {
705738
private Expr qualifier;
706739

707-
ConditionallyQualifiedExpr() { this.isConditional() and qualifier = getExprChild(this, 0) }
740+
ConditionallyQualifiedExpr() {
741+
this.isConditional() and qualifier = getExprChild(this, 0) and not this instanceof WriteAccess
742+
}
708743

709744
final override predicate propagatesAbnormal(AstNode child) { child = qualifier }
710745

0 commit comments

Comments
 (0)