From 569063ca73ce622d77d0af6ee07eaace8e33a6c3 Mon Sep 17 00:00:00 2001 From: Nick Rolfe Date: Wed, 5 May 2021 17:14:32 +0100 Subject: [PATCH 01/18] Make YieldCallTree post-order --- .../controlflow/internal/ControlFlowGraphImpl.qll | 3 +-- ql/test/library-tests/controlflow/graph/Cfg.expected | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll index bd5e91670fd..77664806c7c 100644 --- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -1293,8 +1293,7 @@ module Trees { } } - // TODO: make post-order - private class YieldCallTree extends StandardPreOrderTree, YieldCall { + private class YieldCallTree extends StandardPostOrderTree, YieldCall { final override ControlFlowTree getChildNode(int i) { result = this.getArgument(i) } } } diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected index 3e39856c559..025f50e62e4 100644 --- a/ql/test/library-tests/controlflow/graph/Cfg.expected +++ b/ql/test/library-tests/controlflow/graph/Cfg.expected @@ -2038,7 +2038,7 @@ cfg.rb: #-----| -> ... != ... # 190| enter run_block -#-----| -> yield ... +#-----| -> 42 # 190| run_block #-----| -> self @@ -2049,10 +2049,10 @@ cfg.rb: #-----| -> exit run_block # 191| yield ... -#-----| -> 42 +#-----| -> exit run_block (normal) # 191| 42 -#-----| -> exit run_block (normal) +#-----| -> yield ... # 194| call to run_block #-----| -> exit cfg.rb (normal) From a0084b77325947e050f831a9d65dd501825b2942 Mon Sep 17 00:00:00 2001 From: Nick Rolfe Date: Wed, 5 May 2021 17:18:44 +0100 Subject: [PATCH 02/18] Simplify CFG tree classes for calls --- .../controlflow/internal/ControlFlowGraphImpl.qll | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll index 77664806c7c..4cb35175592 100644 --- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -555,6 +555,10 @@ module Trees { } } + private class CallTree extends StandardPostOrderTree, Call { + override ControlFlowTree getChildNode(int i) { result = this.getArgument(i) } + } + private class CaseTree extends PreOrderTree, CaseExpr { final override predicate propagatesAbnormal(AstNode child) { child = this.getValue() or child = this.getABranch() @@ -913,7 +917,7 @@ module Trees { } } - private class MethodCallTree extends StandardPostOrderTree, MethodCall { + private class MethodCallTree extends CallTree, MethodCall { final override ControlFlowTree getChildNode(int i) { result = this.getReceiver() and i = 0 or @@ -1222,10 +1226,6 @@ module Trees { final override ControlFlowTree getChildNode(int i) { result = this.getComponent(i) } } - private class SuperCallTree extends StandardPostOrderTree, SuperCall { - final override ControlFlowTree getChildNode(int i) { result = this.getArgument(i) } - } - private class ToplevelTree extends BodyStmtTree, Toplevel { final override AstNode getBodyChild(int i, boolean rescuable) { result = this.getBeginBlock(i) and rescuable = true @@ -1292,10 +1292,6 @@ module Trees { ) } } - - private class YieldCallTree extends StandardPostOrderTree, YieldCall { - final override ControlFlowTree getChildNode(int i) { result = this.getArgument(i) } - } } private Scope parent(Scope n) { From d623f47ba02754f41228d5e8947bc7ab322f444b Mon Sep 17 00:00:00 2001 From: Nick Rolfe Date: Thu, 6 May 2021 15:36:25 +0100 Subject: [PATCH 03/18] Make ClassDeclarationTree post-order --- .../internal/ControlFlowGraphImpl.qll | 16 ++++++++++++++- .../controlflow/graph/Cfg.expected | 20 +++++++++---------- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll index 4cb35175592..75309f5abf9 100644 --- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -602,7 +602,21 @@ module Trees { private class CharacterTree extends LeafTree, CharacterLiteral { } - private class ClassDeclarationTree extends BodyStmtPreOrderTree, ClassDeclaration { + private class ClassDeclarationTree extends BodyStmtPostOrderTree, ClassDeclaration { + override predicate first(AstNode first) { + this.firstInner(first) + or + not exists(this.getAChild(_)) and + first = this + } + + final override predicate succ(AstNode pred, AstNode succ, Completion c) { + BodyStmtPostOrderTree.super.succ(pred, succ, c) + or + succ = this and + this.lastInner(pred, c) + } + /** Gets the `i`th child in the body of this block. */ final override AstNode getBodyChild(int i, boolean rescuable) { result = this.getScopeExpr() and i = 0 and rescuable = false diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected index 025f50e62e4..ffad06e886c 100644 --- a/ql/test/library-tests/controlflow/graph/Cfg.expected +++ b/ql/test/library-tests/controlflow/graph/Cfg.expected @@ -845,7 +845,7 @@ cfg.rb: #-----| -> ... = ... # 54| ... = ... -#-----| -> Silly +#-----| -> Object # 54| character #-----| -> ?\x40 @@ -854,7 +854,7 @@ cfg.rb: #-----| -> ... = ... # 58| Silly -#-----| -> Object +#-----| -> x # 58| Object #-----| -> complex @@ -1013,7 +1013,7 @@ cfg.rb: #-----| -> self # 69| print -#-----| -> x +#-----| -> Silly # 69| exit print @@ -1346,7 +1346,7 @@ cfg.rb: #-----| -> call to type # 113| ... if ... -#-----| -> C +#-----| -> @field # 113| call to puts #-----| -> ... if ... @@ -1368,7 +1368,7 @@ cfg.rb: #-----| -> ... > ... # 115| C -#-----| -> @field +#-----| -> swap # 116| ... = ... #-----| -> @@static_field @@ -1380,7 +1380,7 @@ cfg.rb: #-----| -> ... = ... # 117| ... = ... -#-----| -> swap +#-----| -> C # 117| @@static_field #-----| -> 10 @@ -2979,7 +2979,7 @@ loops.rb: raise.rb: # 1| enter raise.rb -#-----| -> ExceptionA +#-----| -> Exception # 1| ExceptionA #-----| -> Exception @@ -2990,13 +2990,13 @@ raise.rb: #-----| -> exit raise.rb # 1| Exception -#-----| -> ExceptionB +#-----| -> ExceptionA # 4| ExceptionB -#-----| -> Exception +#-----| -> m1 # 4| Exception -#-----| -> m1 +#-----| -> ExceptionB # 7| enter m1 #-----| -> x From fd3d50f3408a42742e12492b8af1aaa0d1b18e37 Mon Sep 17 00:00:00 2001 From: Nick Rolfe Date: Thu, 6 May 2021 15:54:11 +0100 Subject: [PATCH 04/18] Make ModuleDeclarationTree post-order --- .../internal/ControlFlowGraphImpl.qll | 20 ++++++++++++++++--- .../controlflow/graph/Cfg.expected | 6 +++--- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll index 75309f5abf9..104c9692296 100644 --- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -603,7 +603,7 @@ module Trees { private class CharacterTree extends LeafTree, CharacterLiteral { } private class ClassDeclarationTree extends BodyStmtPostOrderTree, ClassDeclaration { - override predicate first(AstNode first) { + final override predicate first(AstNode first) { this.firstInner(first) or not exists(this.getAChild(_)) and @@ -952,12 +952,26 @@ module Trees { } } - private class ModuleDeclarationTree extends BodyStmtPreOrderTree, ModuleDeclaration { + private class ModuleDeclarationTree extends BodyStmtPostOrderTree, ModuleDeclaration { + final override predicate first(AstNode first) { + this.firstInner(first) + or + not exists(this.getAChild(_)) and + first = this + } + + final override predicate succ(AstNode pred, AstNode succ, Completion c) { + BodyStmtPostOrderTree.super.succ(pred, succ, c) + or + succ = this and + this.lastInner(pred, c) + } + /** Gets the `i`th child in the body of this block. */ final override AstNode getBodyChild(int i, boolean rescuable) { result = this.getScopeExpr() and i = 0 and rescuable = false or - result = BodyStmtPreOrderTree.super.getBodyChild(i - count(this.getScopeExpr()), rescuable) + result = BodyStmtPostOrderTree.super.getBodyChild(i - count(this.getScopeExpr()), rescuable) } } diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected index ffad06e886c..865191c7259 100644 --- a/ql/test/library-tests/controlflow/graph/Cfg.expected +++ b/ql/test/library-tests/controlflow/graph/Cfg.expected @@ -1389,7 +1389,7 @@ cfg.rb: #-----| -> ... = ... # 120| ... = ... -#-----| -> M +#-----| -> nothing # 120| swap #-----| -> -> { ... } @@ -1424,7 +1424,7 @@ cfg.rb: #-----| -> [...] # 122| M -#-----| -> nothing +#-----| -> EmptyClass # 123| ... = ... #-----| -> some @@ -1529,7 +1529,7 @@ cfg.rb: #-----| -> #{...} # 130| ... = ... -#-----| -> EmptyClass +#-----| -> M # 130| Constant #-----| -> 5 From 9185a93312e28090a9fee2cee74a89ccb4f30696 Mon Sep 17 00:00:00 2001 From: Nick Rolfe Date: Thu, 6 May 2021 16:20:50 +0100 Subject: [PATCH 05/18] Make SingletonClassDeclarationTree post-order --- .../internal/ControlFlowGraphImpl.qll | 18 ++++++++++++++++-- .../controlflow/graph/Cfg.expected | 6 +++--- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll index 104c9692296..c05083afb9d 100644 --- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -1176,13 +1176,27 @@ module Trees { } } - private class SingletonClassTree extends BodyStmtPreOrderTree, SingletonClass { + private class SingletonClassTree extends BodyStmtPostOrderTree, SingletonClass { + final override predicate first(AstNode first) { + this.firstInner(first) + or + not exists(this.getAChild(_)) and + first = this + } + + final override predicate succ(AstNode pred, AstNode succ, Completion c) { + BodyStmtPostOrderTree.super.succ(pred, succ, c) + or + succ = this and + this.lastInner(pred, c) + } + /** Gets the `i`th child in the body of this block. */ final override AstNode getBodyChild(int i, boolean rescuable) { ( result = this.getValue() and i = 0 and rescuable = false or - result = BodyStmtPreOrderTree.super.getBodyChild(i - 1, rescuable) + result = BodyStmtPostOrderTree.super.getBodyChild(i - 1, rescuable) ) } } diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected index 865191c7259..d3731c5e367 100644 --- a/ql/test/library-tests/controlflow/graph/Cfg.expected +++ b/ql/test/library-tests/controlflow/graph/Cfg.expected @@ -1599,10 +1599,10 @@ cfg.rb: #-----| -> call to itself # 141| Constant -#-----| -> class << ... +#-----| -> Silly # 143| class << ... -#-----| -> Silly +#-----| -> silly # 143| call to itself #-----| -> setter= @@ -1617,7 +1617,7 @@ cfg.rb: #-----| -> self # 145| print -#-----| -> silly +#-----| -> class << ... # 145| exit print From 2c7f1e0c1139be8abe84a444f86c2a5245059cbc Mon Sep 17 00:00:00 2001 From: Nick Rolfe Date: Thu, 6 May 2021 16:28:36 +0100 Subject: [PATCH 06/18] Remove unused class --- .../internal/ControlFlowGraphImpl.qll | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll index c05083afb9d..1e7227a24f8 100644 --- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -523,24 +523,6 @@ module Trees { } } - abstract class BodyStmtPreOrderTree extends BodyStmtTree, PreOrderTree { - final override predicate last(AstNode last, Completion c) { - this.lastInner(last, c) - or - not exists(this.getAChild(_)) and - last = this and - c.isValidFor(this) - } - - final override predicate succ(AstNode pred, AstNode succ, Completion c) { - BodyStmtTree.super.succ(pred, succ, c) - or - pred = this and - c instanceof SimpleCompletion and - this.firstInner(succ) - } - } - abstract class BodyStmtPostOrderTree extends BodyStmtTree, PostOrderTree { override predicate first(AstNode first) { first = this } } From 4e80b548c1ef86a3a28f9b5c54a217782beb8ee1 Mon Sep 17 00:00:00 2001 From: Nick Rolfe Date: Thu, 6 May 2021 16:45:27 +0100 Subject: [PATCH 07/18] Make BeginBlock CFG post-order --- .../controlflow/internal/ControlFlowGraphImpl.qll | 6 ++++-- ql/test/library-tests/controlflow/graph/Cfg.expected | 6 +++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll index 1e7227a24f8..c24a559e5ed 100644 --- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -1108,7 +1108,8 @@ module Trees { private class SimplePostOrderStmtSequenceTree extends StmtSequenceTree, PostOrderTree { SimplePostOrderStmtSequenceTree() { this instanceof StringInterpolationComponent or - this instanceof ParenthesizedExpr + this instanceof ParenthesizedExpr or + this instanceof BeginBlock } final override predicate first(AstNode first) { first(this.getStmt(0), first) } @@ -1136,7 +1137,8 @@ module Trees { not this instanceof EndBlock and not this instanceof StringInterpolationComponent and not this instanceof Block and - not this instanceof ParenthesizedExpr + not this instanceof ParenthesizedExpr and + not this instanceof BeginBlock } final override predicate propagatesAbnormal(AstNode child) { child = this.getAStmt() } diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected index d3731c5e367..c04cd93be88 100644 --- a/ql/test/library-tests/controlflow/graph/Cfg.expected +++ b/ql/test/library-tests/controlflow/graph/Cfg.expected @@ -491,7 +491,7 @@ case.rb: cfg.rb: # 1| enter cfg.rb -#-----| -> BEGIN { ... } +#-----| -> self # 1| bar #-----| -> alias ... @@ -559,10 +559,10 @@ cfg.rb: #-----| -> call to puts # 15| BEGIN { ... } -#-----| -> self +#-----| -> bar # 16| call to puts -#-----| -> bar +#-----| -> BEGIN { ... } # 16| self #-----| -> "hello" From 2569bf257fed9c5caa3bd4114906c7febe3b087e Mon Sep 17 00:00:00 2001 From: Nick Rolfe Date: Fri, 7 May 2021 15:40:50 +0100 Subject: [PATCH 08/18] Make CFG for TThen post-order --- .../internal/ControlFlowGraphImpl.qll | 6 +- .../controlflow/graph/Cfg.expected | 354 +++++++----------- .../dataflow/local/DataflowStep.expected | 3 - 3 files changed, 130 insertions(+), 233 deletions(-) diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll index c24a559e5ed..c5645b77c69 100644 --- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -1109,7 +1109,8 @@ module Trees { SimplePostOrderStmtSequenceTree() { this instanceof StringInterpolationComponent or this instanceof ParenthesizedExpr or - this instanceof BeginBlock + this instanceof BeginBlock or + this instanceof ASTInternal::TThen } final override predicate first(AstNode first) { first(this.getStmt(0), first) } @@ -1138,7 +1139,8 @@ module Trees { not this instanceof StringInterpolationComponent and not this instanceof Block and not this instanceof ParenthesizedExpr and - not this instanceof BeginBlock + not this instanceof BeginBlock and + not this instanceof ASTInternal::TThen } final override predicate propagatesAbnormal(AstNode child) { child = this.getAStmt() } diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected index c04cd93be88..ed12e16b6bd 100644 --- a/ql/test/library-tests/controlflow/graph/Cfg.expected +++ b/ql/test/library-tests/controlflow/graph/Cfg.expected @@ -41,8 +41,8 @@ break_ensure.rb: #-----| -> In # 3| ... > ... +#-----| true -> break #-----| false -> if ... -#-----| true -> then ... # 3| element #-----| -> 0 @@ -50,9 +50,6 @@ break_ensure.rb: # 3| 0 #-----| -> ... > ... -# 3| then ... -#-----| -> break - # 4| break #-----| break -> for ... in ... @@ -64,16 +61,16 @@ break_ensure.rb: # 8| call to nil? #-----| false -> if ... -#-----| true -> then ... +#-----| true -> self # 8| elements #-----| -> call to nil? # 8| then ... -#-----| -> self +#-----| -> if ... # 9| call to puts -#-----| -> if ... +#-----| -> then ... # 9| self #-----| -> "elements nil" @@ -115,8 +112,8 @@ break_ensure.rb: #-----| -> ensure ... # 16| ... > ... +#-----| true -> break #-----| false -> if ... -#-----| true -> then ... # 16| element #-----| -> 0 @@ -124,9 +121,6 @@ break_ensure.rb: # 16| 0 #-----| -> ... > ... -# 16| then ... -#-----| -> break - # 17| break #-----| break -> [ensure: break] ensure ... @@ -144,11 +138,11 @@ break_ensure.rb: # 20| call to nil? #-----| false -> if ... -#-----| true -> then ... +#-----| true -> self # 20| [ensure: break] call to nil? #-----| false -> [ensure: break] if ... -#-----| true -> [ensure: break] then ... +#-----| true -> [ensure: break] self # 20| elements #-----| -> call to nil? @@ -157,17 +151,17 @@ break_ensure.rb: #-----| -> [ensure: break] call to nil? # 20| then ... -#-----| -> self - -# 20| [ensure: break] then ... -#-----| -> [ensure: break] self - -# 21| call to puts #-----| -> if ... -# 21| [ensure: break] call to puts +# 20| [ensure: break] then ... #-----| -> [ensure: break] if ... +# 21| call to puts +#-----| -> then ... + +# 21| [ensure: break] call to puts +#-----| -> [ensure: break] then ... + # 21| self #-----| -> "elements nil" @@ -199,14 +193,11 @@ break_ensure.rb: # 29| call to nil? #-----| false -> if ... -#-----| true -> then ... +#-----| true -> return # 29| elements #-----| -> call to nil? -# 29| then ... -#-----| -> return - # 30| return #-----| return -> [ensure: return] ensure ... @@ -255,12 +246,12 @@ break_ensure.rb: #-----| -> [ensure: return] In # 35| ... > ... +#-----| true -> break #-----| false -> if ... -#-----| true -> then ... # 35| [ensure: return] ... > ... +#-----| true -> [ensure: return] break #-----| false -> [ensure: return] if ... -#-----| true -> [ensure: return] then ... # 35| call to x #-----| -> 0 @@ -280,12 +271,6 @@ break_ensure.rb: # 35| [ensure: return] 0 #-----| -> [ensure: return] ... > ... -# 35| then ... -#-----| -> break - -# 35| [ensure: return] then ... -#-----| -> [ensure: return] break - # 36| break #-----| break -> for ... in ... @@ -336,7 +321,7 @@ break_ensure.rb: # 47| ... > ... #-----| false -> if ... -#-----| true -> then ... +#-----| true -> self # 47| element #-----| -> 1 @@ -344,9 +329,6 @@ break_ensure.rb: # 47| 1 #-----| -> ... > ... -# 47| then ... -#-----| -> self - # 48| call to raise #-----| raise -> [ensure: raise] ensure ... @@ -370,11 +352,11 @@ break_ensure.rb: # 51| ... > ... #-----| false -> if ... -#-----| true -> then ... +#-----| true -> 10 # 51| [ensure: raise] ... > ... #-----| false -> [ensure: raise] if ... -#-----| true -> [ensure: raise] then ... +#-----| true -> [ensure: raise] 10 # 51| element #-----| -> 0 @@ -388,12 +370,6 @@ break_ensure.rb: # 51| [ensure: raise] 0 #-----| -> [ensure: raise] ... > ... -# 51| then ... -#-----| -> 10 - -# 51| [ensure: raise] then ... -#-----| -> [ensure: raise] 10 - # 52| break #-----| break -> for ... in ... @@ -439,30 +415,30 @@ case.rb: #-----| -> 1 # 3| 1 -#-----| match -> then ... #-----| no-match -> when ... +#-----| match -> self # 3| then ... -#-----| -> self +#-----| -> exit if_in_case (normal) # 3| ( ... ) -#-----| -> exit if_in_case (normal) +#-----| -> then ... # 3| if ... #-----| -> ( ... ) # 3| call to x2 #-----| false -> if ... -#-----| true -> then ... +#-----| true -> self # 3| self #-----| -> call to x2 # 3| then ... -#-----| -> self +#-----| -> if ... # 3| call to puts -#-----| -> if ... +#-----| -> then ... # 3| self #-----| -> "x2" @@ -474,14 +450,14 @@ case.rb: #-----| -> 2 # 4| 2 -#-----| match -> then ... +#-----| match -> self #-----| no-match -> exit if_in_case (normal) # 4| then ... -#-----| -> self +#-----| -> exit if_in_case (normal) # 4| call to puts -#-----| -> exit if_in_case (normal) +#-----| -> then ... # 4| self #-----| -> "2" @@ -706,14 +682,14 @@ cfg.rb: #-----| -> 1 # 42| 1 -#-----| match -> then ... #-----| no-match -> when ... +#-----| match -> self # 42| then ... -#-----| -> self +#-----| -> case ... # 42| call to puts -#-----| -> case ... +#-----| -> then ... # 42| self #-----| -> "one" @@ -726,21 +702,21 @@ cfg.rb: # 43| 2 #-----| no-match -> 3 -#-----| match -> then ... +#-----| match -> self # 43| 3 #-----| no-match -> 4 -#-----| match -> then ... +#-----| match -> self # 43| 4 #-----| no-match -> else ... -#-----| match -> then ... +#-----| match -> self # 43| then ... -#-----| -> self +#-----| -> case ... # 43| call to puts -#-----| -> case ... +#-----| -> then ... # 43| self #-----| -> "some" @@ -767,8 +743,8 @@ cfg.rb: #-----| -> b # 48| ... == ... -#-----| true -> then ... #-----| false -> when ... +#-----| true -> self # 48| b #-----| -> 1 @@ -777,10 +753,10 @@ cfg.rb: #-----| -> ... == ... # 48| then ... -#-----| -> self +#-----| -> chained # 48| call to puts -#-----| -> chained +#-----| -> then ... # 48| self #-----| -> "one" @@ -792,7 +768,7 @@ cfg.rb: #-----| -> b # 49| ... == ... -#-----| true -> then ... +#-----| true -> self #-----| false -> b # 49| b @@ -802,7 +778,7 @@ cfg.rb: #-----| -> ... == ... # 49| ... > ... -#-----| true -> then ... +#-----| true -> self #-----| false -> chained # 49| b @@ -812,10 +788,10 @@ cfg.rb: #-----| -> ... > ... # 49| then ... -#-----| -> self +#-----| -> chained # 49| call to puts -#-----| -> chained +#-----| -> then ... # 49| self #-----| -> "some" @@ -1042,7 +1018,7 @@ cfg.rb: #-----| -> ; # 75| ... < ... -#-----| true -> then ... +#-----| true -> 0 #-----| false -> x # 75| x @@ -1052,17 +1028,17 @@ cfg.rb: #-----| -> ... < ... # 75| then ... -#-----| -> 0 +#-----| -> if ... # 75| 0 -#-----| -> if ... +#-----| -> then ... # 75| elsif ... #-----| -> if ... # 75| ... > ... #-----| false -> else ... -#-----| true -> then ... +#-----| true -> 10 # 75| x #-----| -> 10 @@ -1071,10 +1047,10 @@ cfg.rb: #-----| -> ... > ... # 75| then ... -#-----| -> 10 +#-----| -> elsif ... # 75| 10 -#-----| -> elsif ... +#-----| -> then ... # 75| else ... #-----| -> x @@ -1154,7 +1130,7 @@ cfg.rb: # 91| ... > ... #-----| false -> if ... -#-----| true -> then ... +#-----| true -> next # 91| x #-----| -> 3 @@ -1162,9 +1138,6 @@ cfg.rb: # 91| 3 #-----| -> ... > ... -# 91| then ... -#-----| -> next - # 91| next #-----| next -> In @@ -1810,7 +1783,7 @@ cfg.rb: # 172| ... == ... #-----| true -> else ... -#-----| false -> then ... +#-----| false -> self # 172| x #-----| -> 10 @@ -1819,10 +1792,10 @@ cfg.rb: #-----| -> ... == ... # 172| then ... -#-----| -> self +#-----| -> unless ... # 172| call to puts -#-----| -> unless ... +#-----| -> then ... # 172| self #-----| -> "hi" @@ -1980,7 +1953,7 @@ cfg.rb: # 184| ... == ... #-----| false -> if ... -#-----| true -> then ... +#-----| true -> redo # 184| x #-----| -> 5 @@ -1988,9 +1961,6 @@ cfg.rb: # 184| 5 #-----| -> ... == ... -# 184| then ... -#-----| -> redo - # 184| redo #-----| redo -> do ... @@ -2114,7 +2084,7 @@ exit.rb: # 2| ... > ... #-----| false -> if ... -#-----| true -> then ... +#-----| true -> self # 2| x #-----| -> 2 @@ -2122,9 +2092,6 @@ exit.rb: # 2| 2 #-----| -> ... > ... -# 2| then ... -#-----| -> self - # 3| call to exit #-----| exit -> exit m1 (abnormal) @@ -2165,7 +2132,7 @@ exit.rb: # 9| ... > ... #-----| false -> if ... -#-----| true -> then ... +#-----| true -> self # 9| x #-----| -> 2 @@ -2173,9 +2140,6 @@ exit.rb: # 9| 2 #-----| -> ... > ... -# 9| then ... -#-----| -> self - # 10| call to abort #-----| exit -> exit m2 (abnormal) @@ -2253,7 +2217,7 @@ ifs.rb: #-----| -> exit m1 (normal) # 2| ... > ... -#-----| true -> then ... +#-----| true -> self #-----| false -> x # 2| x @@ -2263,10 +2227,10 @@ ifs.rb: #-----| -> ... > ... # 2| then ... -#-----| -> self +#-----| -> if ... # 3| call to puts -#-----| -> if ... +#-----| -> then ... # 3| self #-----| -> "x is greater than 2" @@ -2285,7 +2249,7 @@ ifs.rb: #-----| false -> else ... # 4| [true] ... and ... -#-----| true -> then ... +#-----| true -> self # 4| [false] ... and ... #-----| false -> [false] ... and ... @@ -2332,10 +2296,10 @@ ifs.rb: #-----| -> ... == ... # 4| then ... -#-----| -> self +#-----| -> elsif ... # 5| call to puts -#-----| -> elsif ... +#-----| -> then ... # 5| self #-----| -> "x is 1" @@ -2374,10 +2338,7 @@ ifs.rb: # 12| b #-----| false -> if ... -#-----| true -> then ... - -# 12| then ... -#-----| -> 0 +#-----| true -> 0 # 13| return #-----| return -> exit m2 (normal) @@ -2410,7 +2371,7 @@ ifs.rb: # 19| ... < ... #-----| false -> if ... -#-----| true -> then ... +#-----| true -> x # 19| x #-----| -> 0 @@ -2419,7 +2380,7 @@ ifs.rb: #-----| -> ... < ... # 19| then ... -#-----| -> x +#-----| -> if ... # 20| ... = ... #-----| -> x @@ -2434,11 +2395,11 @@ ifs.rb: #-----| -> - ... # 21| if ... -#-----| -> if ... +#-----| -> then ... # 21| ... > ... #-----| false -> if ... -#-----| true -> then ... +#-----| true -> x # 21| x #-----| -> 10 @@ -2447,10 +2408,10 @@ ifs.rb: #-----| -> ... > ... # 21| then ... -#-----| -> x +#-----| -> if ... # 22| ... = ... -#-----| -> if ... +#-----| -> then ... # 22| x #-----| -> x @@ -2562,7 +2523,7 @@ ifs.rb: #-----| false -> else ... # 33| [true] ( ... ) -#-----| true -> then ... +#-----| true -> "b2 || b4 || b5" # 33| [false] if ... #-----| false -> [false] ( ... ) @@ -2571,15 +2532,18 @@ ifs.rb: #-----| true -> [true] ( ... ) # 33| b1 -#-----| true -> then ... +#-----| true -> b2 #-----| false -> b3 -# 33| then ... -#-----| -> b2 +# 33| [false] then ... +#-----| false -> [false] if ... + +# 33| [true] then ... +#-----| true -> [true] if ... # 33| b2 -#-----| false -> [false] if ... -#-----| true -> [true] if ... +#-----| false -> [false] then ... +#-----| true -> [true] then ... # 33| [false] elsif ... #-----| false -> [false] if ... @@ -2589,14 +2553,17 @@ ifs.rb: # 33| b3 #-----| false -> else ... -#-----| true -> then ... +#-----| true -> b4 -# 33| then ... -#-----| -> b4 +# 33| [false] then ... +#-----| false -> [false] elsif ... + +# 33| [true] then ... +#-----| true -> [true] elsif ... # 33| b4 -#-----| false -> [false] elsif ... -#-----| true -> [true] elsif ... +#-----| false -> [false] then ... +#-----| true -> [true] then ... # 33| else ... #-----| -> b5 @@ -2606,10 +2573,10 @@ ifs.rb: #-----| true -> [true] elsif ... # 33| then ... -#-----| -> "b2 || b4 || b5" +#-----| -> if ... # 33| "b2 || b4 || b5" -#-----| -> if ... +#-----| -> then ... # 33| else ... #-----| -> "!b2 || !b4 || !b5" @@ -2689,13 +2656,13 @@ ifs.rb: # 47| b #-----| false -> else ... -#-----| true -> then ... +#-----| true -> self # 47| then ... -#-----| -> self +#-----| -> if ... # 48| call to puts -#-----| -> if ... +#-----| -> then ... # 48| self #-----| -> "true" @@ -2824,7 +2791,7 @@ loops.rb: #-----| -> self # 12| ... > ... -#-----| true -> then ... +#-----| true -> break #-----| false -> x # 12| x @@ -2833,9 +2800,6 @@ loops.rb: # 12| 100 #-----| -> ... > ... -# 12| then ... -#-----| -> break - # 13| break #-----| break -> while ... @@ -2843,7 +2807,7 @@ loops.rb: #-----| -> if ... # 14| ... > ... -#-----| true -> then ... +#-----| true -> next #-----| false -> x # 14| x @@ -2852,9 +2816,6 @@ loops.rb: # 14| 50 #-----| -> ... > ... -# 14| then ... -#-----| -> next - # 15| next #-----| next -> x @@ -2863,7 +2824,7 @@ loops.rb: # 16| ... > ... #-----| false -> elsif ... -#-----| true -> then ... +#-----| true -> redo # 16| x #-----| -> 10 @@ -2871,9 +2832,6 @@ loops.rb: # 16| 10 #-----| -> ... > ... -# 16| then ... -#-----| -> redo - # 17| redo #-----| redo -> do ... @@ -3020,7 +2978,7 @@ raise.rb: # 8| ... > ... #-----| false -> if ... -#-----| true -> then ... +#-----| true -> self # 8| x #-----| -> 2 @@ -3028,9 +2986,6 @@ raise.rb: # 8| 2 #-----| -> ... > ... -# 8| then ... -#-----| -> self - # 9| call to raise #-----| raise -> exit m1 (abnormal) @@ -3071,10 +3026,7 @@ raise.rb: # 16| b #-----| false -> if ... -#-----| true -> then ... - -# 16| then ... -#-----| -> self +#-----| true -> self # 17| call to raise #-----| raise -> rescue ... @@ -3089,14 +3041,14 @@ raise.rb: #-----| -> ExceptionA # 19| ExceptionA -#-----| match -> then ... +#-----| match -> self #-----| raise -> exit m2 (abnormal) # 19| then ... #-----| -> self # 20| call to puts -#-----| -> self +#-----| -> then ... # 20| self #-----| -> "Rescued" @@ -3132,10 +3084,7 @@ raise.rb: # 27| b #-----| false -> if ... -#-----| true -> then ... - -# 27| then ... -#-----| -> self +#-----| true -> self # 28| call to raise #-----| raise -> rescue ... @@ -3147,13 +3096,13 @@ raise.rb: #-----| -> call to raise # 30| rescue ... -#-----| -> then ... +#-----| -> self # 30| then ... #-----| -> self # 31| call to puts -#-----| -> self +#-----| -> then ... # 31| self #-----| -> "Rescued" @@ -3189,10 +3138,7 @@ raise.rb: # 38| b #-----| false -> if ... -#-----| true -> then ... - -# 38| then ... -#-----| -> self +#-----| true -> self # 39| call to raise #-----| raise -> rescue ... @@ -3207,13 +3153,13 @@ raise.rb: #-----| -> e # 41| e -#-----| -> then ... +#-----| -> self # 41| then ... #-----| -> self # 42| call to puts -#-----| -> self +#-----| -> then ... # 42| self #-----| -> "Rescued {e}" @@ -3249,10 +3195,7 @@ raise.rb: # 49| b #-----| false -> if ... -#-----| true -> then ... - -# 49| then ... -#-----| -> self +#-----| true -> self # 50| call to raise #-----| raise -> rescue ... @@ -3300,10 +3243,7 @@ raise.rb: # 59| b #-----| false -> if ... -#-----| true -> then ... - -# 59| then ... -#-----| -> self +#-----| true -> self # 60| call to raise #-----| raise -> rescue ... @@ -3326,13 +3266,13 @@ raise.rb: #-----| raise -> exit m6 (abnormal) # 62| e -#-----| -> then ... +#-----| -> self # 62| then ... #-----| -> self # 63| call to puts -#-----| -> self +#-----| -> then ... # 63| self #-----| -> "Rescued {e}" @@ -3370,7 +3310,7 @@ raise.rb: #-----| -> self # 69| ... > ... -#-----| true -> then ... +#-----| true -> self #-----| false -> x # 69| x @@ -3379,9 +3319,6 @@ raise.rb: # 69| 2 #-----| -> ... > ... -# 69| then ... -#-----| -> self - # 70| call to raise #-----| raise -> [ensure: raise] ensure ... @@ -3396,7 +3333,7 @@ raise.rb: # 71| ... < ... #-----| false -> elsif ... -#-----| true -> then ... +#-----| true -> "x < 0" # 71| x #-----| -> 0 @@ -3404,9 +3341,6 @@ raise.rb: # 71| 0 #-----| -> ... < ... -# 71| then ... -#-----| -> "x < 0" - # 72| return #-----| return -> [ensure: return] ensure ... @@ -3488,7 +3422,7 @@ raise.rb: #-----| -> self # 82| ... > ... -#-----| true -> then ... +#-----| true -> self #-----| false -> x # 82| x @@ -3497,9 +3431,6 @@ raise.rb: # 82| 2 #-----| -> ... > ... -# 82| then ... -#-----| -> self - # 83| call to raise #-----| raise -> [ensure: raise] ensure ... @@ -3514,7 +3445,7 @@ raise.rb: # 84| ... < ... #-----| false -> elsif ... -#-----| true -> then ... +#-----| true -> "x < 0" # 84| x #-----| -> 0 @@ -3522,9 +3453,6 @@ raise.rb: # 84| 0 #-----| -> ... < ... -# 84| then ... -#-----| -> "x < 0" - # 85| return #-----| return -> [ensure: return] ensure ... @@ -3621,7 +3549,7 @@ raise.rb: #-----| -> self # 97| ... > ... -#-----| true -> then ... +#-----| true -> self #-----| false -> x # 97| x @@ -3630,9 +3558,6 @@ raise.rb: # 97| 2 #-----| -> ... > ... -# 97| then ... -#-----| -> self - # 98| call to raise #-----| raise -> [ensure: raise] ensure ... @@ -3647,7 +3572,7 @@ raise.rb: # 99| ... < ... #-----| false -> elsif ... -#-----| true -> then ... +#-----| true -> "x < 0" # 99| x #-----| -> 0 @@ -3655,9 +3580,6 @@ raise.rb: # 99| 0 #-----| -> ... < ... -# 99| then ... -#-----| -> "x < 0" - # 100| return #-----| return -> [ensure: return] ensure ... @@ -3720,24 +3642,15 @@ raise.rb: # 106| b1 #-----| false -> if ... -#-----| true -> then ... +#-----| true -> self # 106| [ensure: return] b1 #-----| false -> [ensure: return] if ... -#-----| true -> [ensure: return] then ... +#-----| true -> [ensure: return] self # 106| [ensure: raise] b1 #-----| false -> [ensure: raise] if ... -#-----| true -> [ensure: raise] then ... - -# 106| then ... -#-----| -> self - -# 106| [ensure: return] then ... -#-----| -> [ensure: return] self - -# 106| [ensure: raise] then ... -#-----| -> [ensure: raise] self +#-----| true -> [ensure: raise] self # 107| call to raise #-----| raise -> [ensure(1): raise] ensure ... @@ -3894,24 +3807,15 @@ raise.rb: # 116| b2 #-----| false -> if ... -#-----| true -> then ... +#-----| true -> self # 116| [ensure: return] b2 #-----| false -> [ensure: return] if ... -#-----| true -> [ensure: return] then ... +#-----| true -> [ensure: return] self # 116| [ensure: raise] b2 #-----| false -> [ensure: raise] if ... -#-----| true -> [ensure: raise] then ... - -# 116| then ... -#-----| -> self - -# 116| [ensure: return] then ... -#-----| -> [ensure: return] self - -# 116| [ensure: raise] then ... -#-----| -> [ensure: raise] self +#-----| true -> [ensure: raise] self # 117| call to raise #-----| raise -> exit m9 (abnormal) @@ -4001,10 +3905,7 @@ raise.rb: # 130| b #-----| false -> if ... -#-----| true -> then ... - -# 130| then ... -#-----| -> self +#-----| true -> self # 131| call to raise #-----| raise -> rescue ... @@ -4027,13 +3928,13 @@ raise.rb: # 134| ExceptionB #-----| raise -> [ensure: raise] ensure ... -#-----| match -> then ... +#-----| match -> self # 134| then ... -#-----| -> self +#-----| -> ensure ... # 135| call to puts -#-----| -> ensure ... +#-----| -> then ... # 135| self #-----| -> "ExceptionB" @@ -4093,10 +3994,7 @@ raise.rb: # 143| b #-----| false -> if ... -#-----| true -> then ... - -# 143| then ... -#-----| -> self +#-----| true -> self # 144| call to raise #-----| raise -> [ensure: raise] ensure ... diff --git a/ql/test/library-tests/dataflow/local/DataflowStep.expected b/ql/test/library-tests/dataflow/local/DataflowStep.expected index a41b0df807f..df3836b3e08 100644 --- a/ql/test/library-tests/dataflow/local/DataflowStep.expected +++ b/ql/test/library-tests/dataflow/local/DataflowStep.expected @@ -32,7 +32,6 @@ | local_dataflow.rb:19:5:19:5 | x | local_dataflow.rb:20:6:20:6 | x | | local_dataflow.rb:19:10:19:14 | array | local_dataflow.rb:19:1:21:3 | for ... in ... | | local_dataflow.rb:20:3:20:25 | if ... | local_dataflow.rb:19:16:21:3 | do ... | -| local_dataflow.rb:20:12:20:21 | then ... | local_dataflow.rb:20:3:20:25 | if ... | | local_dataflow.rb:20:17:20:21 | break | local_dataflow.rb:19:1:21:3 | for ... in ... | | local_dataflow.rb:24:2:24:8 | break | local_dataflow.rb:23:1:25:3 | while ... | | local_dataflow.rb:24:8:24:8 | 5 | local_dataflow.rb:24:2:24:8 | break | @@ -43,10 +42,8 @@ | local_dataflow.rb:32:5:32:25 | bar | local_dataflow.rb:32:1:32:25 | ... = ... | | local_dataflow.rb:32:5:32:25 | bar | local_dataflow.rb:32:1:32:25 | ... = ... | | local_dataflow.rb:34:7:34:7 | x | local_dataflow.rb:35:6:35:6 | x | -| local_dataflow.rb:35:12:36:13 | then ... | local_dataflow.rb:35:3:37:5 | if ... | | local_dataflow.rb:36:13:36:13 | 7 | local_dataflow.rb:36:6:36:13 | return | | local_dataflow.rb:41:7:41:7 | x | local_dataflow.rb:42:6:42:6 | x | -| local_dataflow.rb:42:12:43:13 | then ... | local_dataflow.rb:42:3:44:5 | if ... | | local_dataflow.rb:43:13:43:13 | 7 | local_dataflow.rb:43:6:43:13 | return | | local_dataflow.rb:45:10:45:10 | 6 | local_dataflow.rb:45:3:45:10 | return | | local_dataflow.rb:49:3:53:3 | | local_dataflow.rb:50:18:50:18 | x | From 46c9f858c4de6da394340e3ce430e734c8c26339 Mon Sep 17 00:00:00 2001 From: Nick Rolfe Date: Fri, 7 May 2021 16:47:19 +0100 Subject: [PATCH 09/18] Make CFG for TElse post-order --- .../internal/ControlFlowGraphImpl.qll | 6 +- .../controlflow/graph/Cfg.expected | 55 +++++++++---------- 2 files changed, 31 insertions(+), 30 deletions(-) diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll index c5645b77c69..74e47e5e220 100644 --- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -1110,7 +1110,8 @@ module Trees { this instanceof StringInterpolationComponent or this instanceof ParenthesizedExpr or this instanceof BeginBlock or - this instanceof ASTInternal::TThen + this instanceof ASTInternal::TThen or + this instanceof ASTInternal::TElse } final override predicate first(AstNode first) { first(this.getStmt(0), first) } @@ -1140,7 +1141,8 @@ module Trees { not this instanceof Block and not this instanceof ParenthesizedExpr and not this instanceof BeginBlock and - not this instanceof ASTInternal::TThen + not this instanceof ASTInternal::TThen and + not this instanceof ASTInternal::TElse } final override predicate propagatesAbnormal(AstNode child) { child = this.getAStmt() } diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected index ed12e16b6bd..cf8e69cb208 100644 --- a/ql/test/library-tests/controlflow/graph/Cfg.expected +++ b/ql/test/library-tests/controlflow/graph/Cfg.expected @@ -709,8 +709,8 @@ cfg.rb: #-----| match -> self # 43| 4 -#-----| no-match -> else ... #-----| match -> self +#-----| no-match -> self # 43| then ... #-----| -> case ... @@ -725,10 +725,10 @@ cfg.rb: #-----| -> call to puts # 44| else ... -#-----| -> self +#-----| -> case ... # 44| call to puts -#-----| -> case ... +#-----| -> else ... # 44| self #-----| -> "many" @@ -1037,8 +1037,8 @@ cfg.rb: #-----| -> if ... # 75| ... > ... -#-----| false -> else ... #-----| true -> 10 +#-----| false -> x # 75| x #-----| -> 10 @@ -1053,20 +1053,20 @@ cfg.rb: #-----| -> then ... # 75| else ... -#-----| -> x - -# 75| x #-----| -> elsif ... -# 78| ; +# 75| x #-----| -> else ... -# 82| else ... +# 78| ; #-----| -> self -# 83| call to puts +# 82| else ... #-----| -> ensure ... +# 83| call to puts +#-----| -> else ... + # 83| self #-----| -> "ok" @@ -1782,8 +1782,8 @@ cfg.rb: #-----| -> x # 172| ... == ... -#-----| true -> else ... #-----| false -> self +#-----| true -> self # 172| x #-----| -> 10 @@ -1804,10 +1804,10 @@ cfg.rb: #-----| -> call to puts # 172| else ... -#-----| -> self +#-----| -> unless ... # 172| call to puts -#-----| -> unless ... +#-----| -> else ... # 172| self #-----| -> "bye" @@ -2246,7 +2246,7 @@ ifs.rb: #-----| true -> x # 4| [false] ... and ... -#-----| false -> else ... +#-----| false -> self # 4| [true] ... and ... #-----| true -> self @@ -2308,10 +2308,10 @@ ifs.rb: #-----| -> call to puts # 6| else ... -#-----| -> self +#-----| -> elsif ... # 7| call to puts -#-----| -> elsif ... +#-----| -> else ... # 7| self #-----| -> "I can't guess the number" @@ -2520,7 +2520,7 @@ ifs.rb: #-----| -> exit m5 (normal) # 33| [false] ( ... ) -#-----| false -> else ... +#-----| false -> "!b2 || !b4 || !b5" # 33| [true] ( ... ) #-----| true -> "b2 || b4 || b5" @@ -2552,8 +2552,8 @@ ifs.rb: #-----| true -> [true] if ... # 33| b3 -#-----| false -> else ... #-----| true -> b4 +#-----| false -> b5 # 33| [false] then ... #-----| false -> [false] elsif ... @@ -2565,12 +2565,15 @@ ifs.rb: #-----| false -> [false] then ... #-----| true -> [true] then ... -# 33| else ... -#-----| -> b5 +# 33| [false] else ... +#-----| false -> [false] elsif ... + +# 33| [true] else ... +#-----| true -> [true] elsif ... # 33| b5 -#-----| false -> [false] elsif ... -#-----| true -> [true] elsif ... +#-----| false -> [false] else ... +#-----| true -> [true] else ... # 33| then ... #-----| -> if ... @@ -2579,10 +2582,10 @@ ifs.rb: #-----| -> then ... # 33| else ... -#-----| -> "!b2 || !b4 || !b5" +#-----| -> if ... # 33| "!b2 || !b4 || !b5" -#-----| -> if ... +#-----| -> else ... # 36| enter conditional_method_def #-----| -> self @@ -2655,7 +2658,6 @@ ifs.rb: #-----| -> self # 47| b -#-----| false -> else ... #-----| true -> self # 47| then ... @@ -2670,9 +2672,6 @@ ifs.rb: # 48| "true" #-----| -> call to puts -# 49| else ... -#-----| -> if ... - # 51| call to puts #-----| -> exit empty_else (normal) From 7f6805c82f4db7f84e96130dbad4fdbe390711f6 Mon Sep 17 00:00:00 2001 From: Nick Rolfe Date: Fri, 7 May 2021 17:00:30 +0100 Subject: [PATCH 10/18] Make CFG for TDo post-order --- .../internal/ControlFlowGraphImpl.qll | 2 + .../controlflow/graph/Cfg.expected | 75 +++++++++---------- 2 files changed, 36 insertions(+), 41 deletions(-) diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll index 74e47e5e220..f0943db8025 100644 --- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -1111,6 +1111,7 @@ module Trees { this instanceof ParenthesizedExpr or this instanceof BeginBlock or this instanceof ASTInternal::TThen or + this instanceof ASTInternal::TDo or this instanceof ASTInternal::TElse } @@ -1142,6 +1143,7 @@ module Trees { not this instanceof ParenthesizedExpr and not this instanceof BeginBlock and not this instanceof ASTInternal::TThen and + not this instanceof ASTInternal::TDo and not this instanceof ASTInternal::TElse } diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected index cf8e69cb208..eaeba1bd727 100644 --- a/ql/test/library-tests/controlflow/graph/Cfg.expected +++ b/ql/test/library-tests/controlflow/graph/Cfg.expected @@ -25,7 +25,7 @@ break_ensure.rb: #-----| -> ensure ... # 2| element -#-----| -> do ... +#-----| -> element # 2| In #-----| empty -> for ... in ... @@ -35,10 +35,10 @@ break_ensure.rb: #-----| -> In # 2| do ... -#-----| -> element +#-----| -> In # 3| if ... -#-----| -> In +#-----| -> do ... # 3| ... > ... #-----| true -> break @@ -96,7 +96,7 @@ break_ensure.rb: #-----| -> exit m2 (normal) # 14| element -#-----| -> do ... +#-----| -> element # 14| In #-----| empty -> for ... in ... @@ -106,7 +106,7 @@ break_ensure.rb: #-----| -> In # 14| do ... -#-----| -> element +#-----| -> In # 16| if ... #-----| -> ensure ... @@ -131,7 +131,7 @@ break_ensure.rb: #-----| -> [ensure: break] elements # 20| if ... -#-----| -> In +#-----| -> do ... # 20| [ensure: break] if ... #-----| break -> for ... in ... @@ -214,10 +214,10 @@ break_ensure.rb: #-----| return -> exit m3 (normal) # 33| element -#-----| -> do ... +#-----| -> self # 33| [ensure: return] element -#-----| -> [ensure: return] do ... +#-----| -> [ensure: return] self # 33| In #-----| empty -> for ... in ... @@ -234,17 +234,17 @@ break_ensure.rb: #-----| -> [ensure: return] In # 33| do ... -#-----| -> self - -# 33| [ensure: return] do ... -#-----| -> [ensure: return] self - -# 35| if ... #-----| -> In -# 35| [ensure: return] if ... +# 33| [ensure: return] do ... #-----| -> [ensure: return] In +# 35| if ... +#-----| -> do ... + +# 35| [ensure: return] if ... +#-----| -> [ensure: return] do ... + # 35| ... > ... #-----| true -> break #-----| false -> if ... @@ -304,7 +304,7 @@ break_ensure.rb: #-----| -> exit m4 (normal) # 45| element -#-----| -> do ... +#-----| -> element # 45| In #-----| empty -> for ... in ... @@ -314,7 +314,7 @@ break_ensure.rb: #-----| -> In # 45| do ... -#-----| -> element +#-----| -> In # 47| if ... #-----| -> ensure ... @@ -345,7 +345,7 @@ break_ensure.rb: #-----| -> [ensure: raise] element # 51| if ... -#-----| -> In +#-----| -> do ... # 51| [ensure: raise] if ... #-----| raise -> for ... in ... @@ -646,10 +646,7 @@ cfg.rb: #-----| -> false # 31| true -#-----| true -> do ... - -# 31| do ... -#-----| -> 1 +#-----| true -> 1 # 32| break #-----| break -> while ... @@ -1104,7 +1101,7 @@ cfg.rb: #-----| -> $global # 90| x -#-----| -> do ... +#-----| -> x # 90| In #-----| empty -> for ... in ... @@ -1123,7 +1120,7 @@ cfg.rb: #-----| -> [...] # 90| do ... -#-----| -> x +#-----| -> In # 91| if ... #-----| -> self @@ -1142,7 +1139,7 @@ cfg.rb: #-----| next -> In # 92| call to puts -#-----| -> In +#-----| -> do ... # 92| self #-----| -> x @@ -1841,8 +1838,8 @@ cfg.rb: #-----| -> i # 176| ... > ... -#-----| false -> do ... #-----| true -> until ... +#-----| false -> x # 176| x #-----| -> 10 @@ -1863,7 +1860,7 @@ cfg.rb: #-----| -> ... += ... # 176| call to puts -#-----| -> x +#-----| -> do ... # 176| self #-----| -> "hello" @@ -1927,8 +1924,8 @@ cfg.rb: #-----| -> i # 182| ... < ... -#-----| true -> do ... #-----| false -> while ... +#-----| true -> x # 182| x #-----| -> 10 @@ -1962,10 +1959,10 @@ cfg.rb: #-----| -> ... == ... # 184| redo -#-----| redo -> do ... +#-----| redo -> x # 185| call to puts -#-----| -> x +#-----| -> do ... # 185| self #-----| -> x @@ -2708,8 +2705,8 @@ loops.rb: #-----| -> exit m1 (normal) # 2| ... >= ... -#-----| true -> do ... #-----| false -> while ... +#-----| true -> self # 2| x #-----| -> 0 @@ -2718,7 +2715,7 @@ loops.rb: #-----| -> ... >= ... # 2| do ... -#-----| -> self +#-----| -> x # 3| call to puts #-----| -> x @@ -2730,7 +2727,7 @@ loops.rb: #-----| -> call to puts # 4| ... -= ... -#-----| -> x +#-----| -> do ... # 4| x #-----| -> 1 @@ -2756,8 +2753,8 @@ loops.rb: #-----| -> self # 9| ... >= ... -#-----| true -> do ... #-----| false -> while ... +#-----| true -> self # 9| x #-----| -> 0 @@ -2766,7 +2763,7 @@ loops.rb: #-----| -> ... >= ... # 9| do ... -#-----| -> self +#-----| -> x # 10| call to puts #-----| -> x @@ -2832,10 +2829,10 @@ loops.rb: #-----| -> ... > ... # 17| redo -#-----| redo -> do ... +#-----| redo -> self # 19| call to puts -#-----| -> x +#-----| -> do ... # 19| self #-----| -> "Iter" @@ -2922,7 +2919,6 @@ loops.rb: #-----| -> exit m4 (normal) # 31| ... < ... -#-----| true -> do ... #-----| false -> while ... # 31| x @@ -2931,9 +2927,6 @@ loops.rb: # 31| y #-----| -> ... < ... -# 31| do ... -#-----| -> x - raise.rb: # 1| enter raise.rb #-----| -> Exception From 9def7c2dfe3185e5907f4aebd0af2923991b8ab1 Mon Sep 17 00:00:00 2001 From: Nick Rolfe Date: Fri, 7 May 2021 17:15:10 +0100 Subject: [PATCH 11/18] Make CFG for TEnsure post-order --- .../internal/ControlFlowGraphImpl.qll | 6 +- .../controlflow/graph/Cfg.expected | 221 ++++++++---------- 2 files changed, 106 insertions(+), 121 deletions(-) diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll index f0943db8025..30fba96e2cd 100644 --- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -1112,7 +1112,8 @@ module Trees { this instanceof BeginBlock or this instanceof ASTInternal::TThen or this instanceof ASTInternal::TDo or - this instanceof ASTInternal::TElse + this instanceof ASTInternal::TElse or + this instanceof ASTInternal::TEnsure } final override predicate first(AstNode first) { first(this.getStmt(0), first) } @@ -1144,7 +1145,8 @@ module Trees { not this instanceof BeginBlock and not this instanceof ASTInternal::TThen and not this instanceof ASTInternal::TDo and - not this instanceof ASTInternal::TElse + not this instanceof ASTInternal::TElse and + not this instanceof ASTInternal::TEnsure } final override predicate propagatesAbnormal(AstNode child) { child = this.getAStmt() } diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected index eaeba1bd727..e403081e046 100644 --- a/ql/test/library-tests/controlflow/graph/Cfg.expected +++ b/ql/test/library-tests/controlflow/graph/Cfg.expected @@ -22,7 +22,7 @@ break_ensure.rb: #-----| -> elements # 2| for ... in ... -#-----| -> ensure ... +#-----| -> elements # 2| element #-----| -> element @@ -54,10 +54,10 @@ break_ensure.rb: #-----| break -> for ... in ... # 7| ensure ... -#-----| -> elements +#-----| -> exit m1 (normal) # 8| if ... -#-----| -> exit m1 (normal) +#-----| -> ensure ... # 8| call to nil? #-----| false -> if ... @@ -109,7 +109,7 @@ break_ensure.rb: #-----| -> In # 16| if ... -#-----| -> ensure ... +#-----| -> elements # 16| ... > ... #-----| true -> break @@ -122,20 +122,20 @@ break_ensure.rb: #-----| -> ... > ... # 17| break -#-----| break -> [ensure: break] ensure ... +#-----| break -> [ensure: break] elements # 19| ensure ... -#-----| -> elements - -# 19| [ensure: break] ensure ... -#-----| -> [ensure: break] elements - -# 20| if ... #-----| -> do ... -# 20| [ensure: break] if ... +# 19| [ensure: break] ensure ... #-----| break -> for ... in ... +# 20| if ... +#-----| -> ensure ... + +# 20| [ensure: break] if ... +#-----| -> [ensure: break] ensure ... + # 20| call to nil? #-----| false -> if ... #-----| true -> self @@ -189,7 +189,7 @@ break_ensure.rb: #-----| -> elements # 29| if ... -#-----| -> ensure ... +#-----| -> elements # 29| call to nil? #-----| false -> if ... @@ -199,20 +199,20 @@ break_ensure.rb: #-----| -> call to nil? # 30| return -#-----| return -> [ensure: return] ensure ... +#-----| return -> [ensure: return] elements # 32| ensure ... -#-----| -> elements - -# 32| [ensure: return] ensure ... -#-----| -> [ensure: return] elements - -# 33| for ... in ... #-----| -> self -# 33| [ensure: return] for ... in ... +# 32| [ensure: return] ensure ... #-----| return -> exit m3 (normal) +# 33| for ... in ... +#-----| -> ensure ... + +# 33| [ensure: return] for ... in ... +#-----| -> [ensure: return] ensure ... + # 33| element #-----| -> self @@ -317,7 +317,7 @@ break_ensure.rb: #-----| -> In # 47| if ... -#-----| -> ensure ... +#-----| -> element # 47| ... > ... #-----| false -> if ... @@ -330,7 +330,7 @@ break_ensure.rb: #-----| -> ... > ... # 48| call to raise -#-----| raise -> [ensure: raise] ensure ... +#-----| raise -> [ensure: raise] element # 48| self #-----| -> "" @@ -339,17 +339,17 @@ break_ensure.rb: #-----| -> call to raise # 50| ensure ... -#-----| -> element - -# 50| [ensure: raise] ensure ... -#-----| -> [ensure: raise] element - -# 51| if ... #-----| -> do ... -# 51| [ensure: raise] if ... +# 50| [ensure: raise] ensure ... #-----| raise -> for ... in ... +# 51| if ... +#-----| -> ensure ... + +# 51| [ensure: raise] if ... +#-----| -> [ensure: raise] ensure ... + # 51| ... > ... #-----| false -> if ... #-----| true -> 10 @@ -1059,7 +1059,7 @@ cfg.rb: #-----| -> self # 82| else ... -#-----| -> ensure ... +#-----| -> self # 83| call to puts #-----| -> else ... @@ -1071,10 +1071,10 @@ cfg.rb: #-----| -> call to puts # 84| ensure ... -#-----| -> self +#-----| -> escape # 85| call to puts -#-----| -> escape +#-----| -> ensure ... # 85| self #-----| -> "end" @@ -3312,7 +3312,7 @@ raise.rb: #-----| -> ... > ... # 70| call to raise -#-----| raise -> [ensure: raise] ensure ... +#-----| raise -> [ensure: raise] self # 70| self #-----| -> "x > 2" @@ -3334,13 +3334,13 @@ raise.rb: #-----| -> ... < ... # 72| return -#-----| return -> [ensure: return] ensure ... +#-----| return -> [ensure: return] self # 72| "x < 0" #-----| -> return # 74| call to puts -#-----| -> ensure ... +#-----| -> self # 74| self #-----| -> "0 <= x <= 2" @@ -3349,23 +3349,23 @@ raise.rb: #-----| -> call to puts # 75| ensure ... -#-----| -> self - -# 75| [ensure: return] ensure ... -#-----| -> [ensure: return] self - -# 75| [ensure: raise] ensure ... -#-----| -> [ensure: raise] self - -# 76| call to puts #-----| -> exit m7 (normal) -# 76| [ensure: return] call to puts +# 75| [ensure: return] ensure ... #-----| return -> exit m7 (normal) -# 76| [ensure: raise] call to puts +# 75| [ensure: raise] ensure ... #-----| raise -> exit m7 (abnormal) +# 76| call to puts +#-----| -> ensure ... + +# 76| [ensure: return] call to puts +#-----| -> [ensure: return] ensure ... + +# 76| [ensure: raise] call to puts +#-----| -> [ensure: raise] ensure ... + # 76| self #-----| -> "ensure" @@ -3424,7 +3424,7 @@ raise.rb: #-----| -> ... > ... # 83| call to raise -#-----| raise -> [ensure: raise] ensure ... +#-----| raise -> [ensure: raise] self # 83| self #-----| -> "x > 2" @@ -3446,13 +3446,13 @@ raise.rb: #-----| -> ... < ... # 85| return -#-----| return -> [ensure: return] ensure ... +#-----| return -> [ensure: return] self # 85| "x < 0" #-----| -> return # 87| call to puts -#-----| -> ensure ... +#-----| -> self # 87| self #-----| -> "0 <= x <= 2" @@ -3464,20 +3464,20 @@ raise.rb: #-----| -> self # 88| [ensure: return] ensure ... -#-----| -> [ensure: return] self - -# 88| [ensure: raise] ensure ... -#-----| -> [ensure: raise] self - -# 89| call to puts -#-----| -> self - -# 89| [ensure: return] call to puts #-----| return -> exit m8 (normal) -# 89| [ensure: raise] call to puts +# 88| [ensure: raise] ensure ... #-----| raise -> exit m8 (abnormal) +# 89| call to puts +#-----| -> ensure ... + +# 89| [ensure: return] call to puts +#-----| -> [ensure: return] ensure ... + +# 89| [ensure: raise] call to puts +#-----| -> [ensure: raise] ensure ... + # 89| self #-----| -> "ensure" @@ -3551,7 +3551,7 @@ raise.rb: #-----| -> ... > ... # 98| call to raise -#-----| raise -> [ensure: raise] ensure ... +#-----| raise -> [ensure: raise] self # 98| self #-----| -> "x > 2" @@ -3573,13 +3573,13 @@ raise.rb: #-----| -> ... < ... # 100| return -#-----| return -> [ensure: return] ensure ... +#-----| return -> [ensure: return] self # 100| "x < 0" #-----| -> return # 102| call to puts -#-----| -> ensure ... +#-----| -> self # 102| self #-----| -> "0 <= x <= 2" @@ -3591,10 +3591,10 @@ raise.rb: #-----| -> self # 103| [ensure: return] ensure ... -#-----| -> [ensure: return] self +#-----| return -> [ensure: return] self # 103| [ensure: raise] ensure ... -#-----| -> [ensure: raise] self +#-----| raise -> [ensure: raise] self # 104| call to puts #-----| -> b1 @@ -3624,13 +3624,13 @@ raise.rb: #-----| -> [ensure: raise] call to puts # 106| if ... -#-----| -> ensure ... +#-----| -> self # 106| [ensure: return] if ... -#-----| -> [ensure: return] ensure ... +#-----| -> [ensure: return] self # 106| [ensure: raise] if ... -#-----| -> [ensure: raise] ensure ... +#-----| -> [ensure: raise] self # 106| b1 #-----| false -> if ... @@ -3645,13 +3645,13 @@ raise.rb: #-----| true -> [ensure: raise] self # 107| call to raise -#-----| raise -> [ensure(1): raise] ensure ... +#-----| raise -> [ensure(1): raise] self # 107| [ensure: return] call to raise -#-----| raise -> [ensure: return, ensure(1): raise] ensure ... +#-----| raise -> [ensure: return, ensure(1): raise] self # 107| [ensure: raise] call to raise -#-----| raise -> [ensure: raise, ensure(1): raise] ensure ... +#-----| raise -> [ensure: raise, ensure(1): raise] self # 107| self #-----| -> "b1 is true" @@ -3672,40 +3672,40 @@ raise.rb: #-----| -> [ensure: raise] call to raise # 109| ensure ... -#-----| -> self +#-----| -> ensure ... # 109| [ensure(1): raise] ensure ... -#-----| -> [ensure(1): raise] self +#-----| raise -> [ensure: raise] self # 109| [ensure: return] ensure ... -#-----| -> [ensure: return] self +#-----| -> [ensure: return] ensure ... # 109| [ensure: return, ensure(1): raise] ensure ... -#-----| -> [ensure: return, ensure(1): raise] self +#-----| raise -> [ensure: raise] self # 109| [ensure: raise] ensure ... -#-----| -> [ensure: raise] self +#-----| -> [ensure: raise] ensure ... # 109| [ensure: raise, ensure(1): raise] ensure ... -#-----| -> [ensure: raise, ensure(1): raise] self +#-----| raise -> [ensure: raise] self # 110| call to puts -#-----| -> self +#-----| -> ensure ... # 110| [ensure(1): raise] call to puts -#-----| raise -> [ensure: raise] ensure ... +#-----| -> [ensure(1): raise] ensure ... # 110| [ensure: return] call to puts -#-----| return -> [ensure: return] ensure ... +#-----| -> [ensure: return] ensure ... # 110| [ensure: return, ensure(1): raise] call to puts -#-----| raise -> [ensure: raise] ensure ... +#-----| -> [ensure: return, ensure(1): raise] ensure ... # 110| [ensure: raise] call to puts -#-----| raise -> [ensure: raise] ensure ... +#-----| -> [ensure: raise] ensure ... # 110| [ensure: raise, ensure(1): raise] call to puts -#-----| raise -> [ensure: raise] ensure ... +#-----| -> [ensure: raise, ensure(1): raise] ensure ... # 110| self #-----| -> "inner ensure" @@ -3744,7 +3744,7 @@ raise.rb: #-----| -> [ensure: raise, ensure(1): raise] call to puts # 113| call to puts -#-----| -> ensure ... +#-----| -> self # 113| self #-----| -> "End m9" @@ -3753,13 +3753,13 @@ raise.rb: #-----| -> call to puts # 114| ensure ... -#-----| -> self +#-----| -> exit m9 (normal) # 114| [ensure: return] ensure ... -#-----| -> [ensure: return] self +#-----| return -> exit m9 (normal) # 114| [ensure: raise] ensure ... -#-----| -> [ensure: raise] self +#-----| raise -> exit m9 (abnormal) # 115| call to puts #-----| -> b2 @@ -3789,13 +3789,13 @@ raise.rb: #-----| -> [ensure: raise] call to puts # 116| if ... -#-----| -> exit m9 (normal) +#-----| -> ensure ... # 116| [ensure: return] if ... -#-----| return -> exit m9 (normal) +#-----| -> [ensure: return] ensure ... # 116| [ensure: raise] if ... -#-----| raise -> exit m9 (abnormal) +#-----| -> [ensure: raise] ensure ... # 116| b2 #-----| false -> if ... @@ -3851,8 +3851,8 @@ raise.rb: #-----| -> exit m10 # 121| p -#-----| match -> ensure ... #-----| no-match -> self +#-----| match -> self # 121| call to raise #-----| raise -> exit m10 (abnormal) @@ -3864,10 +3864,10 @@ raise.rb: #-----| -> call to raise # 124| ensure ... -#-----| -> self +#-----| -> exit m10 (normal) # 125| call to puts -#-----| -> exit m10 (normal) +#-----| -> ensure ... # 125| self #-----| -> "Will not get executed if p is..." @@ -3893,7 +3893,7 @@ raise.rb: #-----| -> b # 130| if ... -#-----| -> ensure ... +#-----| -> self # 130| b #-----| false -> if ... @@ -3912,18 +3912,18 @@ raise.rb: #-----| -> ExceptionA # 133| ExceptionA -#-----| match -> ensure ... #-----| no-match -> rescue ... +#-----| match -> self # 134| rescue ... #-----| -> ExceptionB # 134| ExceptionB -#-----| raise -> [ensure: raise] ensure ... #-----| match -> self +#-----| raise -> [ensure: raise] self # 134| then ... -#-----| -> ensure ... +#-----| -> self # 135| call to puts #-----| -> then ... @@ -3938,13 +3938,13 @@ raise.rb: #-----| -> self # 136| [ensure: raise] ensure ... -#-----| -> [ensure: raise] self +#-----| raise -> exit m11 (abnormal) # 137| call to puts -#-----| -> self +#-----| -> ensure ... # 137| [ensure: raise] call to puts -#-----| raise -> exit m11 (abnormal) +#-----| -> [ensure: raise] ensure ... # 137| self #-----| -> "Ensure" @@ -3982,14 +3982,14 @@ raise.rb: #-----| -> b # 143| if ... -#-----| -> ensure ... +#-----| -> 3 # 143| b #-----| false -> if ... #-----| true -> self # 144| call to raise -#-----| raise -> [ensure: raise] ensure ... +#-----| raise -> [ensure: raise] 3 # 144| self #-----| -> "" @@ -3997,12 +3997,6 @@ raise.rb: # 144| "" #-----| -> call to raise -# 146| ensure ... -#-----| -> 3 - -# 146| [ensure: raise] ensure ... -#-----| -> [ensure: raise] 3 - # 147| return #-----| return -> exit m12 (normal) @@ -4015,20 +4009,9 @@ raise.rb: # 147| [ensure: raise] 3 #-----| -> [ensure: raise] return -# 150| enter m13 -#-----| -> ensure ... - # 150| m13 #-----| -> m14 -# 150| exit m13 - -# 150| exit m13 (normal) -#-----| -> exit m13 - -# 151| ensure ... -#-----| -> exit m13 (normal) - # 154| enter m14 #-----| -> element From 94ceb3f237e66780b2f6655bd352cdff78623178 Mon Sep 17 00:00:00 2001 From: Nick Rolfe Date: Fri, 7 May 2021 17:20:51 +0100 Subject: [PATCH 12/18] Remove unused class --- .../internal/ControlFlowGraphImpl.qll | 39 ------------------- 1 file changed, 39 deletions(-) diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll index 30fba96e2cd..166443735b2 100644 --- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -1129,45 +1129,6 @@ module Trees { } } - /** - * Control-flow tree for any pre-order StmtSequence that doesn't have a more - * specific implementation. - * TODO: make all StmtSequence tree classes post-order, and simplify class - * hierarchy. - */ - private class SimplePreOrderStmtSequenceTree extends StmtSequenceTree, PreOrderTree { - SimplePreOrderStmtSequenceTree() { - not this instanceof BodyStmtTree and - not this instanceof EndBlock and - not this instanceof StringInterpolationComponent and - not this instanceof Block and - not this instanceof ParenthesizedExpr and - not this instanceof BeginBlock and - not this instanceof ASTInternal::TThen and - not this instanceof ASTInternal::TDo and - not this instanceof ASTInternal::TElse and - not this instanceof ASTInternal::TEnsure - } - - final override predicate propagatesAbnormal(AstNode child) { child = this.getAStmt() } - - final override predicate last(AstNode last, Completion c) { - last(this.getLastStmt(), last, c) - or - not exists(this.getLastStmt()) and - c.isValidFor(this) and - last = this - } - - final override predicate succ(AstNode pred, AstNode succ, Completion c) { - pred = this and - first(this.getBodyChild(0, _), succ) and - c instanceof SimpleCompletion - or - StmtSequenceTree.super.succ(pred, succ, c) - } - } - private class SingletonClassTree extends BodyStmtPostOrderTree, SingletonClass { final override predicate first(AstNode first) { this.firstInner(first) From 004147984b4fa24fd59777a46387b94b6f2e2265 Mon Sep 17 00:00:00 2001 From: Nick Rolfe Date: Tue, 11 May 2021 18:27:11 +0100 Subject: [PATCH 13/18] Simplify CFG classes for StmtSequences --- .../internal/ControlFlowGraphImpl.qll | 95 ++++++++----------- 1 file changed, 38 insertions(+), 57 deletions(-) diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll index 166443735b2..d8762f66cc8 100644 --- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -303,6 +303,8 @@ module Trees { final override predicate first(AstNode first) { this.firstInner(first) } final override predicate last(AstNode last, Completion c) { this.lastInner(last, c) } + + final override predicate propagatesAbnormal(AstNode child) { none() } } private class BinaryOperationTree extends StandardPostOrderTree, BinaryOperation { @@ -338,11 +340,9 @@ module Trees { private class BlockParameterTree extends NonDefaultValueParameterTree, BlockParameter { } - /** - * TODO: make all StmtSequence tree classes post-order, and simplify class - * hierarchy. - */ abstract class BodyStmtTree extends StmtSequenceTree, BodyStmt { + override predicate first(AstNode first) { first = this } + predicate firstInner(AstNode first) { first(this.getBodyChild(0, _), first) or @@ -523,10 +523,6 @@ module Trees { } } - abstract class BodyStmtPostOrderTree extends BodyStmtTree, PostOrderTree { - override predicate first(AstNode first) { first = this } - } - private class BooleanLiteralTree extends LeafTree, BooleanLiteral { } class BraceBlockTree extends ScopeTree, BraceBlock { @@ -584,7 +580,7 @@ module Trees { private class CharacterTree extends LeafTree, CharacterLiteral { } - private class ClassDeclarationTree extends BodyStmtPostOrderTree, ClassDeclaration { + private class ClassDeclarationTree extends BodyStmtTree, ClassDeclaration { final override predicate first(AstNode first) { this.firstInner(first) or @@ -593,7 +589,7 @@ module Trees { } final override predicate succ(AstNode pred, AstNode succ, Completion c) { - BodyStmtPostOrderTree.super.succ(pred, succ, c) + BodyStmtTree.super.succ(pred, succ, c) or succ = this and this.lastInner(pred, c) @@ -728,13 +724,15 @@ module Trees { } } - private class DoBlockTree extends BodyStmtPostOrderTree, DoBlock { + private class DoBlockTree extends BodyStmtTree, DoBlock { /** Gets the `i`th child in the body of this block. */ final override AstNode getBodyChild(int i, boolean rescuable) { result = this.getParameter(i) and rescuable = false or - result = BodyStmtPostOrderTree.super.getBodyChild(i - this.getNumberOfParameters(), rescuable) + result = BodyStmtTree.super.getBodyChild(i - this.getNumberOfParameters(), rescuable) } + + override predicate propagatesAbnormal(AstNode child) { none() } } private class EmptyStatementTree extends LeafTree, EmptyStmt { } @@ -850,12 +848,12 @@ module Trees { final override AstNode getAccessNode() { result = this.getDefiningAccess() } } - private class LambdaTree extends BodyStmtPostOrderTree, Lambda { + private class LambdaTree extends BodyStmtTree, Lambda { /** Gets the `i`th child in the body of this block. */ final override AstNode getBodyChild(int i, boolean rescuable) { result = this.getParameter(i) and rescuable = false or - result = BodyStmtPostOrderTree.super.getBodyChild(i - this.getNumberOfParameters(), rescuable) + result = BodyStmtTree.super.getBodyChild(i - this.getNumberOfParameters(), rescuable) } } @@ -925,16 +923,18 @@ module Trees { private class MethodNameTree extends LeafTree, MethodName, ASTInternal::TTokenMethodName { } - private class MethodTree extends BodyStmtPostOrderTree, Method { + private class MethodTree extends BodyStmtTree, Method { + final override predicate propagatesAbnormal(AstNode child) { none() } + /** Gets the `i`th child in the body of this block. */ final override AstNode getBodyChild(int i, boolean rescuable) { result = this.getParameter(i) and rescuable = false or - result = BodyStmtPostOrderTree.super.getBodyChild(i - this.getNumberOfParameters(), rescuable) + result = BodyStmtTree.super.getBodyChild(i - this.getNumberOfParameters(), rescuable) } } - private class ModuleDeclarationTree extends BodyStmtPostOrderTree, ModuleDeclaration { + private class ModuleDeclarationTree extends BodyStmtTree, ModuleDeclaration { final override predicate first(AstNode first) { this.firstInner(first) or @@ -943,7 +943,7 @@ module Trees { } final override predicate succ(AstNode pred, AstNode succ, Completion c) { - BodyStmtPostOrderTree.super.succ(pred, succ, c) + BodyStmtTree.super.succ(pred, succ, c) or succ = this and this.lastInner(pred, c) @@ -953,7 +953,7 @@ module Trees { final override AstNode getBodyChild(int i, boolean rescuable) { result = this.getScopeExpr() and i = 0 and rescuable = false or - result = BodyStmtPostOrderTree.super.getBodyChild(i - count(this.getScopeExpr()), rescuable) + result = BodyStmtTree.super.getBodyChild(i - count(this.getScopeExpr()), rescuable) } } @@ -1099,37 +1099,7 @@ module Trees { SimpleParameterTreeDupUnderscore() { not exists(this.getDefiningAccess()) } } - /** - * Control-flow tree for any post-order StmtSequence that doesn't have a more - * specific implementation. - * TODO: make all StmtSequence tree classes post-order, and simplify class - * hierarchy. - */ - private class SimplePostOrderStmtSequenceTree extends StmtSequenceTree, PostOrderTree { - SimplePostOrderStmtSequenceTree() { - this instanceof StringInterpolationComponent or - this instanceof ParenthesizedExpr or - this instanceof BeginBlock or - this instanceof ASTInternal::TThen or - this instanceof ASTInternal::TDo or - this instanceof ASTInternal::TElse or - this instanceof ASTInternal::TEnsure - } - - final override predicate first(AstNode first) { first(this.getStmt(0), first) } - - final override predicate propagatesAbnormal(AstNode child) { child = this.getAStmt() } - - final override predicate succ(AstNode pred, AstNode succ, Completion c) { - succ = this and - last(this.getLastBodyChild(), pred, c) and - c instanceof NormalCompletion - or - StmtSequenceTree.super.succ(pred, succ, c) - } - } - - private class SingletonClassTree extends BodyStmtPostOrderTree, SingletonClass { + private class SingletonClassTree extends BodyStmtTree, SingletonClass { final override predicate first(AstNode first) { this.firstInner(first) or @@ -1138,7 +1108,7 @@ module Trees { } final override predicate succ(AstNode pred, AstNode succ, Completion c) { - BodyStmtPostOrderTree.super.succ(pred, succ, c) + BodyStmtTree.super.succ(pred, succ, c) or succ = this and this.lastInner(pred, c) @@ -1149,23 +1119,23 @@ module Trees { ( result = this.getValue() and i = 0 and rescuable = false or - result = BodyStmtPostOrderTree.super.getBodyChild(i - 1, rescuable) + result = BodyStmtTree.super.getBodyChild(i - 1, rescuable) ) } } - private class SingletonMethodTree extends BodyStmtPostOrderTree, SingletonMethod { + private class SingletonMethodTree extends BodyStmtTree, SingletonMethod { /** Gets the `i`th child in the body of this block. */ final override AstNode getBodyChild(int i, boolean rescuable) { result = this.getParameter(i) and rescuable = false or - result = BodyStmtPostOrderTree.super.getBodyChild(i - this.getNumberOfParameters(), rescuable) + result = BodyStmtTree.super.getBodyChild(i - this.getNumberOfParameters(), rescuable) } override predicate first(AstNode first) { first(this.getObject(), first) } override predicate succ(AstNode pred, AstNode succ, Completion c) { - BodyStmtPostOrderTree.super.succ(pred, succ, c) + BodyStmtTree.super.succ(pred, succ, c) or last(this.getObject(), pred, c) and succ = this and @@ -1179,8 +1149,15 @@ module Trees { private class SplatParameterTree extends NonDefaultValueParameterTree, SplatParameter { } - abstract class StmtSequenceTree extends ControlFlowTree, StmtSequence { - override predicate propagatesAbnormal(AstNode child) { none() } + class StmtSequenceTree extends PostOrderTree, StmtSequence { + StmtSequenceTree() { + not this instanceof BraceBlock and + not this instanceof EndBlock + } + + override predicate propagatesAbnormal(AstNode child) { child = this.getAStmt() } + + override predicate first(AstNode first) { first(this.getStmt(0), first) } /** Gets the `i`th child in the body of this body statement. */ AstNode getBodyChild(int i, boolean rescuable) { @@ -1202,6 +1179,10 @@ module Trees { first(this.getBodyChild(i + 1, _), succ) and c instanceof NormalCompletion ) + or + succ = this and + last(this.getLastBodyChild(), pred, c) and + c instanceof NormalCompletion } } From 5e6dddad3e0405bb78066ebb347e790db2c176be Mon Sep 17 00:00:00 2001 From: Nick Rolfe Date: Thu, 13 May 2021 16:59:05 +0100 Subject: [PATCH 14/18] Replace count(getReceiver()) with 1 --- .../codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll index d8762f66cc8..0415e8339cd 100644 --- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -915,9 +915,9 @@ module Trees { final override ControlFlowTree getChildNode(int i) { result = this.getReceiver() and i = 0 or - result = this.getArgument(i - count(this.getReceiver())) + result = this.getArgument(i - 1) or - result = this.getBlock() and i = count(this.getReceiver()) + this.getNumberOfArguments() + result = this.getBlock() and i = 1 + this.getNumberOfArguments() } } From a46f45440ae3d0b39ba38d80d32492add16f312f Mon Sep 17 00:00:00 2001 From: Nick Rolfe Date: Thu, 13 May 2021 17:52:20 +0100 Subject: [PATCH 15/18] Create NamespaceTree to reduce duplication --- .../internal/ControlFlowGraphImpl.qll | 40 +++++++------------ 1 file changed, 15 insertions(+), 25 deletions(-) diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll index 0415e8339cd..14f67e3c3f3 100644 --- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -580,21 +580,7 @@ module Trees { private class CharacterTree extends LeafTree, CharacterLiteral { } - private class ClassDeclarationTree extends BodyStmtTree, ClassDeclaration { - final override predicate first(AstNode first) { - this.firstInner(first) - or - not exists(this.getAChild(_)) and - first = this - } - - final override predicate succ(AstNode pred, AstNode succ, Completion c) { - BodyStmtTree.super.succ(pred, succ, c) - or - succ = this and - this.lastInner(pred, c) - } - + private class ClassDeclarationTree extends NamespaceTree, ClassDeclaration { /** Gets the `i`th child in the body of this block. */ final override AstNode getBodyChild(int i, boolean rescuable) { result = this.getScopeExpr() and i = 0 and rescuable = false @@ -603,8 +589,10 @@ module Trees { i = count(this.getScopeExpr()) and rescuable = true or - result = this.getStmt(i - count(this.getScopeExpr()) - count(this.getSuperclassExpr())) and - rescuable = true + result = + NamespaceTree.super + .getBodyChild(i - count(this.getScopeExpr()) - count(this.getSuperclassExpr()), + rescuable) } } @@ -934,7 +922,16 @@ module Trees { } } - private class ModuleDeclarationTree extends BodyStmtTree, ModuleDeclaration { + private class ModuleDeclarationTree extends NamespaceTree, ModuleDeclaration { + /** Gets the `i`th child in the body of this block. */ + final override AstNode getBodyChild(int i, boolean rescuable) { + result = this.getScopeExpr() and i = 0 and rescuable = false + or + result = NamespaceTree.super.getBodyChild(i - count(this.getScopeExpr()), rescuable) + } + } + + private class NamespaceTree extends BodyStmtTree, Namespace { final override predicate first(AstNode first) { this.firstInner(first) or @@ -948,13 +945,6 @@ module Trees { succ = this and this.lastInner(pred, c) } - - /** Gets the `i`th child in the body of this block. */ - final override AstNode getBodyChild(int i, boolean rescuable) { - result = this.getScopeExpr() and i = 0 and rescuable = false - or - result = BodyStmtTree.super.getBodyChild(i - count(this.getScopeExpr()), rescuable) - } } private class NilTree extends LeafTree, NilLiteral { } From 6d395230d4ba084b92d65ee05f6c6e1d866e257d Mon Sep 17 00:00:00 2001 From: Nick Rolfe Date: Mon, 17 May 2021 14:54:11 +0100 Subject: [PATCH 16/18] Make BraceBlockTree extend StmtSequenceTree --- .../internal/ControlFlowGraphImpl.qll | 30 ++++++++++++------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll index 14f67e3c3f3..ebb1abbc989 100644 --- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -75,11 +75,11 @@ module CfgScope { private class BraceBlockScope extends Range_, BraceBlock { final override predicate entry(AstNode first) { - first(this.(Trees::BraceBlockTree).getFirstChildNode(), first) + first(this.(Trees::BraceBlockTree).getBodyChild(0, _), first) } final override predicate exit(AstNode last, Completion c) { - last(this.(Trees::BraceBlockTree).getLastChildNode(), last, c) + last(this.(Trees::BraceBlockTree).getLastBodyChild(), last, c) } } } @@ -525,11 +525,24 @@ module Trees { private class BooleanLiteralTree extends LeafTree, BooleanLiteral { } - class BraceBlockTree extends ScopeTree, BraceBlock { - final override ControlFlowTree getChildNode(int i) { - result = this.getParameter(i) + class BraceBlockTree extends StmtSequenceTree, BraceBlock { + final override predicate propagatesAbnormal(AstNode child) { none() } + + final override AstNode getBodyChild(int i, boolean rescuable) { + result = this.getParameter(i) and rescuable = false or - result = this.getStmt(i - this.getNumberOfParameters()) + result = StmtSequenceTree.super.getBodyChild(i - this.getNumberOfParameters(), rescuable) + } + + override predicate first(AstNode first) { first = this } + + override predicate succ(AstNode pred, AstNode succ, Completion c) { + // Normal left-to-right evaluation in the body + exists(int i | + last(this.getBodyChild(i, _), pred, c) and + first(this.getBodyChild(i + 1, _), succ) and + c instanceof NormalCompletion + ) } } @@ -1140,10 +1153,7 @@ module Trees { private class SplatParameterTree extends NonDefaultValueParameterTree, SplatParameter { } class StmtSequenceTree extends PostOrderTree, StmtSequence { - StmtSequenceTree() { - not this instanceof BraceBlock and - not this instanceof EndBlock - } + StmtSequenceTree() { not this instanceof EndBlock } override predicate propagatesAbnormal(AstNode child) { child = this.getAStmt() } From 9a2523e2f91fbd74c52336f742607298c4e03145 Mon Sep 17 00:00:00 2001 From: Nick Rolfe Date: Mon, 17 May 2021 14:57:32 +0100 Subject: [PATCH 17/18] Make EndBlockTree extend StmtSequenceTree --- .../internal/ControlFlowGraphImpl.qll | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll index ebb1abbc989..313bf8807b9 100644 --- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -57,11 +57,11 @@ module CfgScope { private class EndBlockScope extends Range_, EndBlock { final override predicate entry(AstNode first) { - first(this.(Trees::EndBlockTree).getFirstChildNode(), first) + first(this.(Trees::EndBlockTree).getBodyChild(0, _), first) } final override predicate exit(AstNode last, Completion c) { - last(this.(Trees::EndBlockTree).getLastChildNode(), last, c) + last(this.(Trees::EndBlockTree).getLastBodyChild(), last, c) } } @@ -738,8 +738,17 @@ module Trees { private class EmptyStatementTree extends LeafTree, EmptyStmt { } - class EndBlockTree extends ScopeTree, EndBlock { - final override ControlFlowTree getChildNode(int i) { result = this.getStmt(i) } + class EndBlockTree extends StmtSequenceTree, EndBlock { + override predicate first(AstNode first) { first = this } + + override predicate succ(AstNode pred, AstNode succ, Completion c) { + // Normal left-to-right evaluation in the body + exists(int i | + last(this.getBodyChild(i, _), pred, c) and + first(this.getBodyChild(i + 1, _), succ) and + c instanceof NormalCompletion + ) + } } private class ForInTree extends LeafTree, ForIn { } @@ -1153,8 +1162,6 @@ module Trees { private class SplatParameterTree extends NonDefaultValueParameterTree, SplatParameter { } class StmtSequenceTree extends PostOrderTree, StmtSequence { - StmtSequenceTree() { not this instanceof EndBlock } - override predicate propagatesAbnormal(AstNode child) { child = this.getAStmt() } override predicate first(AstNode first) { first(this.getStmt(0), first) } From f3d831c25ed1982b748131277189cc81998aef56 Mon Sep 17 00:00:00 2001 From: Nick Rolfe Date: Mon, 17 May 2021 15:26:53 +0100 Subject: [PATCH 18/18] Remove unnecessary superclass prefix --- .../codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll index 313bf8807b9..acde284662b 100644 --- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -603,7 +603,7 @@ module Trees { rescuable = true or result = - NamespaceTree.super + super .getBodyChild(i - count(this.getScopeExpr()) - count(this.getSuperclassExpr()), rescuable) }