From 3dbc0cf0b60a945d410b06a87590078a71e5e386 Mon Sep 17 00:00:00 2001 From: Kasper Svendsen Date: Fri, 12 May 2023 11:29:46 +0200 Subject: [PATCH 1/2] QL: Make implicit receivers explicit --- ql/ql/src/codeql/Locations.qll | 4 +-- .../codeql_ql/style/UseSetLiteralQuery.qll | 36 ++++++++++--------- ql/ql/test/callgraph/Foo.qll | 2 +- ql/ql/test/callgraph/callgraph.expected | 2 +- 4 files changed, 24 insertions(+), 20 deletions(-) diff --git a/ql/ql/src/codeql/Locations.qll b/ql/ql/src/codeql/Locations.qll index 9a067d89da4..1cfb8a8d41a 100644 --- a/ql/ql/src/codeql/Locations.qll +++ b/ql/ql/src/codeql/Locations.qll @@ -25,13 +25,13 @@ class Location extends @location { int getEndColumn() { locations_default(this, _, _, _, _, result) } /** Gets the number of lines covered by this location. */ - int getNumLines() { result = getEndLine() - getStartLine() + 1 } + int getNumLines() { result = this.getEndLine() - this.getStartLine() + 1 } /** Gets a textual representation of this element. */ cached string toString() { exists(string filepath, int startline, int startcolumn, int endline, int endcolumn | - hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) and + this.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) and result = filepath + "@" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn ) } diff --git a/ql/ql/src/codeql_ql/style/UseSetLiteralQuery.qll b/ql/ql/src/codeql_ql/style/UseSetLiteralQuery.qll index c253f3da7fe..125251c0d62 100644 --- a/ql/ql/src/codeql_ql/style/UseSetLiteralQuery.qll +++ b/ql/ql/src/codeql_ql/style/UseSetLiteralQuery.qll @@ -16,7 +16,7 @@ class DisjunctionChain extends Disjunction { Formula getOperand(int i) { result = rank[i + 1](Formula operand, Location l | - operand = getAnOperand*() and + operand = this.getAnOperand*() and not operand instanceof Disjunction and l = operand.getLocation() | @@ -33,16 +33,16 @@ class DisjunctionChain extends Disjunction { */ class EqualsLiteral extends ComparisonFormula { EqualsLiteral() { - getOperator() = "=" and - getAnOperand() instanceof Literal + this.getOperator() = "=" and + this.getAnOperand() instanceof Literal } AstNode getOther() { - result = getAnOperand() and + result = this.getAnOperand() and not result instanceof Literal } - Literal getLiteral() { result = getAnOperand() } + Literal getLiteral() { result = this.getAnOperand() } } /** @@ -60,29 +60,33 @@ class DisjunctionEqualsLiteral extends DisjunctionChain { DisjunctionEqualsLiteral() { // VarAccess on the same variable exists(VarDef v | - forex(Formula f | f = getOperand(_) | + forex(Formula f | f = this.getOperand(_) | f.(EqualsLiteral).getAnOperand().(VarAccess).getDeclaration() = v ) and - firstOperand = getOperand(0).(EqualsLiteral).getAnOperand() and + firstOperand = this.getOperand(0).(EqualsLiteral).getAnOperand() and firstOperand.(VarAccess).getDeclaration() = v ) or // FieldAccess on the same variable exists(FieldDecl v | - forex(Formula f | f = getOperand(_) | + forex(Formula f | f = this.getOperand(_) | f.(EqualsLiteral).getAnOperand().(FieldAccess).getDeclaration() = v ) and - firstOperand = getOperand(0).(EqualsLiteral).getAnOperand() and + firstOperand = this.getOperand(0).(EqualsLiteral).getAnOperand() and firstOperand.(FieldAccess).getDeclaration() = v ) or // ThisAccess - forex(Formula f | f = getOperand(_) | f.(EqualsLiteral).getAnOperand() instanceof ThisAccess) and - firstOperand = getOperand(0).(EqualsLiteral).getAnOperand().(ThisAccess) + forex(Formula f | f = this.getOperand(_) | + f.(EqualsLiteral).getAnOperand() instanceof ThisAccess + ) and + firstOperand = this.getOperand(0).(EqualsLiteral).getAnOperand().(ThisAccess) or // ResultAccess - forex(Formula f | f = getOperand(_) | f.(EqualsLiteral).getAnOperand() instanceof ResultAccess) and - firstOperand = getOperand(0).(EqualsLiteral).getAnOperand().(ResultAccess) + forex(Formula f | f = this.getOperand(_) | + f.(EqualsLiteral).getAnOperand() instanceof ResultAccess + ) and + firstOperand = this.getOperand(0).(EqualsLiteral).getAnOperand().(ResultAccess) // (in principle something like GlobalValueNumbering could be used to generalize this) } @@ -100,8 +104,8 @@ class DisjunctionEqualsLiteral extends DisjunctionChain { */ class CallLiteral extends Call { CallLiteral() { - getNumberOfArguments() = 1 and - getArgument(0) instanceof Literal + this.getNumberOfArguments() = 1 and + this.getArgument(0) instanceof Literal } } @@ -118,7 +122,7 @@ class DisjunctionPredicateLiteral extends DisjunctionChain { DisjunctionPredicateLiteral() { // Call to the same target exists(PredicateOrBuiltin target | - forex(Formula f | f = getOperand(_) | f.(CallLiteral).getTarget() = target) + forex(Formula f | f = this.getOperand(_) | f.(CallLiteral).getTarget() = target) ) } } diff --git a/ql/ql/test/callgraph/Foo.qll b/ql/ql/test/callgraph/Foo.qll index 70cb53587cf..9d40471f78d 100644 --- a/ql/ql/test/callgraph/Foo.qll +++ b/ql/ql/test/callgraph/Foo.qll @@ -7,7 +7,7 @@ query predicate test() { foo() } class Foo extends AstNode { predicate bar() { none() } - predicate baz() { bar() } + predicate baz() { this.bar() } } class Sub extends Foo { diff --git a/ql/ql/test/callgraph/callgraph.expected b/ql/ql/test/callgraph/callgraph.expected index 823e8d2553a..ad54bd22bab 100644 --- a/ql/ql/test/callgraph/callgraph.expected +++ b/ql/ql/test/callgraph/callgraph.expected @@ -5,7 +5,7 @@ getTarget | Bar.qll:30:12:30:32 | MemberCall | Bar.qll:19:7:19:18 | ClassPredicate getParameter | | Baz.qll:8:18:8:44 | MemberCall | Baz.qll:4:10:4:24 | ClassPredicate getImportedPath | | Foo.qll:5:26:5:30 | PredicateCall | Foo.qll:3:11:3:13 | ClasslessPredicate foo | -| Foo.qll:10:21:10:25 | PredicateCall | Foo.qll:8:13:8:15 | ClassPredicate bar | +| Foo.qll:10:21:10:30 | MemberCall | Foo.qll:8:13:8:15 | ClassPredicate bar | | Foo.qll:14:34:14:44 | MemberCall | Foo.qll:10:13:10:15 | ClassPredicate baz | | Foo.qll:17:27:17:42 | MemberCall | Foo.qll:8:13:8:15 | ClassPredicate bar | | Foo.qll:29:5:29:16 | PredicateCall | Foo.qll:20:13:20:20 | ClasslessPredicate myThing2 | From 1af1bf89177c9dc08023f261e5be6288beefa2dd Mon Sep 17 00:00:00 2001 From: Kasper Svendsen Date: Fri, 12 May 2023 11:30:24 +0200 Subject: [PATCH 2/2] QL: Enable implicit this receiver warnings --- ql/ql/src/qlpack.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/ql/ql/src/qlpack.yml b/ql/ql/src/qlpack.yml index 82ea061b25a..46eb43ff429 100644 --- a/ql/ql/src/qlpack.yml +++ b/ql/ql/src/qlpack.yml @@ -8,3 +8,4 @@ extractor: ql dependencies: codeql/typos: ${workspace} codeql/util: ${workspace} +warnOnImplicitThis: true