Skip to content

Data flow: Add precise call contexts to stage 2#6586

Merged
hvitved merged 5 commits intogithub:mainfrom
hvitved:dataflow/stage2-precise-call-ctx-take2
Sep 10, 2021
Merged

Data flow: Add precise call contexts to stage 2#6586
hvitved merged 5 commits intogithub:mainfrom
hvitved:dataflow/stage2-precise-call-ctx-take2

Conversation

@hvitved
Copy link
Copy Markdown
Contributor

@hvitved hvitved commented Sep 1, 2021

The motivation for this PR is to revert the temporary virtual dispatch cap put in place for C#. The change mostly makes a difference for DBs with huge virtual dispatch fan-out. The timings below are for a proprietary C# database.

Before this PR:
	XSS.ql-33:DataFlowImpl2::additionalJumpStep#fff#shared ....................................................................................... 1m10s
	XSS.ql-33:DataFlowPrivate::NodeImpl::getTypeImpl_dispred#ff_10#join_rhs ...................................................................... 54.6s
	XSS.ql-33:DataFlowImpl2::Stage1::fwdFlow#2#fff ............................................................................................... 54.3s (1756 evaluations with max 1.4s in DataFlowImpl2::Stage1::fwdFlow#2#fff/3@i95#d1de60)
	XSS.ql-33:DataFlowImpl2::viableParamArgEx#fff ................................................................................................ 50.1s
	XSS.ql-33:num#DataFlowImpl2::TNodeNormal#ff .................................................................................................. 44.1s
	XSS.ql-33:num#DataFlowImpl2::TNil#ff ......................................................................................................... 43.9s
	XSS.ql-33:num#DataFlowImpl2::TSummaryCtxNone#f ............................................................................................... 41.1s
	XSS.ql-33:locations_default_102345#join_rhs .................................................................................................. 37.9s
	XSS.ql-33:DataFlowImpl2::localFlowStep#fff ................................................................................................... 35.4s
	XSS.ql-33:#Type::ValueOrRefType::getBaseClass_dispredPlus#ff ................................................................................. 33.1s
	XSS.ql-33:Location::Location::hasLocationInfo_dispred#ffffff ................................................................................. 32s
	XSS.ql-33:DataFlowImpl2::jumpStep#fff#shared ................................................................................................. 27.5s
	XSS.ql-33:DataFlowImpl2::NodeEx::getDataFlowType0#ff ......................................................................................... 25.8s
	XSS.ql-33:project#DataFlowImpl2::viableParamArgEx#fff ........................................................................................ 24.7s
	XSS.ql-33:DataFlowImpl2::NodeEx::getEnclosingCallable0#ff .................................................................................... 24.2s
	XSS.ql-33:ControlFlowGraph::ControlFlow::Node::getEnclosingCallable_dispred#ff ............................................................... 23.2s
	XSS.ql-33:DataFlowImpl2::Stage1::revFlow0#fff ................................................................................................ 22.2s (797 evaluations with max 385ms in DataFlowImpl2::Stage1::revFlow0#fff/3@i243#09cb13)
	XSS.ql-33:XSSSinks::HtmlSinkSink#class#f ..................................................................................................... 21.6s
	XSS.ql-33:DataFlowPublic::Content::toString_dispred#ff ....................................................................................... 18.9s
	XSS.ql-33:m#Assignable::nameOfChild#fb ....................................................................................................... 16.9s (240 evaluations with max 7.5s in m#Assignable::nameOfChild#fb/1@i3#e5ba6w)
	XSS.ql-33:DataFlowImplCommon::Cached::viableParamArg#fff_102#join_rhs ........................................................................ 16.3s
