diff --git a/ql/src/RedundantCode/NegativeLengthCheck.ql b/ql/src/RedundantCode/NegativeLengthCheck.ql index c199d09fbc8..491d286f0b9 100644 --- a/ql/src/RedundantCode/NegativeLengthCheck.ql +++ b/ql/src/RedundantCode/NegativeLengthCheck.ql @@ -16,7 +16,7 @@ where (len = Builtin::len() or len = Builtin::cap()) and ( exists(RelationalComparisonExpr rel | rel = cmp | - rel.getLesserOperand() = len.getACallExpr() and + rel.getLesserOperand() = len.getACall().asExpr() and rel.getGreaterOperand().getIntValue() = ub and ( ub < 0 @@ -27,7 +27,7 @@ where ) or exists(EqualityTestExpr eq | eq = cmp | - eq.getAnOperand() = len.getACallExpr() and + eq.getAnOperand() = len.getACall().asExpr() and eq.getAnOperand().getIntValue() = ub and ub < 0 and r = "equal" diff --git a/ql/src/semmle/go/Expr.qll b/ql/src/semmle/go/Expr.qll index 2fa9559809c..857a7e487ef 100644 --- a/ql/src/semmle/go/Expr.qll +++ b/ql/src/semmle/go/Expr.qll @@ -442,14 +442,6 @@ class CallExpr extends CallOrConversionExpr { /** Gets the expression representing the function being called. */ Expr getCalleeExpr() { result = getChildExpr(0) } - /** Holds if this call is of the form `base.method(...)`. */ - predicate calls(Expr base, string method) { - exists(SelectorExpr callee | callee = getCalleeExpr().stripParens() | - callee.getBase() = base and - method = callee.getSelector().getName() - ) - } - /** Gets the `i`th argument expression of this call (0-based). */ Expr getArgument(int i) { i >= 0 and @@ -462,39 +454,17 @@ class CallExpr extends CallOrConversionExpr { /** Gets the number of argument expressions of this call. */ int getNumArgument() { result = count(getAnArgument()) } - /** Gets the name of the invoked function or method. */ + /** Gets the name of the invoked function or method if it can be determined syntactically. */ string getCalleeName() { - result = getCalleeExpr().stripParens().(Ident).getName() or - calls(_, result) + exists(Expr callee | callee = getCalleeExpr().stripParens() | + result = callee.(Ident).getName() + or + result = callee.(SelectorExpr).getSelector().getName() + ) } /** Gets the declared target of this call. */ - Function getTarget() { this = result.getACallExpr() } - - /** - * Gets the definition of a possible target of this call. - * - * For non-virtual calls, there is at most one possible call target (but there may be none if the - * target has no declaration). - * - * For virtual calls, we look up possible targets in all types that implement the receiver - * interface type. - */ - FuncDef getACallee() { - result = getTarget().(DeclaredFunction).getFuncDecl() - or - exists(SelectorExpr sel, InterfaceType declaredRecv, Type actualRecv | - sel = getCalleeExpr().stripParens() and - declaredRecv = sel.getBase().getType().getUnderlyingType() and - actualRecv.implements(declaredRecv) - | - result = actualRecv - .(PointerType) - .getBaseType() - .(NamedType) - .getMethodDecl(sel.getSelector().getName()) - ) - } + Function getTarget() { getCalleeExpr() = result.getAReference() } override predicate mayHaveOwnSideEffects() { getTarget().mayHaveSideEffects() or diff --git a/ql/src/semmle/go/Scopes.qll b/ql/src/semmle/go/Scopes.qll index dc6e8ef52b9..9e64ec2bea4 100644 --- a/ql/src/semmle/go/Scopes.qll +++ b/ql/src/semmle/go/Scopes.qll @@ -276,11 +276,13 @@ class Field extends Variable { /** A built-in or declared function. */ class Function extends ValueEntity, @functionobject { - /** Gets an expression representing a call to this function. */ - CallExpr getACallExpr() { result.getCalleeExpr() = getAReference() } - /** Gets a call to this function. */ - DataFlow::CallNode getACall() { result.getExpr() = getACallExpr() } + pragma[nomagic] + DataFlow::CallNode getACall() { + this = result.getTarget() + or + this.(DeclaredFunction).getFuncDecl() = result.getACallee() + } /** Holds if this function has no observable side effects. */ predicate mayHaveSideEffects() { none() } diff --git a/ql/src/semmle/go/StringOps.qll b/ql/src/semmle/go/StringOps.qll index ec5ebdb218c..58b7d6d982b 100644 --- a/ql/src/semmle/go/StringOps.qll +++ b/ql/src/semmle/go/StringOps.qll @@ -214,13 +214,16 @@ module StringOps { * width and precision specifiers, but not including `*` specifiers or explicit argument * indices. */ + pragma[noinline] private string getFormatComponentRegex() { - exists(string literal, string opt_flag, string opt_width, string operator, string verb | + exists(string literal, string opt_flag, string width, string prec, string opt_width_and_prec, string operator, string verb | literal = "([^%]|%%)+" and opt_flag = "[-+ #0]?" and - opt_width = "((\\d*|\\*)(\\.(\\d*|\\*))?)?" and + width = "\\d+|\\*" and + prec = "\\.(\\d+|\\*)" and + opt_width_and_prec = "(" + width + ")?(" + prec + ")?" and operator = "[bcdeEfFgGoOpqstTxXUv]" and - verb = "(%" + opt_flag + opt_width + operator + ")" + verb = "(%" + opt_flag + opt_width_and_prec + operator + ")" | result = "(" + literal + "|" + verb + ")" ) diff --git a/ql/src/semmle/go/dataflow/internal/DataFlowDispatch.qll b/ql/src/semmle/go/dataflow/internal/DataFlowDispatch.qll index c85e26316ca..a44a905490b 100644 --- a/ql/src/semmle/go/dataflow/internal/DataFlowDispatch.qll +++ b/ql/src/semmle/go/dataflow/internal/DataFlowDispatch.qll @@ -7,7 +7,7 @@ DataFlowCallable viableImpl(DataFlowCall ma) { result = viableCallable(ma) } * Gets a function that might be called by `call`. */ DataFlowCallable viableCallable(CallExpr ma) { - result = ma.getACallee() + result = DataFlow::exprNode(ma).(DataFlow::CallNode).getACallee() } /** diff --git a/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll b/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll index b51f85d440e..d442bb6520b 100644 --- a/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll +++ b/ql/src/semmle/go/dataflow/internal/DataFlowUtil.qll @@ -264,11 +264,36 @@ class CallNode extends ExprNode { /** Gets the declared target of this call */ Function getTarget() { result = expr.getTarget() } - /** Get the definition of a possible target of this call. See `CallExpr.getACallee`. */ - FuncDef getACallee() { result = expr.getACallee() } + private DataFlow::Node getACalleeSource() { + result.getASuccessor*() = getCalleeNode() + } + + /** + * Gets the definition of a possible target of this call. + * + * For non-virtual calls, there is at most one possible call target (but there may be none if the + * target has no declaration). + * + * For virtual calls, we look up possible targets in all types that implement the receiver + * interface type. + */ + FuncDef getACallee() { + result = getTarget().(DeclaredFunction).getFuncDecl() + or + exists(DataFlow::Node calleeSource | calleeSource = getACalleeSource() | + result = calleeSource.asExpr() + or + exists(Method m, InterfaceType declaredRecv, Type actualRecv | + calleeSource = m.getARead() and + declaredRecv = m.getReceiverType().(NamedType).getBaseType() and + actualRecv.implements(declaredRecv) and + result = actualRecv.getMethod(m.getName()).(DeclaredFunction).getFuncDecl() + ) + ) + } /** Gets the name of the function or method being called, if it can be determined. */ - string getCalleeName() { result = expr.getTarget().getName() } + string getCalleeName() { result = expr.getTarget().getName() or result = expr.getCalleeName() } /** Gets the data flow node specifying the function to be called. */ Node getCalleeNode() { result = exprNode(expr.getCalleeExpr()) } @@ -315,10 +340,7 @@ class CallNode extends ExprNode { /** Gets the data flow node corresponding to the receiver of this call, if any. */ Node getReceiver() { - exists(MethodReadNode mrn | - mrn.getASuccessor*() = this.getCalleeNode() and - result = mrn.getReceiver() - ) + result = getACalleeSource().(MethodReadNode).getReceiver() } } @@ -328,7 +350,7 @@ class MethodCallNode extends CallNode { override Method getTarget() { result = expr.getTarget() } - override MethodDecl getACallee() { result = expr.getACallee() } + override MethodDecl getACallee() { result = super.getACallee() } } /** A representation of a receiver initialization. */ diff --git a/ql/test/library-tests/semmle/go/dataflow/CallGraph/getACall.expected b/ql/test/library-tests/semmle/go/dataflow/CallGraph/getACall.expected new file mode 100644 index 00000000000..553fdf373eb --- /dev/null +++ b/ql/test/library-tests/semmle/go/dataflow/CallGraph/getACall.expected @@ -0,0 +1,2 @@ +missingCall +spuriousCall diff --git a/ql/test/library-tests/semmle/go/dataflow/CallGraph/getACall.ql b/ql/test/library-tests/semmle/go/dataflow/CallGraph/getACall.ql new file mode 100644 index 00000000000..82e50120d5e --- /dev/null +++ b/ql/test/library-tests/semmle/go/dataflow/CallGraph/getACall.ql @@ -0,0 +1,11 @@ +import go + +query predicate missingCall(DeclaredFunction f, DataFlow::CallNode call) { + call.getACallee() = f.getFuncDecl() and + not call = f.getACall() +} + +query predicate spuriousCall(DeclaredFunction f, DataFlow::CallNode call) { + call = f.getACall() and + exists(FuncDecl fd | fd = f.getFuncDecl() | not call.getACallee() = fd) +} diff --git a/ql/test/library-tests/semmle/go/dataflow/CallGraph/getACallee.expected b/ql/test/library-tests/semmle/go/dataflow/CallGraph/getACallee.expected new file mode 100644 index 00000000000..381d576a20b --- /dev/null +++ b/ql/test/library-tests/semmle/go/dataflow/CallGraph/getACallee.expected @@ -0,0 +1,2 @@ +missingCallee +spuriousCallee diff --git a/ql/test/library-tests/semmle/go/dataflow/CallGraph/getACallee.ql b/ql/test/library-tests/semmle/go/dataflow/CallGraph/getACallee.ql new file mode 100644 index 00000000000..99e85587d55 --- /dev/null +++ b/ql/test/library-tests/semmle/go/dataflow/CallGraph/getACallee.ql @@ -0,0 +1,25 @@ +import go + +/** + * Gets a string `val` such that there is a comment on the same line as `l` + * that contains the substring `key: val`. + */ +string metadata(Locatable l, string key) { + exists(string f, int line, Comment c, string kv | + l.hasLocationInfo(f, line, _, _, _) and + c.hasLocationInfo(f, line, _, _, _) and + kv = c.getText().regexpFind("\\b(\\w+: \\S+)", _, _) and + key = kv.regexpCapture("(\\w+): (\\S+)", 1) and + result = kv.regexpCapture("(\\w+): (\\S+)", 2) + ) +} + +query predicate missingCallee(DataFlow::CallNode call, FuncDef callee) { + metadata(call.asExpr(), "callee") = metadata(callee, "name") and + not call.getACallee() = callee +} + +query predicate spuriousCallee(DataFlow::CallNode call, FuncDef callee) { + call.getACallee() = callee and + not metadata(call.asExpr(), "callee") = metadata(callee, "name") +} diff --git a/ql/test/library-tests/semmle/go/dataflow/CallGraph/main.go b/ql/test/library-tests/semmle/go/dataflow/CallGraph/main.go new file mode 100644 index 00000000000..b9bc5e7ada3 --- /dev/null +++ b/ql/test/library-tests/semmle/go/dataflow/CallGraph/main.go @@ -0,0 +1,35 @@ +package main + +type I interface { + m() // name: I.m +} + +type s1 struct{} + +type s2 struct{} + +type s3 struct{} + +type mybool bool + +func (s1) m() {} // name: s1.m + +func (*s2) m() {} // name: s2.m + +func (s3) m(int) {} // name: s3.m + +func (mybool) m() {} // name: mybool.m + +func test(x *s1, y s2, z s3, b mybool, i I) { + x.m() // callee: s1.m + y.m() // callee: s2.m + z.m(0) // callee: s3.m + b.m() // callee: mybool.m + i.m() // callee: s1.m callee: s2.m callee: mybool.m + s1.m(*x) // callee: s1.m + s3.m(z, 0) // callee: s3.m + mybool.m(b) // callee: mybool.m + (func() {})() // name: func(){} callee: func(){} + id := func(x int) int { return x } // name: id + id(1) // callee: id +}