diff --git a/ql/src/semmle/go/controlflow/ControlFlowGraphImpl.qll b/ql/src/semmle/go/controlflow/ControlFlowGraphImpl.qll index ac5f10163..7a6006464 100644 --- a/ql/src/semmle/go/controlflow/ControlFlowGraphImpl.qll +++ b/ql/src/semmle/go/controlflow/ControlFlowGraphImpl.qll @@ -262,6 +262,24 @@ newtype TControlFlowNode = * A control-flow node that represents the implicit max bound of a simple slice expression. */ MkImplicitMaxSliceBound(SliceExpr sl) { not exists(sl.getMax()) } or + /** + * A control-flow node that represents the implicit dereference of the base in a field/method + * access. + */ + MkImplicitDeref(Expr e) { + e.getType().getUnderlyingType() instanceof PointerType and + exists(SelectorExpr sel | e = sel.getBase() | + // field accesses through a pointer always implicitly dereference + sel = any(Field f).getAReference() + or + // method accesses only dereference if the receiver is _not_ a pointer + exists(Method m, Type tp | + sel = m.getAReference() and + tp = m.getReceiver().getType().getUnderlyingType() and + not tp instanceof PointerType + ) + ) + } or /** * A control-flow node that represents the start of the execution of a function or file. */ @@ -1614,21 +1632,36 @@ module CFG { } } - private class SelectorExprTree extends PostOrderTree, SelectorExpr { + private class SelectorExprTree extends ControlFlowTree, SelectorExpr { SelectorExprTree() { getBase() instanceof ValueExpr } - override ControlFlow::Node getNode() { result = mkExprOrSkipNode(this) } + override predicate firstNode(ControlFlow::Node first) { + firstNode(getBase(), first) + } - override Completion getCompletion() { - result = Done() + override predicate lastNode(ControlFlow::Node last, Completion cmpl) { + ControlFlowTree.super.lastNode(last, cmpl) or // panic due to `nil` dereference - getBase() instanceof ValueExpr and - this.(ReferenceExpr).isRvalue() and - result = Panic() + last = MkImplicitDeref(this.getBase()) and + cmpl = Panic() + or + last = mkExprOrSkipNode(this) and + cmpl = Done() } - override ControlFlowTree getChildTree(int i) { i = 0 and result = getBase() } + override predicate succ(ControlFlow::Node pred, ControlFlow::Node succ) { + lastNode(getBase(), pred, normalCompletion()) and + ( + succ = MkImplicitDeref(this.getBase()) + or + not exists(MkImplicitDeref(this.getBase())) and + succ = mkExprOrSkipNode(this) + ) + or + pred = MkImplicitDeref(this.getBase()) and + succ = mkExprOrSkipNode(this) + } } private class SendStmtTree extends ControlFlowTree, SendStmt { diff --git a/ql/src/semmle/go/controlflow/IR.qll b/ql/src/semmle/go/controlflow/IR.qll index 8cd9d8aef..45252fbb8 100644 --- a/ql/src/semmle/go/controlflow/IR.qll +++ b/ql/src/semmle/go/controlflow/IR.qll @@ -46,7 +46,8 @@ module IR { this instanceof MkCaseCheckNode or this instanceof MkImplicitLowerSliceBound or this instanceof MkImplicitUpperSliceBound or - this instanceof MkImplicitMaxSliceBound + this instanceof MkImplicitMaxSliceBound or + this instanceof MkImplicitDeref } /** Holds if this instruction reads the value of variable or constant `v`. */ @@ -169,6 +170,8 @@ module IR { this instanceof MkImplicitUpperSliceBound and result = "implicit upper bound" or this instanceof MkImplicitMaxSliceBound and result = "implicit maximum" + or + this instanceof MkImplicitDeref and result = "implicit dereference" } } @@ -229,6 +232,21 @@ module IR { } } + /** + * Gets the effective base of a selector expression, taking implicit dereferences into account. + * + * For a selector expression `b.f`, this will either be the implicit dereference `*b`, or just + * `b` if there is no implicit dereferencing. + */ + private Instruction selectorBase(SelectorExpr e) { + exists(Expr base | base = e.getBase() | + result = MkImplicitDeref(base) + or + not exists(MkImplicitDeref(base)) and + result = evalExprInstruction(base) + ) + } + /** * An IR instruction that reads the value of a field. * @@ -244,7 +262,7 @@ module IR { } /** Gets the instruction computing the base value on which the field is read. */ - Instruction getBase() { result = evalExprInstruction(e.getBase()) } + Instruction getBase() { result = selectorBase(e) } /** Gets the field being read. */ Field getField() { e.getSelector() = result.getAReference() } @@ -262,7 +280,7 @@ module IR { MethodReadInstruction() { e.getSelector() = method.getAReference() } /** Gets the instruction computing the receiver value on which the method is looked up. */ - Instruction getReceiver() { result = evalExprInstruction(e.getBase()) } + Instruction getReceiver() { result = selectorBase(e) } /** Gets the method being looked up. */ Method getMethod() { result = method } @@ -1189,6 +1207,33 @@ module IR { } } + /** + * An instruction implicitly dereferencing the base in a field or method reference through a + * pointer. + */ + class EvalImplicitDerefInstruction extends Instruction, MkImplicitDeref { + Expr e; + + EvalImplicitDerefInstruction() { this = MkImplicitDeref(e) } + + /** Gets the operand that is being dereferenced. */ + Expr getOperand() { result = e } + + override Type getResultType() { + result = e.getType().getUnderlyingType().(PointerType).getBaseType() + } + + override ControlFlow::Root getRoot() { result.isRootOf(e) } + + override string toString() { result = "implicit dereference" } + + override predicate hasLocationInfo( + string filepath, int startline, int startcolumn, int endline, int endcolumn + ) { + e.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) + } + } + /** A representation of the target of a write instruction. */ class WriteTarget extends TWriteTarget { ControlFlow::Node w; @@ -1294,7 +1339,7 @@ module IR { /** Gets the instruction computing the base value on which this field is accessed. */ Instruction getBase() { - exists(SelectorExpr sel | this = MkLhs(_, sel) | result = evalExprInstruction(sel.getBase())) + exists(SelectorExpr sel | this = MkLhs(_, sel) | result = selectorBase(sel)) or result = w.(InitLiteralStructFieldInstruction).getBase() } diff --git a/ql/test/library-tests/semmle/go/controlflow/ControlFlowGraph/ControlFlowNode_getASuccessor.expected b/ql/test/library-tests/semmle/go/controlflow/ControlFlowGraph/ControlFlowNode_getASuccessor.expected index c1186a462..12bd617db 100644 --- a/ql/test/library-tests/semmle/go/controlflow/ControlFlowGraph/ControlFlowNode_getASuccessor.expected +++ b/ql/test/library-tests/semmle/go/controlflow/ControlFlowGraph/ControlFlowNode_getASuccessor.expected @@ -98,11 +98,9 @@ | exprs.go:15:32:15:43 | init of key-value pair | exprs.go:15:49:15:55 | struct2 | | exprs.go:15:35:15:41 | struct1 | exprs.go:15:35:15:43 | selection of x | | exprs.go:15:35:15:43 | selection of x | exprs.go:15:32:15:43 | init of key-value pair | -| exprs.go:15:35:15:43 | selection of x | exprs.go:26:1:26:1 | exit | | exprs.go:15:46:15:57 | init of key-value pair | exprs.go:15:2:15:8 | assignment to struct3 | | exprs.go:15:49:15:55 | struct2 | exprs.go:15:49:15:57 | selection of x | | exprs.go:15:49:15:57 | selection of x | exprs.go:15:46:15:57 | init of key-value pair | -| exprs.go:15:49:15:57 | selection of x | exprs.go:26:1:26:1 | exit | | exprs.go:16:2:16:5 | assignment to arr1 | exprs.go:17:2:17:5 | skip | | exprs.go:16:2:16:5 | skip | exprs.go:16:10:16:26 | composite literal | | exprs.go:16:10:16:26 | composite literal | exprs.go:16:17:16:17 | element index | @@ -110,7 +108,6 @@ | exprs.go:16:17:16:23 | struct3 | exprs.go:16:17:16:25 | selection of x | | exprs.go:16:17:16:25 | init of selection of x | exprs.go:16:2:16:5 | assignment to arr1 | | exprs.go:16:17:16:25 | selection of x | exprs.go:16:17:16:25 | init of selection of x | -| exprs.go:16:17:16:25 | selection of x | exprs.go:26:1:26:1 | exit | | exprs.go:17:2:17:5 | assignment to arr2 | exprs.go:18:2:18:4 | skip | | exprs.go:17:2:17:5 | skip | exprs.go:17:10:17:40 | composite literal | | exprs.go:17:10:17:40 | composite literal | exprs.go:17:19:17:19 | element index | @@ -118,7 +115,6 @@ | exprs.go:17:19:17:25 | struct3 | exprs.go:17:19:17:27 | selection of x | | exprs.go:17:19:17:27 | init of selection of x | exprs.go:17:30:17:30 | 2 | | exprs.go:17:19:17:27 | selection of x | exprs.go:17:19:17:27 | init of selection of x | -| exprs.go:17:19:17:27 | selection of x | exprs.go:26:1:26:1 | exit | | exprs.go:17:30:17:30 | 2 | exprs.go:17:33:17:36 | arr1 | | exprs.go:17:30:17:39 | init of key-value pair | exprs.go:17:2:17:5 | assignment to arr2 | | exprs.go:17:33:17:36 | arr1 | exprs.go:17:38:17:38 | 0 | @@ -210,7 +206,6 @@ | exprs.go:29:9:29:19 | type assertion | exprs.go:29:9:29:21 | selection of x | | exprs.go:29:9:29:19 | type assertion | exprs.go:30:1:30:1 | exit | | exprs.go:29:9:29:21 | selection of x | exprs.go:29:2:29:21 | return statement | -| exprs.go:29:9:29:21 | selection of x | exprs.go:30:1:30:1 | exit | | exprs.go:32:1:32:1 | entry | exprs.go:32:12:32:14 | argument corresponding to arg | | exprs.go:32:1:37:1 | function declaration | exprs.go:39:6:39:10 | skip | | exprs.go:32:6:32:10 | skip | exprs.go:32:1:37:1 | function declaration | @@ -231,7 +226,6 @@ | exprs.go:34:3:34:12 | return statement | exprs.go:37:1:37:1 | exit | | exprs.go:34:10:34:10 | p | exprs.go:34:10:34:12 | selection of x | | exprs.go:34:10:34:12 | selection of x | exprs.go:34:3:34:12 | return statement | -| exprs.go:34:10:34:12 | selection of x | exprs.go:37:1:37:1 | exit | | exprs.go:36:2:36:10 | return statement | exprs.go:37:1:37:1 | exit | | exprs.go:36:9:36:10 | -... | exprs.go:36:2:36:10 | return statement | | exprs.go:39:1:39:1 | entry | exprs.go:39:12:39:14 | argument corresponding to arg | @@ -260,7 +254,6 @@ | exprs.go:44:3:44:12 | return statement | exprs.go:47:1:47:1 | exit | | exprs.go:44:10:44:10 | p | exprs.go:44:10:44:12 | selection of x | | exprs.go:44:10:44:12 | selection of x | exprs.go:44:3:44:12 | return statement | -| exprs.go:44:10:44:12 | selection of x | exprs.go:47:1:47:1 | exit | | exprs.go:46:2:46:10 | return statement | exprs.go:47:1:47:1 | exit | | exprs.go:46:9:46:10 | -... | exprs.go:46:2:46:10 | return statement | | exprs.go:49:1:49:1 | entry | exprs.go:49:10:49:11 | argument corresponding to xs | @@ -797,7 +790,6 @@ | stmts5.go:8:2:8:3 | me | stmts5.go:8:2:8:7 | selection of val | | stmts5.go:8:2:8:7 | assignment to field val | stmts5.go:9:1:9:1 | exit | | stmts5.go:8:2:8:7 | selection of val | stmts5.go:8:12:8:16 | other | -| stmts5.go:8:2:8:7 | selection of val | stmts5.go:9:1:9:1 | exit | | stmts5.go:8:2:8:16 | ... += ... | stmts5.go:8:2:8:7 | assignment to field val | | stmts5.go:8:12:8:16 | other | stmts5.go:8:2:8:16 | ... += ... | | stmts5.go:11:1:11:1 | entry | stmts5.go:11:20:12:1 | skip | @@ -842,9 +834,10 @@ | stmts7.go:19:7:19:13 | argument corresponding to methods | stmts7.go:19:7:19:13 | initialization of methods | | stmts7.go:19:7:19:13 | initialization of methods | stmts7.go:20:2:20:8 | methods | | stmts7.go:19:26:19:28 | skip | stmts7.go:19:1:21:1 | function declaration | -| stmts7.go:20:2:20:8 | methods | stmts7.go:20:2:20:11 | selection of fn | +| stmts7.go:20:2:20:8 | implicit dereference | stmts7.go:20:2:20:11 | selection of fn | +| stmts7.go:20:2:20:8 | implicit dereference | stmts7.go:21:1:21:1 | exit | +| stmts7.go:20:2:20:8 | methods | stmts7.go:20:2:20:8 | implicit dereference | | stmts7.go:20:2:20:11 | selection of fn | stmts7.go:20:2:20:13 | call to fn | -| stmts7.go:20:2:20:11 | selection of fn | stmts7.go:21:1:21:1 | exit | | stmts7.go:20:2:20:13 | call to fn | stmts7.go:21:1:21:1 | exit | | stmts7.go:23:1:23:1 | entry | stmts7.go:23:16:23:23 | argument corresponding to callback | | stmts7.go:23:1:28:1 | function declaration | stmts7.go:0:0:0:0 | exit | @@ -854,13 +847,13 @@ | stmts7.go:24:2:24:20 | defer statement | stmts7.go:25:10:25:17 | callback | | stmts7.go:24:8:24:15 | callback | stmts7.go:24:8:24:18 | selection of fn | | stmts7.go:24:8:24:18 | selection of fn | stmts7.go:24:2:24:20 | defer statement | -| stmts7.go:24:8:24:18 | selection of fn | stmts7.go:28:1:28:1 | exit | | stmts7.go:24:8:24:20 | call to fn | stmts7.go:28:1:28:1 | exit | | stmts7.go:25:2:25:23 | defer statement | stmts7.go:26:2:26:12 | selection of Println | -| stmts7.go:25:8:25:21 | selection of fn | stmts7.go:24:8:24:20 | call to fn | +| stmts7.go:25:8:25:18 | implicit dereference | stmts7.go:24:8:24:20 | call to fn | +| stmts7.go:25:8:25:18 | implicit dereference | stmts7.go:25:8:25:21 | selection of fn | | stmts7.go:25:8:25:21 | selection of fn | stmts7.go:25:2:25:23 | defer statement | | stmts7.go:25:8:25:23 | call to fn | stmts7.go:24:8:24:20 | call to fn | -| stmts7.go:25:9:25:17 | &... | stmts7.go:25:8:25:21 | selection of fn | +| stmts7.go:25:9:25:17 | &... | stmts7.go:25:8:25:18 | implicit dereference | | stmts7.go:25:10:25:17 | callback | stmts7.go:25:9:25:17 | &... | | stmts7.go:26:2:26:12 | selection of Println | stmts7.go:26:14:26:30 | "print something" | | stmts7.go:26:2:26:31 | call to Println | stmts7.go:25:8:25:23 | call to fn |