Skip to content

C#: Implement ContentSet #17207

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 22, 2024
Merged

C#: Implement ContentSet #17207

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f6ec56a
C#: Implement `ContentSet`
hvitved Aug 13, 2024
d638b5c
Sync shared file
hvitved Aug 13, 2024
e94fabc
Address review comment
hvitved Aug 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
178 changes: 105 additions & 73 deletions csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -908,19 +908,20 @@ private class Argument extends Expr {
*
* `postUpdate` indicates whether the store targets a post-update node.
*/
private predicate fieldOrPropertyStore(Expr e, Content c, Expr src, Expr q, boolean postUpdate) {
private predicate fieldOrPropertyStore(Expr e, ContentSet c, Expr src, Expr q, boolean postUpdate) {
exists(FieldOrProperty f |
c = f.getContent() and
c = f.getContentSet() and
(
f.isFieldLike() and
f instanceof InstanceFieldOrProperty
or
exists(
FlowSummaryImpl::Private::SummarizedCallableImpl sc,
FlowSummaryImpl::Private::SummaryComponentStack input
FlowSummaryImpl::Private::SummaryComponentStack input, ContentSet readSet
|
sc.propagatesFlow(input, _, _, _) and
input.contains(FlowSummaryImpl::Private::SummaryComponent::content(f.getContent()))
input.contains(FlowSummaryImpl::Private::SummaryComponent::content(readSet)) and
c.getAStoreContent() = readSet.getAReadContent()
)
)
|
Expand Down Expand Up @@ -970,28 +971,13 @@ private predicate fieldOrPropertyStore(Expr e, Content c, Expr src, Expr q, bool
)
}

/** Holds if property `p1` overrides or implements source declaration property `p2`. */
private predicate overridesOrImplementsSourceDecl(Property p1, Property p2) {
p1.getOverridee*().getUnboundDeclaration() = p2
or
p1.getAnUltimateImplementee().getUnboundDeclaration() = p2
}

/**
* Holds if `e2` is an expression that reads field or property `c` from
* expression `e1`. This takes overriding into account for properties written
* from library code.
* expression `e1`.
*/
private predicate fieldOrPropertyRead(Expr e1, Content c, FieldOrPropertyRead e2) {
private predicate fieldOrPropertyRead(Expr e1, ContentSet c, FieldOrPropertyRead e2) {
e1 = e2.getQualifier() and
exists(FieldOrProperty ret | c = ret.getContent() |
ret = e2.getTarget()
or
exists(Property target |
target.getGetter() = e2.(PropertyCall).getARuntimeTarget() and
overridesOrImplementsSourceDecl(target, ret)
)
)
c = e2.getTarget().(FieldOrProperty).getContentSet()
}

/**
Expand Down Expand Up @@ -1208,6 +1194,11 @@ private module Cached {
} or
TCapturedVariableContent(VariableCapture::CapturedVariable v)

cached
newtype TContentSet =
TSingletonContent(Content c) { not c instanceof PropertyContent } or
TPropertyContentSet(Property p) { p.isUnboundDeclaration() }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the corresponding TPropertyContent injector be deleted from TContent if Content has been replaced with ContentSet everywhere (it looks like it will require a re-write of ContentApprox)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still need Content, so that can't be deleted. ContentSet is a type used to represent sets of Contents with just a single entity.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I get that we need the Content type. I am just trying to understand whether we need an injector for properties both as a part of Content and ContentSet?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could in principle have just a single injector for TContentSet, but having two makes it more explicit that properties need special treatment.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok - thank you!


cached
newtype TContentApprox =
TFieldApproxContent(string firstChar) { firstChar = approximateFieldContent(_) } or
Expand Down Expand Up @@ -2076,10 +2067,10 @@ class FieldOrProperty extends Assignable, Modifiable {
}

/** Gets the content that matches this field or property. */
Content getContent() {
result.(FieldContent).getField() = this.getUnboundDeclaration()
ContentSet getContentSet() {
result.isField(this.getUnboundDeclaration())
or
result.(PropertyContent).getProperty() = this.getUnboundDeclaration()
result.isProperty(this.getUnboundDeclaration())
}
}

Expand Down Expand Up @@ -2209,8 +2200,8 @@ private class StoreStepConfiguration extends ControlFlowReachabilityConfiguratio
}

pragma[nomagic]
private PropertyContent getResultContent() {
result.getProperty() = any(SystemThreadingTasksTaskTClass c_).getResultProperty()
private ContentSet getResultContent() {
result.isProperty(any(SystemThreadingTasksTaskTClass c_).getResultProperty())
}

private predicate primaryConstructorParameterStore(
Expand All @@ -2224,17 +2215,16 @@ private predicate primaryConstructorParameterStore(
)
}

/**
* Holds if data can flow from `node1` to `node2` via an assignment to
* content `c`.
*/
predicate storeStep(Node node1, ContentSet c, Node node2) {
pragma[nomagic]
private predicate recordParameter(RecordType t, Parameter p, string name) {
p.getName() = name and p.getCallable().getDeclaringType() = t
}

private predicate storeContentStep(Node node1, Content c, Node node2) {
exists(StoreStepConfiguration x, ExprNode node, boolean postUpdate |
hasNodePath(x, node1, node) and
if postUpdate = true then node = node2.(PostUpdateNode).getPreUpdateNode() else node = node2
|
fieldOrPropertyStore(_, c, node1.asExpr(), node.getExpr(), postUpdate)
or
arrayStore(_, node1.asExpr(), node.getExpr(), postUpdate) and c instanceof ElementContent
)
or
Expand All @@ -2255,26 +2245,59 @@ predicate storeStep(Node node1, ContentSet c, Node node2) {
c instanceof ElementContent
)
or
primaryConstructorParameterStore(node1, c, node2)
or
exists(Parameter p, DataFlowCallable callable |
node1 = TExplicitParameterNode(p, callable) and
node2 = TPrimaryConstructorThisAccessNode(p, true, callable) and
not recordParameter(_, p, _) and
c.(PrimaryConstructorParameterContent).getParameter() = p
)
or
VariableCapture::storeStep(node1, c, node2)
}

pragma[nomagic]
private predicate recordProperty(RecordType t, ContentSet c, string name) {
exists(Property p |
c.isProperty(p) and
p.getName() = name and
p.getDeclaringType() = t
)
}

/**
* Holds if data can flow from `node1` to `node2` via an assignment to
* content `c`.
*/
predicate storeStep(Node node1, ContentSet c, Node node2) {
exists(Content cont |
storeContentStep(node1, cont, node2) and
c.isSingleton(cont)
)
or
exists(StoreStepConfiguration x, ExprNode node, boolean postUpdate |
hasNodePath(x, node1, node) and
if postUpdate = true then node = node2.(PostUpdateNode).getPreUpdateNode() else node = node2
|
fieldOrPropertyStore(_, c, node1.asExpr(), node.getExpr(), postUpdate)
)
or
exists(Expr e |
e = node1.asExpr() and
node2.(AsyncReturnNode).getExpr() = e and
c = getResultContent()
)
or
primaryConstructorParameterStore(node1, c, node2)
or
exists(Parameter p, DataFlowCallable callable |
exists(Parameter p, DataFlowCallable callable, RecordType t, string name |
node1 = TExplicitParameterNode(p, callable) and
node2 = TPrimaryConstructorThisAccessNode(p, true, callable) and
if p.getCallable().getDeclaringType() instanceof RecordType
then c.(PropertyContent).getProperty().getName() = p.getName()
else c.(PrimaryConstructorParameterContent).getParameter() = p
recordParameter(t, p, name) and
recordProperty(t, c, name)
)
or
FlowSummaryImpl::Private::Steps::summaryStoreStep(node1.(FlowSummaryNode).getSummaryNode(), c,
node2.(FlowSummaryNode).getSummaryNode())
or
VariableCapture::storeStep(node1, c, node2)
}

private class ReadStepConfiguration extends ControlFlowReachabilityConfiguration {
Expand Down Expand Up @@ -2342,14 +2365,8 @@ private class ReadStepConfiguration extends ControlFlowReachabilityConfiguration
}
}

/**
* Holds if data can flow from `node1` to `node2` via a read of content `c`.
*/
predicate readStep(Node node1, ContentSet c, Node node2) {
private predicate readContentStep(Node node1, Content c, Node node2) {
exists(ReadStepConfiguration x |
hasNodePath(x, node1, node2) and
fieldOrPropertyRead(node1.asExpr(), c, node2.asExpr())
or
hasNodePath(x, node1, node2) and
arrayRead(node1.asExpr(), node2.asExpr()) and
c instanceof ElementContent
Expand All @@ -2361,10 +2378,6 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
c instanceof ElementContent
)
or
hasNodePath(x, node1, node2) and
node2.asExpr().(AwaitExpr).getExpr() = node1.asExpr() and
c = getResultContent()
or
node1 =
any(InstanceParameterAccessPreNode n |
n.getUnderlyingControlFlowNode() = node2.(ExprNode).getControlFlowNode() and
Expand Down Expand Up @@ -2402,10 +2415,41 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
)
)
or
VariableCapture::readStep(node1, c, node2)
}

/**
* Holds if data can flow from `node1` to `node2` via a read of content `c`.
*/
predicate readStep(Node node1, ContentSet c, Node node2) {
exists(Content cont |
readContentStep(node1, cont, node2) and
c.isSingleton(cont)
)
or
exists(ReadStepConfiguration x | hasNodePath(x, node1, node2) |
fieldOrPropertyRead(node1.asExpr(), c, node2.asExpr())
or
node2.asExpr().(AwaitExpr).getExpr() = node1.asExpr() and
c = getResultContent()
)
or
FlowSummaryImpl::Private::Steps::summaryReadStep(node1.(FlowSummaryNode).getSummaryNode(), c,
node2.(FlowSummaryNode).getSummaryNode())
}

private predicate clearsCont(Node n, Content c) {
exists(Argument a, Struct s, Field f |
a = n.(PostUpdateNode).getPreUpdateNode().asExpr() and
a.getType() = s and
f = s.getAField() and
c.(FieldContent).getField() = f.getUnboundDeclaration() and
not f.isRef()
)
or
VariableCapture::readStep(node1, c, node2)
n = any(PostUpdateNode n1 | primaryConstructorParameterStore(_, c, n1)).getPreUpdateNode()
or
VariableCapture::clearsContent(n, c)
}

/**
Expand All @@ -2414,6 +2458,11 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
* in `x.f = newValue`.
*/
predicate clearsContent(Node n, ContentSet c) {
exists(Content cont |
clearsCont(n, cont) and
c.isSingleton(cont)
)
or
fieldOrPropertyStore(_, c, _, n.asExpr(), true)
or
fieldOrPropertyStore(_, c, _, n.(ObjectInitializerNode).getInitializer(), false)
Expand All @@ -2424,20 +2473,8 @@ predicate clearsContent(Node n, ContentSet c) {
oi = we.getInitializer() and
n.asExpr() = oi and
f = oi.getAMemberInitializer().getInitializedMember() and
c = f.getContent()
)
or
exists(Argument a, Struct s, Field f |
a = n.(PostUpdateNode).getPreUpdateNode().asExpr() and
a.getType() = s and
f = s.getAField() and
c.(FieldContent).getField() = f.getUnboundDeclaration() and
not f.isRef()
c = f.getContentSet()
)
or
n = any(PostUpdateNode n1 | primaryConstructorParameterStore(_, c, n1)).getPreUpdateNode()
or
VariableCapture::clearsContent(n, c)
}

/**
Expand All @@ -2447,7 +2484,7 @@ predicate clearsContent(Node n, ContentSet c) {
predicate expectsContent(Node n, ContentSet c) {
FlowSummaryImpl::Private::Steps::summaryExpectsContent(n.(FlowSummaryNode).getSummaryNode(), c)
or
n.asExpr() instanceof SpreadElementExpr and c instanceof ElementContent
n.asExpr() instanceof SpreadElementExpr and c.isElement()
}

class NodeRegion instanceof ControlFlow::BasicBlock {
Expand Down Expand Up @@ -3048,8 +3085,3 @@ abstract class SyntheticField extends string {
/** Gets the type of this synthetic field. */
Type getType() { result instanceof ObjectType }
}

/**
* Holds if the the content `c` is a container.
*/
predicate containerContent(DataFlow::Content c) { c instanceof DataFlow::ElementContent }
70 changes: 65 additions & 5 deletions csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,22 +267,82 @@ class CapturedVariableContent extends Content, TCapturedVariableContent {
override Location getLocation() { result = v.getLocation() }
}

/** Holds if property `p1` overrides or implements source declaration property `p2`. */
private predicate overridesOrImplementsSourceDecl(Property p1, Property p2) {
p1.getOverridee*().getUnboundDeclaration() = p2
or
p1.getAnUltimateImplementee().getUnboundDeclaration() = p2
}

/**
* An entity that represents a set of `Content`s.
*
* The set may be interpreted differently depending on whether it is
* stored into (`getAStoreContent`) or read from (`getAReadContent`).
*/
class ContentSet instanceof Content {
class ContentSet extends TContentSet {
/** Holds if this content set is the singleton `{c}`. */
predicate isSingleton(Content c) { this = TSingletonContent(c) }

/**
* Holds if this content set represents the property `p`.
*
*
* For `getAReadContent`, this set represents all properties that may
* (reflexively and transitively) override/implement `p` (or vice versa).
*/
predicate isProperty(Property p) { this = TPropertyContentSet(p) }

/** Holds if this content set represent the field `f`. */
predicate isField(Field f) { this.isSingleton(TFieldContent(f)) }

/** Holds if this content set represents an element in a collection. */
predicate isElement() { this.isSingleton(TElementContent()) }

/** Gets a content that may be stored into when storing into this set. */
Content getAStoreContent() { result = this }
Content getAStoreContent() {
this.isSingleton(result)
or
this.isProperty(result.(PropertyContent).getProperty())
}

/** Gets a content that may be read from when reading from this set. */
Content getAReadContent() { result = this }
Content getAReadContent() {
this.isSingleton(result)
or
exists(Property p1, Property p2 |
this.isProperty(p1) and
p2 = result.(PropertyContent).getProperty()
|
overridesOrImplementsSourceDecl(p2, p1)
or
overridesOrImplementsSourceDecl(p1, p2)
)
}

/** Gets a textual representation of this content set. */
string toString() { result = super.toString() }
string toString() {
exists(Content c |
this.isSingleton(c) and
result = c.toString()
)
or
exists(Property p |
this.isProperty(p) and
result = "property " + p.getName()
)
}

/** Gets the location of this content set. */
Location getLocation() { result = super.getLocation() }
Location getLocation() {
exists(Content c |
this.isSingleton(c) and
result = c.getLocation()
)
or
exists(Property p |
this.isProperty(p) and
result = p.getLocation()
)
}
}
Loading
Loading