First commit of this PR (revert https://github.com//pull/6394):
	XSS.ql-33:DataFlowImpl2::Stage3::fwdFlowIn#fffffff ........................................ 192m53s (366 evaluations with max 13m37s in DataFlowImpl2::Stage3::fwdFlowIn#fffffff/7@i174#0675c0)
	XSS.ql-33:DataFlowImpl2::Stage3::fwdFlow0#fffff ........................................... 46m40s (369 evaluations with max 2m45s in DataFlowImpl2::Stage3::fwdFlow0#fffff/5@i181#0675c4)
	XSS.ql-33:DataFlowImpl2::Stage3::fwdFlowOutFromArg#fffff#reorder_0_2_4_1_3 ................ 35m46s (365 evaluations with max 2m11s in DataFlowImpl2::Stage3::fwdFlowOutFromArg#fffff#reorder_0_2_4_1_3/5@i174#0675cy)
	XSS.ql-33:DataFlowImpl2::Stage3::fwdFlow#fffff ............................................ 33m23s (367 evaluations with max 40.5s in DataFlowImpl2::Stage3::fwdFlow#fffff/5@i168#0675c3)
	XSS.ql-33:DataFlowImpl2::Stage3::fwdFlowIsEntered#fffff#reorder_0_3_4_1_2 ................. 16m28s (360 evaluations with max 1m16s in DataFlowImpl2::Stage3::fwdFlowIsEntered#fffff#reorder_0_3_4_1_2/5@i181#0675cx)
	XSS.ql-33:DataFlowImpl2::Stage3::revFlowInToReturn#fffff#reorder_0_2_4_1_3 ................ 9m23s (362 evaluations with max 52.8s in DataFlowImpl2::Stage3::revFlowInToReturn#fffff#reorder_0_2_4_1_3/5@i28#6b414z)
	XSS.ql-33:DataFlowImpl2::Stage3::callMayFlowThroughFwd#ff ................................. 3m53s
	XSS.ql-33:DataFlowImpl2::Stage4::fwdFlowIn#fffffff ........................................ 3m32s (403 evaluations with max 29.8s in DataFlowImpl2::Stage4::fwdFlowIn#fffffff/7@i113#b7163z)
	XSS.ql-33:DataFlowImpl2::Stage3::revFlow0#fffff ........................................... 2m49s (380 evaluations with max 6.4s in DataFlowImpl2::Stage3::revFlow0#fffff/5@i19#6b4143)
	XSS.ql-33:DataFlowImpl2::Stage3::fwdFlowOutNotFromArg#fffff ............................... 2m44s (364 evaluations with max 1.9s in DataFlowImpl2::Stage3::fwdFlowOutNotFromArg#fffff/5@i136#0675c2)
	XSS.ql-33:DataFlowImpl2::Stage3::fwdFlowStore#fffffff ..................................... 2m35s (366 evaluations with max 8.2s in DataFlowImpl2::Stage3::fwdFlowStore#fffffff/7@i205#0675c1)
	XSS.ql-33:DataFlowImpl2::viableParamArgEx#fff ............................................. 2m18s
	XSS.ql-33:DataFlowImpl2::viableReturnPosOutEx#fff ......................................... 1m59s
	XSS.ql-33:DataFlowImpl2::Stage1::fwdFlow#2#fff ............................................ 1m37s (1753 evaluations with max 2.9s in DataFlowImpl2::Stage1::fwdFlow#2#fff/3@i30#d93cc0)
	XSS.ql-33:DataFlowImpl2::Stage3::fwdFlowRead#fffffff#reorder_0_1_6_2_3_4_5#join_rhs ....... 1m34s
	XSS.ql-33:DataFlowImpl2::viableReturnPosOutEx#fff_102#join_rhs ............................ 1m21s
	XSS.ql-33:DataFlowImpl2::Stage3::fwdFlowRead#fffffff#reorder_0_1_6_2_3_4_5 ................ 1m17s (363 evaluations with max 5.3s in DataFlowImpl2::Stage3::fwdFlowRead#fffffff#reorder_0_1_6_2_3_4_5/7@i137#0675cz)
	XSS.ql-33:num#DataFlowImpl2::TNodeNormal#ff ............................................... 1m8s
	XSS.ql-33:DataFlowImpl2::Stage3::revFlowOut#ffffff ........................................ 57.7s (361 evaluations with max 7.7s in DataFlowImpl2::Stage3::revFlowOut#ffffff/6@i18#6b4140)
	XSS.ql-33:DataFlowImpl2::Stage1::fwdFlowOut#ffff .......................................... 52.4s (1735 evaluations with max 3.3s in DataFlowImpl2::Stage1::fwdFlowOut#ffff/4@i23#d93cc2)
	XSS.ql-33:project#DataFlowImpl2::viableParamArgEx#fff ..................................... 48.4s
	XSS.ql-33:DataFlowImplCommon::Cached::viableParamArg#fff_102#join_rhs ..................... 46.2s
This PR:
	XSS.ql-33:DataFlowImpl2::Stage3::fwdFlowIn#fffffff .................................. 3m31s (429 evaluations with max 13.6s in DataFlowImpl2::Stage3::fwdFlowIn#fffffff/7@i148#a26e80)
	XSS.ql-33:DataFlowImpl2::viableParamArgEx#fff ....................................... 3m6s
	XSS.ql-33:DataFlowImpl2::additionalJumpStep#fff#shared .............................. 2m33s
	XSS.ql-33:DataFlowImpl2::viableReturnPosOutEx#fff ................................... 2m32s
	XSS.ql-33:DataFlowImpl2::viableReturnPosOutEx#fff_102#join_rhs ...................... 2m29s
	XSS.ql-33:DataFlowImpl2::Stage3::fwdFlow0#fffff ..................................... 2m7s (439 evaluations with max 6.6s in DataFlowImpl2::Stage3::fwdFlow0#fffff/5@i149#a26e84)
	XSS.ql-33:DataFlowImplCommon::Cached::nodeEnclosingCallable#ff ...................... 1m59s
	XSS.ql-33:DataFlowImplCommon::viableCallableExt#ff_10#join_rhs ...................... 1m33s
	XSS.ql-33:DataFlowImpl2::Stage1::fwdFlow#2#fff ...................................... 1m33s (1753 evaluations with max 3s in DataFlowImpl2::Stage1::fwdFlow#2#fff/3@i30#d93cc0)
	XSS.ql-33:project#DataFlowImpl2::viableParamArgEx#fff ............................... 1m22s
	XSS.ql-33:DataFlowImpl2::Stage3::fwdFlow#fffff ...................................... 1m10s (436 evaluations with max 3.2s in DataFlowImpl2::Stage3::fwdFlow#fffff/5@i130#a26e83)
	XSS.ql-33:DataFlowPrivate::NodeImpl::getTypeImpl_dispred#ff_10#join_rhs ............. 1m9s
	XSS.ql-33:DataFlowImpl2::Stage1::fwdFlowOut#ffff .................................... 1m8s (1735 evaluations with max 5.3s in DataFlowImpl2::Stage1::fwdFlowOut#ffff/4@i23#d93cc2)
	XSS.ql-33:num#DataFlowImpl2::TNodeNormal#ff ......................................... 1m1s
	XSS.ql-33:DataFlowImpl2::Stage2::fwdFlowIn#fffffff .................................. 58.3s (1502 evaluations with max 2s in DataFlowImpl2::Stage2::fwdFlowIn#fffffff/7@i71#63d620)
	XSS.ql-33:DataFlowImplCommon::Cached::viableParamArg#fff_102#join_rhs ............... 57.6s
	XSS.ql-33:DataFlowImpl2::Stage3::fwdFlowIsEntered#fffff#reorder_0_3_4_1_2 ........... 56.6s (412 evaluations with max 3.4s in DataFlowImpl2::Stage3::fwdFlowIsEntered#fffff#reorder_0_3_4_1_2/5@i149#a26e8x)
	XSS.ql-33:#Type::ValueOrRefType::getBaseClass_dispredPlus#ff ........................ 54.2s
	XSS.ql-33:DataFlowImpl2::Stage3::fwdFlowOutFromArg#fffff#reorder_0_2_4_1_3 .......... 52.8s (432 evaluations with max 1.6s in DataFlowImpl2::Stage3::fwdFlowOutFromArg#fffff#reorder_0_2_4_1_3/5@i146#a26e8y)
	XSS.ql-33:DataFlowImpl2::Stage3::fwdFlowRead#fffffff#reorder_0_1_6_2_3_4_5#join_rhs . 51.6s

https://jenkins.internal.semmle.com/job/Changes/job/CSharp-Differences/1299/ (first commit against base)
https://jenkins.internal.semmle.com/job/Changes/job/CSharp-Differences/1312/ (PR against first commit)
https://jenkins.internal.semmle.com/job/Changes/job/CPP-Differences/2235/
https://jenkins.internal.semmle.com/job/Changes/job/Java-Differences/1579/
https://jenkins.internal.semmle.com/job/Changes/job/Python-Differences/682/

Before:
```
[2021-08-25 09:56:29] (1395s) Tuple counts for DataFlowImpl2::Stage3::callMayFlowThroughFwd#ff/2@111fb3:
                      15495496   ~5%         {5} r1 = SCAN DataFlowImpl2::Stage3::fwdFlowOutFromArg#fffff#reorder_0_2_4_1_3 OUTPUT In.3, In.4, In.2 'config', In.0 'call', In.1
                      1450611958 ~6335%      {5} r2 = JOIN r1 WITH DataFlowImpl2::Stage3::fwdFlow#fffff_03412#join_rhs ON FIRST 3 OUTPUT Lhs.3 'call', Lhs.4, Lhs.2 'config', Rhs.3, Rhs.4
                      7043648    ~20415%     {2} r3 = JOIN r2 WITH DataFlowImpl2::Stage3::fwdFlowIsEntered#fffff#reorder_0_3_4_1_2 ON FIRST 5 OUTPUT Lhs.0 'call', Lhs.2 'config'
                                             return r3
```

After:
```
[2021-08-25 10:57:02] (2652s) Tuple counts for DataFlowImpl2::Stage3::callMayFlowThroughFwd#ff/2@d3e27b:
                      15495496 ~0%         {6} r1 = SCAN DataFlowImpl2::Stage3::fwdFlowOutFromArg#fffff#reorder_0_2_4_1_3 OUTPUT In.0 'call', In.1, In.2 'config', In.3, In.4, In.2 'config'
                      9236888  ~22%        {7} r2 = JOIN r1 WITH DataFlowImpl2::Stage3::fwdFlowIsEntered#fffff#reorder_0_3_4_1_2 ON FIRST 3 OUTPUT Lhs.3, Rhs.3, Rhs.4, Lhs.4, Lhs.5, Lhs.0 'call', Lhs.2 'config'
                      7043648  ~20415%     {2} r3 = JOIN r2 WITH DataFlowImpl2::Stage3::fwdFlow#fffff ON FIRST 5 OUTPUT Lhs.5 'call', Lhs.6 'config'
                                           return r3
```
@hvitved hvitved marked this pull request as ready for review September 6, 2021 13:51
@hvitved hvitved requested review from a team as code owners September 6, 2021 13:51
tamasvajk
tamasvajk previously approved these changes Sep 9, 2021
Copy link
Copy Markdown
Contributor

@tamasvajk tamasvajk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
This is a low confidence approval, you might want to wait for another approval before merging.

Copy link
Copy Markdown
Contributor

@aschackmull aschackmull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The join-order fix looks broken to me.

@hvitved hvitved merged commit 649c2ce into github:main Sep 10, 2021
@hvitved hvitved deleted the dataflow/stage2-precise-call-ctx-take2 branch September 10, 2021 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants