Skip to content

Add QL for QL query to warn about possible non-inlining across overlay frontier #19590

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.

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
33 changes: 33 additions & 0 deletions ql/ql/src/codeql_ql/ast/Ast.qll
Original file line number Diff line number Diff line change
Expand Up @@ -2538,6 +2538,18 @@ private class NoOptArg extends AnnotationArg {
NoOptArg() { this.getValue() = "noopt" }
}

private class CallerArg extends AnnotationArg {
CallerArg() { this.getValue() = "caller" }
}

private class LocalArg extends AnnotationArg {
LocalArg() { this.getValue() = "local" }
}

private class LocalQArg extends AnnotationArg {
LocalQArg() { this.getValue() = "local?" }
}

private class MonotonicAggregatesArg extends AnnotationArg {
MonotonicAggregatesArg() { this.getValue() = "monotonicAggregates" }
}
Expand Down Expand Up @@ -2597,6 +2609,27 @@ class NoOpt extends Annotation {
override string toString() { result = "noopt" }
}

/** An `overlay[caller]` annotation. */
class OverlayCaller extends Annotation {
OverlayCaller() { this.getName() = "overlay" and this.getArgs(0) instanceof CallerArg }

override string toString() { result = "caller" }
}

/** An `overlay[local]` annotation. */
class OverlayLocal extends Annotation {
OverlayLocal() { this.getName() = "overlay" and this.getArgs(0) instanceof LocalArg }

override string toString() { result = "local" }
}

/** An `overlay[local?]` annotation. */
class OverlayLocalQ extends Annotation {
OverlayLocalQ() { this.getName() = "overlay" and this.getArgs(0) instanceof LocalQArg }

override string toString() { result = "local?" }
}

/** A `language[monotonicAggregates]` annotation. */
class MonotonicAggregates extends Annotation {
MonotonicAggregates() { this.getArgs(0) instanceof MonotonicAggregatesArg }
Expand Down
41 changes: 41 additions & 0 deletions ql/ql/src/queries/overlay/InlineOverlayCaller.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/**
* @name Cannot inline predicate across overlay frontier
* @description Local inline predicates that are not annotated with `overlay[caller]` are
* not inlined across the overlay frontier. This may negatively affect performance.
* @kind problem
* @problem.severity warning
* @id ql/inline-overlay-caller
* @tags performance
* @precision high
*/

import ql

predicate mayBeLocal(AstNode n) {
n.getAnAnnotation() instanceof OverlayLocal
or
n.getAnAnnotation() instanceof OverlayLocalQ
or
// The tree-sitter-ql grammar doesn't handle annotations on file-level
// module declarations correctly. To work around that, we consider any
// node in a file that contains an overlay[local] or overlay[local?]
// annotation to be potentially local.
exists(AstNode m |
n.getLocation().getFile() = m.getLocation().getFile() and
mayBeLocal(m)
)
}

from Predicate p
where
mayBeLocal(p) and
p.getAnAnnotation() instanceof Inline and
not p.getAnAnnotation() instanceof OverlayCaller and
not p.isPrivate()
select p,
"This possibly local non-private inline predicate will not " +
"be inlined across the overlay frontier. This may negatively " +
"affect evaluation performance. Consider adding an " +
"`overlay[caller]` annotation to allow inlining across the " +
"overlay frontier. Note that adding an `overlay[caller]` " +
"annotation affects semantics under overlay evaluation."
Loading