diff --git a/ql/src/semmle/go/Decls.qll b/ql/src/semmle/go/Decls.qll index 4892cd978de..c8ce8f6047a 100644 --- a/ql/src/semmle/go/Decls.qll +++ b/ql/src/semmle/go/Decls.qll @@ -110,25 +110,23 @@ class FuncDef extends @funcdef, StmtParent, ExprParent { /** Gets a result variable of this function. */ ResultVariable getAResultVar() { - result.getDeclaration() = getTypeExpr().getAResultDecl().getANameExpr() + result.getFunction() = this } /** * Gets the `i`th parameter of this function. + * + * The receiver variable, if any, is considered to be the -1st parameter. */ Parameter getParameter(int i) { - result = rank[i + 1](Parameter parm, int j, int k | - parm.getDeclaration() = getTypeExpr().getParameterDecl(j).getNameExpr(k) - | - parm order by j, k - ) + result.isParameterOf(this, i) } /** * Gets a parameter of this function. */ Parameter getAParameter() { - result.getDeclaration() = getTypeExpr().getAParameterDecl().getNameExpr(_) + result.getFunction() = this } /** diff --git a/ql/src/semmle/go/Scopes.qll b/ql/src/semmle/go/Scopes.qll index 4506c94957d..ec0f929fe17 100644 --- a/ql/src/semmle/go/Scopes.qll +++ b/ql/src/semmle/go/Scopes.qll @@ -226,25 +226,51 @@ class LocalVariable extends DeclaredVariable { } /** - * A receiver variable or a parameter. + * A (named) function parameter. + * + * Note that receiver variables are considered parameters. */ -abstract class ParameterOrReceiver extends DeclaredVariable { - FuncDef fn; +class Parameter extends DeclaredVariable { + FuncDef f; + int index; + + Parameter() { + f.(MethodDecl).getReceiverDecl().getNameExpr() = this.getDeclaration() and + index = -1 + or + exists(FuncTypeExpr tp | tp = f.getTypeExpr() | + this = rank[index + 1](DeclaredVariable parm, int j, int k | + parm.getDeclaration() = tp.getParameterDecl(j).getNameExpr(k) + | + parm order by j, k + ) + ) + } /** Gets the function to which this parameter belongs. */ - FuncDef getFunction() { result = fn } + FuncDef getFunction() { result = f } + + /** + * Gets the index of this parameter among all parameters of the function. + * + * The receiver is considered to have index -1. + */ + int getIndex() { result = index } + + /** Holds if this is the `i`th parameter of function `fd`. */ + predicate isParameterOf(FuncDef fd, int i) { + fd = f and i = index + } } /** The receiver variable of a method. */ -class ReceiverVariable extends ParameterOrReceiver { - override MethodDecl fn; +class ReceiverVariable extends Parameter { + override MethodDecl f; - ReceiverVariable() { fn.getReceiverDecl().getNameExpr() = this.getDeclaration() } -} + ReceiverVariable() { index = -1 } -/** A (named) function parameter. */ -class Parameter extends ParameterOrReceiver { - Parameter() { fn.getTypeExpr().getAParameterDecl().getNameExpr(_) = this.getDeclaration() } + /** Holds if this is the receiver variable of method `m`. */ + predicate isReceiverOf(MethodDecl m) { m = f } } /** A (named) function result variable. */ diff --git a/ql/src/semmle/go/controlflow/ControlFlowGraphImpl.qll b/ql/src/semmle/go/controlflow/ControlFlowGraphImpl.qll index 37216e53532..bd205271580 100644 --- a/ql/src/semmle/go/controlflow/ControlFlowGraphImpl.qll +++ b/ql/src/semmle/go/controlflow/ControlFlowGraphImpl.qll @@ -226,11 +226,11 @@ newtype TControlFlowNode = /** * A control-flow node that represents the initialization of a parameter to its corresponding argument. */ - MkParameterInit(ParameterOrReceiver parm) { exists(parm.getFunction().getBody()) } or + MkParameterInit(Parameter parm) { exists(parm.getFunction().getBody()) } or /** * A control-flow node that represents the argument corresponding to a parameter. */ - MkArgumentNode(ParameterOrReceiver parm) { exists(parm.getFunction().getBody()) } or + MkArgumentNode(Parameter parm) { exists(parm.getFunction().getBody()) } or /** * A control-flow node that represents the initialization of a result variable to its zero value. */ @@ -312,7 +312,7 @@ newtype TWriteTarget = or exists(IncDecStmt ids | write = MkIncDecNode(ids) | lhs = ids.getOperand().stripParens()) or - exists(ParameterOrReceiver parm | write = MkParameterInit(parm) | lhs = parm.getDeclaration()) + exists(Parameter parm | write = MkParameterInit(parm) | lhs = parm.getDeclaration()) or exists(ResultVariable res | write = MkResultInit(res) | lhs = res.getDeclaration()) } or @@ -1286,20 +1286,18 @@ module CFG { pragma[noinline] private MkEntryNode getEntry() { result = MkEntryNode(this) } - private ParameterOrReceiver getReceiverOrParameter(int i) { - i = 0 and result.getDeclaration() = this.(MethodDecl).getReceiverDecl().getNameExpr() - or - result = getParameter(i - count(this.(MethodDecl).getReceiverDecl().getNameExpr())) + private Parameter getParameterRanked(int i) { + result = rank[i+1](Parameter p, int j | p = getParameter(j) | p order by j) } private ControlFlow::Node getPrologueNode(int i) { i = -1 and result = getEntry() or exists(int numParm, int numRes | - numParm = count(getReceiverOrParameter(_)) and + numParm = count(getParameter(_)) and numRes = count(getResultVar(_)) | - exists(int j, ParameterOrReceiver p | p = getReceiverOrParameter(j) | + exists(int j, Parameter p | p = getParameterRanked(j) | i = 2 * j and result = MkArgumentNode(p) or i = 2 * j + 1 and result = MkParameterInit(p) diff --git a/ql/src/semmle/go/controlflow/IR.qll b/ql/src/semmle/go/controlflow/IR.qll index 9787aff4591..759bca1b8db 100644 --- a/ql/src/semmle/go/controlflow/IR.qll +++ b/ql/src/semmle/go/controlflow/IR.qll @@ -1021,7 +1021,7 @@ module IR { * An instruction initializing a parameter to the corresponding argument. */ class InitParameterInstruction extends WriteInstruction, MkParameterInit { - ParameterOrReceiver parm; + Parameter parm; InitParameterInstruction() { this = MkParameterInit(parm) } @@ -1042,7 +1042,7 @@ module IR { * An instruction reading the value of a function argument. */ class ReadArgumentInstruction extends Instruction, MkArgumentNode { - ParameterOrReceiver parm; + Parameter parm; ReadArgumentInstruction() { this = MkArgumentNode(parm) } diff --git a/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll b/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll index ceacaf5f372..ba8edbe5561 100644 --- a/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll +++ b/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll @@ -350,20 +350,6 @@ class MethodCallNode extends CallNode { override MethodDecl getACallee() { result = super.getACallee() } } -/** A representation of a receiver initialization. */ -class ReceiverNode extends SsaNode { - override SsaExplicitDefinition ssa; - ReceiverVariable recv; - - ReceiverNode() { ssa.getInstruction() = IR::initRecvInstruction(recv) } - - /** Gets the receiver variable this node initializes. */ - ReceiverVariable asReceiverVariable() { result = recv } - - /** Holds if this node initializes the receiver of `fd`. */ - predicate isReceiverOf(FuncDef fd) { recv = fd.(MethodDecl).getReceiver() } -} - /** A representation of a parameter initialization. */ class ParameterNode extends SsaNode { override SsaExplicitDefinition ssa; @@ -375,7 +361,16 @@ class ParameterNode extends SsaNode { override Parameter asParameter() { result = parm } /** Holds if this node initializes the `i`th parameter of `fd`. */ - predicate isParameterOf(FuncDef fd, int i) { parm = fd.getParameter(i) } + predicate isParameterOf(FuncDef fd, int i) { parm.isParameterOf(fd, i) } +} + +/** A representation of a receiver initialization. */ +class ReceiverNode extends ParameterNode { + override ReceiverVariable parm; + + ReceiverVariable asReceiverVariable() { result = parm } + + predicate isReceiverOf(MethodDecl m) { parm.isReceiverOf(m) } } /** @@ -435,10 +430,17 @@ class ArgumentNode extends Node { * Holds if this argument occurs at the given position in the given call. * * The receiver argument is considered to have index `-1`. + * + * Note that we currently do not track receiver arguments into calls to interface methods. */ predicate argumentOf(CallExpr call, int pos) { call = c.asExpr() and - pos = i + pos = i and + ( + i != -1 + or + exists(c.(MethodCallNode).getTarget().getBody()) + ) } /** diff --git a/ql/test/library-tests/semmle/go/dataflow/InterProceduralDataFlow/Test.expected b/ql/test/library-tests/semmle/go/dataflow/InterProceduralDataFlow/Test.expected index a670304f6c0..bf5ce2f062f 100644 --- a/ql/test/library-tests/semmle/go/dataflow/InterProceduralDataFlow/Test.expected +++ b/ql/test/library-tests/semmle/go/dataflow/InterProceduralDataFlow/Test.expected @@ -16,3 +16,4 @@ | main.go:53:16:53:24 | "source3" | main.go:10:14:10:14 | x | | pointers.go:6:13:6:21 | "source6" | main.go:10:14:10:14 | x | | pointers.go:6:13:6:21 | "source6" | pointers.go:15:11:15:12 | star expression | +| receivers.go:11:12:11:29 | type conversion | receivers.go:6:10:6:13 | recv | diff --git a/ql/test/library-tests/semmle/go/dataflow/InterProceduralDataFlow/receivers.go b/ql/test/library-tests/semmle/go/dataflow/InterProceduralDataFlow/receivers.go new file mode 100644 index 00000000000..9998ebaf2f8 --- /dev/null +++ b/ql/test/library-tests/semmle/go/dataflow/InterProceduralDataFlow/receivers.go @@ -0,0 +1,13 @@ +package main + +type mystring string + +func (recv mystring) sink() mystring { + sink := recv + return sink +} + +func test20() { + source := mystring("source") + source.sink() +}