From 6af5b55d5123660b51b2ecdc463e9f6e49374625 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Wed, 6 Nov 2024 14:24:39 +0000 Subject: [PATCH] Refactor elementAppliesToQualifier This is needed for performance when there are lots of embeddings. --- go/ql/lib/semmle/go/dataflow/ExternalFlow.qll | 10 ++- .../go/dataflow/internal/FlowSummaryImpl.qll | 63 ++++++++++--------- 2 files changed, 38 insertions(+), 35 deletions(-) diff --git a/go/ql/lib/semmle/go/dataflow/ExternalFlow.qll b/go/ql/lib/semmle/go/dataflow/ExternalFlow.qll index c738221c093e..a59c7294e802 100644 --- a/go/ql/lib/semmle/go/dataflow/ExternalFlow.qll +++ b/go/ql/lib/semmle/go/dataflow/ExternalFlow.qll @@ -475,10 +475,9 @@ SourceSinkInterpretationInput::SourceOrSinkElement interpretElement( // Go does not need to distinguish functions with signature signature = "" and exists(string p | p = interpretPackage(pkg) | - result.hasTypeInfo(p, type, subtypes) and - ( - result.asFieldEntity().hasQualifiedName(p, type, name) or - result.asMethodEntity().hasQualifiedName(p, type, name) + exists(Entity e | result.hasFullInfo(e, p, type, subtypes) | + e.(Field).hasQualifiedName(p, type, name) or + e.(Method).hasQualifiedName(p, type, name) ) or subtypes = true and @@ -488,8 +487,7 @@ SourceSinkInterpretationInput::SourceOrSinkElement interpretElement( m2.getName() = name and m2.getReceiverBaseType().hasQualifiedName(pkg2, type2) | - result.asMethodEntity() = m2 and - result.hasTypeInfo(pkg2, type2, subtypes) + result.hasFullInfo(m2, pkg2, type2, subtypes) ) or type = "" and diff --git a/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll b/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll index 792584e1da7a..ed3006a8d740 100644 --- a/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll +++ b/go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll @@ -185,23 +185,23 @@ module SourceSinkInterpretationInput implements /** * Holds if this source or sink element is a method or field that was specified - * with the given values for `pkg`, `type` and `subtypes`. + * with the given values for `e`, `pkg`, `type` and `subtypes`. */ - predicate hasTypeInfo(string pkg, string type, boolean subtypes) { - this = TMethodEntityElement(_, pkg, type, subtypes) or - this = TFieldEntityElement(_, pkg, type, subtypes) + predicate hasFullInfo(Entity e, string pkg, string type, boolean subtypes) { + this = TMethodEntityElement(e, pkg, type, subtypes) or + this = TFieldEntityElement(e, pkg, type, subtypes) } /** Gets a textual representation of this source or sink element. */ string toString() { - not this.hasTypeInfo(_, _, _) and + (this instanceof TOtherEntityElement or this instanceof TAstElement) and result = "element representing " + [this.asEntity().toString(), this.asAstNode().toString()] or - exists(string pkg, string name, boolean subtypes | - this.hasTypeInfo(pkg, name, subtypes) and + exists(Entity e, string pkg, string name, boolean subtypes | + this.hasFullInfo(e, pkg, name, subtypes) and result = - "element representing " + this.asEntity().toString() + " with receiver type " + pkg + "." + - name + " and subtypes=" + subtypes + "element representing " + e.toString() + " with receiver type " + pkg + "." + name + + " and subtypes=" + subtypes ) } @@ -249,8 +249,8 @@ module SourceSinkInterpretationInput implements ( result.asOtherEntity() = callTarget or - result.asMethodEntity() = callTarget and - elementAppliesToQualifier(result, cn.getReceiver()) + callTarget instanceof Method and + result = getElementWithQualifier(callTarget, cn.getReceiver()) ) ) } @@ -278,22 +278,20 @@ module SourceSinkInterpretationInput implements } /** - * Holds if method or field spec `sse` applies in the context of qualifier `qual`. + * Gets a method or field spec for `e` which applies in the context of + * qualifier `qual`. * - * Note that naively checking `sse.asEntity()`'s qualified name is not correct, because - * `Method`s and `Field`s may have multiple qualified names due to embedding. We must instead - * check that the specific name given by `sse.hasTypeInfo` refers to either `qual`'s type - * or to a type it embeds. + * Note that naively checking `e`'s qualified name is not correct, because + * `Method`s and `Field`s may have multiple qualified names due to embedding. + * We must instead check that the package and type name given by + * `result.hasFullInfo` refer to either `qual`'s type or to a type it embeds. */ - bindingset[sse, qual] + bindingset[e, qual] pragma[inline_late] - private predicate elementAppliesToQualifier(SourceOrSinkElement sse, DataFlow::Node qual) { - exists( - string pkg, string typename, boolean subtypes, Type syntacticQualBaseType, Type targetType - | - sse.hasTypeInfo(pkg, typename, subtypes) and - targetType.hasQualifiedName(pkg, typename) and - syntacticQualBaseType = getSyntacticQualifierBaseType(qual) + private SourceOrSinkElement getElementWithQualifier(Entity e, DataFlow::Node qual) { + exists(boolean subtypes, Type syntacticQualBaseType, Type targetType | + syntacticQualBaseType = getSyntacticQualifierBaseType(qual) and + result = constructElement(e, targetType, subtypes) | subtypes = [true, false] and syntacticQualBaseType = targetType @@ -307,12 +305,20 @@ module SourceSinkInterpretationInput implements or // `syntacticQualBaseType`'s underlying type might be a struct type and `sse` // might be a promoted method or field in it. - targetType = - getIntermediateEmbeddedType(sse.asMethodEntity(), syntacticQualBaseType.getUnderlyingType()) + targetType = getIntermediateEmbeddedType(e, syntacticQualBaseType.getUnderlyingType()) ) ) } + bindingset[e, targetType, subtypes] + pragma[inline_late] + private SourceOrSinkElement constructElement(Entity e, Type targetType, boolean subtypes) { + exists(string pkg, string typename | + targetType.hasQualifiedName(pkg, typename) and + result.hasFullInfo(e, pkg, typename, subtypes) + ) + } + /** * Gets the type of an embedded field of `st` which is on the path to `e`, * which is a promoted method or field of `st`, or its base type if it's a @@ -388,8 +394,7 @@ module SourceSinkInterpretationInput implements or exists(DataFlow::FieldReadNode frn | frn = n | c = "" and - frn.getField() = pragma[only_bind_into](e).asFieldEntity() and - elementAppliesToQualifier(pragma[only_bind_into](e), frn.getBase()) + pragma[only_bind_into](e) = getElementWithQualifier(frn.getField(), frn.getBase()) ) ) } @@ -410,7 +415,7 @@ module SourceSinkInterpretationInput implements | c = "" and fw.writesField(base, f, node.asNode()) and - elementAppliesToQualifier(pragma[only_bind_into](e), base) + pragma[only_bind_into](e) = getElementWithQualifier(f, base) ) } }