From 5f65ea60d12dd6b69e1979c269ad52e2873114d3 Mon Sep 17 00:00:00 2001 From: Kasper Svendsen Date: Tue, 27 May 2025 10:16:47 +0200 Subject: [PATCH 1/5] QL AST: Add overlay annotations --- ql/ql/src/codeql_ql/ast/Ast.qll | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/ql/ql/src/codeql_ql/ast/Ast.qll b/ql/ql/src/codeql_ql/ast/Ast.qll index 937c7bc61010..b3bbf41aaefe 100644 --- a/ql/ql/src/codeql_ql/ast/Ast.qll +++ b/ql/ql/src/codeql_ql/ast/Ast.qll @@ -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" } } @@ -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 } From b291b0637e9691371e4c9ffe1ca88835023df2a1 Mon Sep 17 00:00:00 2001 From: Kasper Svendsen Date: Tue, 27 May 2025 10:17:46 +0200 Subject: [PATCH 2/5] Warn about possible non-inlining across overlay frontier --- .../queries/overlay/InlineOverlayCaller.ql | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 ql/ql/src/queries/overlay/InlineOverlayCaller.ql diff --git a/ql/ql/src/queries/overlay/InlineOverlayCaller.ql b/ql/ql/src/queries/overlay/InlineOverlayCaller.ql new file mode 100644 index 000000000000..d27a0ade9bbf --- /dev/null +++ b/ql/ql/src/queries/overlay/InlineOverlayCaller.ql @@ -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." From 89a4df9f366bd846396d7ca2dfb70d2335fe626e Mon Sep 17 00:00:00 2001 From: Kasper Svendsen Date: Wed, 28 May 2025 15:25:54 +0200 Subject: [PATCH 3/5] Mark possibly local predicate inline to test QL-for-QL query --- java/ql/lib/semmle/code/java/Type.qll | 3 +++ 1 file changed, 3 insertions(+) diff --git a/java/ql/lib/semmle/code/java/Type.qll b/java/ql/lib/semmle/code/java/Type.qll index 5036bbea6224..6fc2f2267599 100644 --- a/java/ql/lib/semmle/code/java/Type.qll +++ b/java/ql/lib/semmle/code/java/Type.qll @@ -9,6 +9,8 @@ * Classes and interfaces can also be local (`LocalClassOrInterface`, `LocalClass`) or anonymous (`AnonymousClass`). * Enumerated types (`EnumType`) and records (`Record`) are special kinds of classes. */ +overlay[local?] +module; import Member import Modifier @@ -612,6 +614,7 @@ class RefType extends Type, Annotatable, Modifiable, @reftype { * to the name of the enclosing type, which might be a nested type as well. For example: * `java.lang.Thread$State`. */ + pragma[inline] string getQualifiedName() { exists(string pkgName | pkgName = this.getPackage().getName() | if pkgName = "" From a688d05147629f6d43df7dd1234146b029b2907e Mon Sep 17 00:00:00 2001 From: Kasper Svendsen Date: Wed, 28 May 2025 15:41:22 +0200 Subject: [PATCH 4/5] Add correctness tag --- ql/ql/src/queries/overlay/InlineOverlayCaller.ql | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ql/ql/src/queries/overlay/InlineOverlayCaller.ql b/ql/ql/src/queries/overlay/InlineOverlayCaller.ql index d27a0ade9bbf..cf663f99ddc6 100644 --- a/ql/ql/src/queries/overlay/InlineOverlayCaller.ql +++ b/ql/ql/src/queries/overlay/InlineOverlayCaller.ql @@ -5,7 +5,8 @@ * @kind problem * @problem.severity warning * @id ql/inline-overlay-caller - * @tags performance + * @tags correctness + * performance * @precision high */ From d80f82dfe113c51cfd4b730d76be3e563cc61026 Mon Sep 17 00:00:00 2001 From: Kasper Svendsen Date: Wed, 28 May 2025 15:46:47 +0200 Subject: [PATCH 5/5] Mark predicate inline to test QL-for-QL query --- java/ql/lib/semmle/code/java/Type.qll | 1 + 1 file changed, 1 insertion(+) diff --git a/java/ql/lib/semmle/code/java/Type.qll b/java/ql/lib/semmle/code/java/Type.qll index 6fc2f2267599..57061c9aa41f 100644 --- a/java/ql/lib/semmle/code/java/Type.qll +++ b/java/ql/lib/semmle/code/java/Type.qll @@ -600,6 +600,7 @@ class RefType extends Type, Annotatable, Modifiable, @reftype { /** * Gets the JVM descriptor for this type, as used in bytecode. */ + pragma[inline] override string getTypeDescriptor() { result = "L" + this.getPackage().getName().replaceAll(".", "/") + "/" +