Skip to content

Commit e93140d

Browse files
authored
Merge pull request #959 from hvitved/csharp/dispose-not-called-on-exc-performance
C#: Improve performance of `cs/dispose-not-called-on-throw`
2 parents 1386af4 + c8eb537 commit e93140d

File tree

2 files changed

+17
-6
lines changed

2 files changed

+17
-6
lines changed

csharp/ql/src/API Abuse/DisposeNotCalledOnException.ql

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,18 +63,25 @@ predicate disposeReachableFromDisposableCreation(MethodCall disposeCall, Expr di
6363
disposeCall.getTarget() instanceof DisposeMethod
6464
}
6565

66-
from MethodCall disposeCall, Expr disposableCreation, MethodCall callThatThrows
66+
class MethodCallThatMayThrow extends MethodCall {
67+
MethodCallThatMayThrow() { exists(getAThrownException(this.getARuntimeTarget())) }
68+
}
69+
70+
ControlFlowElement getACatchOrFinallyClauseChild() {
71+
exists(TryStmt ts | result = ts.getACatchClause() or result = ts.getFinally())
72+
or
73+
result = getACatchOrFinallyClauseChild().getAChild()
74+
}
75+
76+
from MethodCall disposeCall, Expr disposableCreation, MethodCallThatMayThrow callThatThrows
6777
where
6878
disposeReachableFromDisposableCreation(disposeCall, disposableCreation) and
6979
// The dispose call is not, itself, within a dispose method.
7080
not disposeCall.getEnclosingCallable() instanceof DisposeMethod and
7181
// Dispose call not within a finally or catch block
72-
not exists(TryStmt ts |
73-
ts.getACatchClause().getAChild*() = disposeCall or ts.getFinally().getAChild*() = disposeCall
74-
) and
82+
not getACatchOrFinallyClauseChild() = disposeCall and
7583
// At least one method call exists between the allocation and disposal that could throw
7684
disposableCreation.getAReachableElement() = callThatThrows and
77-
callThatThrows.getAReachableElement() = disposeCall and
78-
exists(getAThrownException(callThatThrows.getARuntimeTarget()))
85+
callThatThrows.getAReachableElement() = disposeCall
7986
select disposeCall, "Dispose missed if exception is thrown by $@.", callThatThrows,
8087
callThatThrows.toString()

csharp/ql/src/semmle/code/csharp/controlflow/ControlFlowElement.qll

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,13 @@ class ControlFlowElement extends ExprOrStmtParent, @control_flow_element {
5151
predicate isLive() { exists(this.getAControlFlowNode()) }
5252

5353
/** Holds if the current element is reachable from `src`. */
54+
// potentially very large predicate, so must be inlined
55+
pragma[inline]
5456
predicate reachableFrom(ControlFlowElement src) { this = src.getAReachableElement() }
5557

5658
/** Gets an element that is reachable from this element. */
59+
// potentially very large predicate, so must be inlined
60+
pragma[inline]
5761
ControlFlowElement getAReachableElement() {
5862
// Reachable in same basic block
5963
exists(BasicBlock bb, int i, int j |

0 commit comments

Comments
 (0)