Skip to content

Commit 6db7e72

Browse files
committed
Python: Fix bad join in DataFlowDispatch
A case of bad magic. Rather than evaluating separately whether a class has a method of some name, the compiler opted to magick in the fact that this was done as part of the `findFunctionAccordingToMro` predicate. Hilarity ensued. However, _we_ know that magic really isn't needed in this case (the number of results is bounded by `Class.getAMethod` since methods have only a single name), so by factoring it out into a helper predicate, we can help the join-orderer along. Before ``` (377s) Starting to evaluate predicate _DataFlowDispatch::findFunctionAccordingToMro/2#a610c0a3#prev_DataFlowDispatch::getNextClassInMro/1#__#shared/3@i6#L3#f893bw2h (iteration 6) (377s) Tuple counts for _DataFlowDispatch::findFunctionAccordingToMro/2#a610c0a3#prev_DataFlowDispatch::getNextClassInMro/1#__#shared/3@i6#L3#f893bw2h after 16ms: 33363 ~0% {2} r1 = SCAN `DataFlowDispatch::getNextClassInMro/1#e1ee596a#prev_delta` OUTPUT In.1, In.0 'arg1' 159696 ~4% {3} | JOIN WITH `DataFlowDispatch::findFunctionAccordingToMro/2#a610c0a3#prev` ON FIRST 1 OUTPUT Rhs.1 'arg0', Lhs.1 'arg1', Rhs.2 'arg2' return r1 (377s) Starting to evaluate predicate _Class::Class.getAMethod/0#dispred#66416e47_Function::Function.getName/0#dispred#033700ef_10#join_rh__#antijoin_rhs/3@i6#L4#f893bw2h (iteration 6) (382s) Tuple counts for _Class::Class.getAMethod/0#dispred#66416e47_Function::Function.getName/0#dispred#033700ef_10#join_rh__#antijoin_rhs/3@i6#L4#f893bw2h after 4.4s: 1770825904 ~4% {4} r1 = JOIN `_DataFlowDispatch::findFunctionAccordingToMro/2#a610c0a3#prev_DataFlowDispatch::getNextClassInMro/1#__#shared` WITH `Function::Function.getName/0#dispred#033700ef_10#join_rhs` ON FIRST 1 OUTPUT Lhs.1 'arg0', Rhs.1, Lhs.0 'arg1', Lhs.2 'arg2' 34558 ~3% {3} | JOIN WITH `Class::Class.getAMethod/0#dispred#66416e47` ON FIRST 2 OUTPUT Lhs.0 'arg0', Lhs.2 'arg1', Lhs.3 'arg2' return r1 ... (382s) Starting to evaluate predicate DataFlowDispatch::findFunctionAccordingToMro/2#a610c0a3/3@i6#f893b1xh (iteration 6) (382s) - DataFlowDispatch::findFunctionAccordingToMro/2#a610c0a3_delta has 125138 rows (order for disjuncts: delta=<standard>). (382s) Tuple counts for DataFlowDispatch::findFunctionAccordingToMro/2#a610c0a3/3@i6#f893b1xh after 12ms: 33363 ~0% {2} r1 = SCAN `DataFlowDispatch::getNextClassInMro/1#e1ee596a#prev_delta` OUTPUT In.1, In.0 'cls' 159696 ~0% {3} | JOIN WITH `DataFlowDispatch::findFunctionAccordingToMro/2#a610c0a3#prev` ON FIRST 1 OUTPUT Lhs.1 'cls', Rhs.1 'name', Rhs.2 'result' 125138 ~1% {3} | AND NOT `_Class::Class.getAMethod/0#dispred#66416e47_Function::Function.getName/0#dispred#033700ef_10#join_rh__#antijoin_rhs`(FIRST 3) 0 ~0% {3} r2 = JOIN `DataFlowDispatch::findFunctionAccordingToMro/2#a610c0a3#prev_delta` WITH `DataFlowDispatch::getNextClassInMro/1#e1ee596a#reorder_1_0#prev` ON FIRST 1 OUTPUT Lhs.1 'name', Lhs.2 'result', Rhs.1 'cls' {3} | AND NOT `_Class::Class.getAMethod/0#dispred#66416e47_DataFlowDispatch::findFunctionAccordingToMro/2#a610c0a3#__#antijoin_rhs`(FIRST 3) 0 ~0% {3} | SCAN OUTPUT In.2 'cls', In.0 'name', In.1 'result' 125138 ~1% {3} r3 = r1 UNION r2 125138 ~1% {3} | AND NOT `DataFlowDispatch::findFunctionAccordingToMro/2#a610c0a3#prev`(FIRST 3) return r3 ``` And now ``` (18s) Tuple counts for DataFlowDispatch::class_has_method/2#0d2ae9c0/2@ff66c1lr after 18ms: 202279 ~1% {2} r1 = JOIN `Class::Class.getAMethod/0#dispred#66416e47_10#join_rhs` WITH `Function::Function.getName/0#dispred#033700ef` ON FIRST 1 OUTPUT Lhs.1 'cls', Rhs.1 'name' return r1 ... (490s) Tuple counts for DataFlowDispatch::findFunctionAccordingToMro/2#a610c0a3/3@i6#48b6c1xi after 54ms: 0 ~0% {3} r1 = JOIN `DataFlowDispatch::findFunctionAccordingToMro/2#a610c0a3#prev_delta` WITH `DataFlowDispatch::getNextClassInMro/1#e1ee596a#reorder_1_0#prev` ON FIRST 1 OUTPUT Rhs.1 'cls', Lhs.1 'name', Lhs.2 'result' 0 ~0% {3} | AND NOT `DataFlowDispatch::class_has_method/2#0d2ae9c0`(FIRST 2) 33363 ~0% {2} r2 = SCAN `DataFlowDispatch::getNextClassInMro/1#e1ee596a#prev_delta` OUTPUT In.1, In.0 'cls' 159696 ~0% {3} | JOIN WITH `DataFlowDispatch::findFunctionAccordingToMro/2#a610c0a3#prev` ON FIRST 1 OUTPUT Lhs.1 'cls', Rhs.1 'name', Rhs.2 'result' 125138 ~1% {3} | AND NOT `DataFlowDispatch::class_has_method/2#0d2ae9c0`(FIRST 2) 125138 ~1% {3} r3 = r1 UNION r2 125138 ~1% {3} | AND NOT `DataFlowDispatch::findFunctionAccordingToMro/2#a610c0a3#prev`(FIRST 3) return r3 ```
1 parent 988d067 commit 6db7e72

