Skip to content

Commit 14f9997

Browse files
authored
Merge pull request #20862 from MathiasVP/union-content-field-content-common-base-class
C++: Create a common base class for 'FieldContent' and 'UnionContent'
2 parents 240c637 + 6c4def1 commit 14f9997

File tree

4 files changed

+61
-55
lines changed

4 files changed

+61
-55
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The class `DataFlow::FieldContent` now covers both `union` and `struct`/`class` types. A new predicate `FieldContent.getAField` has been added to access the union members associated with the `FieldContent`. The old `FieldContent` has been renamed to `NonUnionFieldContent`.

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll

Lines changed: 19 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -861,6 +861,10 @@ predicate jumpStep(Node n1, Node n2) {
861861
n2.(FlowSummaryNode).getSummaryNode())
862862
}
863863

864+
bindingset[c]
865+
pragma[inline_late]
866+
private int getIndirectionIndexLate(Content c) { result = c.getIndirectionIndex() }
867+
864868
/**
865869
* Holds if data can flow from `node1` to `node2` via an assignment to `f`.
866870
* Thus, `node2` references an object with a field `f` that contains the
@@ -873,23 +877,17 @@ predicate jumpStep(Node n1, Node n2) {
873877
predicate storeStepImpl(Node node1, Content c, Node node2, boolean certain) {
874878
exists(
875879
PostFieldUpdateNode postFieldUpdate, int indirectionIndex1, int numberOfLoads,
876-
StoreInstruction store
880+
StoreInstruction store, FieldContent fc
877881
|
878882
postFieldUpdate = node2 and
879-
nodeHasInstruction(node1, store, pragma[only_bind_into](indirectionIndex1)) and
883+
fc = c and
884+
nodeHasInstruction(node1, pragma[only_bind_into](store),
885+
pragma[only_bind_into](indirectionIndex1)) and
880886
postFieldUpdate.getIndirectionIndex() = 1 and
881887
numberOfLoadsFromOperand(postFieldUpdate.getFieldAddress(),
882-
store.getDestinationAddressOperand(), numberOfLoads, certain)
883-
|
884-
exists(FieldContent fc | fc = c |
885-
fc.getField() = postFieldUpdate.getUpdatedField() and
886-
fc.getIndirectionIndex() = 1 + indirectionIndex1 + numberOfLoads
887-
)
888-
or
889-
exists(UnionContent uc | uc = c |
890-
uc.getAField() = postFieldUpdate.getUpdatedField() and
891-
uc.getIndirectionIndex() = 1 + indirectionIndex1 + numberOfLoads
892-
)
888+
store.getDestinationAddressOperand(), numberOfLoads, certain) and
889+
fc.getAField() = postFieldUpdate.getUpdatedField() and
890+
getIndirectionIndexLate(fc) = 1 + indirectionIndex1 + numberOfLoads
893891
)
894892
or
895893
// models-as-data summarized flow
@@ -965,22 +963,17 @@ predicate nodeHasInstruction(Node node, Instruction instr, int indirectionIndex)
965963
* `node2`.
966964
*/
967965
predicate readStep(Node node1, ContentSet c, Node node2) {
968-
exists(FieldAddress fa1, Operand operand, int numberOfLoads, int indirectionIndex2 |
966+
exists(
967+
FieldAddress fa1, Operand operand, int numberOfLoads, int indirectionIndex2, FieldContent fc
968+
|
969+
fc = c and
969970
nodeHasOperand(node2, operand, indirectionIndex2) and
970971
// The `1` here matches the `node2.getIndirectionIndex() = 1` conjunct
971972
// in `storeStep`.
972973
nodeHasOperand(node1, fa1.getObjectAddressOperand(), 1) and
973-
numberOfLoadsFromOperand(fa1, operand, numberOfLoads, _)
974-
|
975-
exists(FieldContent fc | fc = c |
976-
fc.getField() = fa1.getField() and
977-
fc.getIndirectionIndex() = indirectionIndex2 + numberOfLoads
978-
)
979-
or
980-
exists(UnionContent uc | uc = c |
981-
uc.getAField() = fa1.getField() and
982-
uc.getIndirectionIndex() = indirectionIndex2 + numberOfLoads
983-
)
974+
numberOfLoadsFromOperand(fa1, operand, numberOfLoads, _) and
975+
fc.getAField() = fa1.getField() and
976+
getIndirectionIndexLate(fc) = indirectionIndex2 + numberOfLoads
984977
)
985978
or
986979
// models-as-data summarized flow
@@ -1574,7 +1567,7 @@ pragma[inline]
15741567
ContentApprox getContentApprox(Content c) {
15751568
exists(string prefix, Field f |
15761569
prefix = result.(FieldApproxContent).getPrefix() and
1577-
f = c.(FieldContent).getField() and
1570+
f = c.(NonUnionFieldContent).getField() and
15781571
fieldHasApproxName(f, prefix)
15791572
)
15801573
or

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll

Lines changed: 36 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2093,8 +2093,8 @@ private Field getAFieldWithSize(Union u, int bytes) {
20932093

20942094
cached
20952095
private newtype TContent =
2096-
TFieldContent(Field f, int indirectionIndex) {
2097-
// the indirection index for field content starts at 1 (because `TFieldContent` is thought of as
2096+
TNonUnionContent(Field f, int indirectionIndex) {
2097+
// the indirection index for field content starts at 1 (because `TNonUnionContent` is thought of as
20982098
// the address of the field, `FieldAddress` in the IR).
20992099
indirectionIndex = [1 .. SsaImpl::getMaxIndirectionsForType(f.getUnspecifiedType())] and
21002100
// Reads and writes of union fields are tracked using `UnionContent`.
@@ -2124,14 +2124,14 @@ private newtype TContent =
21242124
*/
21252125
class Content extends TContent {
21262126
/** Gets a textual representation of this element. */
2127-
abstract string toString();
2127+
string toString() { none() } // overridden in subclasses
21282128

21292129
predicate hasLocationInfo(string path, int sl, int sc, int el, int ec) {
21302130
path = "" and sl = 0 and sc = 0 and el = 0 and ec = 0
21312131
}
21322132

21332133
/** Gets the indirection index of this `Content`. */
2134-
abstract int getIndirectionIndex();
2134+
int getIndirectionIndex() { none() } // overridden in subclasses
21352135

21362136
/**
21372137
* INTERNAL: Do not use.
@@ -2142,7 +2142,7 @@ class Content extends TContent {
21422142
* For example, a write to a field `f` implies that any content of
21432143
* the form `*f` is also cleared.
21442144
*/
2145-
abstract predicate impliesClearOf(Content c);
2145+
predicate impliesClearOf(Content c) { none() } // overridden in subclasses
21462146
}
21472147

21482148
/**
@@ -2162,22 +2162,42 @@ private module ContentStars {
21622162

21632163
private import ContentStars
21642164

2165-
/** A reference through a non-union instance field. */
2165+
private class TFieldContent = TNonUnionContent or TUnionContent;
2166+
2167+
/**
2168+
* A `Content` that references a `Field`. This may be a field of a `struct`,
2169+
* `class`, or `union`. In the case of a `union` there may be multiple fields
2170+
* associated with the same `Content`.
2171+
*/
21662172
class FieldContent extends Content, TFieldContent {
2173+
/** Gets a `Field` of this `Content`. */
2174+
Field getAField() { none() }
2175+
2176+
/**
2177+
* Gets the field associated with this `Content`, if a unique one exists.
2178+
*/
2179+
final Field getField() { result = unique( | | this.getAField()) }
2180+
2181+
override int getIndirectionIndex() { none() } // overridden in subclasses
2182+
2183+
override string toString() { none() } // overridden in subclasses
2184+
2185+
override predicate impliesClearOf(Content c) { none() } // overridden in subclasses
2186+
}
2187+
2188+
/** A reference through a non-union instance field. */
2189+
class NonUnionFieldContent extends FieldContent, TNonUnionContent {
21672190
private Field f;
21682191
private int indirectionIndex;
21692192

2170-
FieldContent() { this = TFieldContent(f, indirectionIndex) }
2193+
NonUnionFieldContent() { this = TNonUnionContent(f, indirectionIndex) }
21712194

21722195
override string toString() { result = contentStars(this) + f.toString() }
21732196

2174-
Field getField() { result = f }
2197+
override Field getAField() { result = f }
21752198

21762199
/** Gets the indirection index of this `FieldContent`. */
2177-
pragma[inline]
2178-
override int getIndirectionIndex() {
2179-
pragma[only_bind_into](result) = pragma[only_bind_out](indirectionIndex)
2180-
}
2200+
override int getIndirectionIndex() { result = indirectionIndex }
21812201

21822202
override predicate impliesClearOf(Content c) {
21832203
exists(FieldContent fc |
@@ -2191,7 +2211,7 @@ class FieldContent extends Content, TFieldContent {
21912211
}
21922212

21932213
/** A reference through an instance field of a union. */
2194-
class UnionContent extends Content, TUnionContent {
2214+
class UnionContent extends FieldContent, TUnionContent {
21952215
private Union u;
21962216
private int indirectionIndex;
21972217
private int bytes;
@@ -2201,16 +2221,13 @@ class UnionContent extends Content, TUnionContent {
22012221
override string toString() { result = contentStars(this) + u.toString() }
22022222

22032223
/** Gets a field of the underlying union of this `UnionContent`, if any. */
2204-
Field getAField() { result = u.getAField() and getFieldSize(result) = bytes }
2224+
override Field getAField() { result = u.getAField() and getFieldSize(result) = bytes }
22052225

22062226
/** Gets the underlying union of this `UnionContent`. */
22072227
Union getUnion() { result = u }
22082228

22092229
/** Gets the indirection index of this `UnionContent`. */
2210-
pragma[inline]
2211-
override int getIndirectionIndex() {
2212-
pragma[only_bind_into](result) = pragma[only_bind_out](indirectionIndex)
2213-
}
2230+
override int getIndirectionIndex() { result = indirectionIndex }
22142231

22152232
override predicate impliesClearOf(Content c) {
22162233
exists(UnionContent uc |
@@ -2234,10 +2251,7 @@ class ElementContent extends Content, TElementContent {
22342251

22352252
ElementContent() { this = TElementContent(indirectionIndex) }
22362253

2237-
pragma[inline]
2238-
override int getIndirectionIndex() {
2239-
pragma[only_bind_into](result) = pragma[only_bind_out](indirectionIndex)
2240-
}
2254+
override int getIndirectionIndex() { result = indirectionIndex }
22412255

22422256
override predicate impliesClearOf(Content c) { none() }
22432257

cpp/ql/src/utils/modelgenerator/internal/CaptureModels.qll

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ module ModelGeneratorCommonInput implements ModelGeneratorCommonInputSig<Cpp::Lo
190190
predicate isRelevantType(Type t) { any() }
191191

192192
Type getUnderlyingContentType(DataFlow::ContentSet c) {
193-
result = c.(DataFlow::FieldContent).getField().getUnspecifiedType() or
193+
result = c.(DataFlow::NonUnionFieldContent).getField().getUnspecifiedType() or
194194
result = c.(DataFlow::UnionContent).getUnion().getUnspecifiedType()
195195
}
196196

@@ -340,12 +340,7 @@ private module SummaryModelGeneratorInput implements SummaryModelGeneratorInputS
340340
)
341341
}
342342

343-
predicate isField(DataFlow::ContentSet cs) {
344-
exists(DataFlow::Content c | cs.isSingleton(c) |
345-
c instanceof DataFlow::FieldContent or
346-
c instanceof DataFlow::UnionContent
347-
)
348-
}
343+
predicate isField(DataFlow::ContentSet cs) { cs.isSingleton(any(DataFlow::FieldContent fc)) }
349344

350345
predicate isCallback(DataFlow::ContentSet c) { none() }
351346

0 commit comments

Comments
 (0)