From f2fd1c7931c1cd660428774a706e318dbea8175e Mon Sep 17 00:00:00 2001 From: Arthur Baars Date: Tue, 15 Dec 2020 19:07:15 +0100 Subject: [PATCH] CFG: make def nodes visible --- .../internal/ControlFlowGraphImpl.qll | 24 +- .../controlflow/graph/Cfg.expected | 319 +++++++++++++++++- 2 files changed, 304 insertions(+), 39 deletions(-) diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll index 1d3dd316e8c..00b37dbabe8 100644 --- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -150,14 +150,7 @@ abstract private class StandardNode extends ControlFlowTree { abstract AstNode getChildNode(int i); private AstNode getChildNodeRanked(int i) { - result = - rank[i + 1](AstNode child, int j | - child = this.getChildNode(j) and - // Never descend into children with a separate scope - not child instanceof CfgScope - | - child order by j - ) + result = rank[i + 1](AstNode child, int j | child = this.getChildNode(j) | child order by j) } /** Gets the first child node of this element. */ @@ -301,8 +294,6 @@ module Trees { or result = this.getChild(i - 1) } - - override predicate isHidden() { any() } } private class BlockArgumentTree extends StandardPostOrderTree, BlockArgument { @@ -447,8 +438,6 @@ module Trees { or result = this.getChild(i - 1) and rescuable = true } - - override predicate isHidden() { any() } } private class ElementReferenceTree extends StandardPostOrderTree, ElementReference { @@ -695,8 +684,6 @@ module Trees { or result = this.getBody() and i = 1 } - - override predicate isHidden() { any() } } private class LambdaParametersTree extends StandardPreOrderTree, LambdaParameters { @@ -783,8 +770,6 @@ module Trees { succ = this and c instanceof NormalCompletion } - - override predicate isHidden() { any() } } private class MethodParametersTree extends StandardPreOrderTree, MethodParameters { @@ -952,8 +937,7 @@ module Trees { child = this.getChildNode(j, _) and not result instanceof Rescue and not result instanceof Ensure and - not result instanceof Else and - not child instanceof CfgScope + not result instanceof Else | child order by j ) @@ -1219,8 +1203,6 @@ module Trees { } final override predicate last(AstNode last, Completion c) { this.lastBody(last, c) } - - override predicate isHidden() { any() } } private class SingletonMethodTree extends RescueEnsureBlockTree, PostOrderTree, SingletonMethod { @@ -1246,8 +1228,6 @@ module Trees { succ = this and c instanceof NormalCompletion } - - override predicate isHidden() { any() } } private class SplatArgumentTree extends StandardPostOrderTree, SplatArgument { diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected index 9aecfae2e99..058b8f95af2 100644 --- a/ql/test/library-tests/controlflow/graph/Cfg.expected +++ b/ql/test/library-tests/controlflow/graph/Cfg.expected @@ -1,4 +1,7 @@ break_ensure.rb: +# 1| enter top-level +#-----| -> m1 + # 1| enter m1 #-----| -> elements @@ -12,12 +15,15 @@ break_ensure.rb: #-----| -> elements case.rb: +# 1| enter top-level +#-----| -> if_in_case + # 1| enter if_in_case #-----| -> Case cfg.rb: # 1| enter top-level -#-----| -> Alias +#-----| -> bar # 15| enter BEGIN block #-----| -> BeginBlock @@ -62,6 +68,9 @@ cfg.rb: #-----| -> x exit.rb: +# 1| enter top-level +#-----| -> m1 + # 1| enter m1 #-----| -> x @@ -69,12 +78,15 @@ exit.rb: #-----| -> x heredoc.rb: +# 1| enter top-level +#-----| -> double_heredoc + # 1| enter double_heredoc #-----| -> < 1 +#-----| -> m1 # 1| enter m1 #-----| -> x @@ -95,6 +107,9 @@ ifs.rb: #-----| -> String loops.rb: +# 1| enter top-level +#-----| -> m1 + # 1| enter m1 #-----| -> x @@ -151,6 +166,12 @@ raise.rb: #-----| -> Ensure break_ensure.rb: +# 1| Method +#-----| -> m2 + +# 1| m1 +#-----| -> Method + # 1| elements #-----| -> elements @@ -205,6 +226,12 @@ break_ensure.rb: # 9| String #-----| -> puts +# 13| Method +#-----| -> m3 + +# 13| m2 +#-----| -> Method + # 13| elements #-----| -> elements @@ -284,6 +311,12 @@ break_ensure.rb: # 21| [ensure: break] String #-----| -> [ensure: break] puts +# 27| Method +#-----| -> m4 + +# 27| m3 +#-----| -> Method + # 27| elements #-----| -> elements @@ -370,6 +403,12 @@ break_ensure.rb: # 41| String #-----| -> puts +# 44| Method +#-----| -> exit top-level (normal) + +# 44| m4 +#-----| -> Method + # 44| elements #-----| -> elements @@ -450,6 +489,12 @@ break_ensure.rb: #-----| -> [ensure: raise] Break case.rb: +# 1| Method +#-----| -> exit top-level (normal) + +# 1| if_in_case +#-----| -> Method + # 2| Case #-----| -> x1 @@ -499,6 +544,12 @@ case.rb: #-----| -> puts cfg.rb: +# 1| Method +#-----| -> Alias + +# 1| bar +#-----| -> Method + # 3| Alias #-----| -> foo @@ -548,7 +599,7 @@ cfg.rb: #-----| -> StringArray # 12| Call -#-----| -> 41 +#-----| -> BeginBlock # 12| puts #-----| -> Call @@ -560,6 +611,7 @@ cfg.rb: #-----| -> String # 16| Call +#-----| -> EndBlock #-----| -> exit BEGIN block (normal) # 16| puts @@ -572,6 +624,7 @@ cfg.rb: #-----| -> String # 20| Call +#-----| -> 41 #-----| -> exit END block (normal) # 20| puts @@ -871,7 +924,7 @@ cfg.rb: #-----| -> C # 62| Assignment -#-----| -> 1 +#-----| -> pattern # 62| DestructuredLeftAssignment #-----| -> Assignment @@ -903,6 +956,12 @@ cfg.rb: # 62| 3 #-----| -> Array +# 63| Method +#-----| -> 1 + +# 63| pattern +#-----| -> Method + # 63| DestructuredParameter #-----| -> a @@ -949,7 +1008,7 @@ cfg.rb: #-----| -> Array # 68| Call -#-----| -> 42 +#-----| -> print # 68| puts #-----| -> Call @@ -963,6 +1022,12 @@ cfg.rb: # 68| 2 #-----| -> ElementReference +# 69| Method +#-----| -> 42 + +# 69| print +#-----| -> Method + # 70| Call #-----| -> exit print (normal) @@ -1136,7 +1201,7 @@ cfg.rb: #-----| -> Pair # 98| Assignment -#-----| -> String +#-----| -> parameters # 98| map2 #-----| -> Assignment @@ -1165,6 +1230,12 @@ cfg.rb: # 98| map1 #-----| -> HashSplatArgument +# 101| Method +#-----| -> String + +# 101| parameters +#-----| -> Method + # 101| OptionalParameter #-----| no-match -> 42 #-----| match -> KeywordParameter @@ -1282,7 +1353,7 @@ cfg.rb: #-----| -> @field # 117| Assignment -#-----| -> swap +#-----| -> Lambda # 117| @@static_field #-----| -> Assignment @@ -1296,8 +1367,11 @@ cfg.rb: # 120| swap #-----| -> Assignment +# 120| Lambda +#-----| -> swap + # 120| DestructuredParameter -#-----| -> exit lambda (normal) +#-----| -> Block # 120| x #-----| -> y @@ -1305,6 +1379,9 @@ cfg.rb: # 120| y #-----| -> DestructuredParameter +# 120| Block +#-----| -> exit lambda (normal) + # 120| Array #-----| -> exit block (normal) @@ -1487,7 +1564,7 @@ cfg.rb: #-----| -> ScopeResolution # 138| ScopeResolution -#-----| -> Silly +#-----| -> SingletonClass # 138| Call #-----| -> Constant @@ -1501,15 +1578,30 @@ cfg.rb: # 138| Constant #-----| -> ScopeResolution -# 140| Call +# 140| SingletonClass #-----| -> Silly +# 140| Call +#-----| -> Setter + # 140| Silly #-----| -> itself # 140| itself #-----| -> Call +# 141| Method +#-----| -> print + +# 141| Setter +#-----| -> Method + +# 142| Method +#-----| -> Silly + +# 142| print +#-----| -> Method + # 143| Call #-----| -> super @@ -1535,7 +1627,7 @@ cfg.rb: #-----| -> Call # 148| Assignment -#-----| -> 1 +#-----| -> silly # 148| silly #-----| -> Assignment @@ -1549,6 +1641,15 @@ cfg.rb: # 148| new #-----| -> Call +# 149| SingletonMethod +#-----| -> two_parameters + +# 149| silly +#-----| -> method + +# 149| method +#-----| -> SingletonMethod + # 149| SplatParameter #-----| -> x @@ -1561,6 +1662,12 @@ cfg.rb: # 150| x #-----| -> puts +# 153| Method +#-----| -> 1 + +# 153| two_parameters +#-----| -> Method + # 153| a #-----| -> b @@ -1865,6 +1972,12 @@ cfg.rb: # 183| 0 #-----| -> Binary +# 185| Method +#-----| -> run_block + +# 185| run_block +#-----| -> Method + # 186| Yield #-----| -> 42 @@ -1889,6 +2002,12 @@ cfg.rb: #-----| -> puts exit.rb: +# 1| Method +#-----| -> m2 + +# 1| m1 +#-----| -> Method + # 1| x #-----| -> x @@ -1923,6 +2042,12 @@ exit.rb: # 5| String #-----| -> puts +# 8| Method +#-----| -> exit top-level (normal) + +# 8| m2 +#-----| -> Method + # 8| x #-----| -> x @@ -1958,11 +2083,17 @@ exit.rb: #-----| -> puts heredoc.rb: -# 2| MethodCall +# 1| Method +#-----| -> exit top-level (normal) + +# 1| double_heredoc +#-----| -> Method + +# 2| Call #-----| -> exit double_heredoc (normal) # 2| puts -#-----| -> MethodCall +#-----| -> Call # 2| < HeredocBody @@ -1977,6 +2108,12 @@ heredoc.rb: #-----| -> puts ifs.rb: +# 1| Method +#-----| -> m2 + +# 1| m1 +#-----| -> Method + # 1| x #-----| -> x @@ -2077,6 +2214,12 @@ ifs.rb: # 7| String #-----| -> puts +# 11| Method +#-----| -> m3 + +# 11| m2 +#-----| -> Method + # 11| b #-----| -> b @@ -2099,6 +2242,12 @@ ifs.rb: # 15| 1 #-----| -> Return +# 18| Method +#-----| -> m4 + +# 18| m3 +#-----| -> Method + # 18| x #-----| -> x @@ -2164,6 +2313,12 @@ ifs.rb: # 25| x #-----| -> puts +# 28| Method +#-----| -> m5 + +# 28| m4 +#-----| -> Method + # 28| b1 #-----| -> b2 @@ -2209,6 +2364,12 @@ ifs.rb: # 29| String #-----| -> Conditional +# 32| Method +#-----| -> 1 + +# 32| m5 +#-----| -> Method + # 32| b1 #-----| -> b2 @@ -2274,14 +2435,17 @@ ifs.rb: # 36| UnlessModifier #-----| -> exit top-level (normal) -# 36| conditional_method_def +# 36| Method #-----| -> UnlessModifier -# 37| MethodCall +# 36| conditional_method_def +#-----| -> Method + +# 37| Call #-----| -> exit conditional_method_def (normal) # 37| puts -#-----| -> MethodCall +#-----| -> Call # 37| String #-----| -> puts @@ -2297,6 +2461,12 @@ ifs.rb: #-----| -> Binary loops.rb: +# 1| Method +#-----| -> m2 + +# 1| m1 +#-----| -> Method + # 1| x #-----| -> While @@ -2331,6 +2501,12 @@ loops.rb: # 4| 1 #-----| -> OperatorAssignment +# 8| Method +#-----| -> m3 + +# 8| m2 +#-----| -> Method + # 8| x #-----| -> While @@ -2431,6 +2607,12 @@ loops.rb: # 21| String #-----| -> puts +# 24| Method +#-----| -> exit top-level (normal) + +# 24| m3 +#-----| -> Method + # 25| Call #-----| -> exit m3 (normal) @@ -2481,11 +2663,17 @@ raise.rb: #-----| -> Exception # 4| Superclass -#-----| -> exit top-level (normal) +#-----| -> m1 # 4| Exception #-----| -> Superclass +# 7| Method +#-----| -> m2 + +# 7| m1 +#-----| -> Method + # 7| x #-----| -> x @@ -2520,6 +2708,12 @@ raise.rb: # 11| String #-----| -> puts +# 14| Method +#-----| -> m3 + +# 14| m2 +#-----| -> Method + # 14| b #-----| -> b @@ -2564,6 +2758,12 @@ raise.rb: # 22| String #-----| -> puts +# 25| Method +#-----| -> m4 + +# 25| m3 +#-----| -> Method + # 25| b #-----| -> b @@ -2604,6 +2804,12 @@ raise.rb: # 33| String #-----| -> puts +# 36| Method +#-----| -> m5 + +# 36| m4 +#-----| -> Method + # 36| b #-----| -> b @@ -2647,6 +2853,12 @@ raise.rb: # 44| String #-----| -> puts +# 47| Method +#-----| -> m6 + +# 47| m5 +#-----| -> Method + # 47| b #-----| -> b @@ -2681,6 +2893,12 @@ raise.rb: # 54| String #-----| -> puts +# 57| Method +#-----| -> m7 + +# 57| m6 +#-----| -> Method + # 57| b #-----| -> b @@ -2732,6 +2950,12 @@ raise.rb: # 65| String #-----| -> puts +# 68| Method +#-----| -> m8 + +# 68| m7 +#-----| -> Method + # 68| x #-----| -> x @@ -2821,6 +3045,12 @@ raise.rb: # 76| [ensure: raise] String #-----| -> [ensure: raise] puts +# 79| Method +#-----| -> m9 + +# 79| m8 +#-----| -> Method + # 79| x #-----| -> String @@ -2928,6 +3158,12 @@ raise.rb: # 91| String #-----| -> puts +# 94| Method +#-----| -> m10 + +# 94| m9 +#-----| -> Method + # 94| x #-----| -> b1 @@ -3245,6 +3481,12 @@ raise.rb: # 117| [ensure: raise] String #-----| -> [ensure: raise] raise +# 121| Method +#-----| -> m11 + +# 121| m10 +#-----| -> Method + # 121| OptionalParameter #-----| no-match -> String #-----| match -> Ensure @@ -3270,6 +3512,12 @@ raise.rb: # 125| String #-----| -> puts +# 128| Method +#-----| -> m12 + +# 128| m11 +#-----| -> Method + # 128| b #-----| -> b @@ -3345,6 +3593,12 @@ raise.rb: # 139| String #-----| -> puts +# 142| Method +#-----| -> m13 + +# 142| m12 +#-----| -> Method + # 142| b #-----| -> b @@ -3382,10 +3636,18 @@ raise.rb: # 147| [ensure: raise] 3 #-----| -> [ensure: raise] Return +# 150| Method +#-----| -> exit top-level (normal) + +# 150| m13 +#-----| -> Method + # 151| Ensure #-----| -> exit m13 (normal) break_ensure.rb: +# 1| exit top-level + # 1| exit m1 # 13| exit m2 @@ -3395,6 +3657,8 @@ break_ensure.rb: # 44| exit m4 case.rb: +# 1| exit top-level + # 1| exit if_in_case cfg.rb: @@ -3427,11 +3691,15 @@ cfg.rb: # 189| exit block exit.rb: +# 1| exit top-level + # 1| exit m1 # 8| exit m2 heredoc.rb: +# 1| exit top-level + # 1| exit double_heredoc ifs.rb: @@ -3450,6 +3718,8 @@ ifs.rb: # 36| exit conditional_method_def loops.rb: +# 1| exit top-level + # 1| exit m1 # 8| exit m2 @@ -3488,6 +3758,9 @@ raise.rb: # 150| exit m13 break_ensure.rb: +# 1| exit top-level (normal) +#-----| -> exit top-level + # 1| exit m1 (normal) #-----| -> exit m1 @@ -3504,6 +3777,9 @@ break_ensure.rb: #-----| -> exit m4 case.rb: +# 1| exit top-level (normal) +#-----| -> exit top-level + # 1| exit if_in_case (normal) #-----| -> exit if_in_case @@ -3551,6 +3827,9 @@ cfg.rb: #-----| -> exit block exit.rb: +# 1| exit top-level (normal) +#-----| -> exit top-level + # 1| exit m1 (abnormal) #-----| -> exit m1 @@ -3564,6 +3843,9 @@ exit.rb: #-----| -> exit m2 heredoc.rb: +# 1| exit top-level (normal) +#-----| -> exit top-level + # 1| exit double_heredoc (normal) #-----| -> exit double_heredoc @@ -3590,6 +3872,9 @@ ifs.rb: #-----| -> exit conditional_method_def loops.rb: +# 1| exit top-level (normal) +#-----| -> exit top-level + # 1| exit m1 (normal) #-----| -> exit m1