From 09d2fe391e63e1a128f2c76722445f6dd7cdff33 Mon Sep 17 00:00:00 2001 From: Max Schaefer Date: Wed, 1 Jul 2020 20:13:03 +0100 Subject: [PATCH] Data flow: Replace `getErasedRepr()` and `Node::getTypeBound()` with `getNodeType()`. cf https://github.com/github/codeql/pull/3854 --- .../go/dataflow/internal/DataFlowImpl.qll | 32 +++++++++---------- .../dataflow/internal/DataFlowImplCommon.qll | 29 ++++++++--------- .../go/dataflow/internal/DataFlowPrivate.qll | 12 ++----- 3 files changed, 31 insertions(+), 42 deletions(-) diff --git a/ql/src/semmle/go/dataflow/internal/DataFlowImpl.qll b/ql/src/semmle/go/dataflow/internal/DataFlowImpl.qll index a5c032f2660..5042dce683f 100644 --- a/ql/src/semmle/go/dataflow/internal/DataFlowImpl.qll +++ b/ql/src/semmle/go/dataflow/internal/DataFlowImpl.qll @@ -1124,11 +1124,11 @@ private module LocalFlowBigStep { ( localFlowStepNodeCand1(node1, node2, config) and preservesValue = true and - t = getErasedNodeTypeBound(node1) + t = getNodeType(node1) or additionalLocalFlowStepNodeCand2(node1, node2, config) and preservesValue = false and - t = getErasedNodeTypeBound(node2) + t = getNodeType(node2) ) and node1 != node2 and cc.relevantFor(node1.getEnclosingCallable()) and @@ -1147,7 +1147,7 @@ private module LocalFlowBigStep { additionalLocalFlowStepNodeCand2(mid, node2, config) and not mid instanceof FlowCheckNode and preservesValue = false and - t = getErasedNodeTypeBound(node2) and + t = getNodeType(node2) and nodeCand2(node2, unbind(config)) ) ) @@ -1202,9 +1202,7 @@ private predicate flowCandFwd( ) { flowCandFwd0(node, fromArg, argApf, apf, config) and not apf.isClearedAt(node) and - if node instanceof CastingNode - then compatibleTypes(getErasedNodeTypeBound(node), apf.getType()) - else any() + if node instanceof CastingNode then compatibleTypes(getNodeType(node), apf.getType()) else any() } pragma[nomagic] @@ -1216,7 +1214,7 @@ private predicate flowCandFwd0( config.isSource(node) and fromArg = false and argApf = TAccessPathFrontNone() and - apf = TFrontNil(getErasedNodeTypeBound(node)) + apf = TFrontNil(getNodeType(node)) or exists(Node mid | flowCandFwd(mid, fromArg, argApf, apf, config) and @@ -1242,7 +1240,7 @@ private predicate flowCandFwd0( additionalJumpStep(mid, node, config) and fromArg = false and argApf = TAccessPathFrontNone() and - apf = TFrontNil(getErasedNodeTypeBound(node)) + apf = TFrontNil(getNodeType(node)) ) or // store @@ -1672,7 +1670,7 @@ private predicate flowFwd0( config.isSource(node) and fromArg = false and argAp = TAccessPathNone() and - ap = TNil(getErasedNodeTypeBound(node)) and + ap = TNil(getNodeType(node)) and apf = ap.(AccessPathNil).getFront() or flowCand(node, _, _, _, unbind(config)) and @@ -1700,7 +1698,7 @@ private predicate flowFwd0( additionalJumpStep(mid, node, config) and fromArg = false and argAp = TAccessPathNone() and - ap = TNil(getErasedNodeTypeBound(node)) and + ap = TNil(getNodeType(node)) and apf = ap.(AccessPathNil).getFront() ) ) @@ -2077,7 +2075,7 @@ private newtype TPathNode = config.isSource(node) and cc instanceof CallContextAny and sc instanceof SummaryCtxNone and - ap = TNil(getErasedNodeTypeBound(node)) + ap = TNil(getNodeType(node)) or // ... or a step from an existing PathNode to another node. exists(PathNodeMid mid | @@ -2304,7 +2302,7 @@ private predicate pathStep(PathNodeMid mid, Node node, CallContext cc, SummaryCt cc instanceof CallContextAny and sc instanceof SummaryCtxNone and mid.getAp() instanceof AccessPathNil and - ap = TNil(getErasedNodeTypeBound(node)) + ap = TNil(getNodeType(node)) or exists(TypedContent tc | pathStoreStep(mid, node, pop(tc, ap), tc, cc)) and sc = mid.getSummaryCtx() @@ -2646,7 +2644,7 @@ private module FlowExploration { cc instanceof CallContextAny and sc1 = TSummaryCtx1None() and sc2 = TSummaryCtx2None() and - ap = TPartialNil(getErasedNodeTypeBound(node)) and + ap = TPartialNil(getNodeType(node)) and not fullBarrier(node, config) and exists(config.explorationLimit()) or @@ -2663,7 +2661,7 @@ private module FlowExploration { partialPathStep(mid, node, cc, sc1, sc2, ap, config) and not fullBarrier(node, config) and if node instanceof CastingNode - then compatibleTypes(getErasedNodeTypeBound(node), ap.getType()) + then compatibleTypes(getNodeType(node), ap.getType()) else any() ) } @@ -2776,7 +2774,7 @@ private module FlowExploration { sc1 = mid.getSummaryCtx1() and sc2 = mid.getSummaryCtx2() and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedNodeTypeBound(node)) and + ap = TPartialNil(getNodeType(node)) and config = mid.getConfiguration() ) or @@ -2792,7 +2790,7 @@ private module FlowExploration { sc1 = TSummaryCtx1None() and sc2 = TSummaryCtx2None() and mid.getAp() instanceof PartialAccessPathNil and - ap = TPartialNil(getErasedNodeTypeBound(node)) and + ap = TPartialNil(getNodeType(node)) and config = mid.getConfiguration() or partialPathStoreStep(mid, _, _, node, ap) and @@ -2806,7 +2804,7 @@ private module FlowExploration { sc1 = mid.getSummaryCtx1() and sc2 = mid.getSummaryCtx2() and apConsFwd(ap, tc, ap0, config) and - compatibleTypes(ap.getType(), getErasedNodeTypeBound(node)) + compatibleTypes(ap.getType(), getNodeType(node)) ) or partialPathIntoCallable(mid, node, _, cc, sc1, sc2, _, ap, config) diff --git a/ql/src/semmle/go/dataflow/internal/DataFlowImplCommon.qll b/ql/src/semmle/go/dataflow/internal/DataFlowImplCommon.qll index c02a2f4d6fe..830ca401213 100644 --- a/ql/src/semmle/go/dataflow/internal/DataFlowImplCommon.qll +++ b/ql/src/semmle/go/dataflow/internal/DataFlowImplCommon.qll @@ -22,7 +22,7 @@ private module Cached { exists(int i | viableParam(call, i, p) and arg.argumentOf(call, i) and - compatibleTypes(getErasedNodeTypeBound(arg), getErasedNodeTypeBound(p)) + compatibleTypes(getNodeType(arg), getNodeType(p)) ) } @@ -218,10 +218,10 @@ private module Cached { then // normal flow through read = TReadStepTypesNone() and - compatibleTypes(getErasedNodeTypeBound(p), getErasedNodeTypeBound(node)) + compatibleTypes(getNodeType(p), getNodeType(node)) or // getter - compatibleTypes(read.getContentType(), getErasedNodeTypeBound(node)) + compatibleTypes(read.getContentType(), getNodeType(node)) else any() } @@ -243,7 +243,7 @@ private module Cached { readStepWithTypes(mid, read.getContainerType(), read.getContent(), node, read.getContentType()) and Cand::parameterValueFlowReturnCand(p, _, true) and - compatibleTypes(getErasedNodeTypeBound(p), read.getContainerType()) + compatibleTypes(getNodeType(p), read.getContainerType()) ) or // flow through: no prior read @@ -292,11 +292,11 @@ private module Cached { | // normal flow through read = TReadStepTypesNone() and - compatibleTypes(getErasedNodeTypeBound(arg), getErasedNodeTypeBound(out)) + compatibleTypes(getNodeType(arg), getNodeType(out)) or // getter - compatibleTypes(getErasedNodeTypeBound(arg), read.getContainerType()) and - compatibleTypes(read.getContentType(), getErasedNodeTypeBound(out)) + compatibleTypes(getNodeType(arg), read.getContainerType()) and + compatibleTypes(read.getContentType(), getNodeType(out)) ) } @@ -405,8 +405,8 @@ private module Cached { ) { storeStep(node1, c, node2) and readStep(_, c, _) and - contentType = getErasedNodeTypeBound(node1) and - containerType = getErasedNodeTypeBound(node2) + contentType = getNodeType(node1) and + containerType = getNodeType(node2) or exists(Node n1, Node n2 | n1 = node1.(PostUpdateNode).getPreUpdateNode() and @@ -415,8 +415,8 @@ private module Cached { argumentValueFlowsThrough(n2, TReadStepTypesSome(containerType, c, contentType), n1) or readStep(n2, c, n1) and - contentType = getErasedNodeTypeBound(n1) and - containerType = getErasedNodeTypeBound(n2) + contentType = getNodeType(n1) and + containerType = getNodeType(n2) ) } @@ -507,8 +507,8 @@ private predicate readStepWithTypes( Node n1, DataFlowType container, Content c, Node n2, DataFlowType content ) { readStep(n1, c, n2) and - container = getErasedNodeTypeBound(n1) and - content = getErasedNodeTypeBound(n2) + container = getNodeType(n1) and + content = getNodeType(n2) } private newtype TReadStepTypesOption = @@ -771,9 +771,6 @@ DataFlowCallable resolveCall(DataFlowCall call, CallContext cc) { result = viableCallable(call) and cc instanceof CallContextReturn } -pragma[noinline] -DataFlowType getErasedNodeTypeBound(Node n) { result = getErasedRepr(n.getTypeBound()) } - predicate read = readStep/3; /** An optional Boolean value. */ diff --git a/ql/src/semmle/go/dataflow/internal/DataFlowPrivate.qll b/ql/src/semmle/go/dataflow/internal/DataFlowPrivate.qll index 1d195a0ba35..0bbac21c684 100644 --- a/ql/src/semmle/go/dataflow/internal/DataFlowPrivate.qll +++ b/ql/src/semmle/go/dataflow/internal/DataFlowPrivate.qll @@ -148,16 +148,10 @@ predicate clearsContent(Node n, Content c) { none() // stub implementation } -/** - * Gets a representative (boxed) type for `t` for the purpose of pruning - * possible flow. A single type is used for all numeric types to account for - * numeric conversions, and otherwise the erasure is used. - */ -DataFlowType getErasedRepr(Type t) { - result = t // stub implementation -} +/** Gets the type of `n` used for type pruning. */ +DataFlowType getNodeType(Node n) { result = n.getType() } -/** Gets a string representation of a type returned by `getErasedRepr`. */ +/** Gets a string representation of a type returned by `getNodeType()`. */ string ppReprType(Type t) { result = t.toString() } /**