Skip to content

Commit

Permalink
Fix exception stack lowering in finally handlers (#36480)
Browse files Browse the repository at this point in the history
When using `return` or `break` nested inside `finally` handlers,
exception stack lowering failed to pop exceptions from the stack
correctly:

* For `return`, the exception stack was not popped at all. If done
  inside a loop this could eventually cause the runtime to run out of
  memory.
* For `break`, the exception stack was popped too early, causing subtle
  inconsistency in intermediate finally handlers.

Fix these issues by storing the current exception token stack with the
current finally handler information and using it to pop the stack before
jumping into the finally block.

Fixes #34579
  • Loading branch information
c42f authored Jul 1, 2020
1 parent 90e70ed commit 16ba0dd
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 18 deletions.
44 changes: 29 additions & 15 deletions src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -3711,10 +3711,10 @@ f(x) = yt(x)
(label-counter 0) ;; counter for generating label addresses
(label-map (table)) ;; maps label names to generated addresses
(label-nesting (table)) ;; exception handler and catch block nesting of each label
(finally-handler #f) ;; `(var label map level)` where `map` is a list of `(tag . action)`.
;; To exit the current finally block, set `var` to integer `tag`,
;; jump to `label`, and put `(tag . action)` in the map, where `action`
;; is `(return x)`, `(break x)`, or a call to rethrow.
(finally-handler #f) ;; Current finally block info: `(var label map level tokens)`
;; `map` is a list of `(tag . action)` which will
;; be emitted at the exit of the block. Code
;; should enter the finally block via `enter-finally-block`.
(handler-goto-fixups '()) ;; `goto`s that might need `leave` exprs added
(handler-level 0) ;; exception handler nesting depth
(catch-token-stack '())) ;; tokens identifying handler enter for current catch blocks
Expand All @@ -3732,14 +3732,27 @@ f(x) = yt(x)
(let ((l (make-label)))
(mark-label l)
l)))
(define (leave-finally-block action (need-goto #t))
;; Enter a finally block, either through the landing pad or via a jump if
;; `need-goto` is true. Before entering, the current code path is identified
;; with a tag which labels the action to be taken at finally handler exit.
;; `action` may be `(return x)`, `(break x)`, or a call to rethrow.
(define (enter-finally-block action (need-goto #t))
(let* ((tags (caddr finally-handler))
(tag (if (null? tags) 1 (+ 1 (caar tags)))))
;; To enter the current active finally block, set the tag variable
;; to identify the current code path with the action for this code path
;; which will run at finally block exit.
(set-car! (cddr finally-handler) (cons (cons tag action) tags))
(emit `(= ,(car finally-handler) ,tag))
(if need-goto
(begin (emit `(leave ,(+ 1 (- handler-level (cadddr finally-handler)))))
(emit `(goto ,(cadr finally-handler)))))
(let ((label (cadr finally-handler))
(dest-handler-level (cadddr finally-handler))
(dest-tokens (caddddr finally-handler)))
;; Leave current exception handling scope and jump to finally block
(let ((pexc (pop-exc-expr catch-token-stack dest-tokens)))
(if pexc (emit pexc)))
(emit `(leave ,(+ 1 (- handler-level dest-handler-level))))
(emit `(goto ,label))))
tag))
(define (pop-exc-expr src-tokens dest-tokens)
(if (eq? src-tokens dest-tokens)
Expand Down Expand Up @@ -3768,19 +3781,19 @@ f(x) = yt(x)
(else (make-ssavalue)))))
(if tmp (emit `(= ,tmp ,x)))
(if finally-handler
(leave-finally-block `(return ,(or tmp x)))
(enter-finally-block `(return ,(or tmp x)))
(begin (emit `(leave ,handler-level))
(actually-return (or tmp x))))
(or tmp x))
(actually-return x))))
(define (emit-break labl)
(let ((lvl (caddr labl))
(dest-tokens (cadddr labl)))
(let ((pexc (pop-exc-expr catch-token-stack dest-tokens)))
(if pexc (emit pexc)))
(if (and finally-handler (> (cadddr finally-handler) lvl))
(leave-finally-block `(break ,labl))
(enter-finally-block `(break ,labl))
(begin
(let ((pexc (pop-exc-expr catch-token-stack dest-tokens)))
(if pexc (emit pexc)))
(if (> handler-level lvl)
(emit `(leave ,(- handler-level lvl))))
(emit `(goto ,(cadr labl)))))))
Expand Down Expand Up @@ -4060,7 +4073,7 @@ f(x) = yt(x)
;; handler block entry
(emit `(= ,handler-token (enter ,catch)))
(set! handler-level (+ handler-level 1))
(if finally (begin (set! my-finally-handler (list finally endl '() handler-level))
(if finally (begin (set! my-finally-handler (list finally endl '() handler-level catch-token-stack))
(set! finally-handler my-finally-handler)
(emit `(= ,finally -1))))
(let* ((v1 (compile (cadr e) break-labels value #f)) ;; emit try block code
Expand All @@ -4078,11 +4091,12 @@ f(x) = yt(x)
(mark-label catch)
(emit `(leave 1))
(if finally
(begin (leave-finally-block '(call (top rethrow)) #f)
(if endl (mark-label endl))
(begin (enter-finally-block '(call (top rethrow)) #f) ;; enter block via exception
(mark-label endl) ;; non-exceptional control flow enters here
(set! finally-handler last-finally-handler)
(compile (caddr e) break-labels #f #f)
;; emit actions to be taken at exit of finally block
;; emit actions to be taken at exit of finally
;; block, depending on the tag variable `finally`
(let loop ((actions (caddr my-finally-handler)))
(if (pair? actions)
(let ((skip (if (and tail (null? (cdr actions))
Expand Down
73 changes: 70 additions & 3 deletions test/exceptions.jl
Original file line number Diff line number Diff line change
Expand Up @@ -178,10 +178,77 @@ end
end
end

@testset "Finally handling with exception stacks" begin
# The lowering of finally is quite subtle when combined with break or
# return because each finally block may be entered via multiple code paths
# (eg, different occurrences of return), and these code paths must diverge
# again once the finally block has completed. To complicate matters
# further, the return code path must thread through every nested finally
# block before actually returning, all the while preserving the information
# about which variable to return.

# Issue #34579
(()-> begin
try
throw("err")
catch
# Explicit return => exception should be popped before finally block
return
finally
@test length(Base.catch_stack()) == 0
end
end)()
@test length(Base.catch_stack()) == 0

while true
try
error("err1")
catch
try
# Break target is outside catch block, but finally is inside =>
# exception should not be popped inside finally block
break
finally
@test length(Base.catch_stack()) == 1
end
end
end
@test length(Base.catch_stack()) == 0

# Nested finally handling with `return`: each finally block should observe
# only the active exceptions as according to its nesting depth.
(() -> begin
try
try
error("err1")
catch
try
try
error("err2")
catch
# This return needs to thread control flow through
# multiple finally blocks in the linearized IR.
return
end
finally
# At this point err2 is dealt with
@test length(Base.catch_stack()) == 1
@test Base.catch_stack()[1][1] == ErrorException("err1")
end
end
finally
# At this point err1 is dealt with
@test length(Base.catch_stack()) == 0
end
end)()
@test length(Base.catch_stack()) == 0
end

@testset "Deep exception stacks" begin
# Generate deep exception stack with recursive handlers Note that if you
# let this overflow the program stack (not the exception stack) julia will
# crash. See #28577
# Generate deep exception stack with recursive handlers.
#
# (Note that if you let this overflow the program stack (not the exception
# stack) julia will crash. See #28577.)
function test_exc_stack_deep(n)
n != 1 || error("RootCause")
try
Expand Down

0 comments on commit 16ba0dd

Please sign in to comment.