Skip to content

Commit 711b435

Browse files
felickzCopilotCopilot
authored
fix(hotspots): avoid cartesian product in getImportStringFrom (stacked on #238) (#240)
* chore: bump pinned CodeQL CLI to v2.27.3 and release v0.8.3 * Add Hotspots query resource diagnostics Co-authored-by: felickz <1760475+felickz@users.noreply.github.com> * Cap Hotspots evaluator disk cache Co-authored-by: felickz <1760475+felickz@users.noreply.github.com> * fix(hotspots): avoid cartesian product when resolving config import strings getImportStringFrom compared the config file's dotted path against Import.getImportString() with matches("%" + importString). That non-equality string join gives the optimizer nothing to bind on, and with CodeQL 2.27.3 (github/codeql codeql-cli/v2.27.3) it planned a cartesian product of ~33k import tuples with the 15M-row locations_default relation, filling the runner disk. Materialize the config's dotted-path suffixes in a pragma[nomagic] helper and join on string equality instead. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * ci(hotspots): TEMPORARY run Hotspots CI on PRs stacked on CLI bump branches hotspots-ci.yml only triggers for PRs targeting main, so a PR stacked on chore/update-codeql-cli-* would get no hotspots signal. Drop this commit before merging the fix into the bump branch. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 7a3ca86 commit 711b435

2 files changed

Lines changed: 25 additions & 8 deletions

File tree

‎.github/workflows/hotspots-ci.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ name: "Hotspots Build/Validate"
2525

2626
on:
2727
pull_request:
28-
branches: [main]
28+
branches: [main, "chore/update-codeql-cli-*"]
2929
paths:
3030
- "ql/hotspots/**"
3131
- ".github/actions/install-codeql/**"

‎ql/hotspots/utils/hotspots.qll‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ private string getAbsoluteImportString(Import i) {
2323
result = getFileImport(i.getResolvedModule().getFile()).suffix(1)
2424
}
2525

26+
/** Gets a non-empty suffix of `s`, including `s` itself. */
27+
bindingset[s]
28+
private string getASuffix(string s) { result = s.suffix(any(int n | n in [0 .. s.length() - 1])) }
29+
2630
/**
2731
* A Taint tracking configuration. Either a Class or a Module.
2832
*/
@@ -83,6 +87,25 @@ abstract class TaintTrackingConfiguration extends ModuleDeclaration {
8387
else result = "unknown"
8488
}
8589

90+
/**
91+
* Gets a suffix of the dotted module path of the file this configuration is in.
92+
*
93+
* This is materialized on its own, and compared with `Import.getImportString()` by equality
94+
* rather than with `matches("%" + importString)`: a non-equality string join gives the
95+
* optimizer nothing to bind on, and it has been observed to pick a cartesian product with
96+
* `locations_default` (billions of tuples) instead of joining on this small relation first.
97+
*/
98+
pragma[nomagic]
99+
private string getADottedPathSuffix() {
100+
result =
101+
getASuffix(this.getLocation()
102+
.getFile()
103+
.getRelativePath()
104+
.splitAt(".", 0)
105+
.replaceAll("/", ".")
106+
.suffix(1))
107+
}
108+
86109
/**
87110
* Returns the import string necessary to import this configuration.
88111
*/
@@ -94,13 +117,7 @@ abstract class TaintTrackingConfiguration extends ModuleDeclaration {
94117
else
95118
exists(Import i |
96119
i = tl.getAnImport() and
97-
this.getLocation()
98-
.getFile()
99-
.getRelativePath()
100-
.splitAt(".", 0)
101-
.replaceAll("/", ".")
102-
.suffix(1)
103-
.matches("%" + i.getImportString()) and
120+
i.getImportString() = this.getADottedPathSuffix() and
104121
result = getAbsoluteImportString(i)
105122
)
106123
)

0 commit comments

Comments
 (0)