Skip to content

Commit 0e0ab1e

Browse files
committed
C++: make unresolve a member of ElementBase
Also remove the charpred of ElementBase. This gets rid of many redundant charpred checks. It means that incomplete classes from the db are now `Element`s, which is maybe noisy but should not be harmful. Together, these changes give a great reduction in DIL and should help the optimiser. It brings the DIL of `UncontrolledFormatString.ql` down from 43,908 lines to 35,400 lines.
1 parent 3470ebc commit 0e0ab1e

File tree

4 files changed

+56
-25
lines changed

4 files changed

+56
-25
lines changed

cpp/ql/src/semmle/code/cpp/Class.qll

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,12 @@ private import semmle.code.cpp.internal.ResolveClass
1212
*/
1313
class Class extends UserType {
1414
Class() {
15-
isClass(underlyingElement(this))
15+
isClass(this.underlying()) and
16+
this = resolveClass(_)
17+
}
18+
19+
override @element unresolve() {
20+
resolveClass(result) = this
1621
}
1722

1823
/** Gets a child declaration of this class. */

cpp/ql/src/semmle/code/cpp/Element.qll

Lines changed: 33 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,63 +2,75 @@ import semmle.code.cpp.Location
22
private import semmle.code.cpp.Enclosing
33
private import semmle.code.cpp.internal.ResolveClass
44

5-
/**
6-
* Get the `@element` that represents this `@element`.
7-
* Normally this will simply be `e`, but sometimes it is not.
8-
* For example, for an incomplete struct `e` the result may be a
9-
* complete struct with the same name.
10-
*/
11-
private cached @element resolveElement(@element e) {
12-
if isClass(e)
13-
then result = resolveClass(e)
14-
else result = e
15-
}
16-
175
/**
186
* Get the `Element` that represents this `@element`.
197
* Normally this will simply be a cast of `e`, but sometimes it is not.
208
* For example, for an incomplete struct `e` the result may be a
219
* complete struct with the same name.
2210
*/
11+
pragma[inline]
2312
Element mkElement(@element e) {
24-
result = resolveElement(e)
13+
result.unresolve() = e
2514
}
2615

2716
/**
28-
* Get an `@element` that resolves to the `Element`. This should
17+
* INTERNAL: Do not use.
18+
*
19+
* Gets an `@element` that resolves to the `Element`. This should
2920
* normally only be called from member predicates, where `e` is not
3021
* `this` and you need the result for an argument to a database
3122
* extensional.
3223
* See `underlyingElement` for when `e` is `this`.
3324
*/
25+
pragma[inline]
3426
@element unresolveElement(Element e) {
35-
resolveElement(result) = e
27+
result = e.unresolve()
3628
}
3729

3830
/**
39-
* Get the `@element` that this `Element` extends. This should normally
31+
* INTERNAL: Do not use.
32+
*
33+
* Gets the `@element` that this `Element` extends. This should normally
4034
* only be called from member predicates, where `e` is `this` and you
4135
* need the result for an argument to a database extensional.
4236
* See `unresolveElement` for when `e` is not `this`.
4337
*/
38+
pragma[inline]
4439
@element underlyingElement(Element e) {
4540
result = e
4641
}
4742

4843
/**
49-
* A C/C++ element with no member predicates other than `toString`. Not for
44+
* A C/C++ element with a minimal set of member predicates. Not for
5045
* general use. This class does not define a location, so classes wanting to
5146
* change their location without affecting other classes can extend
5247
* `ElementBase` instead of `Element` to create a new rootdef for `getURL`,
5348
* `getLocation`, or `hasLocationInfo`.
5449
*/
5550
class ElementBase extends @element {
56-
ElementBase() {
57-
this = resolveElement(_)
58-
}
59-
6051
/** Gets a textual representation of this element. */
6152
string toString() { none() }
53+
54+
/**
55+
* INTERNAL: Do not use.
56+
*
57+
* Gets the `@element` that this `Element` extends. This should normally only
58+
* be called from member predicates on `this` where you need the result for
59+
* an argument to a database extensional.
60+
* See `unresolve` for when the qualifier is not `this`.
61+
*/
62+
final @element underlying() { result = this }
63+
64+
/**
65+
* INTERNAL: Do not use.
66+
*
67+
* Gets an `@element` that resolves to the `Element`. This should normally
68+
* only be called from member predicates, where the qualifier is not `this`
69+
* and you need the result for an argument to a database extensional.
70+
* See `underlying` for when the qualifier is `this`.
71+
*/
72+
pragma[inline]
73+
@element unresolve() { result = this }
6274
}
6375

6476
/**

cpp/ql/src/semmle/code/cpp/Specifier.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -294,13 +294,13 @@ class AttributeArgument extends Element, @attribute_arg {
294294
}
295295

296296
override string toString() {
297-
if exists (@attribute_arg_empty self | mkElement(self) = this)
297+
if exists (@attribute_arg_empty self | self = underlyingElement(this))
298298
then result = "empty argument"
299299
else exists (string prefix, string tail
300300
| (if exists(getName())
301301
then prefix = getName() + "="
302302
else prefix = "") and
303-
(if exists (@attribute_arg_type self | mkElement(self) = this)
303+
(if exists (@attribute_arg_type self | self = underlyingElement(this))
304304
then tail = getValueType().getName()
305305
else tail = getValueText()) and
306306
result = prefix + tail)

cpp/ql/src/semmle/code/cpp/pointsto/PointsTo.qll

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -633,12 +633,26 @@ class PointsToExpr extends Expr
633633
pragma[noopt]
634634
Element pointsTo()
635635
{
636-
this.interesting() and exists(int set, @element thisEntity, @element resultEntity | thisEntity = underlyingElement(this) and pointstosets(set, thisEntity) and setlocations(set, resultEntity) and resultEntity = unresolveElement(result))
636+
this.interesting() and
637+
exists(int set, @element thisEntity, @element resultEntity |
638+
thisEntity = this.underlying() and
639+
pointstosets(set, thisEntity) and
640+
setlocations(set, resultEntity) and
641+
resultEntity = localUnresolveElement(result)
642+
)
637643
}
638644

639645
float confidence() { result = 1.0 / count(this.pointsTo()) }
640646
}
641647

648+
/*
649+
* This is used above in a `pragma[noopt]` context, which prevents its
650+
* customary inlining. We materialise it explicitly here.
651+
*/
652+
private @element localUnresolveElement(Element e) {
653+
result = unresolveElement(e)
654+
}
655+
642656
/**
643657
* Holds if anything points to an element, that is, is equivalent to:
644658
* ```

0 commit comments

Comments
 (0)