From 8501e30b6a1f37d82a9185669def6085dc54dfd1 Mon Sep 17 00:00:00 2001 From: Arthur Baars Date: Tue, 15 Dec 2020 12:44:16 +0100 Subject: [PATCH 01/15] CFG: fix linking heredoc start to heredoc body --- .../internal/ControlFlowGraphImpl.qll | 39 +++++++++---------- .../controlflow/graph/Cfg.expected | 30 ++++++++++++++ .../controlflow/graph/heredoc.rb | 7 ++++ 3 files changed, 56 insertions(+), 20 deletions(-) create mode 100644 ql/test/library-tests/controlflow/graph/heredoc.rb diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll index adff80357b8..cc4f3d06811 100644 --- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -37,6 +37,7 @@ private import codeql_ruby.controlflow.ControlFlowGraph private import Completion private import SuccessorTypes private import Splitting +private import codeql.files.FileSystem private AstNode parent(AstNode n) { result.getAFieldOrChild() = n and @@ -606,29 +607,27 @@ module Trees { private class HashSplatParameterTree extends LeafTree, HashSplatParameter { } - private class HeredocBeginningTree extends StandardPreOrderTree, HeredocBeginning { - pragma[noinline] - private string getName() { - result = this.getValue().regexpCapture("^<<[-~]?[`']?(.*)[`']?$", 1) - } + private HeredocBody heredoc(HeredocBeginning start) { + exists(int i, File f | + start = + rank[i](HeredocBeginning b | + f = b.getLocation().getFile() + | + b order by b.getLocation().getStartLine(), b.getLocation().getStartColumn() + ) and + result = + rank[i](HeredocBody b | + f = b.getLocation().getFile() + | + b order by b.getLocation().getStartLine(), b.getLocation().getStartColumn() + ) + ) + } + private class HeredocBeginningTree extends StandardPreOrderTree, HeredocBeginning { final override AstNode getChildNode(int i) { i = 0 and - result = - min(string name, HeredocBody doc, HeredocEnd end | - name = this.getName() and - end = unique(HeredocEnd x | x = doc.getChild(_) | x) and - end.getValue() = name and - doc.getLocation().getFile() = this.getLocation().getFile() and - ( - doc.getLocation().getStartLine() > this.getLocation().getStartLine() - or - doc.getLocation().getStartLine() = this.getLocation().getStartLine() and - doc.getLocation().getStartColumn() > this.getLocation().getStartColumn() - ) - | - doc order by doc.getLocation().getStartLine(), doc.getLocation().getStartColumn() - ) + result = heredoc(this) } } diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected index ca1694f09fc..88146edbf0c 100644 --- a/ql/test/library-tests/controlflow/graph/Cfg.expected +++ b/ql/test/library-tests/controlflow/graph/Cfg.expected @@ -68,6 +68,10 @@ exit.rb: # 8| enter m2 #-----| -> x +heredoc.rb: +# 1| enter double_heredoc +#-----| -> < x @@ -1950,6 +1954,25 @@ exit.rb: # 12| String #-----| -> puts +heredoc.rb: +# 2| MethodCall +#-----| -> exit double_heredoc (normal) + +# 2| puts +#-----| -> MethodCall + +# 2| < HeredocBody + +# 2| < HeredocBody + +# 2| HeredocBody +#-----| -> < puts + ifs.rb: # 1| x #-----| -> x @@ -3380,6 +3403,9 @@ exit.rb: # 8| exit m2 +heredoc.rb: +# 1| exit double_heredoc + ifs.rb: # 1| exit m1 @@ -3505,6 +3531,10 @@ exit.rb: # 8| exit m2 (normal) #-----| -> exit m2 +heredoc.rb: +# 1| exit double_heredoc (normal) +#-----| -> exit double_heredoc + ifs.rb: # 1| exit m1 (normal) #-----| -> exit m1 diff --git a/ql/test/library-tests/controlflow/graph/heredoc.rb b/ql/test/library-tests/controlflow/graph/heredoc.rb new file mode 100644 index 00000000000..09e1b771ea8 --- /dev/null +++ b/ql/test/library-tests/controlflow/graph/heredoc.rb @@ -0,0 +1,7 @@ +def double_heredoc + puts(< Date: Tue, 15 Dec 2020 14:49:50 +0100 Subject: [PATCH 02/15] CFG: drop getObject from flow of singleton method --- .../controlflow/internal/ControlFlowGraphImpl.qll | 8 ++------ ql/test/library-tests/controlflow/graph/Cfg.expected | 5 +---- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll index cc4f3d06811..c9c7364e1cb 100644 --- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -1203,15 +1203,11 @@ module Trees { private class SingletonMethodTree extends RescueEnsureBlockTree, SingletonMethod { final override AstNode getChildNode(int i, boolean rescuable) { - result = this.getObject() and + result = this.getParameters() and i = 0 and rescuable = false or - result = this.getParameters() and - i = 1 and - rescuable = false - or - result = this.getChild(i - 2) and + result = this.getChild(i - 1) and rescuable = true } diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected index 88146edbf0c..204f8781111 100644 --- a/ql/test/library-tests/controlflow/graph/Cfg.expected +++ b/ql/test/library-tests/controlflow/graph/Cfg.expected @@ -50,7 +50,7 @@ cfg.rb: #-----| -> String # 149| enter method -#-----| -> silly +#-----| -> SplatParameter # 153| enter two_parameters #-----| -> a @@ -1543,9 +1543,6 @@ cfg.rb: # 148| new #-----| -> Call -# 149| silly -#-----| -> SplatParameter - # 149| SplatParameter #-----| -> x From 69de81bdd5ae99406b3ad00faf25723a9eda5127 Mon Sep 17 00:00:00 2001 From: Arthur Baars Date: Tue, 15 Dec 2020 16:19:28 +0100 Subject: [PATCH 03/15] CFG: have alternative flow for the definition and call of methods etc. --- .../controlflow/ControlFlowGraph.qll | 12 +++ .../internal/ControlFlowGraphImpl.qll | 90 +++++++++++++------ .../controlflow/internal/Splitting.qll | 2 +- 3 files changed, 75 insertions(+), 29 deletions(-) diff --git a/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll b/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll index 8df84ffd371..89ed39fd8a6 100644 --- a/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll +++ b/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll @@ -38,6 +38,18 @@ class CfgScope extends AstNode, CfgScopeRange { this instanceof Lambda and result = "lambda" } + + predicate entry(AstNode first) { + first(this.(Trees::RescueEnsureBlockTree).firstBody(), first) + or + not this instanceof Trees::RescueEnsureBlockTree and first(this, first) + } + + predicate exit(AstNode last, Completion c) { + this.(Trees::RescueEnsureBlockTree).lastBody(last, c) + or + not this instanceof Trees::RescueEnsureBlockTree and last(this, last, c) + } } /** diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll index c9c7364e1cb..bf8df829838 100644 --- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -124,7 +124,7 @@ predicate succ(AstNode pred, AstNode succ, Completion c) { pragma[nomagic] predicate succEntry(CfgScope scope, AstNode first) { exists(AstNode n | - first(scope, n) and + scope.entry(n) and succImplIfHidden*(n, first) and not isHidden(first) ) @@ -134,7 +134,7 @@ predicate succEntry(CfgScope scope, AstNode first) { pragma[nomagic] predicate succExit(CfgScope scope, AstNode last, Completion c) { exists(AstNode n | - last(scope, n, c) and + scope.exit(n, c) and succImplIfHidden*(last, n) and not isHidden(last) ) @@ -271,11 +271,13 @@ module Trees { final override Interpolation getChildNode(int i) { result = this.getChild(i) } } - private class BeginTree extends RescueEnsureBlockTree, Begin { + private class BeginTree extends RescueEnsureBlockTree, PreOrderTree, Begin { final override AstNode getChildNode(int i, boolean rescuable) { result = this.getChild(i) and rescuable = true } + final override predicate last(AstNode last, Completion c) { this.lastBody(last, c) } + override predicate isHidden() { any() } } @@ -379,12 +381,14 @@ module Trees { private class CharacterTree extends LeafTree, Character { } - private class ClassTree extends RescueEnsureBlockTree, Class { + private class ClassTree extends RescueEnsureBlockTree, PreOrderTree, Class { final override AstNode getChildNode(int i, boolean rescuable) { result = this.getName() and i = 0 and rescuable = false or result = this.getChild(i - 1) and rescuable = true } + + final override predicate last(AstNode last, Completion c) { this.lastBody(last, c) } } private class ClassVariableTree extends LeafTree, ClassVariable { } @@ -435,7 +439,7 @@ module Trees { override predicate isHidden() { any() } } - private class DoBlockTree extends RescueEnsureBlockTree, DoBlock { + private class DoBlockTree extends RescueEnsureBlockTree, PreOrderTree, PostOrderTree, DoBlock { final override AstNode getChildNode(int i, boolean rescuable) { result = this.getParameters() and i = 0 and rescuable = false or @@ -761,13 +765,23 @@ module Trees { } } - private class MethodTree extends RescueEnsureBlockTree, Method { + private class MethodTree extends RescueEnsureBlockTree, PostOrderTree, Method { final override AstNode getChildNode(int i, boolean rescuable) { result = this.getParameters() and i = 0 and rescuable = false or result = this.getChild(i - 1) and rescuable = true } + final override predicate first(AstNode first) { first(this.getName(), first) } + + override predicate succ(AstNode pred, AstNode succ, Completion c) { + RescueEnsureBlockTree.super.succ(pred, succ, c) + or + last(this.getName(), pred, c) and + succ = this and + c instanceof NormalCompletion + } + override predicate isHidden() { any() } } @@ -777,12 +791,14 @@ module Trees { override predicate isHidden() { any() } } - private class ModuleTree extends RescueEnsureBlockTree, Module { + private class ModuleTree extends RescueEnsureBlockTree, PreOrderTree, Module { final override AstNode getChildNode(int i, boolean rescuable) { result = this.getName() and i = 0 and rescuable = false or result = this.getChild(i - 1) and rescuable = true } + + final override predicate last(AstNode last, Completion c) { this.lastBody(last, c) } } private class NextTree extends StandardPostOrderTree, Next { @@ -919,7 +935,7 @@ module Trees { } /** A block that may contain `rescue`/`ensure`. */ - abstract class RescueEnsureBlockTree extends PreOrderTree { + abstract class RescueEnsureBlockTree extends ControlFlowTree { /** * Gets the `i`th child of this block. `rescuable` indicates whether exceptional * execution of the child can be caught by `rescue`/`ensure`. @@ -954,11 +970,7 @@ module Trees { final private predicate hasEnsure() { exists(this.getEnsure()) } - final override predicate propagatesAbnormal(AstNode child) { - child = this.getEnsure() - or - child = this.getBodyChild(_, false) - } + final override predicate propagatesAbnormal(AstNode child) { none() } /** * Gets a descendant that belongs to the `ensure` block of this block, if any. @@ -1069,7 +1081,7 @@ module Trees { nestLevel = this.nestLevel() } - override predicate last(AstNode last, Completion c) { + predicate lastBody(AstNode last, Completion c) { exists(boolean ensurable | last = this.getAnEnsurePredecessor(c, ensurable) | not this.hasEnsure() or @@ -1091,22 +1103,28 @@ module Trees { not exists(this.getBodyChild(_, _)) and not exists(this.getRescue(_)) and this.lastEnsure0(last, c) + or + last([this.getEnsure(), this.getBodyChild(_, false)], last, c) and + not c instanceof NormalCompletion } - final override predicate succ(AstNode pred, AstNode succ, Completion c) { + AstNode firstBody() { + result = this.getBodyChild(0, _) + or + not exists(this.getBodyChild(_, _)) and + ( + result = this.getRescue(0) + or + not exists(this.getRescue(_)) and + result = this.getEnsure() + ) + } + + override predicate succ(AstNode pred, AstNode succ, Completion c) { + this instanceof PreOrderTree and pred = this and c instanceof SimpleCompletion and - ( - first(this.getBodyChild(0, _), succ) - or - not exists(this.getBodyChild(_, _)) and - ( - first(this.getRescue(0), succ) - or - not exists(this.getRescue(_)) and - first(this.getEnsure(), succ) - ) - ) + first(this.firstBody(), succ) or // Normal left-to-right evaluation in the body exists(int i | @@ -1188,7 +1206,7 @@ module Trees { private class SetterTree extends LeafTree, Setter { } - private class SingletonClassTree extends RescueEnsureBlockTree, SingletonClass { + private class SingletonClassTree extends RescueEnsureBlockTree, PreOrderTree, SingletonClass { final override AstNode getChildNode(int i, boolean rescuable) { rescuable = true and ( @@ -1198,10 +1216,12 @@ module Trees { ) } + final override predicate last(AstNode last, Completion c) { this.lastBody(last, c) } + override predicate isHidden() { any() } } - private class SingletonMethodTree extends RescueEnsureBlockTree, SingletonMethod { + private class SingletonMethodTree extends RescueEnsureBlockTree, PostOrderTree, SingletonMethod { final override AstNode getChildNode(int i, boolean rescuable) { result = this.getParameters() and i = 0 and @@ -1211,6 +1231,20 @@ module Trees { rescuable = true } + final override predicate first(AstNode first) { first(this.getObject(), first) } + + override predicate succ(AstNode pred, AstNode succ, Completion c) { + RescueEnsureBlockTree.super.succ(pred, succ, c) + or + last(this.getObject(), pred, c) and + first(this.getName(), succ) and + c instanceof NormalCompletion + or + last(this.getName(), pred, c) and + succ = this and + c instanceof NormalCompletion + } + override predicate isHidden() { any() } } diff --git a/ql/src/codeql_ruby/controlflow/internal/Splitting.qll b/ql/src/codeql_ruby/controlflow/internal/Splitting.qll index a92695e4f3b..7dec2b7150a 100644 --- a/ql/src/codeql_ruby/controlflow/internal/Splitting.qll +++ b/ql/src/codeql_ruby/controlflow/internal/Splitting.qll @@ -384,7 +384,7 @@ module EnsureSplitting { ) { this.appliesToPredecessor(pred) and nestLevel = block.nestLevel() and - last(block, pred, c) + block.lastBody(pred, c) } /** From bc47338b52f8420ddadcd2f38b2147a5372e63eb Mon Sep 17 00:00:00 2001 From: Arthur Baars Date: Tue, 15 Dec 2020 18:05:13 +0100 Subject: [PATCH 04/15] CFG: add test-case for conditional method declarations --- .../controlflow/graph/Cfg.expected | 41 +++++++++++++++++++ .../library-tests/controlflow/graph/ifs.rb | 6 ++- 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected index 204f8781111..9aecfae2e99 100644 --- a/ql/test/library-tests/controlflow/graph/Cfg.expected +++ b/ql/test/library-tests/controlflow/graph/Cfg.expected @@ -73,6 +73,9 @@ heredoc.rb: #-----| -> < 1 + # 1| enter m1 #-----| -> x @@ -88,6 +91,9 @@ ifs.rb: # 32| enter m5 #-----| -> b1 +# 36| enter conditional_method_def +#-----| -> String + loops.rb: # 1| enter m1 #-----| -> x @@ -2265,6 +2271,31 @@ ifs.rb: # 33| String #-----| -> If +# 36| UnlessModifier +#-----| -> exit top-level (normal) + +# 36| conditional_method_def +#-----| -> UnlessModifier + +# 37| MethodCall +#-----| -> exit conditional_method_def (normal) + +# 37| puts +#-----| -> MethodCall + +# 37| String +#-----| -> puts + +# 38| Binary +#-----| true -> UnlessModifier +#-----| false -> conditional_method_def + +# 38| 1 +#-----| -> 2 + +# 38| 2 +#-----| -> Binary + loops.rb: # 1| x #-----| -> While @@ -3404,6 +3435,8 @@ heredoc.rb: # 1| exit double_heredoc ifs.rb: +# 1| exit top-level + # 1| exit m1 # 11| exit m2 @@ -3414,6 +3447,8 @@ ifs.rb: # 32| exit m5 +# 36| exit conditional_method_def + loops.rb: # 1| exit m1 @@ -3533,6 +3568,9 @@ heredoc.rb: #-----| -> exit double_heredoc ifs.rb: +# 1| exit top-level (normal) +#-----| -> exit top-level + # 1| exit m1 (normal) #-----| -> exit m1 @@ -3548,6 +3586,9 @@ ifs.rb: # 32| exit m5 (normal) #-----| -> exit m5 +# 36| exit conditional_method_def (normal) +#-----| -> exit conditional_method_def + loops.rb: # 1| exit m1 (normal) #-----| -> exit m1 diff --git a/ql/test/library-tests/controlflow/graph/ifs.rb b/ql/test/library-tests/controlflow/graph/ifs.rb index bbb8f93de7b..a774f1e8ca6 100644 --- a/ql/test/library-tests/controlflow/graph/ifs.rb +++ b/ql/test/library-tests/controlflow/graph/ifs.rb @@ -31,4 +31,8 @@ end def m5 (b1, b2, b3, b4, b5) if (if b1 then b2 elsif b3 then b4 else b5 end) then "b2 || b4 || b5" else "!b2 || !b4 || !b5" end -end \ No newline at end of file +end + +def conditional_method_def() + puts "bla" +end unless 1 == 2 From 30895e634c32edb173ae07bab732b4772a06961b Mon Sep 17 00:00:00 2001 From: Arthur Baars Date: Tue, 15 Dec 2020 18:32:51 +0100 Subject: [PATCH 05/15] CFG: refactor CfgScope --- .../controlflow/ControlFlowGraph.qll | 124 ++++++++++++------ 1 file changed, 83 insertions(+), 41 deletions(-) diff --git a/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll b/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll index 89ed39fd8a6..ac2eb82653b 100644 --- a/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll +++ b/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll @@ -1,55 +1,97 @@ /** Provides classes representing the control flow graph. */ private import codeql.Locations -private import codeql_ruby.ast.internal.TreeSitter::Generated +private import codeql_ruby.ast.internal.TreeSitter private import codeql_ruby.controlflow.BasicBlocks private import SuccessorTypes private import internal.ControlFlowGraphImpl private import internal.Splitting private import internal.Completion -private class CfgScopeRange = - @program or @begin_block or @end_block or @method or @singleton_method or @block or @do_block or - @lambda; +private module CfgScope { + abstract class Range extends Generated::AstNode { + abstract string getName(); + + predicate entry(Generated::AstNode first) { first(this, first) } + + predicate exit(Generated::AstNode last, Completion c) { last(this, last, c) } + } + + private class ProgramScope extends Range, Generated::Program { + final override string getName() { result = "top-level" } + } + + private class BeginBlockScope extends Range, Generated::BeginBlock { + final override string getName() { result = "BEGIN block" } + } + + private class EndBlockScope extends Range, Generated::EndBlock { + final override string getName() { result = "END block" } + } + + private class MethodScope extends Range, Generated::AstNode { + MethodScope() { this instanceof Generated::Method } + + final override string getName() { result = this.(Generated::Method).getName().toString() } + + final override predicate entry(Generated::AstNode first) { + first(this.(Trees::RescueEnsureBlockTree).firstBody(), first) + } + + final override predicate exit(Generated::AstNode last, Completion c) { + this.(Trees::RescueEnsureBlockTree).lastBody(last, c) + } + } + + private class SingletonMethodScope extends Range, Generated::AstNode { + SingletonMethodScope() { this instanceof Generated::SingletonMethod } + + final override string getName() { + result = this.(Generated::SingletonMethod).getName().toString() + } + + final override predicate entry(Generated::AstNode first) { + first(this.(Trees::RescueEnsureBlockTree).firstBody(), first) + } + + final override predicate exit(Generated::AstNode last, Completion c) { + this.(Trees::RescueEnsureBlockTree).lastBody(last, c) + } + } + + private class DoBlockScope extends Range, Generated::DoBlock { + final override string getName() { result = "do block" } + + final override predicate entry(Generated::AstNode first) { + first(this.(Trees::RescueEnsureBlockTree).firstBody(), first) + } + + final override predicate exit(Generated::AstNode last, Completion c) { + this.(Trees::RescueEnsureBlockTree).lastBody(last, c) + } + } + + private class BlockScope extends Range, Generated::Block { + final override string getName() { result = "block" } + } + + private class LambdaScope extends Range, Generated::Lambda { + final override string getName() { result = "lambda" } + } +} /** An AST node with an associated control-flow graph. */ -class CfgScope extends AstNode, CfgScopeRange { +class CfgScope extends Generated::AstNode { + CfgScope::Range range; + + CfgScope() { range = this } + /** Gets the name of this scope. */ - string getName() { - this instanceof Program and - result = "top-level" - or - this instanceof BeginBlock and - result = "BEGIN block" - or - this instanceof EndBlock and - result = "END block" - or - result = this.(Method).getName().toString() - or - result = this.(SingletonMethod).getName().toString() - or - this instanceof Block and - result = "block" - or - this instanceof DoBlock and - result = "do block" - or - this instanceof Lambda and - result = "lambda" - } + string getName() { result = range.getName() } - predicate entry(AstNode first) { - first(this.(Trees::RescueEnsureBlockTree).firstBody(), first) - or - not this instanceof Trees::RescueEnsureBlockTree and first(this, first) - } + predicate entry(Generated::AstNode first) { range.entry(first) } - predicate exit(AstNode last, Completion c) { - this.(Trees::RescueEnsureBlockTree).lastBody(last, c) - or - not this instanceof Trees::RescueEnsureBlockTree and last(this, last, c) - } + predicate exit(Generated::AstNode last, Completion c) { range.exit(last, c) } } /** @@ -65,7 +107,7 @@ class CfgNode extends TCfgNode { string toString() { none() } /** Gets the AST node that this node corresponds to, if any. */ - AstNode getNode() { none() } + Generated::AstNode getNode() { none() } /** Gets the location of this control flow node. */ Location getLocation() { result = this.getNode().getLocation() } @@ -160,11 +202,11 @@ module CfgNodes { */ class AstCfgNode extends CfgNode, TAstNode { private Splits splits; - private AstNode n; + private Generated::AstNode n; AstCfgNode() { this = TAstNode(n, splits) } - final override AstNode getNode() { result = n } + final override Generated::AstNode getNode() { result = n } final override string toString() { result = "[" + this.getSplitsString() + "] " + n.toString() From f2effce78631f7819b5995580322bcbe071b1faf Mon Sep 17 00:00:00 2001 From: Arthur Baars Date: Tue, 15 Dec 2020 19:01:29 +0100 Subject: [PATCH 06/15] CFG: improve handling of block and lambda --- .../codeql_ruby/controlflow/ControlFlowGraph.qll | 16 ++++++++++++++++ .../internal/ControlFlowGraphImpl.qll | 8 +++++--- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll b/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll index ac2eb82653b..2c8fd98b53b 100644 --- a/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll +++ b/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll @@ -73,10 +73,26 @@ private module CfgScope { private class BlockScope extends Range, Generated::Block { final override string getName() { result = "block" } + + final override predicate entry(Generated::AstNode first) { + first(this.(Trees::BlockTree).getFirstChildNode(), first) + } + + final override predicate exit(Generated::AstNode last, Completion c) { + last(this.(Trees::BlockTree).getLastChildNode(), last, c) + } } private class LambdaScope extends Range, Generated::Lambda { final override string getName() { result = "lambda" } + + final override predicate entry(Generated::AstNode first) { + first(this.(Trees::LambdaTree).getFirstChildNode(), first) + } + + final override predicate exit(Generated::AstNode last, Completion c) { + last(this.(Trees::LambdaTree).getLastChildNode(), last, c) + } } } diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll index bf8df829838..1d3dd316e8c 100644 --- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -295,7 +295,7 @@ module Trees { } } - private class BlockTree extends StandardPreOrderTree, Block { + class BlockTree extends StandardNode, PreOrderTree, PostOrderTree, Block { final override AstNode getChildNode(int i) { result = this.getParameters() and i = 0 or @@ -439,7 +439,9 @@ module Trees { override predicate isHidden() { any() } } - private class DoBlockTree extends RescueEnsureBlockTree, PreOrderTree, PostOrderTree, DoBlock { + private class DoBlockTree extends RescueEnsureBlockTree, PostOrderTree, DoBlock { + final override predicate first(AstNode first) { first = this } + final override AstNode getChildNode(int i, boolean rescuable) { result = this.getParameters() and i = 0 and rescuable = false or @@ -687,7 +689,7 @@ module Trees { final override AstNode getDefaultValue() { result = this.getValue() } } - private class LambdaTree extends StandardPreOrderTree, Lambda { + class LambdaTree extends StandardNode, PreOrderTree, PostOrderTree, Lambda { final override AstNode getChildNode(int i) { result = this.getParameters() and i = 0 or From f2fd1c7931c1cd660428774a706e318dbea8175e Mon Sep 17 00:00:00 2001 From: Arthur Baars Date: Tue, 15 Dec 2020 19:07:15 +0100 Subject: [PATCH 07/15] 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 From 81c907a87aced26d0bbbc6fd7055a915eb8fd0a4 Mon Sep 17 00:00:00 2001 From: Arthur Baars Date: Wed, 16 Dec 2020 09:49:19 +0100 Subject: [PATCH 08/15] CFG: fix BEGIN and END blocks --- .../codeql_ruby/controlflow/ControlFlowGraph.qll | 16 ++++++++++++++++ .../internal/ControlFlowGraphImpl.qll | 4 ++-- .../library-tests/controlflow/graph/Cfg.expected | 10 ++++------ 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll b/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll index 2c8fd98b53b..7d0253410f3 100644 --- a/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll +++ b/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll @@ -23,10 +23,26 @@ private module CfgScope { private class BeginBlockScope extends Range, Generated::BeginBlock { final override string getName() { result = "BEGIN block" } + + final override predicate entry(Generated::AstNode first) { + first(this.(Trees::BeginBlockTree).getFirstChildNode(), first) + } + + final override predicate exit(Generated::AstNode last, Completion c) { + last(this.(Trees::BeginBlockTree).getLastChildNode(), last, c) + } } private class EndBlockScope extends Range, Generated::EndBlock { final override string getName() { result = "END block" } + + final override predicate entry(Generated::AstNode first) { + first(this.(Trees::EndBlockTree).getFirstChildNode(), first) + } + + final override predicate exit(Generated::AstNode last, Completion c) { + last(this.(Trees::EndBlockTree).getLastChildNode(), last, c) + } } private class MethodScope extends Range, Generated::AstNode { diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll index 00b37dbabe8..3b68a91cfc7 100644 --- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -274,7 +274,7 @@ module Trees { override predicate isHidden() { any() } } - private class BeginBlockTree extends StandardPreOrderTree, BeginBlock { + class BeginBlockTree extends PreOrderTree, PostOrderTree, StandardNode, BeginBlock { final override AstNode getChildNode(int i) { result = this.getChild(i) } } @@ -456,7 +456,7 @@ module Trees { private class EmptyStatementTree extends LeafTree, EmptyStatement { } - private class EndBlockTree extends StandardPreOrderTree, EndBlock { + class EndBlockTree extends StandardNode, PreOrderTree, PostOrderTree, EndBlock { final override AstNode getChildNode(int i) { result = this.getChild(i) } } diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected index 058b8f95af2..b2f159da227 100644 --- a/ql/test/library-tests/controlflow/graph/Cfg.expected +++ b/ql/test/library-tests/controlflow/graph/Cfg.expected @@ -26,10 +26,10 @@ cfg.rb: #-----| -> bar # 15| enter BEGIN block -#-----| -> BeginBlock +#-----| -> String # 19| enter END block -#-----| -> EndBlock +#-----| -> String # 25| enter block #-----| -> x @@ -608,10 +608,9 @@ cfg.rb: #-----| -> puts # 15| BeginBlock -#-----| -> String +#-----| -> EndBlock # 16| Call -#-----| -> EndBlock #-----| -> exit BEGIN block (normal) # 16| puts @@ -621,10 +620,9 @@ cfg.rb: #-----| -> puts # 19| EndBlock -#-----| -> String +#-----| -> 41 # 20| Call -#-----| -> 41 #-----| -> exit END block (normal) # 20| puts From d016e3cae08a836479f59cd96b35042b2ae2d36d Mon Sep 17 00:00:00 2001 From: Arthur Baars Date: Wed, 16 Dec 2020 14:53:43 +0100 Subject: [PATCH 09/15] CFG: methods are evaluated before their arguments --- .../internal/ControlFlowGraphImpl.qll | 4 +- .../controlflow/graph/Cfg.expected | 776 +++++++++--------- 2 files changed, 390 insertions(+), 390 deletions(-) diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll index 3b68a91cfc7..a361f94fe54 100644 --- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -317,9 +317,9 @@ module Trees { final override AstNode getChildNode(int i) { result = this.getReceiver() and i = 0 or - result = this.getArguments() and i = 1 + result = this.getMethod() and i = 1 or - result = this.getMethod() and i = 2 + result = this.getArguments() and i = 2 } } diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected index b2f159da227..a3988ce2340 100644 --- a/ql/test/library-tests/controlflow/graph/Cfg.expected +++ b/ql/test/library-tests/controlflow/graph/Cfg.expected @@ -26,10 +26,10 @@ cfg.rb: #-----| -> bar # 15| enter BEGIN block -#-----| -> String +#-----| -> puts # 19| enter END block -#-----| -> String +#-----| -> puts # 25| enter block #-----| -> x @@ -41,7 +41,7 @@ cfg.rb: #-----| -> a # 69| enter print -#-----| -> String +#-----| -> puts # 101| enter parameters #-----| -> OptionalParameter @@ -53,7 +53,7 @@ cfg.rb: #-----| -> y # 142| enter print -#-----| -> String +#-----| -> puts # 149| enter method #-----| -> SplatParameter @@ -82,7 +82,7 @@ heredoc.rb: #-----| -> double_heredoc # 1| enter double_heredoc -#-----| -> < puts ifs.rb: # 1| enter top-level @@ -104,7 +104,7 @@ ifs.rb: #-----| -> b1 # 36| enter conditional_method_def -#-----| -> String +#-----| -> puts loops.rb: # 1| enter top-level @@ -209,7 +209,7 @@ break_ensure.rb: # 8| Call #-----| false -> If -#-----| true -> String +#-----| true -> puts # 8| elements #-----| -> nil? @@ -221,10 +221,10 @@ break_ensure.rb: #-----| -> If # 9| puts -#-----| -> Call +#-----| -> String # 9| String -#-----| -> puts +#-----| -> Call # 13| Method #-----| -> m3 @@ -275,11 +275,11 @@ break_ensure.rb: # 20| Call #-----| false -> If -#-----| true -> String +#-----| true -> puts # 20| [ensure: break] Call #-----| false -> [ensure: break] If -#-----| true -> [ensure: break] String +#-----| true -> [ensure: break] puts # 20| elements #-----| -> nil? @@ -300,16 +300,16 @@ break_ensure.rb: #-----| -> [ensure: break] If # 21| puts -#-----| -> Call +#-----| -> String # 21| [ensure: break] puts -#-----| -> [ensure: break] Call +#-----| -> [ensure: break] String # 21| String -#-----| -> puts +#-----| -> Call # 21| [ensure: break] String -#-----| -> [ensure: break] puts +#-----| -> [ensure: break] Call # 27| Method #-----| -> m4 @@ -344,7 +344,7 @@ break_ensure.rb: # 33| For #-----| non-empty -> element -#-----| empty -> String +#-----| empty -> puts # 33| [ensure: return] For #-----| non-empty -> [ensure: return] element @@ -389,7 +389,7 @@ break_ensure.rb: #-----| -> [ensure: return] Binary # 36| Break -#-----| break -> String +#-----| break -> puts # 36| [ensure: return] Break #-----| return -> exit m3 (normal) @@ -398,10 +398,10 @@ break_ensure.rb: #-----| -> exit m3 (normal) # 41| puts -#-----| -> Call +#-----| -> String # 41| String -#-----| -> puts +#-----| -> Call # 44| Method #-----| -> exit top-level (normal) @@ -427,7 +427,7 @@ break_ensure.rb: # 47| Binary #-----| false -> If -#-----| true -> String +#-----| true -> raise # 47| element #-----| -> 1 @@ -439,10 +439,10 @@ break_ensure.rb: #-----| raise -> [ensure: raise] Ensure # 48| raise -#-----| -> Call +#-----| -> String # 48| String -#-----| -> raise +#-----| -> Call # 50| Ensure #-----| -> element @@ -516,32 +516,32 @@ case.rb: # 3| x2 #-----| false -> If -#-----| true -> String +#-----| true -> puts # 3| Call #-----| -> If # 3| puts -#-----| -> Call +#-----| -> String # 3| String -#-----| -> puts +#-----| -> Call # 4| When #-----| -> 2 # 4| 2 -#-----| match -> String +#-----| match -> puts #-----| no-match -> exit if_in_case (normal) # 4| Call #-----| -> exit if_in_case (normal) # 4| puts -#-----| -> Call +#-----| -> String # 4| String -#-----| -> puts +#-----| -> Call cfg.rb: # 1| Method @@ -584,7 +584,7 @@ cfg.rb: #-----| -> SymbolArray # 9| StringArray -#-----| -> 4 +#-----| -> puts # 9| BareString #-----| -> BareString @@ -602,10 +602,10 @@ cfg.rb: #-----| -> BeginBlock # 12| puts -#-----| -> Call +#-----| -> 4 # 12| 4 -#-----| -> puts +#-----| -> Call # 15| BeginBlock #-----| -> EndBlock @@ -614,10 +614,10 @@ cfg.rb: #-----| -> exit BEGIN block (normal) # 16| puts -#-----| -> Call +#-----| -> String # 16| String -#-----| -> puts +#-----| -> Call # 19| EndBlock #-----| -> 41 @@ -626,10 +626,10 @@ cfg.rb: #-----| -> exit END block (normal) # 20| puts -#-----| -> Call +#-----| -> String # 20| String -#-----| -> puts +#-----| -> Call # 23| Binary #-----| -> 2 @@ -641,7 +641,7 @@ cfg.rb: #-----| -> Binary # 25| Call -#-----| -> Symbol +#-----| -> puts # 25| 2 #-----| -> times @@ -650,25 +650,25 @@ cfg.rb: #-----| -> Call # 25| x -#-----| -> x +#-----| -> puts # 25| Call #-----| -> exit block (normal) # 25| puts -#-----| -> Call +#-----| -> x # 25| x -#-----| -> puts +#-----| -> Call # 27| Call #-----| -> Proc # 27| puts -#-----| -> Call +#-----| -> Symbol # 27| BlockArgument -#-----| -> puts +#-----| -> Call # 27| Symbol #-----| -> BlockArgument @@ -716,13 +716,13 @@ cfg.rb: #-----| -> Case # 39| self -#-----| -> 42 +#-----| -> puts # 39| puts -#-----| -> Call +#-----| -> 42 # 39| 42 -#-----| -> puts +#-----| -> Call # 41| Case #-----| -> 10 @@ -734,50 +734,50 @@ cfg.rb: #-----| -> 1 # 42| 1 -#-----| match -> String +#-----| match -> puts #-----| no-match -> When # 42| Call #-----| -> Case # 42| puts -#-----| -> Call +#-----| -> String # 42| String -#-----| -> puts +#-----| -> Call # 43| When #-----| -> 2 # 43| 2 #-----| no-match -> 3 -#-----| match -> String +#-----| match -> puts # 43| 3 #-----| no-match -> 4 -#-----| match -> String +#-----| match -> puts # 43| 4 -#-----| match -> String -#-----| no-match -> String +#-----| match -> puts +#-----| no-match -> puts # 43| Call #-----| -> Case # 43| puts -#-----| -> Call +#-----| -> String # 43| String -#-----| -> puts +#-----| -> Call # 44| Call #-----| -> Case # 44| puts -#-----| -> Call +#-----| -> String # 44| String -#-----| -> puts +#-----| -> Call # 47| Case #-----| -> When @@ -786,7 +786,7 @@ cfg.rb: #-----| -> b # 48| Binary -#-----| true -> String +#-----| true -> puts #-----| false -> When # 48| b @@ -799,17 +799,17 @@ cfg.rb: #-----| -> String # 48| puts -#-----| -> Call +#-----| -> String # 48| String -#-----| -> puts +#-----| -> Call # 49| When #-----| -> b # 49| Binary #-----| false -> b -#-----| true -> String +#-----| true -> puts # 49| b #-----| -> 0 @@ -818,7 +818,7 @@ cfg.rb: #-----| -> Binary # 49| Binary -#-----| true -> String +#-----| true -> puts #-----| false -> String # 49| b @@ -831,10 +831,10 @@ cfg.rb: #-----| -> String # 49| puts -#-----| -> Call +#-----| -> String # 49| String -#-----| -> puts +#-----| -> Call # 52| Assignment #-----| -> ?\x40 @@ -961,7 +961,7 @@ cfg.rb: #-----| -> Method # 63| DestructuredParameter -#-----| -> a +#-----| -> puts # 63| a #-----| -> b @@ -970,25 +970,25 @@ cfg.rb: #-----| -> DestructuredParameter # 64| Call -#-----| -> b +#-----| -> puts # 64| puts -#-----| -> Call +#-----| -> a # 64| a -#-----| -> puts +#-----| -> Call # 65| Call #-----| -> exit pattern (normal) # 65| puts -#-----| -> Call +#-----| -> b # 65| b -#-----| -> puts +#-----| -> Call # 67| Assignment -#-----| -> items +#-----| -> puts # 67| items #-----| -> Assignment @@ -1009,10 +1009,10 @@ cfg.rb: #-----| -> print # 68| puts -#-----| -> Call +#-----| -> items # 68| ElementReference -#-----| -> puts +#-----| -> Call # 68| items #-----| -> 2 @@ -1030,10 +1030,10 @@ cfg.rb: #-----| -> exit print (normal) # 70| puts -#-----| -> Call +#-----| -> String # 70| String -#-----| -> puts +#-----| -> Call # 74| Assignment #-----| -> x @@ -1080,28 +1080,28 @@ cfg.rb: #-----| -> Elsif # 78| ; -#-----| -> String +#-----| -> puts # 83| Call #-----| -> Ensure # 83| puts -#-----| -> Call +#-----| -> String # 83| String -#-----| -> puts +#-----| -> Call # 84| Ensure -#-----| -> String +#-----| -> puts # 85| Call #-----| -> x # 85| puts -#-----| -> Call +#-----| -> String # 85| String -#-----| -> puts +#-----| -> Call # 88| Assignment #-----| -> 1.4 @@ -1138,7 +1138,7 @@ cfg.rb: #-----| -> Array # 91| If -#-----| -> x +#-----| -> puts # 91| Binary #-----| false -> If @@ -1157,10 +1157,10 @@ cfg.rb: #-----| -> For # 92| puts -#-----| -> Call +#-----| -> x # 92| x -#-----| -> puts +#-----| -> Call # 95| Assignment #-----| -> String @@ -1245,16 +1245,16 @@ cfg.rb: #-----| -> HashSplatParameter # 101| HashSplatParameter -#-----| -> value +#-----| -> puts # 102| Call #-----| -> kwargs # 102| puts -#-----| -> Call +#-----| -> value # 102| value -#-----| -> puts +#-----| -> Call # 103| Return #-----| return -> exit parameters (normal) @@ -1278,7 +1278,7 @@ cfg.rb: #-----| -> type # 107| Assignment -#-----| -> < puts # 107| table #-----| -> Assignment @@ -1290,10 +1290,10 @@ cfg.rb: #-----| -> b # 108| puts -#-----| -> Call +#-----| -> < puts +#-----| -> Call # 108| < table @@ -1320,14 +1320,14 @@ cfg.rb: #-----| -> IfModifier # 113| puts -#-----| -> Call +#-----| -> String # 113| String -#-----| -> puts +#-----| -> Call # 113| Binary #-----| false -> IfModifier -#-----| true -> String +#-----| true -> puts # 113| b #-----| -> 10 @@ -1510,7 +1510,7 @@ cfg.rb: #-----| -> 1 # 133| Binary -#-----| raise -> String +#-----| raise -> puts #-----| -> 1 # 133| 1 @@ -1523,10 +1523,10 @@ cfg.rb: #-----| -> 1 # 133| puts -#-----| -> Call +#-----| -> String # 133| String -#-----| -> puts +#-----| -> Call # 135| Assignment #-----| -> M @@ -1601,22 +1601,22 @@ cfg.rb: #-----| -> Method # 143| Call -#-----| -> super +#-----| -> puts # 143| puts -#-----| -> Call +#-----| -> String # 143| String -#-----| -> puts +#-----| -> Call # 144| Call #-----| -> exit print (normal) # 144| puts -#-----| -> Call +#-----| -> super # 144| Call -#-----| -> puts +#-----| -> Call # 144| super #-----| -> print @@ -1649,19 +1649,19 @@ cfg.rb: #-----| -> SingletonMethod # 149| SplatParameter -#-----| -> x +#-----| -> puts # 150| Call #-----| -> exit method (normal) # 150| puts -#-----| -> Call +#-----| -> x # 150| x -#-----| -> puts +#-----| -> Call # 153| Method -#-----| -> 1 +#-----| -> two_parameters # 153| two_parameters #-----| -> Method @@ -1676,10 +1676,10 @@ cfg.rb: #-----| -> __FILE__ # 155| two_parameters -#-----| -> Call +#-----| -> 1 # 155| SplatArgument -#-----| -> two_parameters +#-----| -> Call # 155| Array #-----| -> SplatArgument @@ -1757,8 +1757,8 @@ cfg.rb: #-----| -> x # 167| Binary -#-----| false -> String -#-----| true -> String +#-----| false -> puts +#-----| true -> puts # 167| x #-----| -> 10 @@ -1770,19 +1770,19 @@ cfg.rb: #-----| -> Unless # 167| puts -#-----| -> Call +#-----| -> String # 167| String -#-----| -> puts +#-----| -> Call # 167| Call #-----| -> Unless # 167| puts -#-----| -> Call +#-----| -> String # 167| String -#-----| -> puts +#-----| -> Call # 169| UnlessModifier #-----| -> Until @@ -1791,14 +1791,14 @@ cfg.rb: #-----| -> UnlessModifier # 169| puts -#-----| -> Call +#-----| -> String # 169| String -#-----| -> puts +#-----| -> Call # 169| Binary #-----| true -> UnlessModifier -#-----| false -> String +#-----| false -> puts # 169| x #-----| -> 0 @@ -1820,7 +1820,7 @@ cfg.rb: #-----| -> Binary # 171| OperatorAssignment -#-----| -> String +#-----| -> puts # 171| x #-----| -> 10 @@ -1832,10 +1832,10 @@ cfg.rb: #-----| -> x # 171| puts -#-----| -> Call +#-----| -> String # 171| String -#-----| -> puts +#-----| -> Call # 173| Assignment #-----| -> UntilModifier @@ -1856,10 +1856,10 @@ cfg.rb: #-----| -> i # 174| puts -#-----| -> Call +#-----| -> String # 174| String -#-----| -> puts +#-----| -> Call # 174| OperatorAssignment #-----| -> ParenthesizedStatements @@ -1871,7 +1871,7 @@ cfg.rb: #-----| -> OperatorAssignment # 174| Binary -#-----| false -> String +#-----| false -> puts #-----| true -> 0 # 174| i @@ -1912,7 +1912,7 @@ cfg.rb: #-----| -> OperatorAssignment # 179| If -#-----| -> x +#-----| -> puts # 179| Binary #-----| false -> If @@ -1931,10 +1931,10 @@ cfg.rb: #-----| -> x # 180| puts -#-----| -> Call +#-----| -> x # 180| x -#-----| -> puts +#-----| -> Call # 183| WhileModifier #-----| -> i @@ -1946,10 +1946,10 @@ cfg.rb: #-----| -> i # 183| puts -#-----| -> Call +#-----| -> String # 183| String -#-----| -> puts +#-----| -> Call # 183| OperatorAssignment #-----| -> ParenthesizedStatements @@ -1961,7 +1961,7 @@ cfg.rb: #-----| -> OperatorAssignment # 183| Binary -#-----| true -> String +#-----| true -> puts #-----| false -> run_block # 183| i @@ -1988,16 +1988,16 @@ cfg.rb: #-----| -> Call # 189| x -#-----| -> x +#-----| -> puts # 189| Call #-----| -> exit block (normal) # 189| puts -#-----| -> Call +#-----| -> x # 189| x -#-----| -> puts +#-----| -> Call exit.rb: # 1| Method @@ -2010,11 +2010,11 @@ exit.rb: #-----| -> x # 2| If -#-----| -> String +#-----| -> puts # 2| Binary #-----| false -> If -#-----| true -> 1 +#-----| true -> exit # 2| x #-----| -> 2 @@ -2026,19 +2026,19 @@ exit.rb: #-----| exit -> exit m1 (abnormal) # 3| exit -#-----| -> Call +#-----| -> 1 # 3| 1 -#-----| -> exit +#-----| -> Call # 5| Call #-----| -> exit m1 (normal) # 5| puts -#-----| -> Call +#-----| -> String # 5| String -#-----| -> puts +#-----| -> Call # 8| Method #-----| -> exit top-level (normal) @@ -2050,11 +2050,11 @@ exit.rb: #-----| -> x # 9| If -#-----| -> String +#-----| -> puts # 9| Binary #-----| false -> If -#-----| true -> String +#-----| true -> abort # 9| x #-----| -> 2 @@ -2066,19 +2066,19 @@ exit.rb: #-----| exit -> exit m2 (abnormal) # 10| abort -#-----| -> Call +#-----| -> String # 10| String -#-----| -> abort +#-----| -> Call # 12| Call #-----| -> exit m2 (normal) # 12| puts -#-----| -> Call +#-----| -> String # 12| String -#-----| -> puts +#-----| -> Call heredoc.rb: # 1| Method @@ -2091,7 +2091,7 @@ heredoc.rb: #-----| -> exit double_heredoc (normal) # 2| puts -#-----| -> Call +#-----| -> < HeredocBody @@ -2103,7 +2103,7 @@ heredoc.rb: #-----| -> < puts +#-----| -> Call ifs.rb: # 1| Method @@ -2119,7 +2119,7 @@ ifs.rb: #-----| -> exit m1 (normal) # 2| Binary -#-----| true -> String +#-----| true -> puts #-----| false -> x # 2| x @@ -2132,19 +2132,19 @@ ifs.rb: #-----| -> If # 3| puts -#-----| -> Call +#-----| -> String # 3| String -#-----| -> puts +#-----| -> Call # 4| Elsif #-----| -> If # 4| [false] Binary -#-----| false -> String +#-----| false -> puts # 4| [true] Binary -#-----| true -> String +#-----| true -> puts # 4| [false] Binary #-----| false -> [false] Binary @@ -2198,19 +2198,19 @@ ifs.rb: #-----| -> Elsif # 5| puts -#-----| -> Call +#-----| -> String # 5| String -#-----| -> puts +#-----| -> Call # 7| Call #-----| -> Elsif # 7| puts -#-----| -> Call +#-----| -> String # 7| String -#-----| -> puts +#-----| -> Call # 11| Method #-----| -> m3 @@ -2250,7 +2250,7 @@ ifs.rb: #-----| -> x # 19| If -#-----| -> x +#-----| -> puts # 19| Binary #-----| false -> If @@ -2306,10 +2306,10 @@ ifs.rb: #-----| -> exit m3 (normal) # 25| puts -#-----| -> Call +#-----| -> x # 25| x -#-----| -> puts +#-----| -> Call # 28| Method #-----| -> m5 @@ -2443,10 +2443,10 @@ ifs.rb: #-----| -> exit conditional_method_def (normal) # 37| puts -#-----| -> Call +#-----| -> String # 37| String -#-----| -> puts +#-----| -> Call # 38| Binary #-----| true -> UnlessModifier @@ -2472,7 +2472,7 @@ loops.rb: #-----| -> x # 2| Binary -#-----| true -> x +#-----| true -> puts #-----| false -> exit m1 (normal) # 2| x @@ -2485,10 +2485,10 @@ loops.rb: #-----| -> x # 3| puts -#-----| -> Call +#-----| -> x # 3| x -#-----| -> puts +#-----| -> Call # 4| OperatorAssignment #-----| -> x @@ -2512,8 +2512,8 @@ loops.rb: #-----| -> x # 9| Binary -#-----| true -> x -#-----| false -> String +#-----| true -> puts +#-----| false -> puts # 9| x #-----| -> 0 @@ -2525,10 +2525,10 @@ loops.rb: #-----| -> x # 10| puts -#-----| -> Call +#-----| -> x # 10| x -#-----| -> puts +#-----| -> Call # 11| OperatorAssignment #-----| -> x @@ -2540,7 +2540,7 @@ loops.rb: #-----| -> OperatorAssignment # 12| If -#-----| -> String +#-----| -> puts # 12| Binary #-----| true -> Break @@ -2553,7 +2553,7 @@ loops.rb: #-----| -> Binary # 13| Break -#-----| break -> String +#-----| break -> puts # 14| Elsif #-----| -> If @@ -2585,25 +2585,25 @@ loops.rb: #-----| -> Binary # 17| Redo -#-----| redo -> x +#-----| redo -> puts # 19| Call #-----| -> x # 19| puts -#-----| -> Call +#-----| -> String # 19| String -#-----| -> puts +#-----| -> Call # 21| Call #-----| -> exit m2 (normal) # 21| puts -#-----| -> Call +#-----| -> String # 21| String -#-----| -> puts +#-----| -> Call # 24| Method #-----| -> exit top-level (normal) @@ -2630,16 +2630,16 @@ loops.rb: #-----| -> Call # 25| x -#-----| -> x +#-----| -> puts # 26| Call #-----| -> exit do block (normal) # 26| puts -#-----| -> Call +#-----| -> x # 26| x -#-----| -> puts +#-----| -> Call raise.rb: # 1| Class @@ -2676,11 +2676,11 @@ raise.rb: #-----| -> x # 8| If -#-----| -> String +#-----| -> puts # 8| Binary #-----| false -> If -#-----| true -> String +#-----| true -> raise # 8| x #-----| -> 2 @@ -2692,19 +2692,19 @@ raise.rb: #-----| raise -> exit m1 (abnormal) # 9| raise -#-----| -> Call +#-----| -> String # 9| String -#-----| -> raise +#-----| -> Call # 11| Call #-----| -> exit m1 (normal) # 11| puts -#-----| -> Call +#-----| -> String # 11| String -#-----| -> puts +#-----| -> Call # 14| Method #-----| -> m3 @@ -2716,45 +2716,45 @@ raise.rb: #-----| -> b # 16| If -#-----| -> String +#-----| -> puts # 16| b #-----| false -> If -#-----| true -> ExceptionA +#-----| true -> raise # 17| Call #-----| raise -> Rescue # 17| raise -#-----| -> Call +#-----| -> ExceptionA # 17| ExceptionA -#-----| -> raise +#-----| -> Call # 19| Rescue #-----| -> ExceptionA # 19| ExceptionA -#-----| match -> String +#-----| match -> puts #-----| raise -> exit m2 (abnormal) # 20| Call -#-----| -> String +#-----| -> puts # 20| puts -#-----| -> Call +#-----| -> String # 20| String -#-----| -> puts +#-----| -> Call # 22| Call #-----| -> exit m2 (normal) # 22| puts -#-----| -> Call +#-----| -> String # 22| String -#-----| -> puts +#-----| -> Call # 25| Method #-----| -> m4 @@ -2766,41 +2766,41 @@ raise.rb: #-----| -> b # 27| If -#-----| -> String +#-----| -> puts # 27| b #-----| false -> If -#-----| true -> ExceptionA +#-----| true -> raise # 28| Call #-----| raise -> Rescue # 28| raise -#-----| -> Call +#-----| -> ExceptionA # 28| ExceptionA -#-----| -> raise - -# 30| Rescue -#-----| -> String - -# 31| Call -#-----| -> String - -# 31| puts #-----| -> Call -# 31| String +# 30| Rescue #-----| -> puts +# 31| Call +#-----| -> puts + +# 31| puts +#-----| -> String + +# 31| String +#-----| -> Call + # 33| Call #-----| -> exit m3 (normal) # 33| puts -#-----| -> Call +#-----| -> String # 33| String -#-----| -> puts +#-----| -> Call # 36| Method #-----| -> m5 @@ -2812,44 +2812,44 @@ raise.rb: #-----| -> b # 38| If -#-----| -> String +#-----| -> puts # 38| b #-----| false -> If -#-----| true -> ExceptionA +#-----| true -> raise # 39| Call #-----| raise -> Rescue # 39| raise -#-----| -> Call +#-----| -> ExceptionA # 39| ExceptionA -#-----| -> raise +#-----| -> Call # 41| Rescue #-----| -> e # 41| e -#-----| -> String +#-----| -> puts # 42| Call -#-----| -> String +#-----| -> puts # 42| puts -#-----| -> Call +#-----| -> String # 42| String -#-----| -> puts +#-----| -> Call # 44| Call #-----| -> exit m4 (normal) # 44| puts -#-----| -> Call +#-----| -> String # 44| String -#-----| -> puts +#-----| -> Call # 47| Method #-----| -> m6 @@ -2861,35 +2861,35 @@ raise.rb: #-----| -> b # 49| If -#-----| -> String +#-----| -> puts # 49| b #-----| false -> If -#-----| true -> ExceptionA +#-----| true -> raise # 50| Call #-----| raise -> Rescue # 50| raise -#-----| -> Call +#-----| -> ExceptionA # 50| ExceptionA -#-----| -> raise +#-----| -> Call # 52| Rescue #-----| -> e # 52| e -#-----| -> String +#-----| -> puts # 54| Call #-----| -> exit m5 (normal) # 54| puts -#-----| -> Call +#-----| -> String # 54| String -#-----| -> puts +#-----| -> Call # 57| Method #-----| -> m7 @@ -2901,20 +2901,20 @@ raise.rb: #-----| -> b # 59| If -#-----| -> String +#-----| -> puts # 59| b #-----| false -> If -#-----| true -> ExceptionA +#-----| true -> raise # 60| Call #-----| raise -> Rescue # 60| raise -#-----| -> Call +#-----| -> ExceptionA # 60| ExceptionA -#-----| -> raise +#-----| -> Call # 62| Rescue #-----| -> ExceptionA @@ -2928,25 +2928,25 @@ raise.rb: #-----| raise -> exit m6 (abnormal) # 62| e -#-----| -> String +#-----| -> puts # 63| Call -#-----| -> String +#-----| -> puts # 63| puts -#-----| -> Call +#-----| -> String # 63| String -#-----| -> puts +#-----| -> Call # 65| Call #-----| -> exit m6 (normal) # 65| puts -#-----| -> Call +#-----| -> String # 65| String -#-----| -> puts +#-----| -> Call # 68| Method #-----| -> m8 @@ -2958,10 +2958,10 @@ raise.rb: #-----| -> x # 69| If -#-----| -> String +#-----| -> puts # 69| Binary -#-----| true -> String +#-----| true -> raise #-----| false -> x # 69| x @@ -2974,10 +2974,10 @@ raise.rb: #-----| raise -> [ensure: raise] Ensure # 70| raise -#-----| -> Call +#-----| -> String # 70| String -#-----| -> raise +#-----| -> Call # 71| Elsif #-----| -> If @@ -3002,19 +3002,19 @@ raise.rb: #-----| -> Ensure # 74| puts -#-----| -> Call - -# 74| String -#-----| -> puts - -# 75| Ensure #-----| -> String +# 74| String +#-----| -> Call + +# 75| Ensure +#-----| -> puts + # 75| [ensure: return] Ensure -#-----| -> [ensure: return] String +#-----| -> [ensure: return] puts # 75| [ensure: raise] Ensure -#-----| -> [ensure: raise] String +#-----| -> [ensure: raise] puts # 76| Call #-----| -> exit m7 (normal) @@ -3026,22 +3026,22 @@ raise.rb: #-----| raise -> exit m7 (abnormal) # 76| puts -#-----| -> Call +#-----| -> String # 76| [ensure: return] puts -#-----| -> [ensure: return] Call +#-----| -> [ensure: return] String # 76| [ensure: raise] puts -#-----| -> [ensure: raise] Call +#-----| -> [ensure: raise] String # 76| String -#-----| -> puts +#-----| -> Call # 76| [ensure: return] String -#-----| -> [ensure: return] puts +#-----| -> [ensure: return] Call # 76| [ensure: raise] String -#-----| -> [ensure: raise] puts +#-----| -> [ensure: raise] Call # 79| Method #-----| -> m9 @@ -3050,22 +3050,22 @@ raise.rb: #-----| -> Method # 79| x -#-----| -> String +#-----| -> puts # 80| Call #-----| -> x # 80| puts -#-----| -> Call - -# 80| String -#-----| -> puts - -# 82| If #-----| -> String +# 80| String +#-----| -> Call + +# 82| If +#-----| -> puts + # 82| Binary -#-----| true -> String +#-----| true -> raise #-----| false -> x # 82| x @@ -3078,10 +3078,10 @@ raise.rb: #-----| raise -> [ensure: raise] Ensure # 83| raise -#-----| -> Call +#-----| -> String # 83| String -#-----| -> raise +#-----| -> Call # 84| Elsif #-----| -> If @@ -3106,22 +3106,22 @@ raise.rb: #-----| -> Ensure # 87| puts -#-----| -> Call +#-----| -> String # 87| String -#-----| -> puts +#-----| -> Call # 88| Ensure -#-----| -> String +#-----| -> puts # 88| [ensure: return] Ensure -#-----| -> [ensure: return] String +#-----| -> [ensure: return] puts # 88| [ensure: raise] Ensure -#-----| -> [ensure: raise] String +#-----| -> [ensure: raise] puts # 89| Call -#-----| -> String +#-----| -> puts # 89| [ensure: return] Call #-----| return -> exit m8 (normal) @@ -3130,31 +3130,31 @@ raise.rb: #-----| raise -> exit m8 (abnormal) # 89| puts -#-----| -> Call +#-----| -> String # 89| [ensure: return] puts -#-----| -> [ensure: return] Call +#-----| -> [ensure: return] String # 89| [ensure: raise] puts -#-----| -> [ensure: raise] Call +#-----| -> [ensure: raise] String # 89| String -#-----| -> puts +#-----| -> Call # 89| [ensure: return] String -#-----| -> [ensure: return] puts +#-----| -> [ensure: return] Call # 89| [ensure: raise] String -#-----| -> [ensure: raise] puts +#-----| -> [ensure: raise] Call # 91| Call #-----| -> exit m8 (normal) # 91| puts -#-----| -> Call +#-----| -> String # 91| String -#-----| -> puts +#-----| -> Call # 94| Method #-----| -> m10 @@ -3169,22 +3169,22 @@ raise.rb: #-----| -> b2 # 94| b2 -#-----| -> String +#-----| -> puts # 95| Call #-----| -> x # 95| puts -#-----| -> Call - -# 95| String -#-----| -> puts - -# 97| If #-----| -> String +# 95| String +#-----| -> Call + +# 97| If +#-----| -> puts + # 97| Binary -#-----| true -> String +#-----| true -> raise #-----| false -> x # 97| x @@ -3197,10 +3197,10 @@ raise.rb: #-----| raise -> [ensure: raise] Ensure # 98| raise -#-----| -> Call +#-----| -> String # 98| String -#-----| -> raise +#-----| -> Call # 99| Elsif #-----| -> If @@ -3225,19 +3225,19 @@ raise.rb: #-----| -> Ensure # 102| puts -#-----| -> Call - -# 102| String -#-----| -> puts - -# 103| Ensure #-----| -> String +# 102| String +#-----| -> Call + +# 103| Ensure +#-----| -> puts + # 103| [ensure: return] Ensure -#-----| -> [ensure: return] String +#-----| -> [ensure: return] puts # 103| [ensure: raise] Ensure -#-----| -> [ensure: raise] String +#-----| -> [ensure: raise] puts # 104| Call #-----| -> b1 @@ -3249,22 +3249,22 @@ raise.rb: #-----| -> [ensure: raise] b1 # 104| puts -#-----| -> Call +#-----| -> String # 104| [ensure: return] puts -#-----| -> [ensure: return] Call +#-----| -> [ensure: return] String # 104| [ensure: raise] puts -#-----| -> [ensure: raise] Call +#-----| -> [ensure: raise] String # 104| String -#-----| -> puts +#-----| -> Call # 104| [ensure: return] String -#-----| -> [ensure: return] puts +#-----| -> [ensure: return] Call # 104| [ensure: raise] String -#-----| -> [ensure: raise] puts +#-----| -> [ensure: raise] Call # 106| If #-----| -> Ensure @@ -3277,15 +3277,15 @@ raise.rb: # 106| b1 #-----| false -> If -#-----| true -> String +#-----| true -> raise # 106| [ensure: return] b1 #-----| false -> [ensure: return] If -#-----| true -> [ensure: return] String +#-----| true -> [ensure: return] raise # 106| [ensure: raise] b1 #-----| false -> [ensure: raise] If -#-----| true -> [ensure: raise] String +#-----| true -> [ensure: raise] raise # 107| Call #-----| raise -> [ensure(1): raise] Ensure @@ -3297,43 +3297,43 @@ raise.rb: #-----| raise -> [ensure: raise, ensure(1): raise] Ensure # 107| raise -#-----| -> Call +#-----| -> String # 107| [ensure: return] raise -#-----| -> [ensure: return] Call - -# 107| [ensure: raise] raise -#-----| -> [ensure: raise] Call - -# 107| String -#-----| -> raise - -# 107| [ensure: return] String -#-----| -> [ensure: return] raise - -# 107| [ensure: raise] String -#-----| -> [ensure: raise] raise - -# 109| Ensure -#-----| -> String - -# 109| [ensure(1): raise] Ensure -#-----| -> [ensure(1): raise] String - -# 109| [ensure: return] Ensure #-----| -> [ensure: return] String -# 109| [ensure: return, ensure(1): raise] Ensure -#-----| -> [ensure: return, ensure(1): raise] String - -# 109| [ensure: raise] Ensure +# 107| [ensure: raise] raise #-----| -> [ensure: raise] String +# 107| String +#-----| -> Call + +# 107| [ensure: return] String +#-----| -> [ensure: return] Call + +# 107| [ensure: raise] String +#-----| -> [ensure: raise] Call + +# 109| Ensure +#-----| -> puts + +# 109| [ensure(1): raise] Ensure +#-----| -> [ensure(1): raise] puts + +# 109| [ensure: return] Ensure +#-----| -> [ensure: return] puts + +# 109| [ensure: return, ensure(1): raise] Ensure +#-----| -> [ensure: return, ensure(1): raise] puts + +# 109| [ensure: raise] Ensure +#-----| -> [ensure: raise] puts + # 109| [ensure: raise, ensure(1): raise] Ensure -#-----| -> [ensure: raise, ensure(1): raise] String +#-----| -> [ensure: raise, ensure(1): raise] puts # 110| Call -#-----| -> String +#-----| -> puts # 110| [ensure(1): raise] Call #-----| raise -> [ensure: raise] Ensure @@ -3351,58 +3351,58 @@ raise.rb: #-----| raise -> [ensure: raise] Ensure # 110| puts -#-----| -> Call +#-----| -> String # 110| [ensure(1): raise] puts -#-----| -> [ensure(1): raise] Call +#-----| -> [ensure(1): raise] String # 110| [ensure: return] puts -#-----| -> [ensure: return] Call +#-----| -> [ensure: return] String # 110| [ensure: return, ensure(1): raise] puts -#-----| -> [ensure: return, ensure(1): raise] Call +#-----| -> [ensure: return, ensure(1): raise] String # 110| [ensure: raise] puts -#-----| -> [ensure: raise] Call +#-----| -> [ensure: raise] String # 110| [ensure: raise, ensure(1): raise] puts -#-----| -> [ensure: raise, ensure(1): raise] Call +#-----| -> [ensure: raise, ensure(1): raise] String # 110| String -#-----| -> puts +#-----| -> Call # 110| [ensure(1): raise] String -#-----| -> [ensure(1): raise] puts +#-----| -> [ensure(1): raise] Call # 110| [ensure: return] String -#-----| -> [ensure: return] puts +#-----| -> [ensure: return] Call # 110| [ensure: return, ensure(1): raise] String -#-----| -> [ensure: return, ensure(1): raise] puts +#-----| -> [ensure: return, ensure(1): raise] Call # 110| [ensure: raise] String -#-----| -> [ensure: raise] puts +#-----| -> [ensure: raise] Call # 110| [ensure: raise, ensure(1): raise] String -#-----| -> [ensure: raise, ensure(1): raise] puts +#-----| -> [ensure: raise, ensure(1): raise] Call # 113| Call #-----| -> Ensure # 113| puts -#-----| -> Call - -# 113| String -#-----| -> puts - -# 114| Ensure #-----| -> String +# 113| String +#-----| -> Call + +# 114| Ensure +#-----| -> puts + # 114| [ensure: return] Ensure -#-----| -> [ensure: return] String +#-----| -> [ensure: return] puts # 114| [ensure: raise] Ensure -#-----| -> [ensure: raise] String +#-----| -> [ensure: raise] puts # 115| Call #-----| -> b2 @@ -3414,22 +3414,22 @@ raise.rb: #-----| -> [ensure: raise] b2 # 115| puts -#-----| -> Call +#-----| -> String # 115| [ensure: return] puts -#-----| -> [ensure: return] Call +#-----| -> [ensure: return] String # 115| [ensure: raise] puts -#-----| -> [ensure: raise] Call +#-----| -> [ensure: raise] String # 115| String -#-----| -> puts +#-----| -> Call # 115| [ensure: return] String -#-----| -> [ensure: return] puts +#-----| -> [ensure: return] Call # 115| [ensure: raise] String -#-----| -> [ensure: raise] puts +#-----| -> [ensure: raise] Call # 116| If #-----| -> exit m9 (normal) @@ -3442,15 +3442,15 @@ raise.rb: # 116| b2 #-----| false -> If -#-----| true -> String +#-----| true -> raise # 116| [ensure: return] b2 #-----| false -> [ensure: return] If -#-----| true -> [ensure: return] String +#-----| true -> [ensure: return] raise # 116| [ensure: raise] b2 #-----| false -> [ensure: raise] If -#-----| true -> [ensure: raise] String +#-----| true -> [ensure: raise] raise # 117| Call #-----| raise -> exit m9 (abnormal) @@ -3462,22 +3462,22 @@ raise.rb: #-----| raise -> exit m9 (abnormal) # 117| raise -#-----| -> Call +#-----| -> String # 117| [ensure: return] raise -#-----| -> [ensure: return] Call +#-----| -> [ensure: return] String # 117| [ensure: raise] raise -#-----| -> [ensure: raise] Call +#-----| -> [ensure: raise] String # 117| String -#-----| -> raise +#-----| -> Call # 117| [ensure: return] String -#-----| -> [ensure: return] raise +#-----| -> [ensure: return] Call # 117| [ensure: raise] String -#-----| -> [ensure: raise] raise +#-----| -> [ensure: raise] Call # 121| Method #-----| -> m11 @@ -3486,29 +3486,29 @@ raise.rb: #-----| -> Method # 121| OptionalParameter -#-----| no-match -> String +#-----| no-match -> raise #-----| match -> Ensure # 121| Call #-----| raise -> exit m10 (abnormal) # 121| raise -#-----| -> Call +#-----| -> String # 121| String -#-----| -> raise +#-----| -> Call # 124| Ensure -#-----| -> String +#-----| -> puts # 125| Call #-----| -> exit m10 (normal) # 125| puts -#-----| -> Call +#-----| -> String # 125| String -#-----| -> puts +#-----| -> Call # 128| Method #-----| -> m12 @@ -3524,16 +3524,16 @@ raise.rb: # 130| b #-----| false -> If -#-----| true -> ExceptionA +#-----| true -> raise # 131| Call #-----| raise -> Rescue # 131| raise -#-----| -> Call +#-----| -> ExceptionA # 131| ExceptionA -#-----| -> raise +#-----| -> Call # 133| Rescue #-----| -> ExceptionA @@ -3546,50 +3546,50 @@ raise.rb: #-----| -> ExceptionB # 134| ExceptionB -#-----| match -> String +#-----| match -> puts #-----| raise -> [ensure: raise] Ensure # 135| Call #-----| -> Ensure # 135| puts -#-----| -> Call +#-----| -> String # 135| String -#-----| -> puts +#-----| -> Call # 136| Ensure -#-----| -> String +#-----| -> puts # 136| [ensure: raise] Ensure -#-----| -> [ensure: raise] String +#-----| -> [ensure: raise] puts # 137| Call -#-----| -> String +#-----| -> puts # 137| [ensure: raise] Call #-----| raise -> exit m11 (abnormal) # 137| puts -#-----| -> Call +#-----| -> String # 137| [ensure: raise] puts -#-----| -> [ensure: raise] Call +#-----| -> [ensure: raise] String # 137| String -#-----| -> puts +#-----| -> Call # 137| [ensure: raise] String -#-----| -> [ensure: raise] puts +#-----| -> [ensure: raise] Call # 139| Call #-----| -> exit m11 (normal) # 139| puts -#-----| -> Call +#-----| -> String # 139| String -#-----| -> puts +#-----| -> Call # 142| Method #-----| -> m13 @@ -3605,16 +3605,16 @@ raise.rb: # 143| b #-----| false -> If -#-----| true -> String +#-----| true -> raise # 144| Call #-----| raise -> [ensure: raise] Ensure # 144| raise -#-----| -> Call +#-----| -> String # 144| String -#-----| -> raise +#-----| -> Call # 146| Ensure #-----| -> 3 From eafec4331b3e232cdcab315956a01961aec5471b Mon Sep 17 00:00:00 2001 From: Arthur Baars Date: Wed, 16 Dec 2020 10:47:51 +0100 Subject: [PATCH 10/15] CFG: add nodes for block arguments --- .../controlflow/internal/ControlFlowGraphImpl.qll | 3 ++- ql/test/library-tests/controlflow/graph/Cfg.expected | 12 ++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll index a361f94fe54..d0f0d255251 100644 --- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -313,13 +313,14 @@ module Trees { } private class CallTree extends StandardPostOrderTree, Call { - // this.getBlock() is not included as it uses a different scope final override AstNode getChildNode(int i) { result = this.getReceiver() and i = 0 or result = this.getMethod() and i = 1 or result = this.getArguments() and i = 2 + or + result = this.getBlock() and i = 3 } } diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected index a3988ce2340..a5bd861ba6b 100644 --- a/ql/test/library-tests/controlflow/graph/Cfg.expected +++ b/ql/test/library-tests/controlflow/graph/Cfg.expected @@ -647,6 +647,9 @@ cfg.rb: #-----| -> times # 25| times +#-----| -> Block + +# 25| Block #-----| -> Call # 25| x @@ -680,6 +683,9 @@ cfg.rb: #-----| -> new # 29| new +#-----| -> Block + +# 29| Block #-----| -> Call # 29| BlockParameter @@ -1985,6 +1991,9 @@ cfg.rb: #-----| -> exit top-level (normal) # 189| run_block +#-----| -> Block + +# 189| Block #-----| -> Call # 189| x @@ -2627,6 +2636,9 @@ loops.rb: #-----| -> Array # 25| each +#-----| -> DoBlock + +# 25| DoBlock #-----| -> Call # 25| x From dd954ea943a2a51646a80810504aea80aed15d3c Mon Sep 17 00:00:00 2001 From: Arthur Baars Date: Wed, 16 Dec 2020 11:17:09 +0100 Subject: [PATCH 11/15] CFG: correct flow for lambda bodies Lambda bodies are parsed as nested do-blocks or normal blocks. This is actually incorrect, as the body of a lambda can't have parameters. However, we can "inline" such blocks to get the desired control flow. --- .../controlflow/ControlFlowGraph.qll | 19 +++++++++++++++++-- .../internal/ControlFlowGraphImpl.qll | 18 +++++++++++++----- .../controlflow/graph/Cfg.expected | 15 ++------------- 3 files changed, 32 insertions(+), 20 deletions(-) diff --git a/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll b/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll index 7d0253410f3..98a2398db09 100644 --- a/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll +++ b/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll @@ -76,6 +76,8 @@ private module CfgScope { } private class DoBlockScope extends Range, Generated::DoBlock { + DoBlockScope() { not this.getParent() instanceof Generated::Lambda } + final override string getName() { result = "do block" } final override predicate entry(Generated::AstNode first) { @@ -88,6 +90,8 @@ private module CfgScope { } private class BlockScope extends Range, Generated::Block { + BlockScope() { not this.getParent() instanceof Generated::Lambda } + final override string getName() { result = "block" } final override predicate entry(Generated::AstNode first) { @@ -103,11 +107,22 @@ private module CfgScope { final override string getName() { result = "lambda" } final override predicate entry(Generated::AstNode first) { - first(this.(Trees::LambdaTree).getFirstChildNode(), first) + first(this.getParameters(), first) + or + not exists(this.getParameters()) and + ( + first(this.getBody().(Trees::DoBlockTree).firstBody(), first) + or + first(this.getBody().(Trees::BlockTree).getFirstChildNode(), first) + ) } final override predicate exit(Generated::AstNode last, Completion c) { - last(this.(Trees::LambdaTree).getLastChildNode(), last, c) + last(this.getBody().(Trees::BlockTree).getLastChildNode(), last, c) + or + this.getBody().(Trees::RescueEnsureBlockTree).lastBody(last, c) + or + not exists(this.getBody()) and last(this.getParameters(), last, c) } } } diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll index d0f0d255251..4474165b4a0 100644 --- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -431,7 +431,7 @@ module Trees { override predicate isHidden() { any() } } - private class DoBlockTree extends RescueEnsureBlockTree, PostOrderTree, DoBlock { + class DoBlockTree extends RescueEnsureBlockTree, PostOrderTree, DoBlock { final override predicate first(AstNode first) { first = this } final override AstNode getChildNode(int i, boolean rescuable) { @@ -679,11 +679,19 @@ module Trees { final override AstNode getDefaultValue() { result = this.getValue() } } - class LambdaTree extends StandardNode, PreOrderTree, PostOrderTree, Lambda { - final override AstNode getChildNode(int i) { - result = this.getParameters() and i = 0 + class LambdaTree extends PreOrderTree, PostOrderTree, Lambda { + final override predicate propagatesAbnormal(AstNode child) { child = this.getParameters() } + + final override predicate succ(AstNode pred, AstNode succ, Completion c) { + this.getBody().(ControlFlowTree).succ(pred, succ, c) or - result = this.getBody() and i = 1 + last(this.getParameters(), pred, c) and + c instanceof NormalCompletion and + ( + first(this.getBody().(DoBlockTree).firstBody(), succ) + or + first(this.getBody().(BlockTree).getFirstChildNode(), succ) + ) } } diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected index a5bd861ba6b..a2fd71aa2eb 100644 --- a/ql/test/library-tests/controlflow/graph/Cfg.expected +++ b/ql/test/library-tests/controlflow/graph/Cfg.expected @@ -49,9 +49,6 @@ cfg.rb: # 120| enter lambda #-----| -> x -# 120| enter block -#-----| -> y - # 142| enter print #-----| -> puts @@ -1375,7 +1372,7 @@ cfg.rb: #-----| -> swap # 120| DestructuredParameter -#-----| -> Block +#-----| -> y # 120| x #-----| -> y @@ -1383,11 +1380,8 @@ cfg.rb: # 120| y #-----| -> DestructuredParameter -# 120| Block -#-----| -> exit lambda (normal) - # 120| Array -#-----| -> exit block (normal) +#-----| -> exit lambda (normal) # 120| y #-----| -> x @@ -3690,8 +3684,6 @@ cfg.rb: # 120| exit lambda -# 120| exit block - # 142| exit print # 149| exit method @@ -3821,9 +3813,6 @@ cfg.rb: # 120| exit lambda (normal) #-----| -> exit lambda -# 120| exit block (normal) -#-----| -> exit block - # 142| exit print (normal) #-----| -> exit print From 91ae23743455e8ec0e14ba3aabe3eece500a04a0 Mon Sep 17 00:00:00 2001 From: Arthur Baars Date: Thu, 17 Dec 2020 10:47:51 +0100 Subject: [PATCH 12/15] Use latest CodeQL for CI --- .github/workflows/dataset_measure.yml | 3 ++- .github/workflows/qltest.yml | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/dataset_measure.yml b/.github/workflows/dataset_measure.yml index adcfbbfbf05..26f799d10ef 100644 --- a/.github/workflows/dataset_measure.yml +++ b/.github/workflows/dataset_measure.yml @@ -22,7 +22,8 @@ jobs: - name: Fetch CodeQL run: | - gh release download --repo https://github.com/github/codeql-cli-binaries --pattern codeql-linux64.zip + LATEST=$(gh release list --repo https://github.com/github/codeql-cli-binaries | cut -f 1 | sort --version-sort | tail -1) + gh release download --repo https://github.com/github/codeql-cli-binaries --pattern codeql-linux64.zip "$LATEST" unzip -q codeql-linux64.zip env: GITHUB_TOKEN: ${{ github.token }} diff --git a/.github/workflows/qltest.yml b/.github/workflows/qltest.yml index 3b8d870a61b..a099f650c39 100644 --- a/.github/workflows/qltest.yml +++ b/.github/workflows/qltest.yml @@ -16,7 +16,8 @@ jobs: - uses: actions/checkout@v2 - name: Fetch CodeQL run: | - gh release download --repo https://github.com/github/codeql-cli-binaries --pattern codeql-linux64.zip + LATEST=$(gh release list --repo https://github.com/github/codeql-cli-binaries | cut -f 1 | sort --version-sort | tail -1) + gh release download --repo https://github.com/github/codeql-cli-binaries --pattern codeql-linux64.zip "$LATEST" unzip -q codeql-linux64.zip env: GITHUB_TOKEN: ${{ github.token }} From 1033b8610aaad16994666e5ff89dac3549f82dbb Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Thu, 17 Dec 2020 10:23:27 +0100 Subject: [PATCH 13/15] CFG: Add more tests --- .../controlflow/graph/Cfg.expected | 71 ++++++++++++++++++- .../library-tests/controlflow/graph/raise.rb | 6 +- 2 files changed, 75 insertions(+), 2 deletions(-) diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected index a2fd71aa2eb..ec9b794b989 100644 --- a/ql/test/library-tests/controlflow/graph/Cfg.expected +++ b/ql/test/library-tests/controlflow/graph/Cfg.expected @@ -162,6 +162,12 @@ raise.rb: # 150| enter m13 #-----| -> Ensure +# 154| enter m14 +#-----| -> element + +# 155| enter block +#-----| -> elem + break_ensure.rb: # 1| Method #-----| -> m2 @@ -3641,7 +3647,7 @@ raise.rb: #-----| -> [ensure: raise] Return # 150| Method -#-----| -> exit top-level (normal) +#-----| -> m14 # 150| m13 #-----| -> Method @@ -3649,6 +3655,53 @@ raise.rb: # 151| Ensure #-----| -> exit m13 (normal) +# 154| Method +#-----| -> exit top-level (normal) + +# 154| m14 +#-----| -> Method + +# 154| element +#-----| -> element + +# 155| Call +#-----| -> exit m14 (normal) + +# 155| element +#-----| -> each + +# 155| each +#-----| -> Block + +# 155| Block +#-----| -> Call + +# 155| elem +#-----| -> element + +# 155| IfModifier +#-----| -> exit block (normal) + +# 155| Call +#-----| raise -> exit m14 (abnormal) +#-----| raise -> exit block (abnormal) + +# 155| raise +#-----| -> String + +# 155| String +#-----| -> Call + +# 155| Call +#-----| false -> IfModifier +#-----| true -> raise + +# 155| element +#-----| -> nil? + +# 155| nil? +#-----| -> Call + break_ensure.rb: # 1| exit top-level @@ -3759,6 +3812,10 @@ raise.rb: # 150| exit m13 +# 154| exit m14 + +# 155| exit block + break_ensure.rb: # 1| exit top-level (normal) #-----| -> exit top-level @@ -3952,3 +4009,15 @@ raise.rb: # 150| exit m13 (normal) #-----| -> exit m13 + +# 154| exit m14 (abnormal) +#-----| -> exit m14 + +# 154| exit m14 (normal) +#-----| -> exit m14 + +# 155| exit block (abnormal) +#-----| -> exit block + +# 155| exit block (normal) +#-----| -> exit block diff --git a/ql/test/library-tests/controlflow/graph/raise.rb b/ql/test/library-tests/controlflow/graph/raise.rb index f75f4fa7e8e..f4e24402649 100644 --- a/ql/test/library-tests/controlflow/graph/raise.rb +++ b/ql/test/library-tests/controlflow/graph/raise.rb @@ -149,4 +149,8 @@ end def m13 ensure -end \ No newline at end of file +end + +def m14 element + element.each { |elem| raise "" if element.nil? } +end From 46fc17da582b26eb2f882731c10214bf891f0304 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Thu, 17 Dec 2020 10:23:52 +0100 Subject: [PATCH 14/15] CFG: Fix multiple abnormal successors --- .../controlflow/ControlFlowGraph.qll | 3 +++ .../internal/ControlFlowGraphImpl.qll | 24 +++++++++++-------- .../controlflow/graph/Cfg.expected | 4 ---- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll b/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll index 98a2398db09..9c059516f72 100644 --- a/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll +++ b/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll @@ -118,6 +118,9 @@ private module CfgScope { } final override predicate exit(Generated::AstNode last, Completion c) { + last(this.getParameters(), last, c) and + not c instanceof NormalCompletion + or last(this.getBody().(Trees::BlockTree).getLastChildNode(), last, c) or this.getBody().(Trees::RescueEnsureBlockTree).lastBody(last, c) diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll index 4474165b4a0..a0762c46d51 100644 --- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -164,7 +164,7 @@ abstract private class StandardNode extends ControlFlowTree { ) } - final override predicate propagatesAbnormal(AstNode child) { child = this.getChildNodeRanked(_) } + override predicate propagatesAbnormal(AstNode child) { child = this.getChildNode(_) } pragma[nomagic] override predicate succ(AstNode pred, AstNode succ, Completion c) { @@ -223,9 +223,17 @@ abstract private class StandardPostOrderTree extends StandardNode, PostOrderTree } abstract private class LeafTree extends PreOrderTree, PostOrderTree { + override predicate propagatesAbnormal(AstNode child) { none() } + + override predicate succ(AstNode pred, AstNode succ, Completion c) { none() } +} + +abstract class ScopeTree extends StandardNode, LeafTree { final override predicate propagatesAbnormal(AstNode child) { none() } - final override predicate succ(AstNode pred, AstNode succ, Completion c) { none() } + final override predicate succ(AstNode pred, AstNode succ, Completion c) { + StandardNode.super.succ(pred, succ, c) + } } /** Defines the CFG by dispatch on the various AST types. */ @@ -274,7 +282,7 @@ module Trees { override predicate isHidden() { any() } } - class BeginBlockTree extends PreOrderTree, PostOrderTree, StandardNode, BeginBlock { + class BeginBlockTree extends ScopeTree, BeginBlock { final override AstNode getChildNode(int i) { result = this.getChild(i) } } @@ -288,7 +296,7 @@ module Trees { } } - class BlockTree extends StandardNode, PreOrderTree, PostOrderTree, Block { + class BlockTree extends ScopeTree, Block { final override AstNode getChildNode(int i) { result = this.getParameters() and i = 0 or @@ -457,7 +465,7 @@ module Trees { private class EmptyStatementTree extends LeafTree, EmptyStatement { } - class EndBlockTree extends StandardNode, PreOrderTree, PostOrderTree, EndBlock { + class EndBlockTree extends ScopeTree, EndBlock { final override AstNode getChildNode(int i) { result = this.getChild(i) } } @@ -679,12 +687,8 @@ module Trees { final override AstNode getDefaultValue() { result = this.getValue() } } - class LambdaTree extends PreOrderTree, PostOrderTree, Lambda { - final override predicate propagatesAbnormal(AstNode child) { child = this.getParameters() } - + class LambdaTree extends LeafTree, Lambda { final override predicate succ(AstNode pred, AstNode succ, Completion c) { - this.getBody().(ControlFlowTree).succ(pred, succ, c) - or last(this.getParameters(), pred, c) and c instanceof NormalCompletion and ( diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected index ec9b794b989..8c8ea06a75c 100644 --- a/ql/test/library-tests/controlflow/graph/Cfg.expected +++ b/ql/test/library-tests/controlflow/graph/Cfg.expected @@ -3683,7 +3683,6 @@ raise.rb: #-----| -> exit block (normal) # 155| Call -#-----| raise -> exit m14 (abnormal) #-----| raise -> exit block (abnormal) # 155| raise @@ -4010,9 +4009,6 @@ raise.rb: # 150| exit m13 (normal) #-----| -> exit m13 -# 154| exit m14 (abnormal) -#-----| -> exit m14 - # 154| exit m14 (normal) #-----| -> exit m14 From b676c952189c7476681e5bbaf0803e00d7c14685 Mon Sep 17 00:00:00 2001 From: Arthur Baars Date: Thu, 17 Dec 2020 12:17:29 +0100 Subject: [PATCH 15/15] Address comments --- .../controlflow/ControlFlowGraph.qll | 138 +--------------- .../internal/ControlFlowGraphImpl.qll | 150 ++++++++++++++++-- .../controlflow/internal/Splitting.qll | 2 +- 3 files changed, 144 insertions(+), 146 deletions(-) diff --git a/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll b/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll index 9c059516f72..40571c701c9 100644 --- a/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll +++ b/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll @@ -1,147 +1,21 @@ /** Provides classes representing the control flow graph. */ private import codeql.Locations -private import codeql_ruby.ast.internal.TreeSitter +private import codeql_ruby.ast.internal.TreeSitter::Generated private import codeql_ruby.controlflow.BasicBlocks private import SuccessorTypes private import internal.ControlFlowGraphImpl private import internal.Splitting private import internal.Completion -private module CfgScope { - abstract class Range extends Generated::AstNode { - abstract string getName(); - - predicate entry(Generated::AstNode first) { first(this, first) } - - predicate exit(Generated::AstNode last, Completion c) { last(this, last, c) } - } - - private class ProgramScope extends Range, Generated::Program { - final override string getName() { result = "top-level" } - } - - private class BeginBlockScope extends Range, Generated::BeginBlock { - final override string getName() { result = "BEGIN block" } - - final override predicate entry(Generated::AstNode first) { - first(this.(Trees::BeginBlockTree).getFirstChildNode(), first) - } - - final override predicate exit(Generated::AstNode last, Completion c) { - last(this.(Trees::BeginBlockTree).getLastChildNode(), last, c) - } - } - - private class EndBlockScope extends Range, Generated::EndBlock { - final override string getName() { result = "END block" } - - final override predicate entry(Generated::AstNode first) { - first(this.(Trees::EndBlockTree).getFirstChildNode(), first) - } - - final override predicate exit(Generated::AstNode last, Completion c) { - last(this.(Trees::EndBlockTree).getLastChildNode(), last, c) - } - } - - private class MethodScope extends Range, Generated::AstNode { - MethodScope() { this instanceof Generated::Method } - - final override string getName() { result = this.(Generated::Method).getName().toString() } - - final override predicate entry(Generated::AstNode first) { - first(this.(Trees::RescueEnsureBlockTree).firstBody(), first) - } - - final override predicate exit(Generated::AstNode last, Completion c) { - this.(Trees::RescueEnsureBlockTree).lastBody(last, c) - } - } - - private class SingletonMethodScope extends Range, Generated::AstNode { - SingletonMethodScope() { this instanceof Generated::SingletonMethod } - - final override string getName() { - result = this.(Generated::SingletonMethod).getName().toString() - } - - final override predicate entry(Generated::AstNode first) { - first(this.(Trees::RescueEnsureBlockTree).firstBody(), first) - } - - final override predicate exit(Generated::AstNode last, Completion c) { - this.(Trees::RescueEnsureBlockTree).lastBody(last, c) - } - } - - private class DoBlockScope extends Range, Generated::DoBlock { - DoBlockScope() { not this.getParent() instanceof Generated::Lambda } - - final override string getName() { result = "do block" } - - final override predicate entry(Generated::AstNode first) { - first(this.(Trees::RescueEnsureBlockTree).firstBody(), first) - } - - final override predicate exit(Generated::AstNode last, Completion c) { - this.(Trees::RescueEnsureBlockTree).lastBody(last, c) - } - } - - private class BlockScope extends Range, Generated::Block { - BlockScope() { not this.getParent() instanceof Generated::Lambda } - - final override string getName() { result = "block" } - - final override predicate entry(Generated::AstNode first) { - first(this.(Trees::BlockTree).getFirstChildNode(), first) - } - - final override predicate exit(Generated::AstNode last, Completion c) { - last(this.(Trees::BlockTree).getLastChildNode(), last, c) - } - } - - private class LambdaScope extends Range, Generated::Lambda { - final override string getName() { result = "lambda" } - - final override predicate entry(Generated::AstNode first) { - first(this.getParameters(), first) - or - not exists(this.getParameters()) and - ( - first(this.getBody().(Trees::DoBlockTree).firstBody(), first) - or - first(this.getBody().(Trees::BlockTree).getFirstChildNode(), first) - ) - } - - final override predicate exit(Generated::AstNode last, Completion c) { - last(this.getParameters(), last, c) and - not c instanceof NormalCompletion - or - last(this.getBody().(Trees::BlockTree).getLastChildNode(), last, c) - or - this.getBody().(Trees::RescueEnsureBlockTree).lastBody(last, c) - or - not exists(this.getBody()) and last(this.getParameters(), last, c) - } - } -} - /** An AST node with an associated control-flow graph. */ -class CfgScope extends Generated::AstNode { - CfgScope::Range range; +class CfgScope extends AstNode { + CfgScope::Range_ range; CfgScope() { range = this } /** Gets the name of this scope. */ string getName() { result = range.getName() } - - predicate entry(Generated::AstNode first) { range.entry(first) } - - predicate exit(Generated::AstNode last, Completion c) { range.exit(last, c) } } /** @@ -157,7 +31,7 @@ class CfgNode extends TCfgNode { string toString() { none() } /** Gets the AST node that this node corresponds to, if any. */ - Generated::AstNode getNode() { none() } + AstNode getNode() { none() } /** Gets the location of this control flow node. */ Location getLocation() { result = this.getNode().getLocation() } @@ -252,11 +126,11 @@ module CfgNodes { */ class AstCfgNode extends CfgNode, TAstNode { private Splits splits; - private Generated::AstNode n; + private AstNode n; AstCfgNode() { this = TAstNode(n, splits) } - final override Generated::AstNode getNode() { result = n } + final override AstNode getNode() { result = n } final override string toString() { result = "[" + this.getSplitsString() + "] " + n.toString() diff --git a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll index a0762c46d51..42be8de3756 100644 --- a/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll +++ b/ql/src/codeql_ruby/controlflow/internal/ControlFlowGraphImpl.qll @@ -39,6 +39,130 @@ private import SuccessorTypes private import Splitting private import codeql.files.FileSystem +module CfgScope { + abstract class Range_ extends AstNode { + abstract string getName(); + + abstract predicate entry(AstNode first); + + abstract predicate exit(AstNode last, Completion c); + } + + private class ProgramScope extends Range_, Program { + final override string getName() { result = "top-level" } + + final override predicate entry(AstNode first) { first(this, first) } + + final override predicate exit(AstNode last, Completion c) { last(this, last, c) } + } + + private class BeginBlockScope extends Range_, BeginBlock { + final override string getName() { result = "BEGIN block" } + + final override predicate entry(AstNode first) { + first(this.(Trees::BeginBlockTree).getFirstChildNode(), first) + } + + final override predicate exit(AstNode last, Completion c) { + last(this.(Trees::BeginBlockTree).getLastChildNode(), last, c) + } + } + + private class EndBlockScope extends Range_, EndBlock { + final override string getName() { result = "END block" } + + final override predicate entry(AstNode first) { + first(this.(Trees::EndBlockTree).getFirstChildNode(), first) + } + + final override predicate exit(AstNode last, Completion c) { + last(this.(Trees::EndBlockTree).getLastChildNode(), last, c) + } + } + + private class MethodScope extends Range_, AstNode { + MethodScope() { this instanceof Method } + + final override string getName() { result = this.(Method).getName().toString() } + + final override predicate entry(AstNode first) { + this.(Trees::RescueEnsureBlockTree).firstInner(first) + } + + final override predicate exit(AstNode last, Completion c) { + this.(Trees::RescueEnsureBlockTree).lastInner(last, c) + } + } + + private class SingletonMethodScope extends Range_, AstNode { + SingletonMethodScope() { this instanceof SingletonMethod } + + final override string getName() { result = this.(SingletonMethod).getName().toString() } + + final override predicate entry(AstNode first) { + this.(Trees::RescueEnsureBlockTree).firstInner(first) + } + + final override predicate exit(AstNode last, Completion c) { + this.(Trees::RescueEnsureBlockTree).lastInner(last, c) + } + } + + private class DoBlockScope extends Range_, DoBlock { + DoBlockScope() { not this.getParent() instanceof Lambda } + + final override string getName() { result = "do block" } + + final override predicate entry(AstNode first) { + this.(Trees::RescueEnsureBlockTree).firstInner(first) + } + + final override predicate exit(AstNode last, Completion c) { + this.(Trees::RescueEnsureBlockTree).lastInner(last, c) + } + } + + private class BlockScope extends Range_, Block { + BlockScope() { not this.getParent() instanceof Lambda } + + final override string getName() { result = "block" } + + final override predicate entry(AstNode first) { + first(this.(Trees::BlockTree).getFirstChildNode(), first) + } + + final override predicate exit(AstNode last, Completion c) { + last(this.(Trees::BlockTree).getLastChildNode(), last, c) + } + } + + private class LambdaScope extends Range_, Lambda { + final override string getName() { result = "lambda" } + + final override predicate entry(AstNode first) { + first(this.getParameters(), first) + or + not exists(this.getParameters()) and + ( + this.getBody().(Trees::DoBlockTree).firstInner(first) + or + first(this.getBody().(Trees::BlockTree).getFirstChildNode(), first) + ) + } + + final override predicate exit(AstNode last, Completion c) { + last(this.getParameters(), last, c) and + not c instanceof NormalCompletion + or + last(this.getBody().(Trees::BlockTree).getLastChildNode(), last, c) + or + this.getBody().(Trees::RescueEnsureBlockTree).lastInner(last, c) + or + not exists(this.getBody()) and last(this.getParameters(), last, c) + } + } +} + private AstNode parent(AstNode n) { result.getAFieldOrChild() = n and not n instanceof CfgScope @@ -122,7 +246,7 @@ predicate succ(AstNode pred, AstNode succ, Completion c) { /** Holds if `first` is first executed when entering `scope`. */ pragma[nomagic] -predicate succEntry(CfgScope scope, AstNode first) { +predicate succEntry(CfgScope::Range_ scope, AstNode first) { exists(AstNode n | scope.entry(n) and succImplIfHidden*(n, first) and @@ -132,7 +256,7 @@ predicate succEntry(CfgScope scope, AstNode first) { /** Holds if `last` with completion `c` can exit `scope`. */ pragma[nomagic] -predicate succExit(CfgScope scope, AstNode last, Completion c) { +predicate succExit(CfgScope::Range_ scope, AstNode last, Completion c) { exists(AstNode n | scope.exit(n, c) and succImplIfHidden*(last, n) and @@ -277,7 +401,7 @@ module Trees { result = this.getChild(i) and rescuable = true } - final override predicate last(AstNode last, Completion c) { this.lastBody(last, c) } + final override predicate last(AstNode last, Completion c) { this.lastInner(last, c) } override predicate isHidden() { any() } } @@ -388,7 +512,7 @@ module Trees { result = this.getChild(i - 1) and rescuable = true } - final override predicate last(AstNode last, Completion c) { this.lastBody(last, c) } + final override predicate last(AstNode last, Completion c) { this.lastInner(last, c) } } private class ClassVariableTree extends LeafTree, ClassVariable { } @@ -692,7 +816,7 @@ module Trees { last(this.getParameters(), pred, c) and c instanceof NormalCompletion and ( - first(this.getBody().(DoBlockTree).firstBody(), succ) + this.getBody().(DoBlockTree).firstInner(succ) or first(this.getBody().(BlockTree).getFirstChildNode(), succ) ) @@ -798,7 +922,7 @@ module Trees { result = this.getChild(i - 1) and rescuable = true } - final override predicate last(AstNode last, Completion c) { this.lastBody(last, c) } + final override predicate last(AstNode last, Completion c) { this.lastInner(last, c) } } private class NextTree extends StandardPostOrderTree, Next { @@ -1080,7 +1204,7 @@ module Trees { nestLevel = this.nestLevel() } - predicate lastBody(AstNode last, Completion c) { + predicate lastInner(AstNode last, Completion c) { exists(boolean ensurable | last = this.getAnEnsurePredecessor(c, ensurable) | not this.hasEnsure() or @@ -1107,15 +1231,15 @@ module Trees { not c instanceof NormalCompletion } - AstNode firstBody() { - result = this.getBodyChild(0, _) + predicate firstInner(AstNode first) { + first(this.getBodyChild(0, _), first) or not exists(this.getBodyChild(_, _)) and ( - result = this.getRescue(0) + first(this.getRescue(_), first) or not exists(this.getRescue(_)) and - result = this.getEnsure() + first(this.getEnsure(), first) ) } @@ -1123,7 +1247,7 @@ module Trees { this instanceof PreOrderTree and pred = this and c instanceof SimpleCompletion and - first(this.firstBody(), succ) + this.firstInner(succ) or // Normal left-to-right evaluation in the body exists(int i | @@ -1215,7 +1339,7 @@ module Trees { ) } - final override predicate last(AstNode last, Completion c) { this.lastBody(last, c) } + final override predicate last(AstNode last, Completion c) { this.lastInner(last, c) } } private class SingletonMethodTree extends RescueEnsureBlockTree, PostOrderTree, SingletonMethod { diff --git a/ql/src/codeql_ruby/controlflow/internal/Splitting.qll b/ql/src/codeql_ruby/controlflow/internal/Splitting.qll index 7dec2b7150a..bacb06148ba 100644 --- a/ql/src/codeql_ruby/controlflow/internal/Splitting.qll +++ b/ql/src/codeql_ruby/controlflow/internal/Splitting.qll @@ -384,7 +384,7 @@ module EnsureSplitting { ) { this.appliesToPredecessor(pred) and nestLevel = block.nestLevel() and - block.lastBody(pred, c) + block.lastInner(pred, c) } /**