Skip to content

Commit ad5bed0

Browse files
hvitvedCopilot
hvitved
and
Copilot
committed
Unified: use type-inferred field resolution in data flow
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 8fbcfb1 commit ad5bed0

6 files changed

Lines changed: 172 additions & 8 deletions

File tree

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

Lines changed: 65 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
private import unified
22
private import AllDataFlow
3+
private import codeql.unified.internal.typeinference.TypeInference as T
4+
private import codeql.unified.internal.typeinference.TypeInferencePlugin as TP
35

46
/**
57
* Holds if a MaD access path token of form `name[arg]` exists.
@@ -8,8 +10,20 @@ predicate isAccessPathTokenPresent(string name, string arg) {
810
arg = any(FlowSummaryOutput::Private::AccessPathToken tok).getAnArgument(name)
911
}
1012

13+
private class ResolvedField extends VariableDeclaration {
14+
ResolvedField() { this = T::resolveFieldAccess(_) }
15+
16+
string getName() {
17+
result = super.getName() and
18+
not this = any(TP::TupleType tt).getField(_)
19+
or
20+
this = any(TP::TupleType tt).getField(result)
21+
}
22+
}
23+
1124
private newtype TContent =
1225
TArrayElement() or
26+
TFieldContent(ResolvedField field) or
1327
TNamedMember(string name) {
1428
name = any(Identifier id).getValue()
1529
or
@@ -23,7 +37,9 @@ private newtype TContent =
2337
} or
2438
TCapturedVariable(CaptureSsaInput::CapturedVariable v)
2539

26-
class Content extends TContent {
40+
private class TContentApprox = TArrayElement or TNamedMember or TCapturedVariable;
41+
42+
class ContentApprox extends TContentApprox {
2743
string asNamedMember() { this = TNamedMember(result) }
2844

2945
predicate isArrayElement() { this = TArrayElement() }
@@ -41,6 +57,32 @@ class Content extends TContent {
4157
Location getLocation() { result = this.asCapturedVariable().getLocation() }
4258
}
4359

60+
class Content extends TContent {
61+
ResolvedField asField() { this = TFieldContent(result) }
62+
63+
string asNamedMember() { this = TNamedMember(result) }
64+
65+
predicate isArrayElement() { this = TArrayElement() }
66+
67+
string toString() {
68+
result = this.asNamedMember()
69+
or
70+
result = this.asField().getName()
71+
or
72+
this.isArrayElement() and result = "ArrayElement"
73+
or
74+
result = this.asCapturedVariable().toString()
75+
}
76+
77+
LocalVariable asCapturedVariable() { this = TCapturedVariable(result) }
78+
79+
Location getLocation() {
80+
result = this.asCapturedVariable().getLocation()
81+
or
82+
result = this.asField().getLocation()
83+
}
84+
}
85+
4486
private newtype TContentSet = TSingleton(Content content)
4587

4688
class ContentSet extends TContentSet {
@@ -52,10 +94,31 @@ class ContentSet extends TContentSet {
5294

5395
Content getAStoreContent() { result = this.asSingleton() }
5496

55-
Content getAReadContent() { result = this.asSingleton() }
97+
Content getAReadContent() {
98+
result = this.asSingleton()
99+
or
100+
result.asNamedMember() = this.asSingleton().asField().getName()
101+
or
102+
result.asField().getName() = this.asSingleton().asNamedMember()
103+
}
56104
}
57105

58106
module ContentSet {
107+
/** Gets resolved field content for `access`, falling back to name-based content. */
108+
ContentSet member(Expr access) {
109+
result.asSingleton().asField() = T::resolveFieldAccess(access)
110+
or
111+
not exists(T::resolveFieldAccess(access)) and
112+
exists(string name |
113+
name =
114+
[
115+
access.(MemberAccessExpr).getMemberName(),
116+
access.(UnqualifiedMemberAccess).getMemberName()
117+
] and
118+
result = namedMember(name)
119+
)
120+
}
121+
59122
ContentSet namedMember(string name) { result.asSingleton().asNamedMember() = name }
60123

