Skip to content

C#: Fix a bug in ThrowingCallable #637

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 12, 2018
Merged

C#: Fix a bug in ThrowingCallable #637

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
243af36
C#: Add more CFG tests with throwing methods
hvitved Dec 7, 2018
c887dc8
C#: Fix a bug in `ThrowingCallable`
hvitved Dec 7, 2018
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
20 changes: 12 additions & 8 deletions csharp/ql/src/semmle/code/csharp/controlflow/ControlFlowGraph.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1871,28 +1871,32 @@ module ControlFlow {
}
}

private predicate directlyThrows(ThrowElement te, ThrowCompletion c) {
c.getExceptionClass() = te.getThrownExceptionType() and
// For stub implementations, there may exist proper implementations that are not seen
// during compilation, so we conservatively rule those out
not isStub(te)
Copy link
Contributor

Choose a reason for hiding this comment

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

I realise this is the previous logic, but doesn't it make sense to have isStub at a higher level, for example the callable or the class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps at the callable level, but usually that would be one-to-one with the throw statement. I don't think the whole class needs to be a stub for one of the callables to be a stub.

}

private ControlFlowElement getAThrowingElement(ThrowCompletion c) {
c = result.(ThrowingCall).getACompletion()
or
result = any(ThrowElement te |
c.getExceptionClass() = te.getThrownExceptionType() and
// For stub implementations, there may exist proper implementations that are not seen
// during compilation, so we conservatively rule those out
not isStub(te)
)
directlyThrows(result, c)
or
result = getAThrowingStmt(c)
}

