From c61b31168294cb154de5a9e1cd39a0f4a2b3d942 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Thu, 27 Sep 2018 09:30:11 +0200 Subject: [PATCH 1/2] C++: Make unresolve dispatch on result, not this This change means that there are no results for `unresolveElement(t)` where `t` is a "junk type" -- a class definition that is not in the image of `resolveClass`. These "junk types" still exist as `Element`s, but they will never be returned by any predicate that goes through `unresolveElement` to query the db. We get a small reduction in DIL size and a significant speed improvement. The DIL for `NewArrayDeleteMismatch.ql` is reduced from 27,630 lines to 27,507 lines, and the total analysis time for the LGTM suite on jdk8u is reduced from 1158s to 984s. --- cpp/ql/src/semmle/code/cpp/Class.qll | 4 ---- cpp/ql/src/semmle/code/cpp/Element.qll | 7 ++++++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/Class.qll b/cpp/ql/src/semmle/code/cpp/Class.qll index e2364744b145..a433c4ac7866 100644 --- a/cpp/ql/src/semmle/code/cpp/Class.qll +++ b/cpp/ql/src/semmle/code/cpp/Class.qll @@ -15,10 +15,6 @@ class Class extends UserType { isClass(this.underlying()) } - override @element unresolve() { - resolveClass(result) = this - } - /** Gets a child declaration of this class. */ override Declaration getADeclaration() { result = this.getAMember() } diff --git a/cpp/ql/src/semmle/code/cpp/Element.qll b/cpp/ql/src/semmle/code/cpp/Element.qll index 13f554960093..febb8609fcc1 100644 --- a/cpp/ql/src/semmle/code/cpp/Element.qll +++ b/cpp/ql/src/semmle/code/cpp/Element.qll @@ -70,7 +70,12 @@ class ElementBase extends @element { * See `underlying` for when the qualifier is `this`. */ pragma[inline] - @element unresolve() { result = this } + final @element unresolve() { + not result instanceof @usertype and + result = this + or + this = resolveClass(result) + } } /** From 0da452d59a1c0b7efad199dd321a31031b3fd45c Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Thu, 27 Sep 2018 10:35:04 +0200 Subject: [PATCH 2/2] C++: Revert object-orientation of unresolveElement The change to make `unresolveElement` a member predicate was helpful for the optimiser when it dispatched on `this`, but now that it "dispatches" on `result` it's just an unnecessary pollution of the `ElementBase` namespace. --- cpp/ql/src/semmle/code/cpp/Class.qll | 2 +- cpp/ql/src/semmle/code/cpp/Element.qll | 36 ++++--------------- cpp/ql/src/semmle/code/cpp/Type.qll | 2 +- .../src/semmle/code/cpp/pointsto/PointsTo.qll | 2 +- 4 files changed, 9 insertions(+), 33 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/Class.qll b/cpp/ql/src/semmle/code/cpp/Class.qll index a433c4ac7866..8d4414f03f38 100644 --- a/cpp/ql/src/semmle/code/cpp/Class.qll +++ b/cpp/ql/src/semmle/code/cpp/Class.qll @@ -12,7 +12,7 @@ private import semmle.code.cpp.internal.ResolveClass */ class Class extends UserType { Class() { - isClass(this.underlying()) + isClass(underlyingElement(this)) } /** Gets a child declaration of this class. */ diff --git a/cpp/ql/src/semmle/code/cpp/Element.qll b/cpp/ql/src/semmle/code/cpp/Element.qll index febb8609fcc1..10ad7135b0a6 100644 --- a/cpp/ql/src/semmle/code/cpp/Element.qll +++ b/cpp/ql/src/semmle/code/cpp/Element.qll @@ -10,7 +10,7 @@ private import semmle.code.cpp.internal.ResolveClass */ pragma[inline] Element mkElement(@element e) { - result.unresolve() = e + unresolveElement(result) = e } /** @@ -24,7 +24,10 @@ Element mkElement(@element e) { */ pragma[inline] @element unresolveElement(Element e) { - result = e.unresolve() + not result instanceof @usertype and + result = e + or + e = resolveClass(result) } /** @@ -35,13 +38,12 @@ pragma[inline] * need the result for an argument to a database extensional. * See `unresolveElement` for when `e` is not `this`. */ -pragma[inline] @element underlyingElement(Element e) { result = e } /** - * A C/C++ element with a minimal set of member predicates. Not for + * A C/C++ element with no member predicates other than `toString`. Not for * general use. This class does not define a location, so classes wanting to * change their location without affecting other classes can extend * `ElementBase` instead of `Element` to create a new rootdef for `getURL`, @@ -50,32 +52,6 @@ pragma[inline] class ElementBase extends @element { /** Gets a textual representation of this element. */ string toString() { none() } - - /** - * INTERNAL: Do not use. - * - * Gets the `@element` that this `Element` extends. This should normally only - * be called from member predicates on `this` where you need the result for - * an argument to a database extensional. - * See `unresolve` for when the qualifier is not `this`. - */ - final @element underlying() { result = this } - - /** - * INTERNAL: Do not use. - * - * Gets an `@element` that resolves to the `Element`. This should normally - * only be called from member predicates, where the qualifier is not `this` - * and you need the result for an argument to a database extensional. - * See `underlying` for when the qualifier is `this`. - */ - pragma[inline] - final @element unresolve() { - not result instanceof @usertype and - result = this - or - this = resolveClass(result) - } } /** diff --git a/cpp/ql/src/semmle/code/cpp/Type.qll b/cpp/ql/src/semmle/code/cpp/Type.qll index 3a78e6c7cf94..552e12369283 100644 --- a/cpp/ql/src/semmle/code/cpp/Type.qll +++ b/cpp/ql/src/semmle/code/cpp/Type.qll @@ -7,7 +7,7 @@ private import semmle.code.cpp.internal.ResolveClass * A C/C++ type. */ class Type extends Locatable, @type { - Type() { isType(this.underlying()) } + Type() { isType(underlyingElement(this)) } /** * Gets the name of this type. diff --git a/cpp/ql/src/semmle/code/cpp/pointsto/PointsTo.qll b/cpp/ql/src/semmle/code/cpp/pointsto/PointsTo.qll index 7c70fd50d505..6f1ab3758262 100644 --- a/cpp/ql/src/semmle/code/cpp/pointsto/PointsTo.qll +++ b/cpp/ql/src/semmle/code/cpp/pointsto/PointsTo.qll @@ -635,7 +635,7 @@ class PointsToExpr extends Expr { this.interesting() and exists(int set, @element thisEntity, @element resultEntity | - thisEntity = this.underlying() and + thisEntity = underlyingElement(this) and pointstosets(set, thisEntity) and setlocations(set, resultEntity) and resultEntity = localUnresolveElement(result)