Skip to content

Commit d41d7c8

Browse files
authored
Merge pull request #17207 from hvitved/csharp/content-set
C#: Implement `ContentSet`
2 parents a213982 + e94fabc commit d41d7c8

File tree

10 files changed

+269
-145
lines changed

10 files changed

+269
-145
lines changed

csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll

+105-73
Original file line numberDiff line numberDiff line change
@@ -908,19 +908,20 @@ private class Argument extends Expr {
908908
*
909909
* `postUpdate` indicates whether the store targets a post-update node.
910910
*/
911-
private predicate fieldOrPropertyStore(Expr e, Content c, Expr src, Expr q, boolean postUpdate) {
911+
private predicate fieldOrPropertyStore(Expr e, ContentSet c, Expr src, Expr q, boolean postUpdate) {
912912
exists(FieldOrProperty f |
913-
c = f.getContent() and
913+
c = f.getContentSet() and
914914
(
915915
f.isFieldLike() and
916916
f instanceof InstanceFieldOrProperty
917917
or
918918
exists(
919919
FlowSummaryImpl::Private::SummarizedCallableImpl sc,
920-
FlowSummaryImpl::Private::SummaryComponentStack input
920+
FlowSummaryImpl::Private::SummaryComponentStack input, ContentSet readSet
921921
|
922922
sc.propagatesFlow(input, _, _, _) and
923-
input.contains(FlowSummaryImpl::Private::SummaryComponent::content(f.getContent()))
923+
input.contains(FlowSummaryImpl::Private::SummaryComponent::content(readSet)) and
924+
c.getAStoreContent() = readSet.getAReadContent()
924925
)
925926
)
926927
|
@@ -970,28 +971,13 @@ private predicate fieldOrPropertyStore(Expr e, Content c, Expr src, Expr q, bool
970971
)
971972
}
972973

973-
/** Holds if property `p1` overrides or implements source declaration property `p2`. */
974-
private predicate overridesOrImplementsSourceDecl(Property p1, Property p2) {
975-
p1.getOverridee*().getUnboundDeclaration() = p2
976-
or
977-
p1.getAnUltimateImplementee().getUnboundDeclaration() = p2
978-
}
979-
980974
/**
981975
* Holds if `e2` is an expression that reads field or property `c` from
982-
* expression `e1`. This takes overriding into account for properties written
983-
* from library code.
976+
* expression `e1`.
984977
*/
985-
private predicate fieldOrPropertyRead(Expr e1, Content c, FieldOrPropertyRead e2) {
978+
private predicate fieldOrPropertyRead(Expr e1, ContentSet c, FieldOrPropertyRead e2) {
986979
e1 = e2.getQualifier() and
987-
exists(FieldOrProperty ret | c = ret.getContent() |
988-
ret = e2.getTarget()
989-
or
990-
exists(Property target |
991-
target.getGetter() = e2.(PropertyCall).getARuntimeTarget() and
992-
overridesOrImplementsSourceDecl(target, ret)
993-
)
994-
)
980+
c = e2.getTarget().(FieldOrProperty).getContentSet()
995981
}
996982

997983
/**
@@ -1208,6 +1194,11 @@ private module Cached {
12081194
} or
12091195
TCapturedVariableContent(VariableCapture::CapturedVariable v)
12101196

1197+
cached
1198+
newtype TContentSet =
1199+
TSingletonContent(Content c) { not c instanceof PropertyContent } or
1200+
TPropertyContentSet(Property p) { p.isUnboundDeclaration() }
1201+
12111202
cached
12121203
newtype TContentApprox =
12131204
TFieldApproxContent(string firstChar) { firstChar = approximateFieldContent(_) } or
@@ -2076,10 +2067,10 @@ class FieldOrProperty extends Assignable, Modifiable {
20762067
}
20772068

20782069
/** Gets the content that matches this field or property. */
2079-
Content getContent() {
2080-
result.(FieldContent).getField() = this.getUnboundDeclaration()
2070+
ContentSet getContentSet() {
2071+
result.isField(this.getUnboundDeclaration())
20812072
or
2082-
result.(PropertyContent).getProperty() = this.getUnboundDeclaration()
2073+
result.isProperty(this.getUnboundDeclaration())
20832074
}
20842075
}
20852076

@@ -2211,8 +2202,8 @@ private class StoreStepConfiguration extends ControlFlowReachabilityConfiguratio
22112202
}
22122203

22132204
pragma[nomagic]
2214-
private PropertyContent getResultContent() {
2215-
result.getProperty() = any(SystemThreadingTasksTaskTClass c_).getResultProperty()
2205+
private ContentSet getResultContent() {
2206+
result.isProperty(any(SystemThreadingTasksTaskTClass c_).getResultProperty())
22162207
}
22172208

22182209
private predicate primaryConstructorParameterStore(
@@ -2226,17 +2217,16 @@ private predicate primaryConstructorParameterStore(
22262217
)
22272218
}
22282219

2229-
/**
2230-
* Holds if data can flow from `node1` to `node2` via an assignment to
2231-
* content `c`.
2232-
*/
2233-
predicate storeStep(Node node1, ContentSet c, Node node2) {
2220+
pragma[nomagic]
2221+
private predicate recordParameter(RecordType t, Parameter p, string name) {
2222+
p.getName() = name and p.getCallable().getDeclaringType() = t
2223+
}
2224+
2225+
private predicate storeContentStep(Node node1, Content c, Node node2) {
22342226
exists(StoreStepConfiguration x, ExprNode node, boolean postUpdate |
22352227
hasNodePath(x, node1, node) and
22362228
if postUpdate = true then node = node2.(PostUpdateNode).getPreUpdateNode() else node = node2
22372229
|
2238-
fieldOrPropertyStore(_, c, node1.asExpr(), node.getExpr(), postUpdate)
2239-
or
22402230
arrayStore(_, node1.asExpr(), node.getExpr(), postUpdate) and c instanceof ElementContent
22412231
)
22422232
or
@@ -2257,26 +2247,59 @@ predicate storeStep(Node node1, ContentSet c, Node node2) {
22572247
c instanceof ElementContent
22582248
)
22592249
or
2250+
primaryConstructorParameterStore(node1, c, node2)
2251+
or
2252+
exists(Parameter p, DataFlowCallable callable |
2253+
node1 = TExplicitParameterNode(p, callable) and
2254+
node2 = TPrimaryConstructorThisAccessNode(p, true, callable) and
2255+
not recordParameter(_, p, _) and
2256+
c.(PrimaryConstructorParameterContent).getParameter() = p
2257+
)
2258+
or
2259+
VariableCapture::storeStep(node1, c, node2)
2260+
}
2261+
2262+
pragma[nomagic]
2263+
private predicate recordProperty(RecordType t, ContentSet c, string name) {
2264+
exists(Property p |
2265+
c.isProperty(p) and
2266+
p.getName() = name and
2267+
p.getDeclaringType() = t
2268+
)
2269+
}
2270+
2271+
/**
2272+
* Holds if data can flow from `node1` to `node2` via an assignment to
2273+
* content `c`.
2274+
*/
2275+
predicate storeStep(Node node1, ContentSet c, Node node2) {
2276+
exists(Content cont |
2277+
storeContentStep(node1, cont, node2) and
2278+
c.isSingleton(cont)
2279+
)
2280+
or
2281+
exists(StoreStepConfiguration x, ExprNode node, boolean postUpdate |
2282+
hasNodePath(x, node1, node) and
2283+
if postUpdate = true then node = node2.(PostUpdateNode).getPreUpdateNode() else node = node2
2284+
|
2285+
fieldOrPropertyStore(_, c, node1.asExpr(), node.getExpr(), postUpdate)
2286+
)
2287+
or
22602288
exists(Expr e |
22612289
e = node1.asExpr() and
22622290
node2.(AsyncReturnNode).getExpr() = e and
22632291
c = getResultContent()
22642292
)
22652293
or
2266-
primaryConstructorParameterStore(node1, c, node2)
2267-
or
2268-
exists(Parameter p, DataFlowCallable callable |
2294+
exists(Parameter p, DataFlowCallable callable, RecordType t, string name |
22692295
node1 = TExplicitParameterNode(p, callable) and
22702296
node2 = TPrimaryConstructorThisAccessNode(p, true, callable) and
2271-
if p.getCallable().getDeclaringType() instanceof RecordType
2272-
then c.(PropertyContent).getProperty().getName() = p.getName()
2273-
else c.(PrimaryConstructorParameterContent).getParameter() = p
2297+
recordParameter(t, p, name) and
2298+
recordProperty(t, c, name)
22742299
)
22752300
or
22762301
FlowSummaryImpl::Private::Steps::summaryStoreStep(node1.(FlowSummaryNode).getSummaryNode(), c,
22772302
node2.(FlowSummaryNode).getSummaryNode())
2278-
or
2279-
VariableCapture::storeStep(node1, c, node2)
22802303
}
22812304

22822305
private class ReadStepConfiguration extends ControlFlowReachabilityConfiguration {
@@ -2344,14 +2367,8 @@ private class ReadStepConfiguration extends ControlFlowReachabilityConfiguration
23442367
}
23452368
}
23462369

2347-
/**
2348-
* Holds if data can flow from `node1` to `node2` via a read of content `c`.
2349-
*/
2350-
predicate readStep(Node node1, ContentSet c, Node node2) {
2370+
private predicate readContentStep(Node node1, Content c, Node node2) {
23512371
exists(ReadStepConfiguration x |
2352-
hasNodePath(x, node1, node2) and
2353-
fieldOrPropertyRead(node1.asExpr(), c, node2.asExpr())
2354-
or
23552372
hasNodePath(x, node1, node2) and
23562373
arrayRead(node1.asExpr(), node2.asExpr()) and
23572374
c instanceof ElementContent
@@ -2363,10 +2380,6 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
23632380
c instanceof ElementContent
23642381
)
23652382
or
2366-
hasNodePath(x, node1, node2) and
2367-
node2.asExpr().(AwaitExpr).getExpr() = node1.asExpr() and
2368-
c = getResultContent()
2369-
or
23702383
node1 =
23712384
any(InstanceParameterAccessPreNode n |
23722385
n.getUnderlyingControlFlowNode() = node2.(ExprNode).getControlFlowNode() and
@@ -2404,10 +2417,41 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
24042417
)
24052418
)
24062419
or
2420+
VariableCapture::readStep(node1, c, node2)
2421+
}
2422+
2423+
/**
2424+
* Holds if data can flow from `node1` to `node2` via a read of content `c`.
2425+
*/
2426+
predicate readStep(Node node1, ContentSet c, Node node2) {
2427+
exists(Content cont |
2428+
readContentStep(node1, cont, node2) and
2429+
c.isSingleton(cont)
2430+
)
2431+
or
2432+
exists(ReadStepConfiguration x | hasNodePath(x, node1, node2) |
2433+
fieldOrPropertyRead(node1.asExpr(), c, node2.asExpr())
2434+
or
2435+
node2.asExpr().(AwaitExpr).getExpr() = node1.asExpr() and
2436+
c = getResultContent()
2437+
)
2438+
or
24072439
FlowSummaryImpl::Private::Steps::summaryReadStep(node1.(FlowSummaryNode).getSummaryNode(), c,
24082440
node2.(FlowSummaryNode).getSummaryNode())
2441+
}
2442+
2443+
private predicate clearsCont(Node n, Content c) {
2444+
exists(Argument a, Struct s, Field f |
2445+
a = n.(PostUpdateNode).getPreUpdateNode().asExpr() and
2446+
a.getType() = s and
2447+
f = s.getAField() and
2448+
c.(FieldContent).getField() = f.getUnboundDeclaration() and
2449+
not f.isRef()
2450+
)
24092451
or
2410-
VariableCapture::readStep(node1, c, node2)
2452+
n = any(PostUpdateNode n1 | primaryConstructorParameterStore(_, c, n1)).getPreUpdateNode()
2453+
or
2454+
VariableCapture::clearsContent(n, c)
24112455
}
24122456

24132457
/**
@@ -2416,6 +2460,11 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
24162460
* in `x.f = newValue`.
24172461
*/
24182462
predicate clearsContent(Node n, ContentSet c) {
2463+
exists(Content cont |
2464+
clearsCont(n, cont) and
2465+
c.isSingleton(cont)
2466+
)
2467+
or
24192468
fieldOrPropertyStore(_, c, _, n.asExpr(), true)
24202469
or
24212470
fieldOrPropertyStore(_, c, _, n.(ObjectInitializerNode).getInitializer(), false)
@@ -2426,20 +2475,8 @@ predicate clearsContent(Node n, ContentSet c) {
24262475
oi = we.getInitializer() and
24272476
n.asExpr() = oi and
24282477
f = oi.getAMemberInitializer().getInitializedMember() and
2429-
c = f.getContent()
2430-
)
2431-
or
2432-
exists(Argument a, Struct s, Field f |
2433-
a = n.(PostUpdateNode).getPreUpdateNode().asExpr() and
2434-
a.getType() = s and
2435-
f = s.getAField() and
2436-
c.(FieldContent).getField() = f.getUnboundDeclaration() and
2437-
not f.isRef()
2478+
c = f.getContentSet()
24382479
)
2439-
or
2440-
n = any(PostUpdateNode n1 | primaryConstructorParameterStore(_, c, n1)).getPreUpdateNode()
2441-
or
2442-
VariableCapture::clearsContent(n, c)
24432480
}
24442481

24452482
/**
@@ -2449,7 +2486,7 @@ predicate clearsContent(Node n, ContentSet c) {
24492486
predicate expectsContent(Node n, ContentSet c) {
24502487
FlowSummaryImpl::Private::Steps::summaryExpectsContent(n.(FlowSummaryNode).getSummaryNode(), c)
24512488
or
2452-
n.asExpr() instanceof SpreadElementExpr and c instanceof ElementContent
2489+
n.asExpr() instanceof SpreadElementExpr and c.isElement()
24532490
}
24542491

24552492
class NodeRegion instanceof ControlFlow::BasicBlock {
@@ -3050,8 +3087,3 @@ abstract class SyntheticField extends string {
30503087
/** Gets the type of this synthetic field. */
30513088
Type getType() { result instanceof ObjectType }
30523089
}
3053-
3054-
/**
3055-
* Holds if the the content `c` is a container.
3056-
*/
3057-
predicate containerContent(DataFlow::Content c) { c instanceof DataFlow::ElementContent }

csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll

+65-5
Original file line numberDiff line numberDiff line change
@@ -267,22 +267,82 @@ class CapturedVariableContent extends Content, TCapturedVariableContent {
267267
override Location getLocation() { result = v.getLocation() }
268268
}
269269

270+
/** Holds if property `p1` overrides or implements source declaration property `p2`. */
271+
private predicate overridesOrImplementsSourceDecl(Property p1, Property p2) {
272+
p1.getOverridee*().getUnboundDeclaration() = p2
273+
or
274+
p1.getAnUltimateImplementee().getUnboundDeclaration() = p2
275+
}
276+
270277
/**
271278
* An entity that represents a set of `Content`s.
272279
*
273280
* The set may be interpreted differently depending on whether it is
274281
* stored into (`getAStoreContent`) or read from (`getAReadContent`).
275282
*/
276-
class ContentSet instanceof Content {
283+
class ContentSet extends TContentSet {
284+
/** Holds if this content set is the singleton `{c}`. */
285+
predicate isSingleton(Content c) { this = TSingletonContent(c) }
286+
287+
/**
288+
* Holds if this content set represents the property `p`.
289+
*
290+
*
291+
* For `getAReadContent`, this set represents all properties that may
292+
* (reflexively and transitively) override/implement `p` (or vice versa).
293+
*/
294+
predicate isProperty(Property p) { this = TPropertyContentSet(p) }
295+
296+
/** Holds if this content set represent the field `f`. */
297+
predicate isField(Field f) { this.isSingleton(TFieldContent(f)) }
298+
299+
/** Holds if this content set represents an element in a collection. */
300+
predicate isElement() { this.isSingleton(TElementContent()) }
301+
277302
/** Gets a content that may be stored into when storing into this set. */
278-
Content getAStoreContent() { result = this }
303+
Content getAStoreContent() {
304+
this.isSingleton(result)
305+
or
306+
this.isProperty(result.(PropertyContent).getProperty())
307+
}
279308

280309
/** Gets a content that may be read from when reading from this set. */
281-
Content getAReadContent() { result = this }
310+
Content getAReadContent() {
311+
this.isSingleton(result)
312+
or
313+
exists(Property p1, Property p2 |
314+
this.isProperty(p1) and
315+
p2 = result.(PropertyContent).getProperty()
316+
|
317+
overridesOrImplementsSourceDecl(p2, p1)
318+
or
319+
overridesOrImplementsSourceDecl(p1, p2)
320+
)
321+
}
282322

283323
/** Gets a textual representation of this content set. */
284-
string toString() { result = super.toString() }
324+
string toString() {
325+
exists(Content c |
326+
this.isSingleton(c) and
327+
result = c.toString()
328+
)
329+
or
330+
exists(Property p |
331+
this.isProperty(p) and
332+
result = "property " + p.getName()
333+
)
334+
}
285335

286336
/** Gets the location of this content set. */
287-
Location getLocation() { result = super.getLocation() }
337+
Location getLocation() {
338+
exists(Content c |
339+
this.isSingleton(c) and
340+
result = c.getLocation()
341+
)
342+
or
343+
exists(Property p |
344+
this.isProperty(p) and
345+
result = p.getLocation()
346+
)
347+
}
288348
}

0 commit comments

Comments
 (0)