Skip to content

Commit

Permalink
Merge remote-tracking branch 'upstream/main' into mathiasvp/replace-a…
Browse files Browse the repository at this point in the history
…st-with-ir-use-usedataflow
  • Loading branch information
jketema committed Mar 16, 2023
2 parents eec1e9f + a13b6ed commit 8aa9207
Show file tree
Hide file tree
Showing 43 changed files with 242 additions and 85 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ module Consistency {
/** Holds if `n` should be excluded from the consistency test `uniqueEnclosingCallable`. */
predicate uniqueEnclosingCallableExclude(Node n) { none() }

/** Holds if `call` should be excluded from the consistency test `uniqueCallEnclosingCallable`. */
predicate uniqueCallEnclosingCallableExclude(DataFlowCall call) { none() }

/** Holds if `n` should be excluded from the consistency test `uniqueNodeLocation`. */
predicate uniqueNodeLocationExclude(Node n) { none() }

Expand Down Expand Up @@ -86,6 +89,15 @@ module Consistency {
)
}

query predicate uniqueCallEnclosingCallable(DataFlowCall call, string msg) {
exists(int c |
c = count(call.getEnclosingCallable()) and
c != 1 and
not any(ConsistencyConfiguration conf).uniqueCallEnclosingCallableExclude(call) and
msg = "Call should have one enclosing callable but has " + c + "."
)
}

