Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions change-notes/1.18/analysis-csharp.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@

## Changes to code extraction

* The `into` part of `join` clauses is now extracted.
* The `when` part of constant cases is now extracted.
* Fixed a bug where `while(x is T y) ...` was not extracted correctly.

* *Series of bullet points*

## Changes to QL libraries
Expand All @@ -59,3 +63,4 @@
- `ControlFlowEdgeGotoCase` has been renamed to `ControlFlow::SuccessorTypes::GotoCaseSuccessor`.
- `ControlFlowEdgeGotoDefault` has been renamed to `ControlFlow::SuccessorTypes::GotoDefaultSuccessor`.
- `ControlFlowEdgeException` has been renamed to `ControlFlow::SuccessorTypes::ExceptionSuccessor`.
* The predicate `getCondition()` has been moved from `TypeCase` to `CaseStmt`. It is now possible to get the condition of a `ConstCase` using its `getCondition()` predicate.
52 changes: 27 additions & 25 deletions csharp/ql/src/semmle/code/csharp/Stmt.qll
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,25 @@ class SwitchStmt extends SelectionStmt, @switch_stmt {
* A `case` statement. Either a constant case (`ConstCase`), a type matching
* case (`TypeCase`), or a `default` case (`DefaultCase`).
*/
class CaseStmt extends Stmt, @case { }
class CaseStmt extends Stmt, @case {
/**
* Gets the condition on this case, if any. For example, the type case on line 3
* has no condition, and the type case on line 4 has condition `s.Length > 0`, in
*
* ```
* switch(p)
* {
* case int i:
* case string s when s.Length > 0:
* break;
* ...
* }
* ```
*/
Expr getCondition() {
result = this.getChild(2)
}
}

/**
* A constant case of a `switch` statement, for example `case OpCode.Nop:`
Expand Down Expand Up @@ -280,15 +298,16 @@ class ConstCase extends LabeledStmt, CaseStmt {
}

/**
* A type matching case in a `switch` statement, for example `case int i:` on line 2 or
* `case string s when s.Length>0:` on line 3 in
* A type matching case in a `switch` statement, for example `case int i:` on line 3 or
* `case string s when s.Length > 0:` on line 4 in
*
* ```
* switch(p) {
* case int i:
* case string s when s.Length>0:
* break;
* ...
* switch(p)
* {
* case int i:
* case string s when s.Length > 0:
* break;
* ...
* }
* ```
*/
Expand Down Expand Up @@ -350,23 +369,6 @@ class TypeCase extends LabeledStmt, CaseStmt {
result = getTypeAccess().getType()
}

/**
* Gets the condition on this case, if any. For example, the type case on line 2
* has no condition, and the type case on line 3 has condition `s.Length>0`, in
*
* ```
* switch(p) {
* case int i:
* case string s when s.Length>0:
* break;
* ...
* }
* ```
*/
Expr getCondition() {
result = getChild(2)
}

override string toString() {
exists(string var |
if exists(this.getVariableDeclExpr()) then
Expand Down
4 changes: 2 additions & 2 deletions csharp/ql/src/semmle/code/csharp/controlflow/Completion.qll
Original file line number Diff line number Diff line change
Expand Up @@ -309,8 +309,8 @@ private predicate inBooleanContext(Expr e, boolean isBooleanCompletionForParent)
isBooleanCompletionForParent = false
)
or
exists(TypeCase tc |
tc.getCondition() = e |
exists(CaseStmt cs |
cs.getCondition() = e |
isBooleanCompletionForParent = false
)
or
Expand Down
48 changes: 27 additions & 21 deletions csharp/ql/src/semmle/code/csharp/controlflow/ControlFlowGraph.qll
Original file line number Diff line number Diff line change
Expand Up @@ -1272,25 +1272,24 @@ module ControlFlow {
// Case expression exits abnormally
result = lastConstCaseExpr(cc, c) and
not c instanceof NormalCompletion
or
// Case statement exits with any completion
result = lastConstCaseStmt(cc, c)
)
or
cfe = any(TypeCase tc |
// Type test exits with a non-match
result = lastTypeCaseNoMatch(tc, c)
or
)
or
cfe = any(CaseStmt cs |
// Condition exists with a `false` completion
result = lastTypeCaseCondition(tc, c) and
result = lastCaseCondition(cs, c) and
c instanceof FalseCompletion
or
// Condition exists abnormally
result = lastTypeCaseCondition(tc, c) and
result = lastCaseCondition(cs, c) and
not c instanceof NormalCompletion
or
// Case statement exits with any completion
result = lastTypeCaseStmt(tc, c)
result = lastCaseStmt(cs, c)
)
or
exists(LoopStmt ls |
Expand Down Expand Up @@ -1576,25 +1575,22 @@ module ControlFlow {
}

pragma [nomagic]
private ControlFlowElement lastConstCaseStmt(ConstCase cc, Completion c) {
result = last(cc.getStmt(), c)
private ControlFlowElement lastCaseStmt(CaseStmt cs, Completion c) {
result = last(cs.(TypeCase).getStmt(), c)
or
result = last(cs.(ConstCase).getStmt(), c)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could replace with

result = last(cs.(LabeledStmt).getStmt(), c)

Feel free to change if you like.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did that initially but additional edges appeared.

}

pragma [nomagic]
private ControlFlowElement lastTypeCaseCondition(TypeCase tc, Completion c) {
result = last(tc.getCondition(), c)
private ControlFlowElement lastCaseCondition(CaseStmt cs, Completion c) {
result = last(cs.getCondition(), c)
}

pragma [nomagic]
private ControlFlowElement lastTypeCaseVariableDeclExpr(TypeCase tc, Completion c) {
result = last(tc.getVariableDeclExpr(), c)
}

pragma [nomagic]
private ControlFlowElement lastTypeCaseStmt(TypeCase tc, Completion c) {
result = last(tc.getStmt(), c)
}

pragma [nomagic]
private ControlFlowElement lastLoopStmtCondition(LoopStmt ls, Completion c) {
result = last(ls.getCondition(), c)
Expand Down Expand Up @@ -2032,9 +2028,9 @@ module ControlFlow {
)
or
// Flow from last element of condition to next case
exists(TypeCase tc, int i |
exists(CaseStmt tc, int i |
tc = ss.getCase(i) |
cfe = lastTypeCaseCondition(tc, c) and
cfe = lastCaseCondition(tc, c) and
c instanceof FalseCompletion and
result = first(ss.getCase(i + 1))
)
Expand Down Expand Up @@ -2063,9 +2059,19 @@ module ControlFlow {
result = first(cc.getExpr()) and
c instanceof SimpleCompletion
or
// Flow from last element of case expression to first element of statement
cfe = lastConstCaseExpr(cc, c) and
c.(MatchingCompletion).isMatch() and
c.(MatchingCompletion).isMatch() and (
if exists(cc.getCondition()) then
// Flow from the last element of case expression to the condition
result = first(cc.getCondition())
else
// Flow from last element of case expression to first element of statement
result = first(cc.getStmt())
)
or
// Flow from last element of case condition to first element of statement
cfe = lastCaseCondition(cc, c) and
c instanceof TrueCompletion and
result = first(cc.getStmt())
)
or
Expand Down Expand Up @@ -2105,7 +2111,7 @@ module ControlFlow {
result = first(tc.getStmt())
or
// Flow from condition to first element of statement
cfe = lastTypeCaseCondition(tc, c) and
cfe = lastCaseCondition(tc, c) and
c instanceof TrueCompletion and
result = first(tc.getStmt())
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,14 @@
| Switch.cs:106:29:106:29 | 1 | Switch.cs:106:22:106:30 | return ...; | 2 |
| Switch.cs:108:17:108:17 | 1 | Switch.cs:108:9:108:18 | return ...; | 3 |
| Switch.cs:111:17:111:21 | enter Throw | Switch.cs:111:17:111:21 | exit Throw | 4 |
| Switch.cs:113:9:113:11 | enter M10 | Switch.cs:117:18:117:18 | 3 | 7 |
| Switch.cs:113:9:113:11 | exit M10 | Switch.cs:113:9:113:11 | exit M10 | 1 |
| Switch.cs:117:25:117:25 | access to parameter s | Switch.cs:117:25:117:32 | ... == ... | 3 |
| Switch.cs:117:43:117:43 | 1 | Switch.cs:117:36:117:44 | return ...; | 2 |
| Switch.cs:118:13:118:33 | case ...: | Switch.cs:118:18:118:18 | 2 | 2 |
| Switch.cs:118:25:118:25 | access to parameter s | Switch.cs:118:25:118:31 | ... == ... | 3 |
| Switch.cs:118:42:118:42 | 2 | Switch.cs:118:35:118:43 | return ...; | 2 |
| Switch.cs:120:17:120:17 | 1 | Switch.cs:120:9:120:18 | return ...; | 3 |
| TypeAccesses.cs:3:10:3:10 | enter M | TypeAccesses.cs:7:13:7:22 | ... is ... | 16 |
| TypeAccesses.cs:7:25:7:25 | ; | TypeAccesses.cs:7:25:7:25 | ; | 1 |
| TypeAccesses.cs:8:9:8:28 | ... ...; | TypeAccesses.cs:3:10:3:10 | exit M | 5 |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,21 @@
| post | Switch.cs:106:29:106:29 | 1 | Switch.cs:106:29:106:29 | 1 |
| post | Switch.cs:108:17:108:17 | 1 | Switch.cs:108:17:108:17 | 1 |
| post | Switch.cs:111:17:111:21 | enter Throw | Switch.cs:111:17:111:21 | enter Throw |
| post | Switch.cs:113:9:113:11 | enter M10 | Switch.cs:113:9:113:11 | enter M10 |
| post | Switch.cs:113:9:113:11 | exit M10 | Switch.cs:113:9:113:11 | enter M10 |
| post | Switch.cs:113:9:113:11 | exit M10 | Switch.cs:113:9:113:11 | exit M10 |
| post | Switch.cs:113:9:113:11 | exit M10 | Switch.cs:117:25:117:25 | access to parameter s |
| post | Switch.cs:113:9:113:11 | exit M10 | Switch.cs:117:43:117:43 | 1 |
| post | Switch.cs:113:9:113:11 | exit M10 | Switch.cs:118:13:118:33 | case ...: |
| post | Switch.cs:113:9:113:11 | exit M10 | Switch.cs:118:25:118:25 | access to parameter s |
| post | Switch.cs:113:9:113:11 | exit M10 | Switch.cs:118:42:118:42 | 2 |
| post | Switch.cs:113:9:113:11 | exit M10 | Switch.cs:120:17:120:17 | 1 |
| post | Switch.cs:117:25:117:25 | access to parameter s | Switch.cs:117:25:117:25 | access to parameter s |
| post | Switch.cs:117:43:117:43 | 1 | Switch.cs:117:43:117:43 | 1 |
| post | Switch.cs:118:13:118:33 | case ...: | Switch.cs:118:13:118:33 | case ...: |
| post | Switch.cs:118:25:118:25 | access to parameter s | Switch.cs:118:25:118:25 | access to parameter s |
| post | Switch.cs:118:42:118:42 | 2 | Switch.cs:118:42:118:42 | 2 |
| post | Switch.cs:120:17:120:17 | 1 | Switch.cs:120:17:120:17 | 1 |
| post | TypeAccesses.cs:3:10:3:10 | enter M | TypeAccesses.cs:3:10:3:10 | enter M |
| post | TypeAccesses.cs:7:25:7:25 | ; | TypeAccesses.cs:7:25:7:25 | ; |
| post | TypeAccesses.cs:8:9:8:28 | ... ...; | TypeAccesses.cs:3:10:3:10 | enter M |
Expand Down Expand Up @@ -1709,6 +1724,26 @@
| pre | Switch.cs:106:29:106:29 | 1 | Switch.cs:106:29:106:29 | 1 |
| pre | Switch.cs:108:17:108:17 | 1 | Switch.cs:108:17:108:17 | 1 |
| pre | Switch.cs:111:17:111:21 | enter Throw | Switch.cs:111:17:111:21 | enter Throw |
| pre | Switch.cs:113:9:113:11 | enter M10 | Switch.cs:113:9:113:11 | enter M10 |
| pre | Switch.cs:113:9:113:11 | enter M10 | Switch.cs:113:9:113:11 | exit M10 |
| pre | Switch.cs:113:9:113:11 | enter M10 | Switch.cs:117:25:117:25 | access to parameter s |
| pre | Switch.cs:113:9:113:11 | enter M10 | Switch.cs:117:43:117:43 | 1 |
| pre | Switch.cs:113:9:113:11 | enter M10 | Switch.cs:118:13:118:33 | case ...: |
| pre | Switch.cs:113:9:113:11 | enter M10 | Switch.cs:118:25:118:25 | access to parameter s |
| pre | Switch.cs:113:9:113:11 | enter M10 | Switch.cs:118:42:118:42 | 2 |
| pre | Switch.cs:113:9:113:11 | enter M10 | Switch.cs:120:17:120:17 | 1 |
| pre | Switch.cs:113:9:113:11 | exit M10 | Switch.cs:113:9:113:11 | exit M10 |
| pre | Switch.cs:117:25:117:25 | access to parameter s | Switch.cs:117:25:117:25 | access to parameter s |
| pre | Switch.cs:117:25:117:25 | access to parameter s | Switch.cs:117:43:117:43 | 1 |
| pre | Switch.cs:117:43:117:43 | 1 | Switch.cs:117:43:117:43 | 1 |
| pre | Switch.cs:118:13:118:33 | case ...: | Switch.cs:118:13:118:33 | case ...: |
| pre | Switch.cs:118:13:118:33 | case ...: | Switch.cs:118:25:118:25 | access to parameter s |
| pre | Switch.cs:118:13:118:33 | case ...: | Switch.cs:118:42:118:42 | 2 |
| pre | Switch.cs:118:13:118:33 | case ...: | Switch.cs:120:17:120:17 | 1 |
| pre | Switch.cs:118:25:118:25 | access to parameter s | Switch.cs:118:25:118:25 | access to parameter s |
| pre | Switch.cs:118:25:118:25 | access to parameter s | Switch.cs:118:42:118:42 | 2 |
| pre | Switch.cs:118:42:118:42 | 2 | Switch.cs:118:42:118:42 | 2 |
| pre | Switch.cs:120:17:120:17 | 1 | Switch.cs:120:17:120:17 | 1 |
| pre | TypeAccesses.cs:3:10:3:10 | enter M | TypeAccesses.cs:3:10:3:10 | enter M |
| pre | TypeAccesses.cs:3:10:3:10 | enter M | TypeAccesses.cs:7:25:7:25 | ; |
| pre | TypeAccesses.cs:3:10:3:10 | enter M | TypeAccesses.cs:8:9:8:28 | ... ...; |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@
| Switch.cs:50:30:50:38 | ... != ... | Switch.cs:51:17:51:22 | break; | true |
| Switch.cs:84:19:84:23 | ... > ... | Switch.cs:85:17:85:22 | break; | true |
| Switch.cs:84:19:84:23 | ... > ... | Switch.cs:86:22:86:25 | true | false |
| Switch.cs:117:25:117:32 | ... == ... | Switch.cs:117:43:117:43 | 1 | true |
| Switch.cs:118:25:118:31 | ... == ... | Switch.cs:118:42:118:42 | 2 | true |
| TypeAccesses.cs:7:13:7:22 | ... is ... | TypeAccesses.cs:7:25:7:25 | ; | true |
| VarDecls.cs:25:20:25:20 | access to parameter b | VarDecls.cs:25:24:25:24 | access to local variable x | true |
| VarDecls.cs:25:20:25:20 | access to parameter b | VarDecls.cs:25:28:25:28 | access to local variable y | false |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,10 @@
| 108 | 13 | cflow.cs:108:13:108:21 | ... != ... | false | 116 | 9 | cflow.cs:116:9:116:29 | ...; |
| 108 | 13 | cflow.cs:108:13:108:21 | ... != ... | true | 109 | 9 | cflow.cs:109:9:115:9 | {...} |
| 110 | 20 | cflow.cs:110:20:110:23 | true | true | 111 | 13 | cflow.cs:111:13:113:13 | {...} |
| 117 | 25 | Switch.cs:117:25:117:32 | ... == ... | false | 118 | 13 | Switch.cs:118:13:118:33 | case ...: |
| 117 | 25 | Switch.cs:117:25:117:32 | ... == ... | true | 117 | 43 | Switch.cs:117:43:117:43 | 1 |
| 118 | 25 | Switch.cs:118:25:118:31 | ... == ... | false | 120 | 17 | Switch.cs:120:17:120:17 | 1 |
| 118 | 25 | Switch.cs:118:25:118:31 | ... == ... | true | 118 | 42 | Switch.cs:118:42:118:42 | 2 |
| 127 | 32 | cflow.cs:127:32:127:44 | ... == ... | false | 127 | 53 | cflow.cs:127:53:127:57 | this access |
| 127 | 32 | cflow.cs:127:32:127:44 | ... == ... | true | 127 | 48 | cflow.cs:127:48:127:49 | "" |
| 162 | 48 | cflow.cs:162:48:162:51 | [exception: Exception] true | true | 163 | 9 | cflow.cs:163:9:165:9 | {...} |
Expand Down
39 changes: 39 additions & 0 deletions csharp/ql/test/library-tests/controlflow/graph/Dominance.expected
Original file line number Diff line number Diff line change
Expand Up @@ -819,6 +819,24 @@
| post | Switch.cs:111:17:111:21 | exit Throw | Switch.cs:111:28:111:48 | throw ... |
| post | Switch.cs:111:28:111:48 | throw ... | Switch.cs:111:34:111:48 | object creation of type Exception |
| post | Switch.cs:111:34:111:48 | object creation of type Exception | Switch.cs:111:17:111:21 | enter Throw |
| post | Switch.cs:113:9:113:11 | exit M10 | Switch.cs:117:36:117:44 | return ...; |
| post | Switch.cs:113:9:113:11 | exit M10 | Switch.cs:118:35:118:43 | return ...; |
| post | Switch.cs:113:9:113:11 | exit M10 | Switch.cs:120:9:120:18 | return ...; |
| post | Switch.cs:114:5:121:5 | {...} | Switch.cs:113:9:113:11 | enter M10 |
| post | Switch.cs:115:9:119:9 | switch (...) {...} | Switch.cs:114:5:121:5 | {...} |
| post | Switch.cs:115:17:115:17 | access to parameter s | Switch.cs:115:9:119:9 | switch (...) {...} |
| post | Switch.cs:115:17:115:24 | access to property Length | Switch.cs:115:17:115:17 | access to parameter s |
| post | Switch.cs:117:13:117:34 | case ...: | Switch.cs:115:17:115:24 | access to property Length |
| post | Switch.cs:117:18:117:18 | 3 | Switch.cs:117:13:117:34 | case ...: |
| post | Switch.cs:117:25:117:32 | ... == ... | Switch.cs:117:28:117:32 | "foo" |
| post | Switch.cs:117:28:117:32 | "foo" | Switch.cs:117:25:117:25 | access to parameter s |
| post | Switch.cs:117:36:117:44 | return ...; | Switch.cs:117:43:117:43 | 1 |
| post | Switch.cs:118:18:118:18 | 2 | Switch.cs:118:13:118:33 | case ...: |
| post | Switch.cs:118:25:118:31 | ... == ... | Switch.cs:118:28:118:31 | "fu" |
| post | Switch.cs:118:28:118:31 | "fu" | Switch.cs:118:25:118:25 | access to parameter s |
| post | Switch.cs:118:35:118:43 | return ...; | Switch.cs:118:42:118:42 | 2 |
| post | Switch.cs:120:9:120:18 | return ...; | Switch.cs:120:16:120:17 | -... |
| post | Switch.cs:120:16:120:17 | -... | Switch.cs:120:17:120:17 | 1 |
| post | TypeAccesses.cs:3:10:3:10 | exit M | TypeAccesses.cs:8:13:8:27 | Type t = ... |
| post | TypeAccesses.cs:4:5:9:5 | {...} | TypeAccesses.cs:3:10:3:10 | enter M |
| post | TypeAccesses.cs:5:9:5:26 | ... ...; | TypeAccesses.cs:4:5:9:5 | {...} |
Expand Down Expand Up @@ -2643,6 +2661,27 @@
| pre | Switch.cs:111:17:111:21 | enter Throw | Switch.cs:111:34:111:48 | object creation of type Exception |
| pre | Switch.cs:111:28:111:48 | throw ... | Switch.cs:111:17:111:21 | exit Throw |
| pre | Switch.cs:111:34:111:48 | object creation of type Exception | Switch.cs:111:28:111:48 | throw ... |
| pre | Switch.cs:113:9:113:11 | enter M10 | Switch.cs:114:5:121:5 | {...} |
| pre | Switch.cs:114:5:121:5 | {...} | Switch.cs:115:9:119:9 | switch (...) {...} |
| pre | Switch.cs:115:9:119:9 | switch (...) {...} | Switch.cs:115:17:115:17 | access to parameter s |
| pre | Switch.cs:115:17:115:17 | access to parameter s | Switch.cs:115:17:115:24 | access to property Length |
| pre | Switch.cs:115:17:115:24 | access to property Length | Switch.cs:117:13:117:34 | case ...: |
| pre | Switch.cs:117:13:117:34 | case ...: | Switch.cs:117:18:117:18 | 3 |
| pre | Switch.cs:117:18:117:18 | 3 | Switch.cs:117:25:117:25 | access to parameter s |
| pre | Switch.cs:117:18:117:18 | 3 | Switch.cs:118:13:118:33 | case ...: |
| pre | Switch.cs:117:25:117:25 | access to parameter s | Switch.cs:117:28:117:32 | "foo" |
| pre | Switch.cs:117:25:117:32 | ... == ... | Switch.cs:117:43:117:43 | 1 |
| pre | Switch.cs:117:28:117:32 | "foo" | Switch.cs:117:25:117:32 | ... == ... |
| pre | Switch.cs:117:43:117:43 | 1 | Switch.cs:117:36:117:44 | return ...; |
| pre | Switch.cs:118:13:118:33 | case ...: | Switch.cs:118:18:118:18 | 2 |
| pre | Switch.cs:118:18:118:18 | 2 | Switch.cs:118:25:118:25 | access to parameter s |
| pre | Switch.cs:118:18:118:18 | 2 | Switch.cs:120:17:120:17 | 1 |
| pre | Switch.cs:118:25:118:25 | access to parameter s | Switch.cs:118:28:118:31 | "fu" |
| pre | Switch.cs:118:25:118:31 | ... == ... | Switch.cs:118:42:118:42 | 2 |
| pre | Switch.cs:118:28:118:31 | "fu" | Switch.cs:118:25:118:31 | ... == ... |
| pre | Switch.cs:118:42:118:42 | 2 | Switch.cs:118:35:118:43 | return ...; |
| pre | Switch.cs:120:16:120:17 | -... | Switch.cs:120:9:120:18 | return ...; |
| pre | Switch.cs:120:17:120:17 | 1 | Switch.cs:120:16:120:17 | -... |
| pre | TypeAccesses.cs:3:10:3:10 | enter M | TypeAccesses.cs:4:5:9:5 | {...} |
| pre | TypeAccesses.cs:4:5:9:5 | {...} | TypeAccesses.cs:5:9:5:26 | ... ...; |
| pre | TypeAccesses.cs:5:9:5:26 | ... ...; | TypeAccesses.cs:5:13:5:13 | access to local variable s |
Expand Down
Loading