Skip to content

Commit 364e982

Browse files
asgerfasgerf
committed
unified: Also resolve accessors on instances
1 parent 32979b2 commit 364e982

5 files changed

Lines changed: 60 additions & 8 deletions

File tree

‎unified/ql/lib/codeql/unified/internal/dataflow/CallGraph.qll‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,4 +16,12 @@ DataFlowCallable viableCallable(DataFlowCall c) {
1616

1717
private AccessorDeclaration resolveAccessor(Expr access) {
1818
result.getNameNode() = N::getStaticBindingTargetFromRef(access)
19+
or
20+
not exists(N::getStaticBindingTargetFromRef(access)) and
21+
exists(MemberAccessExpr m, N::NamespaceNode namespace |
22+
access = m and
23+
// TODO: Make type inference own accessor-resolution. It also needs it internally.
24+
namespace.isInstanceMemberNamespace(T::inferType(m.getBase()).asClass()) and
25+
namespace.getMember(m.getMemberName()).isMember(result)
26+
)
1927
}

‎unified/ql/lib/codeql/unified/internal/dataflow/DataFlowCall.qll‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,19 @@ private newtype TDataFlowCall =
99
} or
1010
TGetterCall(Expr expr) {
1111
hasResultValue(expr) and
12-
N::getStaticBindingTargetFromRef(expr) = any(GetterDeclaration g).getNameNode()
12+
(
13+
N::getStaticBindingTargetFromRef(expr) = any(GetterDeclaration g).getNameNode()
14+
or
15+
expr instanceof MemberAccess
16+
)
1317
} or
1418
TSetterCall(Expr expr) {
1519
isInAssignmentContext(expr, _) and
16-
N::getStaticBindingTargetFromRef(expr) = any(SetterDeclaration g).getNameNode()
20+
(
21+
N::getStaticBindingTargetFromRef(expr) = any(SetterDeclaration g).getNameNode()
22+
or
23+
expr instanceof MemberAccess
24+
)
1725
} or
1826
TSummaryCall(SummarizedCallable callable, SummaryNode callee) {
1927
FlowSummaryOutput::summaryCallbackRange(callable, callee)

‎unified/ql/lib/codeql/unified/internal/typeinference/Type.qll‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,9 @@ abstract private class TypeImpl extends TType {
8888

8989
/** Gets the location of this type. */
9090
abstract Location getLocation();
91+
92+
/** Gets the class whose instances are described by this type. */
93+
ClassLikeDeclaration asClass() { this = TClassLikeDeclarationType(result) }
9194
}
9295

9396
/**

‎unified/ql/test/library-tests/dataflow/accessors_instance.swift‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ func t1() {
77
class C {
88
var x: String { source("t1.1") }
99
}
10-
sink(C().x) // $ MISSING: hasValueFlow=t1.1
10+
sink(C().x) // $ hasValueFlow=t1.1
1111
}
1212

1313
func t2() {
@@ -18,7 +18,7 @@ func t2() {
1818
get { source("t2.1") }
1919
}
2020
}
21-
sink(C().x) // $ MISSING: hasValueFlow=t2.1
21+
sink(C().x) // $ hasValueFlow=t2.1
2222
}
2323

2424
func t3() {
@@ -28,14 +28,14 @@ func t3() {
2828
set {}
2929
}
3030
}
31-
sink(C().x) // $ MISSING: hasValueFlow=t3.1
31+
sink(C().x) // $ hasValueFlow=t3.1
3232
}
3333

3434
func t4() {
3535
class C {
3636
var x: String {
3737
get {}
38-
set { sink(newValue) } // $ MISSING: hasValueFlow=t4.1
38+
set { sink(newValue) } // $ hasValueFlow=t4.1
3939
}
4040
}
4141
C().x = source("t4.1")
@@ -45,7 +45,7 @@ func t5() {
4545
class C {
4646
var x: String {
4747
get { source("t5.1") }
48-
set { sink(newValue) } // $ MISSING: hasValueFlow=t5.1
48+
set { sink(newValue) } // $ hasValueFlow=t5.1
4949
}
5050
}
5151
let c = C()
@@ -57,7 +57,7 @@ func t6() {
5757
var x: String { source("t6.1") }
5858

5959
func method() {
60-
sink(self.x) // $ MISSING: hasValueFlow=t6.1
60+
sink(self.x) // $ hasValueFlow=t6.1
6161
sink(x) // $ hasValueFlow=t6.1
6262
}
6363
}

‎unified/ql/test/library-tests/dataflow/test.expected‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,16 @@
11
models
22
edges
3+
| accessors_instance.swift:8:25:8:38 | source(...) | accessors_instance.swift:10:10:10:14 | ... .x | provenance | |
4+
| accessors_instance.swift:18:19:18:32 | source(...) | accessors_instance.swift:21:10:21:14 | ... .x | provenance | |
5+
| accessors_instance.swift:27:19:27:32 | source(...) | accessors_instance.swift:31:10:31:14 | ... .x | provenance | |
6+
| accessors_instance.swift:38:13:38:15 | newValue | accessors_instance.swift:38:24:38:31 | newValue | provenance | |
7+
| accessors_instance.swift:41:5:41:9 | ... .x | accessors_instance.swift:38:13:38:15 | newValue | provenance | |
8+
| accessors_instance.swift:41:13:41:26 | source(...) | accessors_instance.swift:41:5:41:9 | ... .x | provenance | |
9+
| accessors_instance.swift:47:19:47:32 | source(...) | accessors_instance.swift:52:11:52:13 | ... .x | provenance | |
10+
| accessors_instance.swift:48:13:48:15 | newValue | accessors_instance.swift:48:24:48:31 | newValue | provenance | |
11+
| accessors_instance.swift:52:5:52:7 | ... .x | accessors_instance.swift:48:13:48:15 | newValue | provenance | |
12+
| accessors_instance.swift:52:11:52:13 | ... .x | accessors_instance.swift:52:5:52:7 | ... .x | provenance | |
13+
| accessors_instance.swift:57:25:57:38 | source(...) | accessors_instance.swift:60:18:60:23 | ... .x | provenance | |
314
| accessors_instance.swift:57:25:57:38 | source(...) | accessors_instance.swift:61:18:61:18 | x | provenance | |
415
| accessors_local.swift:7:21:7:34 | source(...) | accessors_local.swift:8:10:8:10 | x | provenance | |
516
| accessors_local.swift:15:15:15:28 | source(...) | accessors_local.swift:17:10:17:10 | x | provenance | |
@@ -483,7 +494,23 @@ edges
483494
| test.swift:271:20:271:35 | source(...) | test.swift:271:9:271:9 | x | provenance | |
484495
| test.swift:273:30:273:45 | source(...) | test.swift:274:10:274:12 | f(...) | provenance | |
485496
nodes
497+
| accessors_instance.swift:8:25:8:38 | source(...) | semmle.label | source(...) |
498+
| accessors_instance.swift:10:10:10:14 | ... .x | semmle.label | ... .x |
499+
| accessors_instance.swift:18:19:18:32 | source(...) | semmle.label | source(...) |
500+
| accessors_instance.swift:21:10:21:14 | ... .x | semmle.label | ... .x |
501+
| accessors_instance.swift:27:19:27:32 | source(...) | semmle.label | source(...) |
502+
| accessors_instance.swift:31:10:31:14 | ... .x | semmle.label | ... .x |
503+
| accessors_instance.swift:38:13:38:15 | newValue | semmle.label | newValue |
504+
| accessors_instance.swift:38:24:38:31 | newValue | semmle.label | newValue |
505+
| accessors_instance.swift:41:5:41:9 | ... .x | semmle.label | ... .x |
506+
| accessors_instance.swift:41:13:41:26 | source(...) | semmle.label | source(...) |
507+
| accessors_instance.swift:47:19:47:32 | source(...) | semmle.label | source(...) |
508+
| accessors_instance.swift:48:13:48:15 | newValue | semmle.label | newValue |
509+
| accessors_instance.swift:48:24:48:31 | newValue | semmle.label | newValue |
510+
| accessors_instance.swift:52:5:52:7 | ... .x | semmle.label | ... .x |
511+
| accessors_instance.swift:52:11:52:13 | ... .x | semmle.label | ... .x |
486512
| accessors_instance.swift:57:25:57:38 | source(...) | semmle.label | source(...) |
513+
| accessors_instance.swift:60:18:60:23 | ... .x | semmle.label | ... .x |
487514
| accessors_instance.swift:61:18:61:18 | x | semmle.label | x |
488515
| accessors_local.swift:7:21:7:34 | source(...) | semmle.label | source(...) |
489516
| accessors_local.swift:8:10:8:10 | x | semmle.label | x |
@@ -1096,6 +1123,12 @@ subpaths
10961123
| test.swift:214:43:214:57 | source(...) | test.swift:193:30:193:30 | s | test.swift:193:73:193:73 | s | test.swift:214:21:214:58 | asyncThrowingIdentity(...) |
10971124
testFailures
10981125
#select
1126+
| accessors_instance.swift:10:10:10:14 | ... .x | accessors_instance.swift:8:25:8:38 | source(...) | accessors_instance.swift:10:10:10:14 | ... .x | $@ | accessors_instance.swift:8:25:8:38 | source(...) | source(...) |
1127+
| accessors_instance.swift:21:10:21:14 | ... .x | accessors_instance.swift:18:19:18:32 | source(...) | accessors_instance.swift:21:10:21:14 | ... .x | $@ | accessors_instance.swift:18:19:18:32 | source(...) | source(...) |
1128+
| accessors_instance.swift:31:10:31:14 | ... .x | accessors_instance.swift:27:19:27:32 | source(...) | accessors_instance.swift:31:10:31:14 | ... .x | $@ | accessors_instance.swift:27:19:27:32 | source(...) | source(...) |
1129+
| accessors_instance.swift:38:24:38:31 | newValue | accessors_instance.swift:41:13:41:26 | source(...) | accessors_instance.swift:38:24:38:31 | newValue | $@ | accessors_instance.swift:41:13:41:26 | source(...) | source(...) |
1130+
| accessors_instance.swift:48:24:48:31 | newValue | accessors_instance.swift:47:19:47:32 | source(...) | accessors_instance.swift:48:24:48:31 | newValue | $@ | accessors_instance.swift:47:19:47:32 | source(...) | source(...) |
1131+
| accessors_instance.swift:60:18:60:23 | ... .x | accessors_instance.swift:57:25:57:38 | source(...) | accessors_instance.swift:60:18:60:23 | ... .x | $@ | accessors_instance.swift:57:25:57:38 | source(...) | source(...) |
10991132
| accessors_instance.swift:61:18:61:18 | x | accessors_instance.swift:57:25:57:38 | source(...) | accessors_instance.swift:61:18:61:18 | x | $@ | accessors_instance.swift:57:25:57:38 | source(...) | source(...) |
11001133
| accessors_local.swift:8:10:8:10 | x | accessors_local.swift:7:21:7:34 | source(...) | accessors_local.swift:8:10:8:10 | x | $@ | accessors_local.swift:7:21:7:34 | source(...) | source(...) |
11011134
| accessors_local.swift:17:10:17:10 | x | accessors_local.swift:15:15:15:28 | source(...) | accessors_local.swift:17:10:17:10 | x | $@ | accessors_local.swift:15:15:15:28 | source(...) | source(...) |

0 commit comments

Comments
 (0)