From 2f74e3e765535918713dd4dab77d7214dce5df4e Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Thu, 27 May 2021 16:40:11 +0200 Subject: [PATCH 1/2] QL: Resolve predicate expressions --- ql/consistency-queries/PredicateResolution.ql | 1 + ql/src/codeql_ql/ast/Ast.qll | 55 +++++++++++++------ ql/src/codeql_ql/ast/internal/AstNodes.qll | 8 ++- ql/src/codeql_ql/ast/internal/Module.qll | 22 ++++---- ql/src/codeql_ql/ast/internal/Predicate.qll | 52 ++++++++++++++++++ ql/src/codeql_ql/ast/internal/Type.qll | 4 -- .../localDefinitions.ql | 8 +++ .../ide-contextual-queries/localReferences.ql | 8 +++ 8 files changed, 127 insertions(+), 31 deletions(-) create mode 100644 ql/consistency-queries/PredicateResolution.ql create mode 100644 ql/src/codeql_ql/ast/internal/Predicate.qll diff --git a/ql/consistency-queries/PredicateResolution.ql b/ql/consistency-queries/PredicateResolution.ql new file mode 100644 index 00000000000..b62a01d720b --- /dev/null +++ b/ql/consistency-queries/PredicateResolution.ql @@ -0,0 +1 @@ +import codeql_ql.ast.internal.Predicate::PredConsistency diff --git a/ql/src/codeql_ql/ast/Ast.qll b/ql/src/codeql_ql/ast/Ast.qll index 94535bfc5a4..0cc407e4d68 100644 --- a/ql/src/codeql_ql/ast/Ast.qll +++ b/ql/src/codeql_ql/ast/Ast.qll @@ -1,6 +1,7 @@ import ql private import codeql_ql.ast.internal.AstNodes private import codeql_ql.ast.internal.Module +private import codeql_ql.ast.internal.Predicate private import codeql_ql.ast.internal.Type /** An AST node of a QL program */ @@ -82,6 +83,21 @@ class PredicateExpr extends TPredicateExpr, AstNode { override string toString() { result = "predicate" } + string getName() { + exists(Generated::AritylessPredicateExpr ape, Generated::LiteralId id | + ape.getParent() = pe and + id.getParent() = ape and + result = id.getValue() + ) + } + + int getArity() { + exists(Generated::Integer i | + i.getParent() = pe and + result = i.getValue().toInt() + ) + } + ModuleExpr getQualifier() { exists(Generated::AritylessPredicateExpr ape | ape.getParent() = pe and @@ -89,6 +105,8 @@ class PredicateExpr extends TPredicateExpr, AstNode { ) } + Predicate getResolvedPredicate() { resolvePredicateExpr(this, result) } + override AstNode getParent() { this in [result.(ClasslessPredicate).getAlias(), result.(HigherOrderFormula).getInput(_)] } @@ -99,7 +117,7 @@ class PredicateExpr extends TPredicateExpr, AstNode { /** * A classless predicate. */ -class ClasslessPredicate extends TClasslessPredicate, Predicate, ModuleMember { +class ClasslessPredicate extends TClasslessPredicate, Predicate, ModuleDeclaration { Generated::ModuleMember member; Generated::ClasslessPredicate pred; @@ -253,7 +271,7 @@ class TypeExpr extends TType, AstNode { /** * A QL module. */ -class Module extends TModule, AstNode, ModuleMember { +class Module extends TModule, ModuleDeclaration { Generated::Module mod; Module() { this = TModule(mod) } @@ -267,10 +285,7 @@ class Module extends TModule, AstNode, ModuleMember { ) } - /** - * Gets the name of the module. - */ - string getName() { result = mod.getName().(Generated::ModuleName).getChild().getValue() } + override string getName() { result = mod.getName().(Generated::ModuleName).getChild().getValue() } /** * Gets a member of the module. @@ -297,10 +312,22 @@ class ModuleMember extends TModuleMember, AstNode { predicate isPrivate() { none() } // TODO: Implement. } +/** A declaration. */ +class Declaration extends TDeclaration, AstNode { + /** Gets the name of this declaration. */ + string getName() { none() } +} + +/** An entity that can be declared in a module. */ +class ModuleDeclaration extends TModuleDeclaration, Declaration, ModuleMember { } + +/** An type declaration. Either a `class` or a `newtype`. */ +class TypeDeclaration extends TTypeDeclaration, Declaration { } + /** * A QL class. */ -class Class extends TClass, AstNode, ModuleMember { +class Class extends TClass, TypeDeclaration, ModuleDeclaration { Generated::Dataclass cls; Class() { this = TClass(cls) } @@ -314,10 +341,7 @@ class Class extends TClass, AstNode, ModuleMember { ) } - /** - * Gets the name of the class. - */ - string getName() { result = cls.getName().getValue() } + override string getName() { result = cls.getName().getValue() } /** * Gets the charateristic predicate for this class. @@ -365,12 +389,12 @@ class Class extends TClass, AstNode, ModuleMember { /** * A `newtype Foo` declaration. */ -class NewType extends TNewType, ModuleMember { +class NewType extends TNewType, TypeDeclaration, ModuleDeclaration { Generated::Datatype type; NewType() { this = TNewType(type) } - string getName() { result = type.getName().getValue() } + override string getName() { result = type.getName().getValue() } override string getAPrimaryQlClass() { result = "NewType" } @@ -390,15 +414,14 @@ class NewType extends TNewType, ModuleMember { /** * A branch in a `newtype`. */ -class NewTypeBranch extends TNewTypeBranch, AstNode { +class NewTypeBranch extends TNewTypeBranch, TypeDeclaration { Generated::DatatypeBranch branch; NewTypeBranch() { this = TNewTypeBranch(branch) } override string getAPrimaryQlClass() { result = "NewTypeBranch" } - /** Gets the name of this branch. */ - string getName() { result = branch.getName().getValue() } + override string getName() { result = branch.getName().getValue() } /** Gets a field in this branch. */ VarDecl getField(int i) { diff --git a/ql/src/codeql_ql/ast/internal/AstNodes.qll b/ql/src/codeql_ql/ast/internal/AstNodes.qll index 00f9582a62b..1ccf944d27a 100644 --- a/ql/src/codeql_ql/ast/internal/AstNodes.qll +++ b/ql/src/codeql_ql/ast/internal/AstNodes.qll @@ -154,4 +154,10 @@ Generated::AstNode toGenerated(AST::AstNode n) { class TPredicate = TCharPred or TClasslessPredicate or TClassPredicate; -class TModuleMember = TClasslessPredicate or TClass or TModule or TNewType or TImport or TSelect; +class TModuleMember = TModuleDeclaration or TImport or TSelect; + +class TDeclaration = TTypeDeclaration or TModuleDeclaration; + +class TTypeDeclaration = TClass or TNewType or TNewTypeBranch; + +class TModuleDeclaration = TClasslessPredicate or TModule or TClass or TNewType; diff --git a/ql/src/codeql_ql/ast/internal/Module.qll b/ql/src/codeql_ql/ast/internal/Module.qll index 8cb6d9f322b..2083cb514a9 100644 --- a/ql/src/codeql_ql/ast/internal/Module.qll +++ b/ql/src/codeql_ql/ast/internal/Module.qll @@ -121,7 +121,7 @@ private predicate resolveQualifiedName(Import imp, ContainerOrModule m, int i) { m = TFolder(c) ) or - defines(getEnclosingModule(imp).getEnclosing*(), q, m, _) + definesModule(getEnclosingModule(imp).getEnclosing*(), q, m, _) ) or exists(Folder_ mid | @@ -142,7 +142,7 @@ private predicate resolveSelectionName(Import imp, ContainerOrModule m, int i) { or exists(ContainerOrModule mid | resolveSelectionName(imp, mid, i - 1) and - defines(mid, imp.getSelectionName(i), m, true) + definesModule(mid, imp.getSelectionName(i), m, true) ) } @@ -158,20 +158,22 @@ predicate resolve(Import imp, FileOrModule m) { predicate resolveModuleExpr(ModuleExpr me, FileOrModule m) { not m = TFile(any(File f | f.getExtension() = "ql")) and not exists(me.getQualifier()) and - defines(getEnclosingModule(me).getEnclosing*(), me.getName(), m, _) + definesModule(getEnclosingModule(me).getEnclosing*(), me.getName(), m, _) or exists(FileOrModule mid | resolveModuleExpr(me.getQualifier(), mid) and - defines(mid, me.getName(), m, true) + definesModule(mid, me.getName(), m, true) ) } -private boolean getPublicBool(ModuleMember m) { - if m.isPrivate() then result = false else result = true -} +boolean getPublicBool(ModuleMember m) { if m.isPrivate() then result = false else result = true } -/** Holds if `container` defines module `m` with name `name`. */ -private predicate defines( +/** + * Holds if `container` defines module `m` with name `name`. + * + * `m` may be defined either directly or through `import`s. + */ +private predicate definesModule( ContainerOrModule container, string name, ContainerOrModule m, boolean public ) { container = m.getEnclosing() and @@ -188,7 +190,7 @@ private predicate defines( container = getEnclosingModule(imp) and resolve(imp, m0) and not exists(imp.importedAs()) and - defines(m0, name, m, true) and + definesModule(m0, name, m, true) and public = getPublicBool(imp) ) or diff --git a/ql/src/codeql_ql/ast/internal/Predicate.qll b/ql/src/codeql_ql/ast/internal/Predicate.qll new file mode 100644 index 00000000000..34aab1af6fb --- /dev/null +++ b/ql/src/codeql_ql/ast/internal/Predicate.qll @@ -0,0 +1,52 @@ +import ql +private import codeql_ql.ast.internal.Module + +private predicate definesPredicate(FileOrModule m, string name, Predicate p, boolean public) { + m = getEnclosingModule(p) and + name = p.getName() and + public = getPublicBool(p) + or + // import X + exists(Import imp, FileOrModule m0 | + m = getEnclosingModule(imp) and + m0 = imp.getResolvedModule() and + not exists(imp.importedAs()) and + definesPredicate(m0, name, p, true) and + public = getPublicBool(imp) + ) + or + // predicate X = Y + exists(ClasslessPredicate alias | + m = getEnclosingModule(alias) and + name = alias.getName() and + resolvePredicateExpr(alias.getAlias(), p) and + public = getPublicBool(alias) + ) +} + +predicate resolvePredicateExpr(PredicateExpr pe, Predicate p) { + exists(FileOrModule m, boolean public | + not exists(pe.getQualifier()) and + m = getEnclosingModule(pe).getEnclosing*() and + public = [false, true] + or + m = pe.getQualifier().getResolvedModule() and + public = true + | + definesPredicate(m, pe.getName(), p, public) and + count(p.getParameter(_)) = pe.getArity() + ) +} + +module PredConsistency { + query predicate noResolvePredicateExpr(PredicateExpr pe) { + not resolvePredicateExpr(pe, _) and + not pe.getLocation().getFile().getAbsolutePath().regexpMatch(".*/(test|examples)/.*") + } + + query predicate multipleResolvePredicateExpr(PredicateExpr pe, int c, Predicate p) { + c = strictcount(Predicate p0 | resolvePredicateExpr(pe, p0)) and + c > 1 and + resolvePredicateExpr(pe, p) + } +} diff --git a/ql/src/codeql_ql/ast/internal/Type.qll b/ql/src/codeql_ql/ast/internal/Type.qll index a47f0c6fdd7..baacb7ba7d3 100644 --- a/ql/src/codeql_ql/ast/internal/Type.qll +++ b/ql/src/codeql_ql/ast/internal/Type.qll @@ -207,10 +207,6 @@ private predicate qualifier(TypeExpr te, FileOrModule m, boolean public) { ) } -private boolean getPublicBool(ModuleMember m) { - if m.isPrivate() then result = false else result = true -} - private predicate defines(FileOrModule m, string name, Type t, boolean public) { exists(Class ty | t = TClass(ty) | getEnclosingModule(ty) = m and diff --git a/ql/src/ide-contextual-queries/localDefinitions.ql b/ql/src/ide-contextual-queries/localDefinitions.ql index 99049df8305..cdd6602d473 100644 --- a/ql/src/ide-contextual-queries/localDefinitions.ql +++ b/ql/src/ide-contextual-queries/localDefinitions.ql @@ -50,10 +50,18 @@ predicate resolveType(TypeExpr ref, AstNode target, string kind) { ref.getLocation().getFile() = getFileBySourceArchiveName(selectedSourceFile()) } +predicate resolvePredicate(PredicateExpr ref, Predicate target, string kind) { + target = ref.getResolvedPredicate() and + kind = "predicate" and + ref.getLocation().getFile() = getFileBySourceArchiveName(selectedSourceFile()) +} + predicate resolve(Loc ref, Loc target, string kind) { resolveModule(ref.asAst(), target.asMod(), kind) or resolveType(ref.asAst(), target.asAst(), kind) + or + resolvePredicate(ref.asAst(), target.asAst(), kind) } from Loc ref, Loc target, string kind diff --git a/ql/src/ide-contextual-queries/localReferences.ql b/ql/src/ide-contextual-queries/localReferences.ql index ecaed17ef7d..a6b23ee187d 100644 --- a/ql/src/ide-contextual-queries/localReferences.ql +++ b/ql/src/ide-contextual-queries/localReferences.ql @@ -49,10 +49,18 @@ predicate resolveType(TypeExpr ref, AstNode target, string kind) { ref.getLocation().getFile() = getFileBySourceArchiveName(selectedSourceFile()) } +predicate resolvePredicate(PredicateExpr ref, Predicate target, string kind) { + target = ref.getResolvedPredicate() and + kind = "predicate" and + ref.getLocation().getFile() = getFileBySourceArchiveName(selectedSourceFile()) +} + predicate resolve(Loc ref, Loc target, string kind) { resolveModule(ref.asAst(), target.asMod(), kind) or resolveType(ref.asAst(), target.asAst(), kind) + or + resolvePredicate(ref.asAst(), target.asAst(), kind) } from Loc ref, Loc target, string kind From 0eabd4fcd9efa90402d9e847fc81a3c257da9328 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Thu, 27 May 2021 20:28:46 +0200 Subject: [PATCH 2/2] QL: Restrict `resolvePredicateExpr` to `ClasslessPredicate`s --- ql/src/codeql_ql/ast/internal/Predicate.qll | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ql/src/codeql_ql/ast/internal/Predicate.qll b/ql/src/codeql_ql/ast/internal/Predicate.qll index 34aab1af6fb..8b01438c369 100644 --- a/ql/src/codeql_ql/ast/internal/Predicate.qll +++ b/ql/src/codeql_ql/ast/internal/Predicate.qll @@ -1,7 +1,7 @@ import ql private import codeql_ql.ast.internal.Module -private predicate definesPredicate(FileOrModule m, string name, Predicate p, boolean public) { +private predicate definesPredicate(FileOrModule m, string name, ClasslessPredicate p, boolean public) { m = getEnclosingModule(p) and name = p.getName() and public = getPublicBool(p) @@ -24,7 +24,7 @@ private predicate definesPredicate(FileOrModule m, string name, Predicate p, boo ) } -predicate resolvePredicateExpr(PredicateExpr pe, Predicate p) { +predicate resolvePredicateExpr(PredicateExpr pe, ClasslessPredicate p) { exists(FileOrModule m, boolean public | not exists(pe.getQualifier()) and m = getEnclosingModule(pe).getEnclosing*() and @@ -44,8 +44,8 @@ module PredConsistency { not pe.getLocation().getFile().getAbsolutePath().regexpMatch(".*/(test|examples)/.*") } - query predicate multipleResolvePredicateExpr(PredicateExpr pe, int c, Predicate p) { - c = strictcount(Predicate p0 | resolvePredicateExpr(pe, p0)) and + query predicate multipleResolvePredicateExpr(PredicateExpr pe, int c, ClasslessPredicate p) { + c = strictcount(ClasslessPredicate p0 | resolvePredicateExpr(pe, p0)) and c > 1 and resolvePredicateExpr(pe, p) }