Skip to content

Commit efff86e

Browse files
authored
QL: Merge pull request #125 from github/erik-krogh/fix-my-own-mistake
fixing the callgraph
2 parents 9a02a22 + 865e4f0 commit efff86e

File tree

9 files changed

+78
-31
lines changed

9 files changed

+78
-31
lines changed
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import ql
2+
private import codeql_ql.ast.internal.AstNodes as AstNodes
3+
4+
query AstNode nonTotalGetParent() {
5+
exists(AstNodes::toQL(result).getParent()) and
6+
not exists(result.getParent()) and
7+
not result.getLocation().getStartColumn() = 1 and // startcolumn = 1 <=> top level in file <=> fine to have no parent
8+
not result instanceof YAML::YAMLNode and // parents for YAML doens't work
9+
not (result instanceof QLDoc and result.getLocation().getFile().getExtension() = "dbscheme") // qldoc in dbschemes are not hooked up
10+
}

ql/src/codeql_ql/ast/Ast.qll

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,11 @@ class AstNode extends TAstNode {
5656
* named `pred`.
5757
*/
5858
cached
59-
AstNode getAChild(string pred) { none() }
59+
AstNode getAChild(string pred) {
60+
pred = directMember("getAnAnnotation") and result = getAnAnnotation()
61+
or
62+
pred = directMember("getQLDoc") and result = getQLDoc()
63+
}
6064

6165
/**
6266
* Gets the primary QL class for the ast node.
@@ -70,10 +74,7 @@ class AstNode extends TAstNode {
7074
predicate hasAnnotation(string name) { this.getAnAnnotation().getName() = name }
7175

7276
/** Gets an annotation of this AST node. */
73-
cached
74-
Annotation getAnAnnotation() {
75-
toQL(this).getParent() = pragma[only_bind_out](toQL(result)).getParent()
76-
}
77+
Annotation getAnAnnotation() { toQL(this).getParent() = toQL(result).getParent() }
7778

7879
/**
7980
* Gets the predicate that contains this AST node.
@@ -119,8 +120,6 @@ class TopLevel extends TTopLevel, AstNode {
119120
NewType getANewType() { result = this.getAMember() }
120121

121122
override ModuleMember getAChild(string pred) {
122-
pred = directMember("getQLDoc") and result = this.getQLDoc()
123-
or
124123
pred = directMember("getAnImport") and result = this.getAnImport()
125124
or
126125
pred = directMember("getAClass") and result = this.getAClass()
@@ -214,7 +213,7 @@ class BuiltinPredicate extends PredicateOrBuiltin, TBuiltin {
214213
override string getAPrimaryQlClass() { result = "BuiltinPredicate" }
215214
}
216215

217-
private class BuiltinClassless extends BuiltinPredicate, TBuiltinClassless {
216+
class BuiltinClassless extends BuiltinPredicate, TBuiltinClassless {
218217
string name;
219218
string ret;
220219
string args;
@@ -228,7 +227,7 @@ private class BuiltinClassless extends BuiltinPredicate, TBuiltinClassless {
228227
override PrimitiveType getParameterType(int i) { result.getName() = getArgType(args, i) }
229228
}
230229

231-
private class BuiltinMember extends BuiltinPredicate, TBuiltinMember {
230+
class BuiltinMember extends BuiltinPredicate, TBuiltinMember {
232231
string name;
233232
string qual;
234233
string ret;
@@ -626,7 +625,7 @@ class TypeExpr extends TType, AstNode {
626625
*/
627626
Type getResolvedType() { resolveTypeExpr(this, result) }
628627

629-
override ModuleExpr getAChild(string pred) {
628+
override AstNode getAChild(string pred) {
630629
result = super.getAChild(pred)
631630
or
632631
pred = directMember("getModule") and result = this.getModule()
@@ -691,11 +690,7 @@ class Declaration extends TDeclaration, AstNode {
691690
result = any(Class c).getQLDocFor(this)
692691
}
693692

694-
override AstNode getAChild(string pred) {
695-
result = super.getAChild(pred)
696-
or
697-
pred = directMember("getQLDoc") and result = this.getQLDoc()
698-
}
693+
override AstNode getAChild(string pred) { result = super.getAChild(pred) }
699694
}
700695

701696
/** An entity that can be declared in a module. */
@@ -784,6 +779,8 @@ class Class extends TClass, TypeDeclaration, ModuleDeclaration {
784779
or
785780
pred = directMember("getASuperType") and result = this.getASuperType()
786781
or
782+
pred = directMember("getAnInstanceofType") and result = this.getAnInstanceofType()
783+
or
787784
exists(string name |
788785
pred = stringIndexedMember("getClassPredicate", name) and
789786
result = this.getClassPredicate(name)
@@ -860,8 +857,6 @@ class NewTypeBranch extends TNewTypeBranch, PredicateOrBuiltin, TypeDeclaration
860857
pred = directMember("getBody") and result = this.getBody()
861858
or
862859
exists(int i | pred = indexedMember("getField", i) and result = this.getField(i))
863-
or
864-
pred = directMember("getQLDoc") and result = this.getQLDoc()
865860
}
866861
}
867862

@@ -2196,6 +2191,12 @@ class Annotation extends TAnnotation, AstNode {
21962191

21972192
/** Gets the node corresponding to the field `name`. */
21982193
string getName() { result = annot.getName().getValue() }
2194+
2195+
override AstNode getAChild(string pred) {
2196+
result = super.getAChild(pred)
2197+
or
2198+
exists(int i | pred = indexedMember("getArgs", i) and result = this.getArgs(i))
2199+
}
21992200
}
22002201

22012202
/** A `pragma[noinline]` annotation. */
@@ -2414,8 +2415,8 @@ module YAML {
24142415

24152416
/** Gets the database scheme of this qlpack */
24162417
File getDBScheme() {
2417-
result.getBaseName() = this.getProperty("dbscheme") and
2418-
result = file.getParentContainer().getFile(any(string s | s.matches("%.dbscheme")))
2418+
result.getAbsolutePath() =
2419+
file.getParentContainer().getAbsolutePath() + "/" + this.getProperty("dbscheme")
24192420
}
24202421

24212422
pragma[noinline]

ql/src/codeql_ql/ast/internal/Builtins.qll

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,9 @@ predicate isBuiltinMember(string sig) {
4444
"string string.toLowerCase()", "string string.toUpperCase()", "string string.trim()",
4545
"int date.daysTo(date)", "int date.getDay()", "int date.getHours()", "int date.getMinutes()",
4646
"int date.getMonth()", "int date.getSeconds()", "int date.getYear()",
47-
"string date.toString()", "string date.toISO()", "string int.toUnicode()"
47+
"string date.toString()", "string date.toISO()", "string int.toUnicode()",
48+
"string any.getAQlClass()"
49+
/* getAQlClass is special , see Predicate.qll*/
4850
]
4951
}
5052

ql/src/codeql_ql/ast/internal/Module.qll

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -154,18 +154,13 @@ private predicate resolveSelectionName(Import imp, ContainerOrModule m, int i) {
154154

155155
cached
156156
private module Cached {
157-
private Module getEnclosingModule0(AstNode n) {
158-
not n instanceof Module and
159-
(
160-
n = result.getAChild(_)
161-
or
162-
exists(AstNode prev |
163-
result = getEnclosingModule0(prev) and
164-
n = prev.getAChild(_)
165-
)
166-
)
157+
private AstNode parent(AstNode n) {
158+
result = n.getParent() and
159+
not n instanceof Module
167160
}
168161

162+
private Module getEnclosingModule0(AstNode n) { result = parent*(n.getParent()) }
163+
169164
cached
170165
ContainerOrModule getEnclosingModule(AstNode n) {
171166
result = TModule(getEnclosingModule0(n))

ql/src/codeql_ql/ast/internal/Predicate.qll

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ private module Cached {
8181
// super calls
8282
exists(Super sup, ClassType type, Type supertype |
8383
mc.getBase() = sup and
84-
sup.getEnclosingPredicate().(ClassPredicate).getParent().getType() = type and
84+
sup.getEnclosingPredicate().getParent().(Class).getType() = type and
8585
supertype in [type.getASuperType(), type.getAnInstanceofType()] and
8686
p = supertype.getClassPredicate(mc.getMemberName(), mc.getNumberOfArguments())
8787
)
@@ -110,14 +110,26 @@ private module Cached {
110110
)
111111
}
112112

113+
private predicate resolveBuildinPredicateCall(PredicateCall call, BuiltinClassless pred) {
114+
call.getNumberOfArguments() = pred.getArity() and
115+
call.getPredicateName() = pred.getName()
116+
}
117+
113118
cached
114119
predicate resolveCall(Call c, PredicateOrBuiltin p) {
115120
resolvePredicateCall(c, p)
116121
or
122+
not resolvePredicateCall(c, _) and
123+
resolveBuildinPredicateCall(c, p)
124+
or
117125
resolveMemberCall(c, p)
118126
or
119127
not resolvePredicateCall(c, _) and
120128
resolveDBRelation(c, p)
129+
or
130+
// getAQlClass() is special
131+
c.(MemberCall).getMemberName() = "getAQlClass" and
132+
p.(BuiltinMember).getName() = "getAQlClass"
121133
}
122134
}
123135

ql/test/callgraph/Bar.qll

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import ql
2+
3+
module Firebase {
4+
module Database {
5+
query predicate ref(int i) { i = snapshot() }
6+
}
7+
8+
int snapshot() { result = 2 }
9+
}

ql/test/callgraph/Baz.qll

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
class Foo extends string {
2+
Foo() { this = "Foo" }
3+
4+
string getImportedPath() { none() }
5+
}
6+
7+
class Bar extends string, Foo {
8+
Bar() { exists(Foo.super.getImportedPath()) }
9+
10+
override string getImportedPath() { none() }
11+
}

ql/test/callgraph/callgraph.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
getTarget
2+
| Bar.qll:5:38:5:47 | PredicateCall | Bar.qll:8:3:8:31 | ClasslessPredicate snapshot |
3+
| Baz.qll:8:18:8:44 | MemberCall | Baz.qll:4:3:4:37 | ClassPredicate getImportedPath |
24
| Foo.qll:5:26:5:30 | PredicateCall | Foo.qll:3:1:3:26 | ClasslessPredicate foo |
35
| Foo.qll:10:21:10:25 | PredicateCall | Foo.qll:8:3:8:28 | ClassPredicate bar |
46
| Foo.qll:14:30:14:40 | MemberCall | Foo.qll:10:3:10:27 | ClassPredicate baz |

ql/test/printAst/printAst.expected

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,7 @@ nodes
196196
| file://:0:0:0:0 | exp | semmle.label | [BuiltinPredicate] exp |
197197
| file://:0:0:0:0 | floor | semmle.label | [BuiltinPredicate] floor |
198198
| file://:0:0:0:0 | gcd | semmle.label | [BuiltinPredicate] gcd |
199+
| file://:0:0:0:0 | getAQlClass | semmle.label | [BuiltinPredicate] getAQlClass |
199200
| file://:0:0:0:0 | getDay | semmle.label | [BuiltinPredicate] getDay |
200201
| file://:0:0:0:0 | getHours | semmle.label | [BuiltinPredicate] getHours |
201202
| file://:0:0:0:0 | getMinutes | semmle.label | [BuiltinPredicate] getMinutes |
@@ -288,6 +289,10 @@ edges
288289
| Foo.qll:6:23:6:36 | ComparisonFormula | Foo.qll:6:30:6:30 | ComparisonOp | semmle.order | 14 |
289290
| Foo.qll:6:23:6:36 | ComparisonFormula | Foo.qll:6:32:6:36 | String | semmle.label | getRightOperand() |
290291
| Foo.qll:6:23:6:36 | ComparisonFormula | Foo.qll:6:32:6:36 | String | semmle.order | 15 |
292+
| Foo.qll:9:1:9:5 | annotation | Foo.qll:9:1:9:5 | annotation | semmle.label | getAnAnnotation() |
293+
| Foo.qll:9:1:9:5 | annotation | Foo.qll:9:1:9:5 | annotation | semmle.order | 16 |
294+
| Foo.qll:9:7:11:1 | ClasslessPredicate foo | Foo.qll:9:1:9:5 | annotation | semmle.label | getAnAnnotation() |
295+
| Foo.qll:9:7:11:1 | ClasslessPredicate foo | Foo.qll:9:1:9:5 | annotation | semmle.order | 16 |
291296
| Foo.qll:9:7:11:1 | ClasslessPredicate foo | Foo.qll:9:21:9:25 | f | semmle.label | getParameter(_) |
292297
| Foo.qll:9:7:11:1 | ClasslessPredicate foo | Foo.qll:9:21:9:25 | f | semmle.order | 18 |
293298
| Foo.qll:9:7:11:1 | ClasslessPredicate foo | Foo.qll:10:3:10:85 | ComparisonFormula | semmle.label | getBody() |

0 commit comments

Comments
 (0)