Skip to content

Commit e663abd

Browse files
committed
C#: Avoid using ExceptionClass in deliberate Cartesian products
Using the class `ExceptionClass` in combination with a deliberate Cartesian product can lead to bad join orders, for example ``` EVALUATE NONRECURSIVE RELATION: Completion::TriedControlFlowElement::getAThrownException_dispred#ff(int this, int result) :- {1} r1 = JOIN Expr::Expr::getType_dispred#ff_10#join_rhs WITH @integral_type#f ON Expr::Expr::getType_dispred#ff_10#join_rhs.<0>=@integral_type#f.<0> OUTPUT FIELDS {Expr::Expr::getType_dispred#ff_10#join_rhs.<1>} {1} r2 = JOIN r1 WITH @un_op#f ON r1.<0>=@un_op#f.<0> OUTPUT FIELDS {r1.<0>} {1} r3 = JOIN r2 WITH Stmt::TryStmt::getATriedElement#ff_1#join_rhs ON r2.<0>=Stmt::TryStmt::getATriedElement#ff_1#join_rhs.<0> OUTPUT FIELDS {r2.<0>} {2} r4 = JOIN r3 WITH Stmt::ExceptionClass#f CARTESIAN PRODUCT OUTPUT FIELDS {Stmt::ExceptionClass#f.<0>,r3.<0>} {2} r5 = JOIN r4 WITH System::SystemOverflowExceptionClass#class#f ON r4.<0>=System::SystemOverflowExceptionClass#class#f.<0> OUTPUT FIELDS {r4.<1>,r4.<0>} ``` where the CP is made with `ExceptionClass` rather than `SystemOverflowExceptionClass` directly.
1 parent 87c5872 commit e663abd

File tree

5 files changed

+21
-19
lines changed

5 files changed

+21
-19
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import semmle.code.csharp.frameworks.System
2323
* Gets an exception type that may be thrown during the execution of method `m`.
2424
* Assumes any exception may be thrown by library types.
2525
*/
26-
ExceptionClass getAThrownException(Method m) {
26+
Class getAThrownException(Method m) {
2727
m.fromLibrary() and
2828
result = any(SystemExceptionClass sc)
2929
or

csharp/ql/src/semmle/code/csharp/commons/Assertions.qll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ abstract class AssertMethod extends Method {
1515
final Parameter getAssertedParameter() { result = this.getParameter(this.getAssertionIndex()) }
1616

1717
/** Gets the exception being thrown if the assertion fails, if any. */
18-
abstract ExceptionClass getExceptionClass();
18+
abstract Class getExceptionClass();
1919
}
2020

2121
/** A positive assertion method. */
@@ -122,7 +122,7 @@ class SystemDiagnosticsDebugAssertTrueMethod extends AssertTrueMethod {
122122

123123
override int getAssertionIndex() { result = 0 }
124124

125-
override ExceptionClass getExceptionClass() {
125+
override Class getExceptionClass() {
126126
// A failing assertion generates a message box, see
127127
// https://docs.microsoft.com/en-us/dotnet/api/system.diagnostics.debug.assert
128128
none()
@@ -182,7 +182,7 @@ class ForwarderAssertMethod extends AssertMethod {
182182

183183
override int getAssertionIndex() { result = p.getPosition() }
184184

185-
override ExceptionClass getExceptionClass() {
185+
override Class getExceptionClass() {
186186
result = this.getUnderlyingAssertMethod().getExceptionClass()
187187
}
188188

csharp/ql/src/semmle/code/csharp/controlflow/internal/Completion.qll

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -178,27 +178,29 @@ private predicate isMatchingConstant(Expr e, boolean value) {
178178
)
179179
}
180180

181+
private class Overflowable extends UnaryOperation {
182+
Overflowable() {
183+
not this instanceof UnaryBitwiseOperation and
184+
this.getType() instanceof IntegralType
185+
}
186+
}
187+
181188
/** A control flow element that is inside a `try` block. */
182189
private class TriedControlFlowElement extends ControlFlowElement {
183190
TriedControlFlowElement() { this = any(TryStmt try).getATriedElement() }
184191

185192
/**
186193
* Gets an exception class that is potentially thrown by this element, if any.
187194
*/
188-
ExceptionClass getAThrownException() {
189-
this = any(UnaryOperation uo |
190-
not uo instanceof UnaryBitwiseOperation and
191-
uo.getType() instanceof IntegralType and
192-
result instanceof SystemOverflowExceptionClass
193-
)
195+
Class getAThrownException() {
196+
this instanceof Overflowable and
197+
result instanceof SystemOverflowExceptionClass
194198
or
195-
this = any(CastExpr ce |
196-
ce.getType() instanceof IntegralType and
197-
result instanceof SystemOverflowExceptionClass
198-
or
199-
invalidCastCandidate(ce) and
200-
result instanceof SystemInvalidCastExceptionClass
201-
)
199+
this.(CastExpr).getType() instanceof IntegralType and
200+
result instanceof SystemOverflowExceptionClass
201+
or
202+
invalidCastCandidate(this) and
203+
result instanceof SystemInvalidCastExceptionClass
202204
or
203205
this instanceof Call and
204206
result instanceof SystemExceptionClass

csharp/ql/src/semmle/code/csharp/exprs/Expr.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -558,7 +558,7 @@ class ThrowElement extends ControlFlowElement, DotNet::Throw, @throw_element {
558558
override Expr getExpr() { result = this.getChild(0) }
559559

560560
/** Gets the type of exception being thrown. */
561-
ExceptionClass getThrownExceptionType() {
561+
Class getThrownExceptionType() {
562562
result = getExpr().getType()
563563
or
564564
// Corner case: `throw null`

csharp/ql/src/semmle/code/csharp/frameworks/test/VisualStudio.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ class VSTestAssertClass extends Class {
8989
}
9090

9191
/** The `Microsoft.VisualStudio.TestTools.UnitTesting.AssertFailedException` class. */
92-
class AssertFailedExceptionClass extends ExceptionClass {
92+
class AssertFailedExceptionClass extends Class {
9393
AssertFailedExceptionClass() {
9494
this.getNamespace() instanceof VSTestNamespace and
9595
this.hasName("AssertFailedException")

0 commit comments

Comments
 (0)