diff --git a/ql/src/semmle/go/controlflow/ControlFlowGraphImpl.qll b/ql/src/semmle/go/controlflow/ControlFlowGraphImpl.qll index a97de50e488..9d30f3728a3 100644 --- a/ql/src/semmle/go/controlflow/ControlFlowGraphImpl.qll +++ b/ql/src/semmle/go/controlflow/ControlFlowGraphImpl.qll @@ -262,20 +262,26 @@ newtype TControlFlowNode = 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. + * access, element access, or slice expression. */ 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 + ( + 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 + e = any(IndexExpr ie).getBase() + or + e = any(SliceExpr se).getBase() ) } or /** @@ -1139,20 +1145,34 @@ module CFG { } } - private class IndexExprTree extends PostOrderTree, IndexExpr { - override ControlFlow::Node getNode() { result = mkExprOrSkipNode(this) } + private class IndexExprTree extends ControlFlowTree, IndexExpr { + 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 - this.(ReferenceExpr).isRvalue() and - result = Panic() + // panic due to `nil` dereference + 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 + firstNode(this.getIndex(), succ) + ) or - i = 1 and result = getIndex() + pred = MkImplicitDeref(this.getBase()) and + firstNode(this.getIndex(), succ) + or + lastNode(getIndex(), pred, normalCompletion()) and + succ = mkExprOrSkipNode(this) } } @@ -1678,6 +1698,10 @@ module CFG { override predicate lastNode(ControlFlow::Node last, Completion cmpl) { ControlFlowTree.super.lastNode(last, cmpl) or + // panic due to `nil` dereference + last = MkImplicitDeref(getBase()) and + cmpl = Panic() + or last = MkExprNode(this) and (cmpl = Done() or cmpl = Panic()) } @@ -1686,6 +1710,14 @@ module CFG { ControlFlowTree.super.succ(pred, succ) or lastNode(getBase(), pred, normalCompletion()) and + ( + succ = MkImplicitDeref(getBase()) + or + not exists(MkImplicitDeref(getBase())) and + (firstNode(getLow(), succ) or succ = MkImplicitLowerSliceBound(this)) + ) + or + pred = MkImplicitDeref(getBase()) and (firstNode(getLow(), succ) or succ = MkImplicitLowerSliceBound(this)) or (lastNode(getLow(), pred, normalCompletion()) or pred = MkImplicitLowerSliceBound(this)) and diff --git a/ql/src/semmle/go/controlflow/IR.qll b/ql/src/semmle/go/controlflow/IR.qll index 1c504bcb1b2..2693f021744 100644 --- a/ql/src/semmle/go/controlflow/IR.qll +++ b/ql/src/semmle/go/controlflow/IR.qll @@ -233,13 +233,18 @@ module IR { } /** - * Gets the effective base of a selector expression, taking implicit dereferences into account. + * Gets the effective base of a selector, index or slice 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() | + private Instruction selectorBase(Expr e) { + exists(Expr base | + base = e.(SelectorExpr).getBase() or + base = e.(IndexExpr).getBase() or + base = e.(SliceExpr).getBase() + | result = MkImplicitDeref(base) or not exists(MkImplicitDeref(base)) and @@ -297,7 +302,7 @@ module IR { override IndexExpr e; /** Gets the instruction computing the base value on which the element is looked up. */ - Instruction getBase() { result = evalExprInstruction(e.getBase()) } + Instruction getBase() { result = selectorBase(e) } /** Gets the instruction computing the index of the element being looked up. */ Instruction getIndex() { result = evalExprInstruction(e.getIndex()) } @@ -1237,7 +1242,7 @@ module IR { /** * An instruction implicitly dereferencing the base in a field or method reference through a - * pointer. + * pointer, or the base in an element or slice reference through a pointer. */ class EvalImplicitDerefInstruction extends Instruction, MkImplicitDeref { Expr e; @@ -1417,7 +1422,7 @@ module IR { /** Gets the instruction computing the base value of this element reference. */ Instruction getBase() { - exists(IndexExpr idx | this = MkLhs(_, idx) | result = evalExprInstruction(idx.getBase())) + exists(IndexExpr idx | this = MkLhs(_, idx) | result = selectorBase(idx)) or result = w.(InitLiteralComponentInstruction).getBase() }