From 4b7813b98e8775ef9a15fe92ac2369f2e5bcc42c Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Wed, 21 Aug 2019 09:27:01 +0200 Subject: [PATCH 1/4] C++/C#/Java: Split localFlowStep predicate in two There's now a `localFlowStep` predicate for use directly in queries and other libraries and a `simpleLocalFlowStep` for use only by the global data flow library. The former predicate is intended to include field flow, but the latter may not. This will let Java and C# (and possibly C++ IR) avoid getting two kinds of field flow at the same time, both from SSA and from the global data flow library. It should let C++ AST add some form of field flow to `localFlowStep` without making it an input to the global data flow library. --- .../code/cpp/dataflow/internal/DataFlowImpl.qll | 2 +- .../code/cpp/dataflow/internal/DataFlowImpl2.qll | 2 +- .../code/cpp/dataflow/internal/DataFlowImpl3.qll | 2 +- .../code/cpp/dataflow/internal/DataFlowImpl4.qll | 2 +- .../code/cpp/dataflow/internal/DataFlowUtil.qll | 12 +++++++++++- .../code/cpp/ir/dataflow/internal/DataFlowImpl.qll | 2 +- .../code/cpp/ir/dataflow/internal/DataFlowImpl2.qll | 2 +- .../code/cpp/ir/dataflow/internal/DataFlowImpl3.qll | 2 +- .../code/cpp/ir/dataflow/internal/DataFlowImpl4.qll | 2 +- .../code/cpp/ir/dataflow/internal/DataFlowUtil.qll | 10 ++++++++++ .../code/csharp/dataflow/internal/DataFlowImpl.qll | 2 +- .../code/csharp/dataflow/internal/DataFlowImpl2.qll | 2 +- .../code/csharp/dataflow/internal/DataFlowImpl3.qll | 2 +- .../code/csharp/dataflow/internal/DataFlowImpl4.qll | 2 +- .../code/csharp/dataflow/internal/DataFlowImpl5.qll | 2 +- .../csharp/dataflow/internal/DataFlowPrivate.qll | 11 +++++++++++ .../code/java/dataflow/internal/DataFlowImpl.qll | 2 +- .../code/java/dataflow/internal/DataFlowImpl2.qll | 2 +- .../code/java/dataflow/internal/DataFlowImpl3.qll | 2 +- .../code/java/dataflow/internal/DataFlowImpl4.qll | 2 +- .../code/java/dataflow/internal/DataFlowImpl5.qll | 2 +- .../code/java/dataflow/internal/DataFlowUtil.qll | 11 +++++++++++ 22 files changed, 61 insertions(+), 19 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl.qll index f797b3bcf35b..64edab03d692 100644 --- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl.qll +++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl.qll @@ -162,7 +162,7 @@ private predicate isAdditionalFlowStep( * Holds if data can flow in one local step from `node1` to `node2`. */ private predicate localFlowStep(Node node1, Node node2, Configuration config) { - localFlowStep(node1, node2) and + simpleLocalFlowStep(node1, node2) and not outBarrier(node1, config) and not inBarrier(node2, config) and not fullBarrier(node1, config) and diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll index f797b3bcf35b..64edab03d692 100644 --- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll +++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll @@ -162,7 +162,7 @@ private predicate isAdditionalFlowStep( * Holds if data can flow in one local step from `node1` to `node2`. */ private predicate localFlowStep(Node node1, Node node2, Configuration config) { - localFlowStep(node1, node2) and + simpleLocalFlowStep(node1, node2) and not outBarrier(node1, config) and not inBarrier(node2, config) and not fullBarrier(node1, config) and diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll index f797b3bcf35b..64edab03d692 100644 --- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll +++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll @@ -162,7 +162,7 @@ private predicate isAdditionalFlowStep( * Holds if data can flow in one local step from `node1` to `node2`. */ private predicate localFlowStep(Node node1, Node node2, Configuration config) { - localFlowStep(node1, node2) and + simpleLocalFlowStep(node1, node2) and not outBarrier(node1, config) and not inBarrier(node2, config) and not fullBarrier(node1, config) and diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl4.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl4.qll index f797b3bcf35b..64edab03d692 100644 --- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl4.qll +++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl4.qll @@ -162,7 +162,7 @@ private predicate isAdditionalFlowStep( * Holds if data can flow in one local step from `node1` to `node2`. */ private predicate localFlowStep(Node node1, Node node2, Configuration config) { - localFlowStep(node1, node2) and + simpleLocalFlowStep(node1, node2) and not outBarrier(node1, config) and not inBarrier(node2, config) and not fullBarrier(node1, config) and diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll index 8f8a7a9369a2..955208cacc6e 100644 --- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll +++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowUtil.qll @@ -422,8 +422,18 @@ private module ThisFlow { * Holds if data flows from `nodeFrom` to `nodeTo` in exactly one local * (intra-procedural) step. */ -cached predicate localFlowStep(Node nodeFrom, Node nodeTo) { + simpleLocalFlowStep(nodeFrom, nodeTo) +} + +/** + * INTERNAL: do not use. + * + * This is the local flow predicate that's used as a building block in global + * data flow. It may have less flow than the `localFlowStep` predicate. + */ +cached +predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) { // Expr -> Expr exprToExprStep_nocfg(nodeFrom.asExpr(), nodeTo.asExpr()) or diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl.qll index f797b3bcf35b..64edab03d692 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl.qll @@ -162,7 +162,7 @@ private predicate isAdditionalFlowStep( * Holds if data can flow in one local step from `node1` to `node2`. */ private predicate localFlowStep(Node node1, Node node2, Configuration config) { - localFlowStep(node1, node2) and + simpleLocalFlowStep(node1, node2) and not outBarrier(node1, config) and not inBarrier(node2, config) and not fullBarrier(node1, config) and diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl2.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl2.qll index f797b3bcf35b..64edab03d692 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl2.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl2.qll @@ -162,7 +162,7 @@ private predicate isAdditionalFlowStep( * Holds if data can flow in one local step from `node1` to `node2`. */ private predicate localFlowStep(Node node1, Node node2, Configuration config) { - localFlowStep(node1, node2) and + simpleLocalFlowStep(node1, node2) and not outBarrier(node1, config) and not inBarrier(node2, config) and not fullBarrier(node1, config) and diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl3.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl3.qll index f797b3bcf35b..64edab03d692 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl3.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl3.qll @@ -162,7 +162,7 @@ private predicate isAdditionalFlowStep( * Holds if data can flow in one local step from `node1` to `node2`. */ private predicate localFlowStep(Node node1, Node node2, Configuration config) { - localFlowStep(node1, node2) and + simpleLocalFlowStep(node1, node2) and not outBarrier(node1, config) and not inBarrier(node2, config) and not fullBarrier(node1, config) and diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl4.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl4.qll index f797b3bcf35b..64edab03d692 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl4.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl4.qll @@ -162,7 +162,7 @@ private predicate isAdditionalFlowStep( * Holds if data can flow in one local step from `node1` to `node2`. */ private predicate localFlowStep(Node node1, Node node2, Configuration config) { - localFlowStep(node1, node2) and + simpleLocalFlowStep(node1, node2) and not outBarrier(node1, config) and not inBarrier(node2, config) and not fullBarrier(node1, config) and diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll index 5ceb4e79682f..e9715925432a 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll @@ -156,6 +156,16 @@ UninitializedNode uninitializedNode(LocalVariable v) { result.getLocalVariable() * (intra-procedural) step. */ predicate localFlowStep(Node nodeFrom, Node nodeTo) { + simpleLocalFlowStep(nodeFrom, nodeTo) +} + +/** + * INTERNAL: do not use. + * + * This is the local flow predicate that's used as a building block in global + * data flow. It may have less flow than the `localFlowStep` predicate. + */ +predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) { nodeTo.(CopyInstruction).getSourceValue() = nodeFrom or nodeTo.(PhiInstruction).getAnOperand().getDef() = nodeFrom or // Treat all conversions as flow, even conversions between different numeric types. diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl.qll index f797b3bcf35b..64edab03d692 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl.qll @@ -162,7 +162,7 @@ private predicate isAdditionalFlowStep( * Holds if data can flow in one local step from `node1` to `node2`. */ private predicate localFlowStep(Node node1, Node node2, Configuration config) { - localFlowStep(node1, node2) and + simpleLocalFlowStep(node1, node2) and not outBarrier(node1, config) and not inBarrier(node2, config) and not fullBarrier(node1, config) and diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl2.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl2.qll index f797b3bcf35b..64edab03d692 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl2.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl2.qll @@ -162,7 +162,7 @@ private predicate isAdditionalFlowStep( * Holds if data can flow in one local step from `node1` to `node2`. */ private predicate localFlowStep(Node node1, Node node2, Configuration config) { - localFlowStep(node1, node2) and + simpleLocalFlowStep(node1, node2) and not outBarrier(node1, config) and not inBarrier(node2, config) and not fullBarrier(node1, config) and diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl3.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl3.qll index f797b3bcf35b..64edab03d692 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl3.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl3.qll @@ -162,7 +162,7 @@ private predicate isAdditionalFlowStep( * Holds if data can flow in one local step from `node1` to `node2`. */ private predicate localFlowStep(Node node1, Node node2, Configuration config) { - localFlowStep(node1, node2) and + simpleLocalFlowStep(node1, node2) and not outBarrier(node1, config) and not inBarrier(node2, config) and not fullBarrier(node1, config) and diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl4.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl4.qll index f797b3bcf35b..64edab03d692 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl4.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl4.qll @@ -162,7 +162,7 @@ private predicate isAdditionalFlowStep( * Holds if data can flow in one local step from `node1` to `node2`. */ private predicate localFlowStep(Node node1, Node node2, Configuration config) { - localFlowStep(node1, node2) and + simpleLocalFlowStep(node1, node2) and not outBarrier(node1, config) and not inBarrier(node2, config) and not fullBarrier(node1, config) and diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl5.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl5.qll index f797b3bcf35b..64edab03d692 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl5.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl5.qll @@ -162,7 +162,7 @@ private predicate isAdditionalFlowStep( * Holds if data can flow in one local step from `node1` to `node2`. */ private predicate localFlowStep(Node node1, Node node2, Configuration config) { - localFlowStep(node1, node2) and + simpleLocalFlowStep(node1, node2) and not outBarrier(node1, config) and not inBarrier(node2, config) and not fullBarrier(node1, config) and diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll index 7f78c580cb6f..10561b7c773e 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll @@ -255,6 +255,17 @@ private module Cached { */ cached predicate localFlowStepImpl(Node nodeFrom, Node nodeTo) { + simpleLocalFlowStep(nodeFrom, nodeTo) + } + + /** + * INTERNAL: do not use. + * + * This is the local flow predicate that's used as a building block in global + * data flow. It may have less flow than the `localFlowStep` predicate. + */ + cached + predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) { any(LocalFlow::LocalExprStepConfiguration x).hasNodePath(nodeFrom, nodeTo) or // Flow from SSA definition to first read diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl.qll index f797b3bcf35b..64edab03d692 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl.qll @@ -162,7 +162,7 @@ private predicate isAdditionalFlowStep( * Holds if data can flow in one local step from `node1` to `node2`. */ private predicate localFlowStep(Node node1, Node node2, Configuration config) { - localFlowStep(node1, node2) and + simpleLocalFlowStep(node1, node2) and not outBarrier(node1, config) and not inBarrier(node2, config) and not fullBarrier(node1, config) and diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl2.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl2.qll index f797b3bcf35b..64edab03d692 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl2.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl2.qll @@ -162,7 +162,7 @@ private predicate isAdditionalFlowStep( * Holds if data can flow in one local step from `node1` to `node2`. */ private predicate localFlowStep(Node node1, Node node2, Configuration config) { - localFlowStep(node1, node2) and + simpleLocalFlowStep(node1, node2) and not outBarrier(node1, config) and not inBarrier(node2, config) and not fullBarrier(node1, config) and diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl3.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl3.qll index f797b3bcf35b..64edab03d692 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl3.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl3.qll @@ -162,7 +162,7 @@ private predicate isAdditionalFlowStep( * Holds if data can flow in one local step from `node1` to `node2`. */ private predicate localFlowStep(Node node1, Node node2, Configuration config) { - localFlowStep(node1, node2) and + simpleLocalFlowStep(node1, node2) and not outBarrier(node1, config) and not inBarrier(node2, config) and not fullBarrier(node1, config) and diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl4.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl4.qll index f797b3bcf35b..64edab03d692 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl4.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl4.qll @@ -162,7 +162,7 @@ private predicate isAdditionalFlowStep( * Holds if data can flow in one local step from `node1` to `node2`. */ private predicate localFlowStep(Node node1, Node node2, Configuration config) { - localFlowStep(node1, node2) and + simpleLocalFlowStep(node1, node2) and not outBarrier(node1, config) and not inBarrier(node2, config) and not fullBarrier(node1, config) and diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl5.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl5.qll index f797b3bcf35b..64edab03d692 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl5.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl5.qll @@ -162,7 +162,7 @@ private predicate isAdditionalFlowStep( * Holds if data can flow in one local step from `node1` to `node2`. */ private predicate localFlowStep(Node node1, Node node2, Configuration config) { - localFlowStep(node1, node2) and + simpleLocalFlowStep(node1, node2) and not outBarrier(node1, config) and not inBarrier(node2, config) and not fullBarrier(node1, config) and diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll index 98e4fd214c71..be8e8dc32f94 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll @@ -352,6 +352,17 @@ predicate hasNonlocalValue(FieldRead fr) { */ cached predicate localFlowStep(Node node1, Node node2) { + simpleLocalFlowStep(node1, node2) +} + +/** + * INTERNAL: do not use. + * + * This is the local flow predicate that's used as a building block in global + * data flow. It may have less flow than the `localFlowStep` predicate. + */ +cached +predicate simpleLocalFlowStep(Node node1, Node node2) { // Variable flow steps through adjacent def-use and use-use pairs. exists(SsaExplicitUpdate upd | upd.getDefiningExpr().(VariableAssign).getSource() = node1.asExpr() or From c9ea5ad9a345ca027d8c4f80f894cc2eb16df6ac Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Wed, 21 Aug 2019 09:43:13 +0200 Subject: [PATCH 2/4] C#/Java: Remove `cached` from wrapper predicate --- .../dataflow/internal/DataFlowPrivate.qll | 17 ++++++++--------- .../java/dataflow/internal/DataFlowUtil.qll | 1 - 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll index 10561b7c773e..b216795bd879 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll @@ -220,6 +220,14 @@ module LocalFlow { } } +/** + * Holds if data flows from `nodeFrom` to `nodeTo` in exactly one local + * (intra-procedural) step. + */ +predicate localFlowStepImpl(Node nodeFrom, Node nodeTo) { + simpleLocalFlowStep(nodeFrom, nodeTo) +} + /** A collection of cached types and predicates to be evaluated in the same stage. */ cached private module Cached { @@ -249,15 +257,6 @@ private module Cached { } or TMallocNode(ControlFlow::Nodes::ElementNode cfn) { cfn.getElement() instanceof ObjectCreation } - /** - * Holds if data flows from `nodeFrom` to `nodeTo` in exactly one local - * (intra-procedural) step. - */ - cached - predicate localFlowStepImpl(Node nodeFrom, Node nodeTo) { - simpleLocalFlowStep(nodeFrom, nodeTo) - } - /** * INTERNAL: do not use. * diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll index be8e8dc32f94..6303f3660730 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll @@ -350,7 +350,6 @@ predicate hasNonlocalValue(FieldRead fr) { /** * Holds if data can flow from `node1` to `node2` in one local step. */ -cached predicate localFlowStep(Node node1, Node node2) { simpleLocalFlowStep(node1, node2) } From ec2cc5a80e4432cf7f7ab10b1d47ad579ba16d13 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Wed, 21 Aug 2019 10:05:54 +0200 Subject: [PATCH 3/4] C#: Refactor how simpleLocalFlowStep is called `localFlowStep` is no longer an alias because it should not have the same QLDoc as `simpleLocalFlowStep`. --- .../code/csharp/dataflow/internal/DataFlowPrivate.qll | 10 ---------- .../code/csharp/dataflow/internal/DataFlowPublic.qll | 6 +++++- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll index b216795bd879..15c7b055d97b 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll @@ -220,14 +220,6 @@ module LocalFlow { } } -/** - * Holds if data flows from `nodeFrom` to `nodeTo` in exactly one local - * (intra-procedural) step. - */ -predicate localFlowStepImpl(Node nodeFrom, Node nodeTo) { - simpleLocalFlowStep(nodeFrom, nodeTo) -} - /** A collection of cached types and predicates to be evaluated in the same stage. */ cached private module Cached { @@ -258,8 +250,6 @@ private module Cached { TMallocNode(ControlFlow::Nodes::ElementNode cfn) { cfn.getElement() instanceof ObjectCreation } /** - * INTERNAL: do not use. - * * This is the local flow predicate that's used as a building block in global * data flow. It may have less flow than the `localFlowStep` predicate. */ diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll index dafb7a0c0300..559fc79026bb 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll @@ -147,7 +147,11 @@ ExprNode exprNode(DotNet::Expr e) { result.getExpr() = e } */ ParameterNode parameterNode(DotNet::Parameter p) { result.getParameter() = p } -predicate localFlowStep = localFlowStepImpl/2; +/** + * Holds if data flows from `nodeFrom` to `nodeTo` in exactly one local + * (intra-procedural) step. + */ +predicate localFlowStep(Node nodeFrom, Node nodeTo) { simpleLocalFlowStep(nodeFrom, nodeTo) } /** * Holds if data flows from `source` to `sink` in zero or more local From 6fc3a62edbbee40c9623f2367c8a9c04ab80bf80 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Wed, 21 Aug 2019 10:20:15 +0200 Subject: [PATCH 4/4] C++/C#/Java: Change another caller of localFlow There was also a use of `localFlowStep` in `DataFlowImplCommon` that should now be `simpleLocalFlowStep`. --- .../code/cpp/dataflow/internal/DataFlowImplCommon.qll | 6 +++--- .../code/cpp/ir/dataflow/internal/DataFlowImplCommon.qll | 6 +++--- .../code/csharp/dataflow/internal/DataFlowImplCommon.qll | 6 +++--- .../code/java/dataflow/internal/DataFlowImplCommon.qll | 6 +++--- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImplCommon.qll b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImplCommon.qll index 94623af2a6ac..9bb6a70c7425 100644 --- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImplCommon.qll +++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImplCommon.qll @@ -35,7 +35,7 @@ private module ImplCommon { or exists(Node mid | parameterValueFlowNoCtx(p, mid) and - localFlowStep(mid, node) and + simpleLocalFlowStep(mid, node) and compatibleTypes(p.getType(), node.getType()) ) or @@ -152,7 +152,7 @@ private module ImplCommon { or exists(Node mid | parameterValueFlow(p, mid, cc) and - localFlowStep(mid, node) and + simpleLocalFlowStep(mid, node) and compatibleTypes(p.getType(), node.getType()) ) or @@ -209,7 +209,7 @@ private module ImplCommon { * through a value-preserving method. */ private predicate localValueStep(Node node1, Node node2) { - localFlowStep(node1, node2) or + simpleLocalFlowStep(node1, node2) or argumentValueFlowsThrough(node1, node2, _) } diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImplCommon.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImplCommon.qll index 94623af2a6ac..9bb6a70c7425 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImplCommon.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImplCommon.qll @@ -35,7 +35,7 @@ private module ImplCommon { or exists(Node mid | parameterValueFlowNoCtx(p, mid) and - localFlowStep(mid, node) and + simpleLocalFlowStep(mid, node) and compatibleTypes(p.getType(), node.getType()) ) or @@ -152,7 +152,7 @@ private module ImplCommon { or exists(Node mid | parameterValueFlow(p, mid, cc) and - localFlowStep(mid, node) and + simpleLocalFlowStep(mid, node) and compatibleTypes(p.getType(), node.getType()) ) or @@ -209,7 +209,7 @@ private module ImplCommon { * through a value-preserving method. */ private predicate localValueStep(Node node1, Node node2) { - localFlowStep(node1, node2) or + simpleLocalFlowStep(node1, node2) or argumentValueFlowsThrough(node1, node2, _) } diff --git a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImplCommon.qll b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImplCommon.qll index 94623af2a6ac..9bb6a70c7425 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImplCommon.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImplCommon.qll @@ -35,7 +35,7 @@ private module ImplCommon { or exists(Node mid | parameterValueFlowNoCtx(p, mid) and - localFlowStep(mid, node) and + simpleLocalFlowStep(mid, node) and compatibleTypes(p.getType(), node.getType()) ) or @@ -152,7 +152,7 @@ private module ImplCommon { or exists(Node mid | parameterValueFlow(p, mid, cc) and - localFlowStep(mid, node) and + simpleLocalFlowStep(mid, node) and compatibleTypes(p.getType(), node.getType()) ) or @@ -209,7 +209,7 @@ private module ImplCommon { * through a value-preserving method. */ private predicate localValueStep(Node node1, Node node2) { - localFlowStep(node1, node2) or + simpleLocalFlowStep(node1, node2) or argumentValueFlowsThrough(node1, node2, _) } diff --git a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll index 94623af2a6ac..9bb6a70c7425 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll @@ -35,7 +35,7 @@ private module ImplCommon { or exists(Node mid | parameterValueFlowNoCtx(p, mid) and - localFlowStep(mid, node) and + simpleLocalFlowStep(mid, node) and compatibleTypes(p.getType(), node.getType()) ) or @@ -152,7 +152,7 @@ private module ImplCommon { or exists(Node mid | parameterValueFlow(p, mid, cc) and - localFlowStep(mid, node) and + simpleLocalFlowStep(mid, node) and compatibleTypes(p.getType(), node.getType()) ) or @@ -209,7 +209,7 @@ private module ImplCommon { * through a value-preserving method. */ private predicate localValueStep(Node node1, Node node2) { - localFlowStep(node1, node2) or + simpleLocalFlowStep(node1, node2) or argumentValueFlowsThrough(node1, node2, _) }