61124
ContentSet arrayElement() { result.asSingleton().isArrayElement() }

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,11 @@ predicate step(BuilderNode node1, Step step, BuilderNode node2) {
9292
or
9393
exists(UnqualifiedMemberAccess access | access.isInstanceAccess() |
9494
node1.isLocalVariableRead(access, access.getImplicitQualifierVariable()) and
95-
step.readName(access.getMemberName()) and
95+
step.read(ContentSet::member(access)) and
9696
node2.isResultValue(access)
9797
or
9898
(node1.isIncomingValue(access) or node1.isPostUpdate(access)) and
99-
step.storeName(access.getMemberName()) and
99+
step.store(ContentSet::member(access)) and
100100
node2.isLocalVariablePostUpdate(access, access.getImplicitQualifierVariable())
101101
or
102102
// Pass the receiver along to the getter/setter invoked by this expression
@@ -129,11 +129,11 @@ predicate step(BuilderNode node1, Step step, BuilderNode node2) {
129129
or
130130
exists(MemberAccessExpr expr |
131131
node1.isResultValue(expr.getBase()) and
132-
step.readName(expr.getMemberName()) and
132+
step.read(ContentSet::member(expr)) and
133133
node2.isResultValue(expr)
134134
or
135135
(node1.isIncomingValue(expr) or node1.isPostUpdate(expr)) and
136-
step.storeName(expr.getMemberName()) and
136+
step.store(ContentSet::member(expr)) and
137137
node2.isPostUpdate(expr.getBase())
138138
or
139139
// Pass the receiver along to the getter/setter invoked by this expression

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,13 @@ module DataFlowInput implements InputSig<Location> {
1515

1616
class ContentSet = D::ContentSet;
1717

18-
class ContentApprox = Content; // TODO
18+
class ContentApprox = D::ContentApprox;
1919

20-
ContentApprox getContentApprox(Content c) { result = c } // TODO
20+
ContentApprox getContentApprox(Content c) {
21+
result = c
22+
or
23+
result.asNamedMember() = c.asField().getName()
24+
}
2125

2226
//
2327
// Parameter, argument, return, and out nodes and their positions/kinds

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
class FieldBox {
2+
var field: String = ""
3+
}
4+
5+
class OtherFieldBox {
6+
var field: String = ""
7+
}
8+
9+
class InheritedFieldBox: FieldBox {}
10+
11+
func resolvedFields() {
12+
let box = FieldBox()
13+
box.field = source("fields.resolved")
14+
sink(box.field) // $ hasValueFlow=fields.resolved
15+
sink((box as! OtherFieldBox).field) // no flow
16+
}
17+
18+
func inheritedFields() {
19+
let box = InheritedFieldBox()
20+
box.field = source("fields.inherited")
21+
sink((box as! FieldBox).field) // $ hasValueFlow=fields.inherited
22+
}
23+
24+
func unresolvedFieldRead() {
25+
let box = FieldBox()
26+
box.field = source("fields.unresolved-read")
27+
sink((box as! UnknownFieldBox).field) // $ hasValueFlow=fields.unresolved-read
28+
}
29+
30+
func unresolvedFieldStore() {
31+
let box = FieldBox()
32+
(box as! UnknownFieldBox).field = source("fields.unresolved-store")
33+
sink(box.field) // $ hasValueFlow=fields.unresolved-store
34+
}
35+
36+
func summarizedFieldRead() {
37+
let box = FieldBox()
38+
box.field = source("fields.summary")
39+
sink(TestType.readField(box)) // $ hasValueFlow=fields.summary
40+
}

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

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ edges
3232
| accessors_instance.swift:119:13:119:17 | field | accessors_instance.swift:125:18:125:22 | field | provenance | |
3333
| accessors_instance.swift:119:21:119:35 | source(...) | accessors_instance.swift:119:13:119:17 | field | provenance | |
3434
| accessors_instance.swift:134:24:134:31 | newValue [0] | accessors_instance.swift:134:24:134:33 | ... .0 | provenance | |
35+
| accessors_instance.swift:134:24:134:31 | newValue [0] | accessors_instance.swift:134:24:134:33 | ... .0 | provenance | |
3536
| accessors_instance.swift:143:5:143:7 | [post] ... .p [0] | accessors_instance.swift:134:24:134:31 | newValue [0] | provenance | |
3637
| accessors_instance.swift:143:5:143:9 | ... .0 | accessors_instance.swift:143:5:143:7 | [post] ... .p [0] | provenance | |
3738
| accessors_instance.swift:143:13:143:27 | source(...) | accessors_instance.swift:143:5:143:9 | ... .0 | provenance | |
@@ -243,6 +244,28 @@ edges
243244
| enums.swift:82:12:82:12 | e [foo.0] | enums.swift:83:10:83:23 | ... .foo(...) [foo.0] | provenance | |
244245
| enums.swift:83:10:83:23 | ... .foo(...) [foo.0] | enums.swift:83:22:83:22 | x | provenance | |
245246
| enums.swift:83:22:83:22 | x | enums.swift:84:14:84:14 | x | provenance | |
247+
| fields.swift:13:5:13:7 | [post] box [field] | fields.swift:14:10:14:12 | box [field] | provenance | |
248+
| fields.swift:13:5:13:13 | ... .field | fields.swift:13:5:13:7 | [post] box [field] | provenance | |
249+
| fields.swift:13:17:13:41 | source(...) | fields.swift:13:5:13:13 | ... .field | provenance | |
250+
| fields.swift:14:10:14:12 | box [field] | fields.swift:14:10:14:18 | ... .field | provenance | |
251+
| fields.swift:20:5:20:7 | [post] box [field] | fields.swift:21:11:21:13 | box [field] | provenance | |
252+
| fields.swift:20:5:20:13 | ... .field | fields.swift:20:5:20:7 | [post] box [field] | provenance | |
253+
| fields.swift:20:17:20:42 | source(...) | fields.swift:20:5:20:13 | ... .field | provenance | |
254+
| fields.swift:21:11:21:13 | box [field] | fields.swift:21:11:21:26 | TypeCastExpr [field] | provenance | |
255+
| fields.swift:21:11:21:26 | TypeCastExpr [field] | fields.swift:21:10:21:33 | ... .field | provenance | |
256+
| fields.swift:26:5:26:7 | [post] box [field] | fields.swift:27:11:27:13 | box [field] | provenance | |
257+
| fields.swift:26:5:26:13 | ... .field | fields.swift:26:5:26:7 | [post] box [field] | provenance | |
258+
| fields.swift:26:17:26:48 | source(...) | fields.swift:26:5:26:13 | ... .field | provenance | |
259+
| fields.swift:27:11:27:13 | box [field] | fields.swift:27:11:27:33 | TypeCastExpr [field] | provenance | |
260+
| fields.swift:27:11:27:33 | TypeCastExpr [field] | fields.swift:27:10:27:40 | ... .field | provenance | |
261+
| fields.swift:32:5:32:35 | ... .field | fields.swift:32:6:32:28 | [post] TypeCastExpr [field] | provenance | |
262+
| fields.swift:32:6:32:28 | [post] TypeCastExpr [field] | fields.swift:33:10:33:12 | box [field] | provenance | |
263+
| fields.swift:32:39:32:71 | source(...) | fields.swift:32:5:32:35 | ... .field | provenance | |
264+
| fields.swift:33:10:33:12 | box [field] | fields.swift:33:10:33:18 | ... .field | provenance | |
265+
| fields.swift:38:5:38:7 | [post] box [field] | fields.swift:39:29:39:31 | box [field] | provenance | |
266+
| fields.swift:38:5:38:13 | ... .field | fields.swift:38:5:38:7 | [post] box [field] | provenance | |
267+
| fields.swift:38:17:38:40 | source(...) | fields.swift:38:5:38:13 | ... .field | provenance | |
268+
| fields.swift:39:29:39:31 | box [field] | fields.swift:39:10:39:32 | ... .readField(...) | provenance | |
246269
| flow-summaries.swift:2:9:2:9 | x | flow-summaries.swift:3:35:3:35 | x | provenance | |
247270
| flow-summaries.swift:2:13:2:26 | source(...) | flow-summaries.swift:2:9:2:9 | x | provenance | |
248271
| flow-summaries.swift:3:9:3:9 | y | flow-summaries.swift:4:10:4:10 | y | provenance | |
@@ -568,6 +591,7 @@ nodes
568591
| accessors_instance.swift:119:21:119:35 | source(...) | semmle.label | source(...) |
569592
| accessors_instance.swift:125:18:125:22 | field | semmle.label | field |
570593
| accessors_instance.swift:134:24:134:31 | newValue [0] | semmle.label | newValue [0] |
594+
| accessors_instance.swift:134:24:134:31 | newValue [0] | semmle.label | newValue [0] |
571595
| accessors_instance.swift:134:24:134:33 | ... .0 | semmle.label | ... .0 |
572596
| accessors_instance.swift:143:5:143:7 | [post] ... .p [0] | semmle.label | [post] ... .p [0] |
573597
| accessors_instance.swift:143:5:143:9 | ... .0 | semmle.label | ... .0 |
@@ -832,6 +856,33 @@ nodes
832856
| enums.swift:83:10:83:23 | ... .foo(...) [foo.0] | semmle.label | ... .foo(...) [foo.0] |
833857
| enums.swift:83:22:83:22 | x | semmle.label | x |
834858
| enums.swift:84:14:84:14 | x | semmle.label | x |
859+
| fields.swift:13:5:13:7 | [post] box [field] | semmle.label | [post] box [field] |
860+
| fields.swift:13:5:13:13 | ... .field | semmle.label | ... .field |
861+
| fields.swift:13:17:13:41 | source(...) | semmle.label | source(...) |
862+
| fields.swift:14:10:14:12 | box [field] | semmle.label | box [field] |
863+
| fields.swift:14:10:14:18 | ... .field | semmle.label | ... .field |
864+
| fields.swift:20:5:20:7 | [post] box [field] | semmle.label | [post] box [field] |
865+
| fields.swift:20:5:20:13 | ... .field | semmle.label | ... .field |
866+
| fields.swift:20:17:20:42 | source(...) | semmle.label | source(...) |
867+
| fields.swift:21:10:21:33 | ... .field | semmle.label | ... .field |
868+
| fields.swift:21:11:21:13 | box [field] | semmle.label | box [field] |
869+
| fields.swift:21:11:21:26 | TypeCastExpr [field] | semmle.label | TypeCastExpr [field] |
870+
| fields.swift:26:5:26:7 | [post] box [field] | semmle.label | [post] box [field] |
871+
| fields.swift:26:5:26:13 | ... .field | semmle.label | ... .field |
872+
| fields.swift:26:17:26:48 | source(...) | semmle.label | source(...) |
873+
| fields.swift:27:10:27:40 | ... .field | semmle.label | ... .field |
874+
| fields.swift:27:11:27:13 | box [field] | semmle.label | box [field] |
875+
| fields.swift:27:11:27:33 | TypeCastExpr [field] | semmle.label | TypeCastExpr [field] |
876+
| fields.swift:32:5:32:35 | ... .field | semmle.label | ... .field |
877+
| fields.swift:32:6:32:28 | [post] TypeCastExpr [field] | semmle.label | [post] TypeCastExpr [field] |
878+
| fields.swift:32:39:32:71 | source(...) | semmle.label | source(...) |
879+
| fields.swift:33:10:33:12 | box [field] | semmle.label | box [field] |
880+
| fields.swift:33:10:33:18 | ... .field | semmle.label | ... .field |
881+
| fields.swift:38:5:38:7 | [post] box [field] | semmle.label | [post] box [field] |
882+
| fields.swift:38:5:38:13 | ... .field | semmle.label | ... .field |
883+
| fields.swift:38:17:38:40 | source(...) | semmle.label | source(...) |
884+
| fields.swift:39:10:39:32 | ... .readField(...) | semmle.label | ... .readField(...) |
885+
| fields.swift:39:29:39:31 | box [field] | semmle.label | box [field] |
835886
| flow-summaries.swift:2:9:2:9 | x | semmle.label | x |
836887
| flow-summaries.swift:2:13:2:26 | source(...) | semmle.label | source(...) |
837888
| flow-summaries.swift:3:9:3:9 | y | semmle.label | y |
@@ -1288,6 +1339,11 @@ testFailures
12881339
| enums.swift:65:10:65:14 | opt_x | enums.swift:63:31:63:44 | source(...) | enums.swift:65:10:65:14 | opt_x | $@ | enums.swift:63:31:63:44 | source(...) | source(...) |
12891340
| enums.swift:76:14:76:14 | x | enums.swift:73:34:73:47 | source(...) | enums.swift:76:14:76:14 | x | $@ | enums.swift:73:34:73:47 | source(...) | source(...) |
12901341
| enums.swift:84:14:84:14 | x | enums.swift:73:34:73:47 | source(...) | enums.swift:84:14:84:14 | x | $@ | enums.swift:73:34:73:47 | source(...) | source(...) |
1342+
| fields.swift:14:10:14:18 | ... .field | fields.swift:13:17:13:41 | source(...) | fields.swift:14:10:14:18 | ... .field | $@ | fields.swift:13:17:13:41 | source(...) | source(...) |
1343+
| fields.swift:21:10:21:33 | ... .field | fields.swift:20:17:20:42 | source(...) | fields.swift:21:10:21:33 | ... .field | $@ | fields.swift:20:17:20:42 | source(...) | source(...) |
1344+
| fields.swift:27:10:27:40 | ... .field | fields.swift:26:17:26:48 | source(...) | fields.swift:27:10:27:40 | ... .field | $@ | fields.swift:26:17:26:48 | source(...) | source(...) |
1345+
| fields.swift:33:10:33:18 | ... .field | fields.swift:32:39:32:71 | source(...) | fields.swift:33:10:33:18 | ... .field | $@ | fields.swift:32:39:32:71 | source(...) | source(...) |
1346+
| fields.swift:39:10:39:32 | ... .readField(...) | fields.swift:38:17:38:40 | source(...) | fields.swift:39:10:39:32 | ... .readField(...) | $@ | fields.swift:38:17:38:40 | source(...) | source(...) |
12911347
| flow-summaries.swift:4:10:4:10 | y | flow-summaries.swift:2:13:2:26 | source(...) | flow-summaries.swift:4:10:4:10 | y | $@ | flow-summaries.swift:2:13:2:26 | source(...) | source(...) |
12921348
| flow-summaries.swift:11:14:11:14 | y | flow-summaries.swift:8:13:8:26 | source(...) | flow-summaries.swift:11:14:11:14 | y | $@ | flow-summaries.swift:8:13:8:26 | source(...) | source(...) |
12931349
| flow-summaries.swift:18:10:18:10 | y | flow-summaries.swift:16:18:16:31 | source(...) | flow-summaries.swift:18:10:18:10 | y | $@ | flow-summaries.swift:16:18:16:31 | source(...) | source(...) |

‎unified/ql/test/library-tests/dataflow/test.ext.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,3 +18,4 @@ extensions:
1818
- ["", "TestType", true, "swap(_:_:)", "", "", "Argument[1]", "Argument[0]", "value", "manual"]
1919
- ["", "TestType", true, "getField()", "", "", "Argument[self].Member[field]", "ReturnValue", "value", "manual"]
2020
- ["", "TestType", true, "setField(_:)", "", "", "Argument[0]", "Argument[self].Member[field]", "value", "manual"]
21+
- ["", "TestType", true, "readField(_:)", "", "", "Argument[0].Member[field]", "ReturnValue", "value", "manual"]

0 commit comments

Comments
 (0)