Skip to content

Commit ecb80cb

Browse files
committed
C++: Represent field content using a column that is shared by all template instantiations.
1 parent 14f9997 commit ecb80cb

File tree

1 file changed

+147
-23
lines changed

1 file changed

+147
-23
lines changed

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

Lines changed: 147 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2078,38 +2078,150 @@ predicate localExprFlow(Expr e1, Expr e2) {
20782078
localExprFlowPlus(e1, e2)
20792079
}
20802080

2081+
/**
2082+
* A canonical representation of a field.
2083+
*
2084+
* For performance reasons we we want a unique `Content` that represents
2085+
* a given field across any template instantiation of a class.
2086+
*
2087+
* This is possible in _almost_ all cases, but there are cases where it is
2088+
* not possible to map between a field in the uninstantiated template to a
2089+
* field in the instantiated template. This problem appears to be only in the
2090+
* case of a local class definition. So this abstract class has two
2091+
* implementations: a non-local case (where we can represent a canonical field
2092+
* as the field declaration from an uninstantiated class template or a non-
2093+
* templated class), and a local case (where we simply use the field from the
2094+
* instantiated class).
2095+
*/
2096+
abstract private class CanonicalField extends Field {
2097+
/** Gets a field represented by this canonical field. */
2098+
abstract Field getAField();
2099+
2100+
/**
2101+
* Gets a class that declares a field represented by this canonical field.
2102+
*/
2103+
abstract Class getADeclaringType();
2104+
2105+
/**
2106+
* Gets a type that this canonical field may have. Note that this may
2107+
* not be a unique type. For example, consider this case:
2108+
* ```
2109+
* template<typename T>
2110+
* struct S { T x; };
2111+
*
2112+
* S<int> s1;
2113+
* S<char> s2;
2114+
* ```
2115+
* In this case the canonical field corresponding to `S::x` has two types:
2116+
* `int` and `char`.
2117+
*/
2118+
Type getAType() { result = this.getAField().getType() }
2119+
2120+
Type getAnUnspecifiedType() { result = this.getAType().getUnspecifiedType() }
2121+
}
2122+
2123+
private class NonLocalCanonicalField extends CanonicalField {
2124+
Class declaringType;
2125+
2126+
NonLocalCanonicalField() {
2127+
declaringType = this.getDeclaringType() and
2128+
not declaringType.isFromTemplateInstantiation(_) and
2129+
not declaringType.isLocal() // handled in LocalCanonicalField
2130+
}
2131+
2132+
override Field getAField() {
2133+
exists(Class c | result.getDeclaringType() = c |
2134+
// Either the declaring class of the field is a template instantiation
2135+
// that has been constructed from this canonical declaration
2136+
c.isConstructedFrom(declaringType) and
2137+
pragma[only_bind_out](result.getName()) = pragma[only_bind_out](this.getName())
2138+
or
2139+
// or this canonical declaration is not a template.
2140+
not c.isConstructedFrom(_) and
2141+
result = this
2142+
)
2143+
}
2144+
2145+
override Class getADeclaringType() {
2146+
result = this.getDeclaringType()
2147+
or
2148+
result.isConstructedFrom(this.getDeclaringType())
2149+
}
2150+
}
2151+
2152+
private class LocalCanonicalField extends CanonicalField {
2153+
Class declaringType;
2154+
2155+
LocalCanonicalField() {
2156+
declaringType = this.getDeclaringType() and
2157+
declaringType.isLocal()
2158+
}
2159+
2160+
override Field getAField() { result = this }
2161+
2162+
override Class getADeclaringType() { result = declaringType }
2163+
}
2164+
2165+
/**
2166+
* A canonical representation of a `Union`. See `CanonicalField` for the explanation for
2167+
* why we need a canonical representation.
2168+
*/
2169+
abstract private class CanonicalUnion extends Union {
2170+
/** Gets a union represented by this canonical union. */
2171+
abstract Union getAUnion();
2172+
2173+
/** Gets a canonical field of this canonical union. */
2174+
CanonicalField getACanonicalField() { result.getDeclaringType() = this }
2175+
}
2176+
2177+
private class NonLocalCanonicalUnion extends CanonicalUnion {
2178+
NonLocalCanonicalUnion() { not this.isFromTemplateInstantiation(_) and not this.isLocal() }
2179+
2180+
override Union getAUnion() {
2181+
result = this
2182+
or
2183+
result.isConstructedFrom(this)
2184+
}
2185+
}
2186+
2187+
private class LocalCanonicalUnion extends CanonicalUnion {
2188+
LocalCanonicalUnion() { this.isLocal() }
2189+
2190+
override Union getAUnion() { result = this }
2191+
}
2192+
20812193
bindingset[f]
20822194
pragma[inline_late]
2083-
private int getFieldSize(Field f) { result = f.getType().getSize() }
2195+
private int getFieldSize(CanonicalField f) { result = max(f.getAType().getSize()) }
20842196

