Skip to content

Commit e571138

Browse files
committed
C#: Make postupdate notes for conditional branches.
1 parent 7b6e684 commit e571138

File tree

1 file changed

+40
-8
lines changed

1 file changed

+40
-8
lines changed

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

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,32 @@ predicate hasNodePath(ControlFlowReachabilityConfiguration conf, ExprNode n1, No
177177
)
178178
}
179179

180+
/**
181+
* Gets a node that may execute last in `n`, and which, when it executes last,
182+
* will be the value of `n`.
183+
*/
184+
private ControlFlow::Nodes::ExprNode getALastEvalNode(ControlFlow::Nodes::ExprNode cfn) {
185+
exists(ConditionalExpr e | cfn.getExpr() = e | result.getExpr() = [e.getThen(), e.getElse()])
186+
}
187+
188+
private predicate relevantArgumentType(ControlFlow::Nodes::ExprNode cfn) {
189+
exists(Expr e | cfn.getExpr() = e |
190+
exists(Type t | t = e.stripCasts().getType() |
191+
t instanceof RefType and
192+
not t instanceof NullType
193+
or
194+
t = any(TypeParameter tp | not tp.isValueType())
195+
)
196+
)
197+
}
198+
199+
/** Gets a node for which to construct a post-update node for argument `arg`. */
200+
private ControlFlow::Nodes::ExprNode getAPostUpdateNodeForArg(Argument arg) {
201+
result = getALastEvalNode*(arg.getAControlFlowNode()) and
202+
relevantArgumentType(result) and
203+
not exists(getALastEvalNode(result))
204+
}
205+
180206
/** Provides predicates related to local data flow. */
181207
module LocalFlow {
182208
private class LocalExprStepConfiguration extends ControlFlowReachabilityConfiguration {
@@ -719,14 +745,9 @@ private module Cached {
719745
cfn.getElement().(ObjectCreation).hasInitializer()
720746
} or
721747
TExprPostUpdateNode(ControlFlow::Nodes::ExprNode cfn) {
748+
cfn = getAPostUpdateNodeForArg(_)
749+
or
722750
exists(Expr e | e = cfn.getExpr() |
723-
exists(Type t | t = e.(Argument).stripCasts().getType() |
724-
t instanceof RefType and
725-
not t instanceof NullType
726-
or
727-
t = any(TypeParameter tp | not tp.isValueType())
728-
)
729-
or
730751
fieldOrPropertyStore(_, _, _, e, true)
731752
or
732753
arrayStore(_, _, e, true)
@@ -1921,7 +1942,18 @@ private module PostUpdateNodes {
19211942

19221943
ExprPostUpdateNode() { this = TExprPostUpdateNode(cfn) }
19231944

1924-
override ExprNode getPreUpdateNode() { cfn = result.getControlFlowNode() }
1945+
override ExprNode getPreUpdateNode() {
1946+
// For compund arguments, such as `m(if b then x else y)`, we want the leaf nodes
1947+
// `[post] x` and `[post] y` to have two pre-update nodes: (1) the compund argument,
1948+
// `if b then x else y`; and the (2) the underlying expressions; `x` and `y`,
1949+
// respectively.
1950+
//
1951+
// This ensures that we get flow out of the call into both leafs (1), while still
1952+
// maintaining the invariant that the underlying expression is a pre-update node (2).
1953+
cfn = getAPostUpdateNodeForArg(result.asExpr())
1954+
or
1955+
cfn = result.getControlFlowNode()
1956+
}
19251957

19261958
override DataFlowCallable getEnclosingCallableImpl() {
19271959
result.asCallable() = cfn.getEnclosingCallable()

0 commit comments

Comments
 (0)