Go: Make the models-as-data subtypes column do something more sensible for promoted methods #17618
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| --- | ||
| category: fix | ||
| --- | ||
| * The behaviour of the `subtypes` column in models-as-data now matches other languages more closely. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -149,21 +149,63 @@ module SourceSinkInterpretationInput implements | |||||
| ) | ||||||
| } | ||||||
|
|
||||||
| // Note that due to embedding, which is currently implemented via some | ||||||
| // Methods having multiple qualified names, a given Method is liable to have | ||||||
| // more than one SourceOrSinkElement, one for each of the names it claims. | ||||||
| private newtype TSourceOrSinkElement = | ||||||
| TMethodEntityElement(Method m, string pkg, string type, boolean subtypes) { | ||||||
| m.hasQualifiedName(pkg, type, _) and | ||||||
| subtypes = [true, false] | ||||||
| } or | ||||||
| TFieldEntityElement(Field f, string pkg, string type, boolean subtypes) { | ||||||
| f.hasQualifiedName(pkg, type, _) and | ||||||
| subtypes = [true, false] | ||||||
| } or | ||||||
| TOtherEntityElement(Entity e) { | ||||||
| not e instanceof Method and | ||||||
| not e instanceof Field | ||||||
| } or | ||||||
| TAstElement(AstNode n) | ||||||
|
|
||||||
| /** An element representable by CSV modeling. */ | ||||||
| class SourceOrSinkElement extends TSourceOrSinkElement { | ||||||
| /** Gets this source or sink element as an entity, if it is one. */ | ||||||
| Entity asEntity() { | ||||||
| result = [this.asMethodEntity(), this.asFieldEntity(), this.asOtherEntity()] | ||||||
| } | ||||||
|
|
||||||
| /** Gets this source or sink element as a method, if it is one. */ | ||||||
| Method asMethodEntity() { this = TMethodEntityElement(result, _, _, _) } | ||||||
|
|
||||||
| /** Gets this source or sink element as a field, if it is one. */ | ||||||
| Field asFieldEntity() { this = TFieldEntityElement(result, _, _, _) } | ||||||
|
|
||||||
| /** Gets this source or sink element as an entity which isn't a field or method, if it is one. */ | ||||||
| Entity asOtherEntity() { this = TOtherEntityElement(result) } | ||||||
|
|
||||||
| /** Gets this source or sink element as an AST node, if it is one. */ | ||||||
| AstNode asAstNode() { this = TAstElement(result) } | ||||||
|
|
||||||
| /** | ||||||
| * Holds if this source or sink element is a method or field that was specified | ||||||
| * with the given values for `e`, `pkg`, `type` and `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() { | ||||||
| (this instanceof TOtherEntityElement or this instanceof TAstElement) and | ||||||
| result = "element representing " + [this.asEntity().toString(), this.asAstNode().toString()] | ||||||
| or | ||||||
| exists(Entity e, string pkg, string name, boolean subtypes | | ||||||
| this.hasFullInfo(e, pkg, name, subtypes) and | ||||||
| result = | ||||||
| "element representing " + e.toString() + " with receiver type " + pkg + "." + name + | ||||||
| " and subtypes=" + subtypes | ||||||
| ) | ||||||
| } | ||||||
|
|
||||||
| /** Gets the location of this element. */ | ||||||
|
|
@@ -203,7 +245,17 @@ module SourceSinkInterpretationInput implements | |||||
|
|
||||||
| /** Gets the target of this call, if any. */ | ||||||
| SourceOrSinkElement getCallTarget() { | ||||||
| exists(DataFlow::CallNode cn, Function callTarget | | ||||||
| cn = this.asCall().getNode() and | ||||||
| callTarget = cn.getTarget() | ||||||
| | | ||||||
| ( | ||||||
| result.asOtherEntity() = callTarget | ||||||
| or | ||||||
| callTarget instanceof Method and | ||||||
| result = getElementWithQualifier(callTarget, cn.getReceiver()) | ||||||
| ) | ||||||
| ) | ||||||
| } | ||||||
|
|
||||||
| /** Gets a textual representation of this node. */ | ||||||
|
|
@@ -228,6 +280,105 @@ module SourceSinkInterpretationInput implements | |||||
| } | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Gets a method or field spec for `e` which applies in the context of | ||||||
| * qualifier `qual`. | ||||||
| * | ||||||
| * 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. | ||||||
|
There was a problem hiding this comment. I think I was wrong about this -- I think it's only Methods that can have multiple qnames like this, and field embedding is represented more explicitly
Suggested change
There was a problem hiding this comment. I looked into it and actually it is true for fields as well. In our code this is clear because Field.hasQualifiedName calls Type.getField, whose qldoc says "This includes fields promoted from an embedded field." |
||||||
| * 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[e, qual] | ||||||
| pragma[inline_late] | ||||||
| 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 | ||||||
| or | ||||||
| subtypes = true and | ||||||
| ( | ||||||
| // `syntacticQualBaseType`'s underlying type might be an interface type and `sse` | ||||||
| // might refer to a method defined on an interface embedded within it. | ||||||
| targetType = | ||||||
| syntacticQualBaseType.getUnderlyingType().(InterfaceType).getAnEmbeddedInterface() | ||||||
| or | ||||||
| // `syntacticQualBaseType`'s underlying type might be a struct type and `sse` | ||||||
| // might be a promoted method or field in it. | ||||||
| targetType = getAnIntermediateEmbeddedType(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 | ||||||
| * pointer type. | ||||||
| */ | ||||||
| private Type getAnIntermediateEmbeddedType(Entity e, StructType st) { | ||||||
| exists(Field field1, Field field2, int depth1, int depth2, Type t2 | | ||||||
| field1 = st.getFieldAtDepth(_, depth1) and | ||||||
| field2 = st.getFieldAtDepth(_, depth2) and | ||||||
| result = lookThroughPointerType(field1.getType()) and | ||||||
| t2 = lookThroughPointerType(field2.getType()) and | ||||||
| ( | ||||||
| field1 = field2 | ||||||
| or | ||||||
| field2 = result.getUnderlyingType().(StructType).getFieldAtDepth(_, depth2 - depth1 - 1) | ||||||
| ) | ||||||
| | | ||||||
| e.(Method).getReceiverBaseType() = t2 | ||||||
| or | ||||||
| e.(Field).getDeclaringType() = t2.getUnderlyingType() | ||||||
| ) | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Gets the base type of `underlying`, where `n` is of the form | ||||||
| * `implicitDeref?(underlying.implicitFieldRead1.implicitFieldRead2...)` | ||||||
| * | ||||||
| * For Go syntax like `qualifier.method()` or `qualifier.field`, this is the type of `qualifier`, before any | ||||||
| * implicit dereference is interposed because `qualifier` is of pointer type, or implicit field accesses | ||||||
| * navigate to any embedded struct types that truly host `field`. | ||||||
| */ | ||||||
| private Type getSyntacticQualifierBaseType(DataFlow::Node n) { | ||||||
| exists(DataFlow::Node n2 | | ||||||
| // look through implicit dereference, if there is one | ||||||
| not exists(n.asInstruction().(IR::EvalImplicitDerefInstruction).getOperand()) and | ||||||
| n2 = n | ||||||
| or | ||||||
| n2.asExpr() = n.asInstruction().(IR::EvalImplicitDerefInstruction).getOperand() | ||||||
| | | ||||||
| result = lookThroughPointerType(skipImplicitFieldReads(n2).getType()) | ||||||
| ) | ||||||
| } | ||||||
|
|
||||||
| private DataFlow::Node skipImplicitFieldReads(DataFlow::Node n) { | ||||||
| not exists(lookThroughImplicitFieldRead(n)) and result = n | ||||||
| or | ||||||
| result = skipImplicitFieldReads(lookThroughImplicitFieldRead(n)) | ||||||
| } | ||||||
|
|
||||||
| private DataFlow::Node lookThroughImplicitFieldRead(DataFlow::Node n) { | ||||||
| result.asInstruction() = | ||||||
| n.(DataFlow::InstructionNode) | ||||||
| .asInstruction() | ||||||
| .(IR::ImplicitFieldReadInstruction) | ||||||
| .getBaseInstruction() | ||||||
| } | ||||||
|
|
||||||
| /** Provides additional sink specification logic. */ | ||||||
| bindingset[c] | ||||||
| predicate interpretOutput(string c, InterpretNode mid, InterpretNode node) { | ||||||
|
|
@@ -242,10 +393,12 @@ module SourceSinkInterpretationInput implements | |||||
| e = mid.asElement() | ||||||
| | | ||||||
| (c = "Parameter" or c = "") and | ||||||
| n.asParameter() = pragma[only_bind_into](e).asEntity() | ||||||
| or | ||||||
| exists(DataFlow::FieldReadNode frn | frn = n | | ||||||
| c = "" and | ||||||
| pragma[only_bind_into](e) = getElementWithQualifier(frn.getField(), frn.getBase()) | ||||||
| ) | ||||||
| ) | ||||||
| } | ||||||
|
|
||||||
|
|
@@ -259,10 +412,13 @@ module SourceSinkInterpretationInput implements | |||||
| mid.asCallable() = getNodeEnclosingCallable(ret) | ||||||
| ) | ||||||
| or | ||||||
| exists(SourceOrSinkElement e, DataFlow::Write fw, DataFlow::Node base, Field f | | ||||||
| e = mid.asElement() and | ||||||
| f = e.asFieldEntity() | ||||||
| | | ||||||
| c = "" and | ||||||
| fw.writesField(base, f, node.asNode()) and | ||||||
| pragma[only_bind_into](e) = getElementWithQualifier(f, base) | ||||||
| ) | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
File renamed without changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
File renamed without changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
File renamed without changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
File renamed without changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.