File tree

1 file changed

+8
-2
lines changed

1 file changed

+8
-2
lines changed

python/ql/lib/semmle/python/dataflow/new/internal/DataFlowDispatch.qll

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -829,10 +829,16 @@ Function findFunctionAccordingToMro(Class cls, string name) {
829829
result = cls.getAMethod() and
830830
result.getName() = name
831831
or
832-
not cls.getAMethod().getName() = name and
832+
not class_has_method(cls, name) and
833833
result = findFunctionAccordingToMro(getNextClassInMro(cls), name)
834834
}
835835

836+
/**
837+
* Join-order helper for `findFunctionAccordingToMro` and `findFunctionAccordingToMroKnownStartingClass`.
838+
*/
839+
pragma[nomagic]
840+
private predicate class_has_method(Class cls, string name) { cls.getAMethod().getName() = name }
841+
836842
/**
837843
* Gets a class that, from an approximated MRO calculation, might be the next class
838844
* after `cls` in the MRO for `startingClass`.
@@ -860,7 +866,7 @@ private Function findFunctionAccordingToMroKnownStartingClass(
860866
result.getName() = name and
861867
cls = getADirectSuperclass*(startingClass)
862868
or
863-
not cls.getAMethod().getName() = name and
869+
not class_has_method(cls, name) and
864870
result =
865871
findFunctionAccordingToMroKnownStartingClass(getNextClassInMroKnownStartingClass(cls,
866872
startingClass), startingClass, name)

0 commit comments

Comments
 (0)