Skip to content

Commit 3e2bf23

Browse files
authored
Merge pull request github#12118 from michaelnebel/telemetry/performancefix
C#/Java: Materialize sink/source/summary predicates to avoid bad join order.
2 parents f2904ca + f6a0231 commit 3e2bf23

File tree

2 files changed

+30
-20
lines changed

2 files changed

+30
-20
lines changed

csharp/ql/src/Telemetry/ExternalApi.qll

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,17 +11,19 @@ private import semmle.code.csharp.dataflow.internal.DataFlowDispatch as DataFlow
1111
private import semmle.code.csharp.dataflow.internal.TaintTrackingPrivate
1212
private import semmle.code.csharp.security.dataflow.flowsources.Remote
1313

14+
pragma[nomagic]
15+
private predicate isTestNamespace(Namespace ns) {
16+
ns.getFullName()
17+
.matches([
18+
"NUnit.Framework%", "Xunit%", "Microsoft.VisualStudio.TestTools.UnitTesting%", "Moq%"
19+
])
20+
}
21+
1422
/**
1523
* A test library.
1624
*/
1725
class TestLibrary extends RefType {
18-
TestLibrary() {
19-
this.getNamespace()
20-
.getFullName()
21-
.matches([
22-
"NUnit.Framework%", "Xunit%", "Microsoft.VisualStudio.TestTools.UnitTesting%", "Moq%"
23-
])
24-
}
26+
TestLibrary() { isTestNamespace(this.getNamespace()) }
2527
}
2628

2729
/** Holds if the given callable is not worth supporting. */
@@ -85,18 +87,21 @@ class ExternalApi extends DotNet::Callable {
8587
}
8688

8789
/** Holds if this API has a supported summary. */
90+
pragma[nomagic]
8891
predicate hasSummary() {
8992
this instanceof SummarizedCallable
9093
or
9194
defaultAdditionalTaintStep(this.getAnInput(), _)
9295
}
9396

9497
/** Holds if this API is a known source. */
98+
pragma[nomagic]
9599
predicate isSource() {
96100
this.getAnOutput() instanceof RemoteFlowSource or sourceNode(this.getAnOutput(), _)
97101
}
98102

99103
/** Holds if this API is a known sink. */
104+
pragma[nomagic]
100105
predicate isSink() { sinkNode(this.getAnInput(), _) }
101106

102107
/** Holds if this API is supported by existing CodeQL libraries, that is, it is either a recognized source or sink or has a flow summary. */

java/ql/src/Telemetry/ExternalApi.qll

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,23 +8,25 @@ private import semmle.code.java.dataflow.FlowSummary
88
private import semmle.code.java.dataflow.internal.DataFlowPrivate
99
private import semmle.code.java.dataflow.TaintTracking
1010

11+
pragma[nomagic]
12+
private predicate isTestPackage(Package p) {
13+
p.getName()
14+
.matches([
15+
"org.junit%", "junit.%", "org.mockito%", "org.assertj%",
16+
"com.github.tomakehurst.wiremock%", "org.hamcrest%", "org.springframework.test.%",
17+
"org.springframework.mock.%", "org.springframework.boot.test.%", "reactor.test%",
18+
"org.xmlunit%", "org.testcontainers.%", "org.opentest4j%", "org.mockserver%",
19+
"org.powermock%", "org.skyscreamer.jsonassert%", "org.rnorth.visibleassertions",
20+
"org.openqa.selenium%", "com.gargoylesoftware.htmlunit%", "org.jboss.arquillian.testng%",
21+
"org.testng%"
22+
])
23+
}
24+
1125
/**
1226
* A test library.
1327
*/
1428
private class TestLibrary extends RefType {
15-
TestLibrary() {
16-
this.getPackage()
17-
.getName()
18-
.matches([
19-
"org.junit%", "junit.%", "org.mockito%", "org.assertj%",
20-
"com.github.tomakehurst.wiremock%", "org.hamcrest%", "org.springframework.test.%",
21-
"org.springframework.mock.%", "org.springframework.boot.test.%", "reactor.test%",
22-
"org.xmlunit%", "org.testcontainers.%", "org.opentest4j%", "org.mockserver%",
23-
"org.powermock%", "org.skyscreamer.jsonassert%", "org.rnorth.visibleassertions",
24-
"org.openqa.selenium%", "com.gargoylesoftware.htmlunit%",
25-
"org.jboss.arquillian.testng%", "org.testng%"
26-
])
27-
}
29+
TestLibrary() { isTestPackage(this.getPackage()) }
2830
}
2931

3032
private string containerAsJar(Container container) {
@@ -74,16 +76,19 @@ class ExternalApi extends Callable {
7476
}
7577

7678
/** Holds if this API has a supported summary. */
79+
pragma[nomagic]
7780
predicate hasSummary() {
7881
this = any(SummarizedCallable sc).asCallable() or
7982
TaintTracking::localAdditionalTaintStep(this.getAnInput(), _)
8083
}
8184

85+
pragma[nomagic]
8286
predicate isSource() {
8387
this.getAnOutput() instanceof RemoteFlowSource or sourceNode(this.getAnOutput(), _)
8488
}
8589

8690
/** Holds if this API is a known sink. */
91+
pragma[nomagic]
8792
predicate isSink() { sinkNode(this.getAnInput(), _) }
8893

8994
/** Holds if this API is supported by existing CodeQL libraries, that is, it is either a recognized source or sink or has a flow summary. */

0 commit comments

Comments
 (0)