From 89e6c0e8385b3efdc2115fd46b56bbe160787720 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Wed, 1 Sep 2021 14:07:28 +0200 Subject: [PATCH] CFG: Model calls that may raise an exception In order to avoid dead `rescue`s, we assume that any call that happens in a `rescue`/`ensure` context may raise an exception. --- .../ruby/controlflow/internal/Completion.qll | 16 ++++++ .../controlflow/graph/Cfg.expected | 56 +++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/ql/lib/codeql/ruby/controlflow/internal/Completion.qll b/ql/lib/codeql/ruby/controlflow/internal/Completion.qll index 31608fa3810..e7f64d1318e 100644 --- a/ql/lib/codeql/ruby/controlflow/internal/Completion.qll +++ b/ql/lib/codeql/ruby/controlflow/internal/Completion.qll @@ -70,6 +70,19 @@ private predicate completionIsValidForStmt(AstNode n, Completion c) { c = TReturnCompletion() } +/** + * Holds if `c` happens in an exception-aware context, that is, it may be + * `rescue`d or `ensure`d. In such cases, we assume that the target of `c` + * may raise an exception (in addition to evaluating normally). + */ +private predicate mayRaise(Call c) { + exists(Trees::BodyStmtTree bst | c = bst.getBodyChild(_, true).getAChild*() | + exists(bst.getARescue()) + or + exists(bst.getEnsure()) + ) +} + /** A completion of a statement or an expression. */ abstract class Completion extends TCompletion { /** Holds if this completion is valid for node `n`. */ @@ -91,6 +104,9 @@ abstract class Completion extends TCompletion { or n = any(RescueModifierExpr parent).getBody() and this = TRaiseCompletion() or + mayRaise(n) and + this = TRaiseCompletion() + or not n instanceof NonReturningCall and not completionIsValidForStmt(n, _) and not mustHaveBooleanCompletion(n) and diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected index d86b1bad240..c0d7b6ad0b8 100644 --- a/ql/test/library-tests/controlflow/graph/Cfg.expected +++ b/ql/test/library-tests/controlflow/graph/Cfg.expected @@ -182,6 +182,9 @@ break_ensure.rb: # 27| exit m3 +# 27| exit m3 (abnormal) +#-----| -> exit m3 + # 27| exit m3 (normal) #-----| -> exit m3 @@ -194,6 +197,7 @@ break_ensure.rb: # 29| call to nil? #-----| false -> if ... #-----| true -> return +#-----| raise -> [ensure: raise] elements # 29| elements #-----| -> call to nil? @@ -204,18 +208,27 @@ break_ensure.rb: # 32| ensure ... #-----| -> self +# 32| [ensure: raise] ensure ... +#-----| raise -> exit m3 (abnormal) + # 32| [ensure: return] ensure ... #-----| return -> exit m3 (normal) # 33| for ... in ... #-----| -> ensure ... +# 33| [ensure: raise] for ... in ... +#-----| -> [ensure: raise] ensure ... + # 33| [ensure: return] for ... in ... #-----| -> [ensure: return] ensure ... # 33| element #-----| -> self +# 33| [ensure: raise] element +#-----| -> [ensure: raise] self + # 33| [ensure: return] element #-----| -> [ensure: return] self @@ -223,6 +236,10 @@ break_ensure.rb: #-----| empty -> for ... in ... #-----| non-empty -> element +# 33| [ensure: raise] In +#-----| empty -> [ensure: raise] for ... in ... +#-----| non-empty -> [ensure: raise] element + # 33| [ensure: return] In #-----| empty -> [ensure: return] for ... in ... #-----| non-empty -> [ensure: return] element @@ -230,18 +247,27 @@ break_ensure.rb: # 33| elements #-----| -> In +# 33| [ensure: raise] elements +#-----| -> [ensure: raise] In + # 33| [ensure: return] elements #-----| -> [ensure: return] In # 33| do ... #-----| -> In +# 33| [ensure: raise] do ... +#-----| -> [ensure: raise] In + # 33| [ensure: return] do ... #-----| -> [ensure: return] In # 35| if ... #-----| -> do ... +# 35| [ensure: raise] if ... +#-----| -> [ensure: raise] do ... + # 35| [ensure: return] if ... #-----| -> [ensure: return] do ... @@ -249,6 +275,10 @@ break_ensure.rb: #-----| true -> break #-----| false -> if ... +# 35| [ensure: raise] ... > ... +#-----| true -> [ensure: raise] break +#-----| false -> [ensure: raise] if ... + # 35| [ensure: return] ... > ... #-----| true -> [ensure: return] break #-----| false -> [ensure: return] if ... @@ -256,24 +286,36 @@ break_ensure.rb: # 35| call to x #-----| -> 0 +# 35| [ensure: raise] call to x +#-----| -> [ensure: raise] 0 + # 35| [ensure: return] call to x #-----| -> [ensure: return] 0 # 35| self #-----| -> call to x +# 35| [ensure: raise] self +#-----| -> [ensure: raise] call to x + # 35| [ensure: return] self #-----| -> [ensure: return] call to x # 35| 0 #-----| -> ... > ... +# 35| [ensure: raise] 0 +#-----| -> [ensure: raise] ... > ... + # 35| [ensure: return] 0 #-----| -> [ensure: return] ... > ... # 36| break #-----| break -> for ... in ... +# 36| [ensure: raise] break +#-----| break -> [ensure: raise] for ... in ... + # 36| [ensure: return] break #-----| break -> [ensure: return] for ... in ... @@ -4213,6 +4255,7 @@ raise.rb: # 74| call to puts #-----| -> self +#-----| raise -> [ensure: raise] self # 74| self #-----| -> "0 <= x <= 2" @@ -4325,6 +4368,7 @@ raise.rb: # 87| call to puts #-----| -> self +#-----| raise -> [ensure: raise] self # 87| self #-----| -> "0 <= x <= 2" @@ -4402,6 +4446,7 @@ raise.rb: # 95| call to puts #-----| -> x +#-----| raise -> [ensure: raise] self # 95| self #-----| -> "Begin m9" @@ -4452,6 +4497,7 @@ raise.rb: # 102| call to puts #-----| -> self +#-----| raise -> [ensure: raise] self # 102| self #-----| -> "0 <= x <= 2" @@ -4470,12 +4516,15 @@ raise.rb: # 104| call to puts #-----| -> b1 +#-----| raise -> [ensure: raise] self # 104| [ensure: raise] call to puts #-----| -> [ensure: raise] b1 +#-----| raise -> [ensure: raise] self # 104| [ensure: return] call to puts #-----| -> [ensure: return] b1 +#-----| raise -> [ensure: raise] self # 104| self #-----| -> "outer ensure" @@ -4563,21 +4612,27 @@ raise.rb: # 110| call to puts #-----| -> ensure ... +#-----| raise -> [ensure: raise] self # 110| [ensure(1): raise] call to puts #-----| -> [ensure(1): raise] ensure ... +#-----| raise -> [ensure: raise] self # 110| [ensure: raise] call to puts #-----| -> [ensure: raise] ensure ... +#-----| raise -> [ensure: raise] self # 110| [ensure: raise, ensure(1): raise] call to puts #-----| -> [ensure: raise, ensure(1): raise] ensure ... +#-----| raise -> [ensure: raise] self # 110| [ensure: return] call to puts #-----| -> [ensure: return] ensure ... +#-----| raise -> [ensure: raise] self # 110| [ensure: return, ensure(1): raise] call to puts #-----| -> [ensure: return, ensure(1): raise] ensure ... +#-----| raise -> [ensure: raise] self # 110| self #-----| -> "inner ensure" @@ -4617,6 +4672,7 @@ raise.rb: # 113| call to puts #-----| -> self +#-----| raise -> [ensure: raise] self # 113| self #-----| -> "End m9"