20852197
/**
20862198
* Gets a field in the union `u` whose size
20872199
* is `bytes` number of bytes.
20882200
*/
2089-
private Field getAFieldWithSize(Union u, int bytes) {
2090-
result = u.getAField() and
2201+
private CanonicalField getAFieldWithSize(CanonicalUnion u, int bytes) {
2202+
result = u.getACanonicalField() and
20912203
bytes = getFieldSize(result)
20922204
}
20932205

20942206
cached
20952207
private newtype TContent =
2096-
TNonUnionContent(Field f, int indirectionIndex) {
2208+
TNonUnionContent(CanonicalField f, int indirectionIndex) {
20972209
// the indirection index for field content starts at 1 (because `TNonUnionContent` is thought of as
20982210
// the address of the field, `FieldAddress` in the IR).
2099-
indirectionIndex = [1 .. SsaImpl::getMaxIndirectionsForType(f.getUnspecifiedType())] and
2211+
indirectionIndex = [1 .. max(SsaImpl::getMaxIndirectionsForType(f.getAnUnspecifiedType()))] and
21002212
// Reads and writes of union fields are tracked using `UnionContent`.
21012213
not f.getDeclaringType() instanceof Union
21022214
} or
2103-
TUnionContent(Union u, int bytes, int indirectionIndex) {
2104-
exists(Field f |
2105-
f = u.getAField() and
2215+
TUnionContent(CanonicalUnion u, int bytes, int indirectionIndex) {
2216+
exists(CanonicalField f |
2217+
f = u.getACanonicalField() and
21062218
bytes = getFieldSize(f) and
21072219
// We key `UnionContent` by the union instead of its fields since a write to one
21082220
// field can be read by any read of the union's fields. Again, the indirection index
21092221
// is 1-based (because 0 is considered the address).
21102222
indirectionIndex =
21112223
[1 .. max(SsaImpl::getMaxIndirectionsForType(getAFieldWithSize(u, bytes)
2112-
.getUnspecifiedType())
2224+
.getAnUnspecifiedType())
21132225
)]
21142226
)
21152227
} or
@@ -2175,8 +2287,12 @@ class FieldContent extends Content, TFieldContent {
21752287

21762288
/**
21772289
* Gets the field associated with this `Content`, if a unique one exists.
2290+
*
2291+
* For fields from template instantiations this predicate may still return
2292+
* more than field, but all the fields will be constructed from the same
2293+
* template.
21782294
*/
2179-
final Field getField() { result = unique( | | this.getAField()) }
2295+
Field getField() { none() } // overridden in subclasses
21802296

21812297
override int getIndirectionIndex() { none() } // overridden in subclasses
21822298

@@ -2187,57 +2303,65 @@ class FieldContent extends Content, TFieldContent {
21872303

21882304
/** A reference through a non-union instance field. */
21892305
class NonUnionFieldContent extends FieldContent, TNonUnionContent {
2190-
private Field f;
2306+
private CanonicalField f;
21912307
private int indirectionIndex;
21922308

21932309
NonUnionFieldContent() { this = TNonUnionContent(f, indirectionIndex) }
21942310

21952311
override string toString() { result = contentStars(this) + f.toString() }
21962312

2197-
override Field getAField() { result = f }
2313+
final override Field getField() { result = f.getAField() }
2314+
2315+
override Field getAField() { result = this.getField() }
21982316

21992317
/** Gets the indirection index of this `FieldContent`. */
22002318
override int getIndirectionIndex() { result = indirectionIndex }
22012319

22022320
override predicate impliesClearOf(Content c) {
2203-
exists(FieldContent fc |
2204-
fc = c and
2205-
fc.getField() = f and
2321+
exists(int i |
2322+
c = TNonUnionContent(f, i) and
22062323
// If `this` is `f` then `c` is cleared if it's of the
22072324
// form `*f`, `**f`, etc.
2208-
fc.getIndirectionIndex() >= indirectionIndex
2325+
i >= indirectionIndex
22092326
)
22102327
}
22112328
}
22122329

22132330
/** A reference through an instance field of a union. */
22142331
class UnionContent extends FieldContent, TUnionContent {
2215-
private Union u;
2332+
private CanonicalUnion u;
22162333
private int indirectionIndex;
22172334
private int bytes;
22182335

22192336
UnionContent() { this = TUnionContent(u, bytes, indirectionIndex) }
22202337

22212338
override string toString() { result = contentStars(this) + u.toString() }
22222339

2340+
final override Field getField() { result = unique( | | u.getACanonicalField()).getAField() }
2341+
22232342
/** Gets a field of the underlying union of this `UnionContent`, if any. */
2224-
override Field getAField() { result = u.getAField() and getFieldSize(result) = bytes }
2343+
override Field getAField() {
2344+
exists(CanonicalField cf |
2345+
cf = u.getACanonicalField() and
2346+
result = cf.getAField() and
2347+
getFieldSize(cf) = bytes
2348+
)
2349+
}
22252350

22262351
/** Gets the underlying union of this `UnionContent`. */
2227-
Union getUnion() { result = u }
2352+
Union getUnion() { result = u.getAUnion() }
22282353

22292354
/** Gets the indirection index of this `UnionContent`. */
22302355
override int getIndirectionIndex() { result = indirectionIndex }
22312356

22322357
override predicate impliesClearOf(Content c) {
2233-
exists(UnionContent uc |
2234-
uc = c and
2235-
uc.getUnion() = u and
2358+
exists(int i |
2359+
c = TUnionContent(u, _, i) and
22362360
// If `this` is `u` then `c` is cleared if it's of the
22372361
// form `*u`, `**u`, etc. (and we ignore `bytes` because
22382362
// we know the entire union is overwritten because it's a
22392363
// union).
2240-
uc.getIndirectionIndex() >= indirectionIndex
2364+
i >= indirectionIndex
22412365
)
22422366
}
22432367
}

0 commit comments

Comments
 (0)