Skip to content

Commit 9595fc2

Browse files
committed
C#: Some more precise flow.
1 parent c6ae475 commit 9595fc2

File tree

3 files changed

+44
-26
lines changed

3 files changed

+44
-26
lines changed

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

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,9 @@ module AssignableInternal {
286286
/** Holds if the local variable definition is at the top level of the pattern. */
287287
predicate isTopLevel() { this = pm.getPattern().(BindingPatternExpr).getVariableDeclExpr() }
288288

289+
/** Holds of this local variable definition is a part of a tuple pattern. */
290+
predicate isInTuple() { this.getParent() instanceof TuplePatternExpr }
291+
289292
/** Gets the pattern match that this local variable declaration (pattern) belongs to. */
290293
PatternMatch getMatch() { result = pm }
291294
}
@@ -720,12 +723,30 @@ module AssignableDefinitions {
720723
/** Gets the underlying local variable declaration. */
721724
LocalVariableDeclExpr getDeclaration() { result = lvpd }
722725

723-
override Expr getSource() { this.isTopLevel() and result = this.getMatch().getExpr() }
726+
override Expr getSource() { result = this.getMatch().getExpr() }
724727

725728
override string toString() { result = this.getDeclaration().toString() }
729+
}
726730

727-
/** Holds if the local variable definition is at the top level of the pattern. */
728-
predicate isTopLevel() { lvpd.isTopLevel() }
731+
/**
732+
* A local variable definition at the top level of a pattern.
733+
*/
734+
class TopLevelPatternDefinition extends PatternDefinition {
735+
TopLevelPatternDefinition() { lvpd.isTopLevel() }
736+
}
737+
738+
/**
739+
* A local variable definition in a tuple pattern.
740+
*/
741+
class TuplePatternDefinition extends PatternDefinition {
742+
TuplePatternDefinition() { lvpd.isInTuple() }
743+
}
744+
745+
/**
746+
* A local variable definition in a property pattern.
747+
*/
748+
class PropertyPatternDefinition extends PatternDefinition {
749+
PropertyPatternDefinition() { lvpd.getParent() instanceof PropertyPatternExpr }
729750
}
730751

731752
/**

csharp/ql/lib/semmle/code/csharp/dataflow/Nullness.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ private predicate nonNullDef(Ssa::ExplicitDefinition def) {
104104
def.getADefinition().getSource() instanceof NonNullExpr
105105
or
106106
exists(AssignableDefinition ad | ad = def.getADefinition() |
107-
ad instanceof AssignableDefinitions::PatternDefinition
107+
ad instanceof AssignableDefinitions::TopLevelPatternDefinition
108108
or
109109
ad =
110110
any(AssignableDefinitions::LocalVariableDefinition d |

csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -640,31 +640,30 @@ module LocalFlow {
640640
def.getSource() = e and
641641
(
642642
scope = def.getExpr() and
643-
isSuccessor = true
643+
isSuccessor = true and
644+
(
645+
not def instanceof AssignableDefinitions::PatternDefinition or
646+
def instanceof AssignableDefinitions::TopLevelPatternDefinition
647+
)
648+
or
649+
scope = def.(AssignableDefinitions::TopLevelPatternDefinition).getMatch().(IsExpr) and
650+
isSuccessor = false
644651
or
645-
exists(AssignableDefinitions::PatternDefinition def0 | def = def0 and def0.isTopLevel() |
646-
scope = def0.getMatch().(IsExpr) and
647-
isSuccessor = false
652+
exists(Switch s |
653+
s.getACase() = def.(AssignableDefinitions::TopLevelPatternDefinition).getMatch() and
654+
isSuccessor = true
655+
|
656+
scope = s.getExpr()
648657
or
649-
exists(Switch s |
650-
s.getACase() = def0.getMatch() and
651-
isSuccessor = true
652-
|
653-
scope = s.getExpr()
654-
or
655-
scope = s.getACase()
656-
)
658+
scope = s.getACase()
657659
)
658660
)
659661
or
660662
// Needed for read steps for pattern matching involving properties.
661663
scope = def.getExpr() and
662664
exactScope = false and
663665
isSuccessor = false and
664-
def =
665-
any(AssignableDefinitions::PatternDefinition apd |
666-
e = apd.getDeclaration() and not apd.isTopLevel()
667-
)
666+
e = def.(AssignableDefinitions::PropertyPatternDefinition).getDeclaration()
668667
}
669668
}
670669

@@ -2508,7 +2507,7 @@ private class ReadStepConfiguration extends ControlFlowReachabilityConfiguration
25082507
or
25092508
scope =
25102509
any(TuplePatternExpr te |
2511-
te.getAnArgument() = defTo.(AssignableDefinitions::PatternDefinition).getDeclaration() and
2510+
te.getAnArgument() = defTo.(AssignableDefinitions::TuplePatternDefinition).getDeclaration() and
25122511
e = te and
25132512
exactScope = false and
25142513
isSuccessor = false
@@ -2558,7 +2557,7 @@ private predicate readContentStep(Node node1, Content c, Node node2) {
25582557
or
25592558
// item = variable in node1 = (..., variable, ...) in a case/is var (..., ...)
25602559
te = any(TuplePatternExpr pe).getAChildExpr*() and
2561-
exists(AssignableDefinitions::PatternDefinition lvd |
2560+
exists(AssignableDefinitions::TuplePatternDefinition lvd |
25622561
node2.(AssignableDefinitionNode).getDefinition() = lvd and
25632562
lvd.getDeclaration() = item and
25642563
hasNodePath(x, node1, node2)
@@ -2990,10 +2989,8 @@ class CastNode extends Node {
29902989
CastNode() {
29912990
this.asExpr() instanceof Cast
29922991
or
2993-
this.(AssignableDefinitionNode)
2994-
.getDefinition()
2995-
.(AssignableDefinitions::PatternDefinition)
2996-
.isTopLevel()
2992+
this.(AssignableDefinitionNode).getDefinition() instanceof
2993+
AssignableDefinitions::TopLevelPatternDefinition
29972994
}
29982995
}
29992996

0 commit comments

Comments
 (0)