From b3af3ad12f7dad601e3db268ec29da3187ce4986 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Thu, 6 Feb 2020 19:06:40 +0100 Subject: [PATCH] Data flow: Fix bad join order in `getReturnPosition()` Joining on the enclosing callable before the kind is crucial, as witnessed by this pipeline: ``` [2020-02-06 17:58:21] (1086s) Starting to evaluate predicate DataFlowImplCommon::getReturnPosition#ff/2@83c546 [2020-02-06 18:53:16] (4382s) Tuple counts for DataFlowImplCommon::getReturnPosition#ff: 385478 ~1% {3} r1 = SCAN DataFlowImplCommon::Cached::TReturnPosition0#fff@staged_ext AS I OUTPUT I.<2>, I.<0>, I.<1> 385478 ~2% {3} r2 = JOIN r1 WITH DataFlowImplCommon::Cached::TReturnPosition0#fff_2#join_rhs AS R ON FIRST 1 OUTPUT r1.<2>, r1.<1>, r1.<0> 58638116860 ~0% {3} r3 = JOIN r2 WITH DataFlowImplCommon::ReturnNodeExt::getKind_dispred#ff_10#join_rhs AS R ON FIRST 1 OUTPUT R.<1>, r2.<1>, r2.<2> 914049 ~0% {2} r4 = JOIN r3 WITH DataFlowImplCommon::returnNodeGetEnclosingCallable#ff AS R ON FIRST 2 OUTPUT r3.<0>, r3.<2> return r4 ``` --- .../code/cpp/dataflow/internal/DataFlowImplCommon.qll | 8 +++++++- .../code/cpp/ir/dataflow/internal/DataFlowImplCommon.qll | 8 +++++++- .../code/csharp/dataflow/internal/DataFlowImplCommon.qll | 8 +++++++- .../code/java/dataflow/internal/DataFlowImplCommon.qll | 8 +++++++- 4 files changed, 28 insertions(+), 4 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 3a685110a6d..196c6108350 100644 --- a/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImplCommon.qll +++ b/cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImplCommon.qll @@ -753,9 +753,15 @@ private DataFlowCallable returnNodeGetEnclosingCallable(ReturnNodeExt ret) { result = ret.getEnclosingCallable() } +pragma[noinline] +private ReturnPosition getReturnPosition0(ReturnNodeExt ret, ReturnKindExt kind) { + result.getCallable() = returnNodeGetEnclosingCallable(ret) and + kind = result.getKind() +} + pragma[noinline] ReturnPosition getReturnPosition(ReturnNodeExt ret) { - result = TReturnPosition0(returnNodeGetEnclosingCallable(ret), ret.getKind()) + result = getReturnPosition0(ret, ret.getKind()) } bindingset[cc, callable] 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 3a685110a6d..196c6108350 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 @@ -753,9 +753,15 @@ private DataFlowCallable returnNodeGetEnclosingCallable(ReturnNodeExt ret) { result = ret.getEnclosingCallable() } +pragma[noinline] +private ReturnPosition getReturnPosition0(ReturnNodeExt ret, ReturnKindExt kind) { + result.getCallable() = returnNodeGetEnclosingCallable(ret) and + kind = result.getKind() +} + pragma[noinline] ReturnPosition getReturnPosition(ReturnNodeExt ret) { - result = TReturnPosition0(returnNodeGetEnclosingCallable(ret), ret.getKind()) + result = getReturnPosition0(ret, ret.getKind()) } bindingset[cc, callable] 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 3a685110a6d..196c6108350 100644 --- a/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImplCommon.qll +++ b/csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImplCommon.qll @@ -753,9 +753,15 @@ private DataFlowCallable returnNodeGetEnclosingCallable(ReturnNodeExt ret) { result = ret.getEnclosingCallable() } +pragma[noinline] +private ReturnPosition getReturnPosition0(ReturnNodeExt ret, ReturnKindExt kind) { + result.getCallable() = returnNodeGetEnclosingCallable(ret) and + kind = result.getKind() +} + pragma[noinline] ReturnPosition getReturnPosition(ReturnNodeExt ret) { - result = TReturnPosition0(returnNodeGetEnclosingCallable(ret), ret.getKind()) + result = getReturnPosition0(ret, ret.getKind()) } bindingset[cc, callable] 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 3a685110a6d..196c6108350 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll @@ -753,9 +753,15 @@ private DataFlowCallable returnNodeGetEnclosingCallable(ReturnNodeExt ret) { result = ret.getEnclosingCallable() } +pragma[noinline] +private ReturnPosition getReturnPosition0(ReturnNodeExt ret, ReturnKindExt kind) { + result.getCallable() = returnNodeGetEnclosingCallable(ret) and + kind = result.getKind() +} + pragma[noinline] ReturnPosition getReturnPosition(ReturnNodeExt ret) { - result = TReturnPosition0(returnNodeGetEnclosingCallable(ret), ret.getKind()) + result = getReturnPosition0(ret, ret.getKind()) } bindingset[cc, callable]