query predicate uniqueType(Node n, string msg) {
exists(int c |
n instanceof RelevantNode and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ module Consistency {
/** Holds if `n` should be excluded from the consistency test `uniqueEnclosingCallable`. */
predicate uniqueEnclosingCallableExclude(Node n) { none() }

/** Holds if `call` should be excluded from the consistency test `uniqueCallEnclosingCallable`. */
predicate uniqueCallEnclosingCallableExclude(DataFlowCall call) { none() }

/** Holds if `n` should be excluded from the consistency test `uniqueNodeLocation`. */
predicate uniqueNodeLocationExclude(Node n) { none() }

Expand Down Expand Up @@ -86,6 +89,15 @@ module Consistency {
)
}

query predicate uniqueCallEnclosingCallable(DataFlowCall call, string msg) {
exists(int c |
c = count(call.getEnclosingCallable()) and
c != 1 and
not any(ConsistencyConfiguration conf).uniqueCallEnclosingCallableExclude(call) and
msg = "Call should have one enclosing callable but has " + c + "."
)
}

query predicate uniqueType(Node n, string msg) {
exists(int c |
n instanceof RelevantNode and
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
uniqueEnclosingCallable
uniqueCallEnclosingCallable
uniqueType
uniqueNodeLocation
missingLocation
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
uniqueEnclosingCallable
uniqueCallEnclosingCallable
uniqueType
uniqueNodeLocation
missingLocation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ uniqueEnclosingCallable
| C.cpp:10:20:10:29 | new | Node should have one enclosing callable but has 0. |
| C.cpp:35:24:35:33 | 0 | Node should have one enclosing callable but has 0. |
| C.cpp:35:24:35:33 | new | Node should have one enclosing callable but has 0. |
uniqueCallEnclosingCallable
uniqueType
uniqueNodeLocation
missingLocation
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
uniqueEnclosingCallable
uniqueCallEnclosingCallable
uniqueType
uniqueNodeLocation
missingLocation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ uniqueEnclosingCallable
| misc.c:210:24:210:24 | 0 | Node should have one enclosing callable but has 0. |
| misc.c:210:24:210:28 | ... + ... | Node should have one enclosing callable but has 0. |
| misc.c:210:28:210:28 | 1 | Node should have one enclosing callable but has 0. |
uniqueCallEnclosingCallable
uniqueType
uniqueNodeLocation
| file://:0:0:0:0 | (unnamed parameter 2) | Node should have one location but has 0. |
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
uniqueEnclosingCallable
uniqueCallEnclosingCallable
uniqueType
uniqueNodeLocation
missingLocation
Expand Down
10 changes: 10 additions & 0 deletions csharp/ql/consistency-queries/DataFlowConsistency.ql
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import csharp
import cil
import semmle.code.csharp.dataflow.internal.DataFlowPrivate
import semmle.code.csharp.dataflow.internal.DataFlowPublic
import semmle.code.csharp.dataflow.internal.DataFlowDispatch
import semmle.code.csharp.dataflow.internal.DataFlowImplConsistency::Consistency

private class MyConsistencyConfiguration extends ConsistencyConfiguration {
Expand All @@ -14,6 +15,15 @@ private class MyConsistencyConfiguration extends ConsistencyConfiguration {
)
}

override predicate uniqueCallEnclosingCallableExclude(DataFlowCall call) {
// TODO: Remove once static initializers are folded into the
// static constructors
exists(ControlFlow::Node cfn |
cfn.getElement() = any(FieldOrProperty f | f.isStatic()).getAChild+() and
cfn = call.getControlFlowNode()
)
}

override predicate uniqueNodeLocationExclude(Node n) {
// Methods with multiple implementations
n instanceof ParameterNode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ module Consistency {
/** Holds if `n` should be excluded from the consistency test `uniqueEnclosingCallable`. */
predicate uniqueEnclosingCallableExclude(Node n) { none() }

/** Holds if `call` should be excluded from the consistency test `uniqueCallEnclosingCallable`. */
predicate uniqueCallEnclosingCallableExclude(DataFlowCall call) { none() }

/** Holds if `n` should be excluded from the consistency test `uniqueNodeLocation`. */
predicate uniqueNodeLocationExclude(Node n) { none() }

Expand Down Expand Up @@ -86,6 +89,15 @@ module Consistency {
)
}

query predicate uniqueCallEnclosingCallable(DataFlowCall call, string msg) {
exists(int c |
c = count(call.getEnclosingCallable()) and
c != 1 and
not any(ConsistencyConfiguration conf).uniqueCallEnclosingCallableExclude(call) and
msg = "Call should have one enclosing callable but has " + c + "."
)
}

query predicate uniqueType(Node n, string msg) {
exists(int c |
n instanceof RelevantNode and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ module Consistency {
/** Holds if `n` should be excluded from the consistency test `uniqueEnclosingCallable`. */
predicate uniqueEnclosingCallableExclude(Node n) { none() }

/** Holds if `call` should be excluded from the consistency test `uniqueCallEnclosingCallable`. */
predicate uniqueCallEnclosingCallableExclude(DataFlowCall call) { none() }

/** Holds if `n` should be excluded from the consistency test `uniqueNodeLocation`. */
predicate uniqueNodeLocationExclude(Node n) { none() }

Expand Down Expand Up @@ -86,6 +89,15 @@ module Consistency {
)
}

query predicate uniqueCallEnclosingCallable(DataFlowCall call, string msg) {
exists(int c |
c = count(call.getEnclosingCallable()) and
c != 1 and
not any(ConsistencyConfiguration conf).uniqueCallEnclosingCallableExclude(call) and
msg = "Call should have one enclosing callable but has " + c + "."
)
}

query predicate uniqueType(Node n, string msg) {
exists(int c |
n instanceof RelevantNode and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,14 @@ newtype TParameterPosition =
index = any(Parameter p).getPosition() + 1
} or
TSynthStarArgsElementParameterPosition(int index) { exists(TStarArgsParameterPosition(index)) } or
TDictSplatParameterPosition()
TDictSplatParameterPosition() or
// To get flow from a **kwargs argument to a keyword parameter, we add a read-step
// from a synthetic **kwargs parameter. We need this separate synthetic ParameterNode,
// since we clear content of the normal **kwargs parameter for the names that
// correspond to normal keyword parameters. Since we cannot re-use the same parameter
// position for multiple parameter nodes in the same callable, we introduce this
// synthetic parameter position.
TSynthDictSplatParameterPosition()

/** A parameter position. */
class ParameterPosition extends TParameterPosition {
Expand Down Expand Up @@ -92,6 +99,12 @@ class ParameterPosition extends TParameterPosition {
/** Holds if this position represents a `**kwargs` parameter. */
predicate isDictSplat() { this = TDictSplatParameterPosition() }

/**
* Holds if this position represents a **synthetic** `**kwargs` parameter
* (see comment for `TSynthDictSplatParameterPosition`).
*/
predicate isSynthDictSplat() { this = TSynthDictSplatParameterPosition() }

/** Gets a textual representation of this element. */
string toString() {
this.isSelf() and result = "self"
Expand All @@ -108,6 +121,8 @@ class ParameterPosition extends TParameterPosition {
)
or
this.isDictSplat() and result = "**"
or
this.isSynthDictSplat() and result = "synthetic **"
}
}

Expand Down Expand Up @@ -179,6 +194,8 @@ predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) {
)
or
ppos.isDictSplat() and apos.isDictSplat()
or
ppos.isSynthDictSplat() and apos.isDictSplat()
}

// =============================================================================
Expand Down Expand Up @@ -324,16 +341,9 @@ abstract class DataFlowFunction extends DataFlowCallable, TFunction {
)
or
// `**kwargs`
// since the dataflow library has the restriction that we can only have ONE result per
// parameter position, if there is both a synthetic **kwargs and a real **kwargs
// parameter, we only give the result for the synthetic, and add local flow from the
// synthetic to the real. It might seem more natural to do it in the other
// direction, but since we have a clearStep on the real **kwargs parameter, we would have that
// content-clearing would also affect the synthetic parameter, which we don't want.
ppos.isDictSplat() and
if exists(func.getArgByName(_))
then result = TSynthDictSplatParameterNode(this)
else result.getParameter() = func.getKwarg()
ppos.isDictSplat() and result.getParameter() = func.getKwarg()
or
ppos.isSynthDictSplat() and result = TSynthDictSplatParameterNode(this)
}
}

Expand Down Expand Up @@ -1460,16 +1470,7 @@ class SummaryParameterNode extends ParameterNodeImpl, TSummaryParameterNode {
override Parameter getParameter() { none() }

override predicate isParameterOf(DataFlowCallable c, ParameterPosition ppos) {
sc = c.asLibraryCallable() and
ppos = pos and
// avoid overlap with `SynthDictSplatParameterNode`
not (
pos.isDictSplat() and
exists(ParameterPosition keywordPos |
FlowSummaryImpl::Private::summaryParameterNodeRange(sc, keywordPos) and
keywordPos.isKeyword(_)
)
)
sc = c.asLibraryCallable() and ppos = pos
}

override DataFlowCallable getEnclosingCallable() { result.asLibraryCallable() = sc }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ module Consistency {
/** Holds if `n` should be excluded from the consistency test `uniqueEnclosingCallable`. */
predicate uniqueEnclosingCallableExclude(Node n) { none() }

/** Holds if `call` should be excluded from the consistency test `uniqueCallEnclosingCallable`. */
predicate uniqueCallEnclosingCallableExclude(DataFlowCall call) { none() }

/** Holds if `n` should be excluded from the consistency test `uniqueNodeLocation`. */
predicate uniqueNodeLocationExclude(Node n) { none() }

Expand Down Expand Up @@ -86,6 +89,15 @@ module Consistency {
)
}

query predicate uniqueCallEnclosingCallable(DataFlowCall call, string msg) {
exists(int c |
c = count(call.getEnclosingCallable()) and
c != 1 and
not any(ConsistencyConfiguration conf).uniqueCallEnclosingCallableExclude(call) and
msg = "Call should have one enclosing callable but has " + c + "."
)
}

query predicate uniqueType(Node n, string msg) {
exists(int c |
n instanceof RelevantNode and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,11 +182,7 @@ private predicate synthDictSplatArgumentNodeStoreStep(
private predicate dictSplatParameterNodeClearStep(ParameterNode n, DictionaryElementContent c) {
exists(DataFlowCallable callable, ParameterPosition dictSplatPos, ParameterPosition keywordPos |
dictSplatPos.isDictSplat() and
(
n.getParameter() = callable.(DataFlowFunction).getScope().getKwarg()
or
n = TSummaryParameterNode(callable.asLibraryCallable(), dictSplatPos)
) and
n = callable.getParameter(dictSplatPos) and
exists(callable.getParameter(keywordPos)) and
keywordPos.isKeyword(c.getKey())
)
Expand Down Expand Up @@ -237,28 +233,6 @@ class SynthDictSplatParameterNode extends ParameterNodeImpl, TSynthDictSplatPara
override Parameter getParameter() { none() }
}

/**
* Flow step from the synthetic `**kwargs` parameter to the real `**kwargs` parameter.
* Due to restriction in dataflow library, we can only give one of them as result for
* `DataFlowCallable.getParameter`, so this is a workaround to ensure there is flow to
* _both_ of them.
*/
private predicate dictSplatParameterNodeFlowStep(
ParameterNodeImpl nodeFrom, ParameterNodeImpl nodeTo
) {
exists(DataFlowCallable callable |
nodeFrom = TSynthDictSplatParameterNode(callable) and
(
nodeTo.getParameter() = callable.(DataFlowFunction).getScope().getKwarg()
or
exists(ParameterPosition pos |
nodeTo = TSummaryParameterNode(callable.asLibraryCallable(), pos) and
pos.isDictSplat()
)
)
)
}

/**
* Reads from the synthetic **kwargs parameter to each keyword parameter.
*/
Expand Down Expand Up @@ -404,8 +378,6 @@ predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) {
simpleLocalFlowStepForTypetracking(nodeFrom, nodeTo)
or
summaryFlowSteps(nodeFrom, nodeTo)
or
dictSplatParameterNodeFlowStep(nodeFrom, nodeTo)
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,6 @@ private class MyConsistencyConfiguration extends ConsistencyConfiguration {
n instanceof SynthDictSplatParameterNode
}

override predicate uniqueParameterNodeAtPositionExclude(
DataFlowCallable c, ParameterPosition pos, Node p
) {
// TODO: This can be removed once we solve the overlap of dictionary splat parameters
c.getParameter(pos) = p and
pos.isDictSplat() and
not exists(p.getLocation().getFile().getRelativePath())
}

override predicate uniqueParameterNodePositionExclude(
DataFlowCallable c, ParameterPosition pos, Node p
) {
Expand All @@ -44,4 +35,8 @@ private class MyConsistencyConfiguration extends ConsistencyConfiguration {
param = func.getArgByName(_)
)
}

override predicate uniqueCallEnclosingCallableExclude(DataFlowCall call) {
not exists(call.getLocation().getFile().getRelativePath())
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
uniqueEnclosingCallable
uniqueCallEnclosingCallable
uniqueType
uniqueNodeLocation
missingLocation
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
uniqueEnclosingCallable
uniqueCallEnclosingCallable
uniqueType
uniqueNodeLocation
missingLocation
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
uniqueEnclosingCallable
uniqueCallEnclosingCallable
| new_cls_param.py:14:6:14:16 | classmethod() | Call should have one enclosing callable but has 0. |
| test.py:21:6:21:17 | staticmethod() | Call should have one enclosing callable but has 0. |
| test.py:25:6:25:16 | classmethod() | Call should have one enclosing callable but has 0. |
| test.py:29:6:29:16 | classmethod() | Call should have one enclosing callable but has 0. |
uniqueType
uniqueNodeLocation
missingLocation
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
uniqueEnclosingCallable
uniqueCallEnclosingCallable
uniqueType
uniqueNodeLocation
missingLocation
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
uniqueEnclosingCallable
uniqueCallEnclosingCallable
| datamodel.py:71:6:71:16 | classmethod() | Call should have one enclosing callable but has 0. |
| datamodel.py:76:6:76:17 | staticmethod() | Call should have one enclosing callable but has 0. |
uniqueType
uniqueNodeLocation
missingLocation
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
uniqueEnclosingCallable
uniqueCallEnclosingCallable
uniqueType
uniqueNodeLocation
missingLocation
Expand Down
Loading

0 comments on commit 8aa9207

Please sign in to comment.