From 5007666d6e2a734d509426c345d221a23b570925 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Thu, 17 Oct 2024 11:26:24 +0100 Subject: [PATCH 1/3] Add helper predicate `lookThroughPointerType` --- go/ql/lib/semmle/go/Decls.qll | 5 +---- go/ql/lib/semmle/go/Scopes.qll | 8 +------- go/ql/lib/semmle/go/Types.qll | 20 +++++++++---------- go/ql/lib/semmle/go/controlflow/IR.qll | 6 +----- .../LengthComparisonOffByOne.ql | 2 +- 5 files changed, 14 insertions(+), 27 deletions(-) diff --git a/go/ql/lib/semmle/go/Decls.qll b/go/ql/lib/semmle/go/Decls.qll index 8e3df22cc800..6c66b085575b 100644 --- a/go/ql/lib/semmle/go/Decls.qll +++ b/go/ql/lib/semmle/go/Decls.qll @@ -212,10 +212,7 @@ class MethodDecl extends FuncDecl { * * is `Rectangle`. */ - NamedType getReceiverBaseType() { - result = this.getReceiverType() or - result = this.getReceiverType().(PointerType).getBaseType() - } + NamedType getReceiverBaseType() { result = lookThroughPointerType(this.getReceiverType()) } /** * Gets the receiver variable of this method. diff --git a/go/ql/lib/semmle/go/Scopes.qll b/go/ql/lib/semmle/go/Scopes.qll index 191534759ea6..f9b9e3a26b9a 100644 --- a/go/ql/lib/semmle/go/Scopes.qll +++ b/go/ql/lib/semmle/go/Scopes.qll @@ -519,13 +519,7 @@ class Method extends Function { * Gets the receiver base type of this method, that is, either the base type of the receiver type * if it is a pointer type, or the receiver type itself if it is not a pointer type. */ - Type getReceiverBaseType() { - exists(Type recv | recv = this.getReceiverType() | - if recv instanceof PointerType - then result = recv.(PointerType).getBaseType() - else result = recv - ) - } + Type getReceiverBaseType() { result = lookThroughPointerType(this.getReceiverType()) } /** Holds if this method has name `m` and belongs to the method set of type `tp` or `*tp`. */ private predicate isIn(NamedType tp, string m) { diff --git a/go/ql/lib/semmle/go/Types.qll b/go/ql/lib/semmle/go/Types.qll index 4818db2f774d..e86acd5a3e84 100644 --- a/go/ql/lib/semmle/go/Types.qll +++ b/go/ql/lib/semmle/go/Types.qll @@ -446,11 +446,7 @@ class StructType extends @structtype, CompositeType { if n = "" then ( isEmbedded = true and - ( - name = tp.(NamedType).getName() - or - name = tp.(PointerType).getBaseType().(NamedType).getName() - ) + name = lookThroughPointerType(tp).(NamedType).getName() ) else ( isEmbedded = false and name = n @@ -489,8 +485,7 @@ class StructType extends @structtype, CompositeType { */ private predicate hasEmbeddedField(Type tp, int depth) { exists(Field f | this.hasFieldCand(_, f, depth, true) | - tp = f.getType() or - tp = f.getType().(PointerType).getBaseType() + tp = lookThroughPointerType(f.getType()) ) } @@ -518,9 +513,7 @@ class StructType extends @structtype, CompositeType { this.hasFieldCand(_, embeddedParent, depth - 1, true) and result.getName() = name and ( - result.getReceiverBaseType() = embeddedParent.getType() - or - result.getReceiverBaseType() = embeddedParent.getType().(PointerType).getBaseType() + result.getReceiverBaseType() = lookThroughPointerType(embeddedParent.getType()) or methodhosts(result, embeddedParent.getType()) ) @@ -644,6 +637,13 @@ class PointerType extends @pointertype, CompositeType { override string toString() { result = "pointer type" } } +Type lookThroughPointerType(Type t) { + not t instanceof PointerType and + result = t + or + result = t.(PointerType).getBaseType() +} + private newtype TTypeSetTerm = MkTypeSetTerm(TypeSetLiteralType tslit, int index) { component_types(tslit, index, _, _) } diff --git a/go/ql/lib/semmle/go/controlflow/IR.qll b/go/ql/lib/semmle/go/controlflow/IR.qll index b036ddf6d0f5..addd85a36c4c 100644 --- a/go/ql/lib/semmle/go/controlflow/IR.qll +++ b/go/ql/lib/semmle/go/controlflow/IR.qll @@ -358,11 +358,7 @@ module IR { override predicate reads(ValueEntity v) { v = field } - override Type getResultType() { - if field.getType() instanceof PointerType - then result = field.getType().(PointerType).getBaseType() - else result = field.getType() - } + override Type getResultType() { result = lookThroughPointerType(field.getType()) } override ControlFlow::Root getRoot() { result.isRootOf(e) } diff --git a/go/ql/src/InconsistentCode/LengthComparisonOffByOne.ql b/go/ql/src/InconsistentCode/LengthComparisonOffByOne.ql index 39c951f0150b..fbb1965c5f62 100644 --- a/go/ql/src/InconsistentCode/LengthComparisonOffByOne.ql +++ b/go/ql/src/InconsistentCode/LengthComparisonOffByOne.ql @@ -73,7 +73,7 @@ predicate isRegexpMethodCall(DataFlow::MethodCallNode c) { exists(NamedType regexp, Type recvtp | regexp.getName() = "Regexp" and recvtp = c.getReceiver().getType() | - recvtp = regexp or recvtp.(PointerType).getBaseType() = regexp + lookThroughPointerType(recvtp) = regexp ) } From 87992fac889d06101bde85a161bbade3bacc90d3 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Thu, 17 Oct 2024 11:50:17 +0100 Subject: [PATCH 2/3] Revert change to `hasEmbeddedField` --- go/ql/lib/semmle/go/Types.qll | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/go/ql/lib/semmle/go/Types.qll b/go/ql/lib/semmle/go/Types.qll index e86acd5a3e84..9f0b1c845121 100644 --- a/go/ql/lib/semmle/go/Types.qll +++ b/go/ql/lib/semmle/go/Types.qll @@ -485,7 +485,8 @@ class StructType extends @structtype, CompositeType { */ private predicate hasEmbeddedField(Type tp, int depth) { exists(Field f | this.hasFieldCand(_, f, depth, true) | - tp = lookThroughPointerType(f.getType()) + tp = f.getType() or + tp = f.getType().(PointerType).getBaseType() ) } From 1318504aa57986091317f27f4de759fbf4cc13f9 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Thu, 17 Oct 2024 12:06:46 +0100 Subject: [PATCH 3/3] Add QLDoc --- go/ql/lib/semmle/go/Types.qll | 3 +++ 1 file changed, 3 insertions(+) diff --git a/go/ql/lib/semmle/go/Types.qll b/go/ql/lib/semmle/go/Types.qll index 9f0b1c845121..1b09ea466cc4 100644 --- a/go/ql/lib/semmle/go/Types.qll +++ b/go/ql/lib/semmle/go/Types.qll @@ -638,6 +638,9 @@ class PointerType extends @pointertype, CompositeType { override string toString() { result = "pointer type" } } +/** + * Gets the base type if `t` is a pointer type, otherwise `t` itself. + */ Type lookThroughPointerType(Type t) { not t instanceof PointerType and result = t