private Stmt getAThrowingStmt(ThrowCompletion c) {
directlyThrows(result, c)
or
result.(ExprStmt).getExpr() = getAThrowingElement(c)
or
result.(BlockStmt).getFirstStmt() = getAThrowingStmt(c)
or
exists(IfStmt ifStmt, ThrowCompletion c1, ThrowCompletion c2 |
result = ifStmt and
ifStmt.getThen() = getAThrowingElement(c1) and
ifStmt.getElse() = getAThrowingElement(c2) |
ifStmt.getThen() = getAThrowingStmt(c1) and
ifStmt.getElse() = getAThrowingStmt(c2) |
c = c1
or
c = c2
Expand Down
48 changes: 26 additions & 22 deletions csharp/ql/test/library-tests/controlflow/graph/BasicBlock.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,28 +187,32 @@
| ExitMethods.cs:43:9:46:9 | [exception: Exception] catch (...) {...} | ExitMethods.cs:43:9:46:9 | [exception: Exception] catch (...) {...} | 1 |
| ExitMethods.cs:44:9:46:9 | {...} | ExitMethods.cs:45:13:45:19 | return ...; | 2 |
| ExitMethods.cs:47:9:50:9 | [exception: Exception] catch (...) {...} | ExitMethods.cs:49:13:49:19 | return ...; | 3 |
| ExitMethods.cs:53:17:53:26 | enter ErrorMaybe | ExitMethods.cs:55:13:55:13 | access to parameter b | 4 |
| ExitMethods.cs:53:17:53:26 | exit ErrorMaybe | ExitMethods.cs:53:17:53:26 | exit ErrorMaybe | 1 |
| ExitMethods.cs:56:19:56:33 | object creation of type Exception | ExitMethods.cs:56:13:56:34 | throw ...; | 2 |
| ExitMethods.cs:59:17:59:27 | enter ErrorAlways | ExitMethods.cs:61:13:61:13 | access to parameter b | 4 |
| ExitMethods.cs:59:17:59:27 | exit ErrorAlways | ExitMethods.cs:59:17:59:27 | exit ErrorAlways | 1 |
| ExitMethods.cs:62:19:62:33 | object creation of type Exception | ExitMethods.cs:62:13:62:34 | throw ...; | 2 |
| ExitMethods.cs:64:41:64:43 | "b" | ExitMethods.cs:64:13:64:45 | throw ...; | 3 |
| ExitMethods.cs:67:10:67:13 | enter Exit | ExitMethods.cs:67:10:67:13 | exit Exit | 6 |
| ExitMethods.cs:72:10:72:18 | enter ExitInTry | ExitMethods.cs:72:10:72:18 | exit ExitInTry | 8 |
| ExitMethods.cs:85:10:85:24 | enter ApplicationExit | ExitMethods.cs:85:10:85:24 | exit ApplicationExit | 5 |
| ExitMethods.cs:90:13:90:21 | enter ThrowExpr | ExitMethods.cs:92:16:92:25 | ... != ... | 7 |
| ExitMethods.cs:90:13:90:21 | exit ThrowExpr | ExitMethods.cs:90:13:90:21 | exit ThrowExpr | 1 |
| ExitMethods.cs:92:29:92:29 | 1 | ExitMethods.cs:92:9:92:77 | return ...; | 5 |
| ExitMethods.cs:92:69:92:75 | "input" | ExitMethods.cs:92:41:92:76 | throw ... | 3 |
| ExitMethods.cs:95:16:95:34 | enter ExtensionMethodCall | ExitMethods.cs:97:16:97:30 | call to method Contains | 6 |
| ExitMethods.cs:97:9:97:39 | return ...; | ExitMethods.cs:95:16:95:34 | exit ExtensionMethodCall | 2 |
| ExitMethods.cs:97:34:97:34 | 0 | ExitMethods.cs:97:34:97:34 | 0 | 1 |
| ExitMethods.cs:97:38:97:38 | 1 | ExitMethods.cs:97:38:97:38 | 1 | 1 |
| ExitMethods.cs:100:17:100:32 | enter FailingAssertion | ExitMethods.cs:100:17:100:32 | exit FailingAssertion | 6 |
| ExitMethods.cs:106:17:106:33 | enter FailingAssertion2 | ExitMethods.cs:106:17:106:33 | exit FailingAssertion2 | 6 |
| ExitMethods.cs:112:10:112:20 | enter AssertFalse | ExitMethods.cs:112:10:112:20 | exit AssertFalse | 4 |
| ExitMethods.cs:114:17:114:33 | enter FailingAssertion3 | ExitMethods.cs:114:17:114:33 | exit FailingAssertion3 | 7 |
| ExitMethods.cs:53:10:53:11 | enter M7 | ExitMethods.cs:53:10:53:11 | exit M7 | 5 |
| ExitMethods.cs:59:10:59:11 | enter M8 | ExitMethods.cs:59:10:59:11 | exit M8 | 5 |
| ExitMethods.cs:65:17:65:26 | enter ErrorMaybe | ExitMethods.cs:67:13:67:13 | access to parameter b | 4 |
| ExitMethods.cs:65:17:65:26 | exit ErrorMaybe | ExitMethods.cs:65:17:65:26 | exit ErrorMaybe | 1 |
| ExitMethods.cs:68:19:68:33 | object creation of type Exception | ExitMethods.cs:68:13:68:34 | throw ...; | 2 |
| ExitMethods.cs:71:17:71:27 | enter ErrorAlways | ExitMethods.cs:73:13:73:13 | access to parameter b | 4 |
| ExitMethods.cs:71:17:71:27 | exit ErrorAlways | ExitMethods.cs:71:17:71:27 | exit ErrorAlways | 1 |
| ExitMethods.cs:74:19:74:33 | object creation of type Exception | ExitMethods.cs:74:13:74:34 | throw ...; | 2 |
| ExitMethods.cs:76:41:76:43 | "b" | ExitMethods.cs:76:13:76:45 | throw ...; | 3 |
| ExitMethods.cs:79:17:79:28 | enter ErrorAlways2 | ExitMethods.cs:79:17:79:28 | exit ErrorAlways2 | 5 |
| ExitMethods.cs:84:17:84:28 | enter ErrorAlways3 | ExitMethods.cs:84:17:84:28 | exit ErrorAlways3 | 4 |
| ExitMethods.cs:86:10:86:13 | enter Exit | ExitMethods.cs:86:10:86:13 | exit Exit | 6 |
| ExitMethods.cs:91:10:91:18 | enter ExitInTry | ExitMethods.cs:91:10:91:18 | exit ExitInTry | 8 |
| ExitMethods.cs:104:10:104:24 | enter ApplicationExit | ExitMethods.cs:104:10:104:24 | exit ApplicationExit | 5 |
| ExitMethods.cs:109:13:109:21 | enter ThrowExpr | ExitMethods.cs:111:16:111:25 | ... != ... | 7 |
| ExitMethods.cs:109:13:109:21 | exit ThrowExpr | ExitMethods.cs:109:13:109:21 | exit ThrowExpr | 1 |
| ExitMethods.cs:111:29:111:29 | 1 | ExitMethods.cs:111:9:111:77 | return ...; | 5 |
| ExitMethods.cs:111:69:111:75 | "input" | ExitMethods.cs:111:41:111:76 | throw ... | 3 |
| ExitMethods.cs:114:16:114:34 | enter ExtensionMethodCall | ExitMethods.cs:116:16:116:30 | call to method Contains | 6 |
| ExitMethods.cs:116:9:116:39 | return ...; | ExitMethods.cs:114:16:114:34 | exit ExtensionMethodCall | 2 |
| ExitMethods.cs:116:34:116:34 | 0 | ExitMethods.cs:116:34:116:34 | 0 | 1 |
| ExitMethods.cs:116:38:116:38 | 1 | ExitMethods.cs:116:38:116:38 | 1 | 1 |
| ExitMethods.cs:119:17:119:32 | enter FailingAssertion | ExitMethods.cs:119:17:119:32 | exit FailingAssertion | 6 |
| ExitMethods.cs:125:17:125:33 | enter FailingAssertion2 | ExitMethods.cs:125:17:125:33 | exit FailingAssertion2 | 6 |
| ExitMethods.cs:131:10:131:20 | enter AssertFalse | ExitMethods.cs:131:10:131:20 | exit AssertFalse | 4 |
| ExitMethods.cs:133:17:133:33 | enter FailingAssertion3 | ExitMethods.cs:133:17:133:33 | exit FailingAssertion3 | 7 |
| Extensions.cs:5:23:5:29 | enter ToInt32 | Extensions.cs:5:23:5:29 | exit ToInt32 | 6 |
| Extensions.cs:10:24:10:29 | enter ToBool | Extensions.cs:10:24:10:29 | exit ToBool | 7 |
| Extensions.cs:15:23:15:33 | enter CallToInt32 | Extensions.cs:15:23:15:33 | exit CallToInt32 | 4 |
Expand Down
Loading