Merge pull request #951 from skyfex/devel

Tests and fixes for return in except and finally statements

compiler/ccgstmts.nim

@@ -263,33 +263,33 @@ proc genIf(p: BProc, n: PNode, d: var TLoc) =
 
 
 proc blockLeaveActions(p: BProc, howManyTrys, howManyExcepts: int) = 
-  # This is called by return and break stmts.
+  # Called by return and break stmts.
-  # When jumping out of try/except/finally stmts,
+  # Deals with issues faced when jumping out of try/except/finally stmts,
-  # we need to pop safe points from try statements,
-  # execute finally-stmts, and pop exceptions
-  # from except stmts
 
-  let L = p.nestedTryStmts.len
-
-  # danger of endless recursion! we workaround this here by a temp stack
   var stack: seq[PNode]
-  newSeq(stack, howManyTrys)
+  newSeq(stack, 0)
-  for i in countup(1, howManyTrys): 
-    stack[i-1] = p.nestedTryStmts[L-i]
-  setLen(p.nestedTryStmts, L-howManyTrys)
   
   var alreadyPoppedCnt = p.inExceptBlock
-  for tryStmt in items(stack):
+  for i in countup(1, howManyTrys):
+
     if gCmd != cmdCompileToCpp:
+      # Pop safe points generated by try
       if alreadyPoppedCnt > 0:
         dec alreadyPoppedCnt
       else:
         linefmt(p, cpsStmts, "#popSafePoint();$n")
-    # Find finally-stmts for this try-stmt
+
-    # and generate a copy of the finally stmts here
+    # Pop this try-stmt of the list of nested trys
+    # so we don't infinite recurse on it in the next step.
+    var tryStmt = p.nestedTryStmts.pop
+    stack.add(tryStmt)
+
+    # Find finally-stmt for this try-stmt
+    # and generate a copy of its sons
     var finallyStmt = lastSon(tryStmt)
     if finallyStmt.kind == nkFinally: 
       genStmts(p, finallyStmt.sons[0])
+
   # push old elements again:
   for i in countdown(howManyTrys-1, 0): 
     p.nestedTryStmts.add(stack[i])
@@ -304,7 +304,14 @@ proc genReturnStmt(p: BProc, t: PNode) =
   p.beforeRetNeeded = true
   genLineDir(p, t)
   if (t.sons[0].kind != nkEmpty): genStmts(p, t.sons[0])
-  blockLeaveActions(p, min(1, p.nestedTryStmts.len), p.inExceptBlock)
+  blockLeaveActions(p, 
+    howManyTrys    = p.nestedTryStmts.len,
+    howManyExcepts = p.inExceptBlock)
+  if (p.finallySafePoints.len > 0):
+    # If we're in a finally block, and we came here by exception
+    # consume it before we return.
+    var safePoint = p.finallySafePoints[p.finallySafePoints.len-1]
+    linefmt(p, cpsStmts, "if ($1.status != 0) #popCurrentException();$n", safePoint)    
   lineFF(p, cpsStmts, "goto BeforeRet;$n", "br label %BeforeRet$n", [])
 
 proc genComputedGoto(p: BProc; n: PNode) =
@@ -843,7 +850,9 @@ proc genTry(p: BProc, t: PNode, d: var TLoc) =
   discard pop(p.nestedTryStmts)
   endBlock(p) # end of else block
   if i < length and t.sons[i].kind == nkFinally:
+    p.finallySafePoints.add(safePoint)
     exprBlock(p, t.sons[i].sons[0], d)
+    discard pop(p.finallySafePoints)
   linefmt(p, cpsStmts, "if ($1.status != 0) #reraiseException();$n", safePoint)
 
 proc genAsmOrEmitStmt(p: BProc, t: PNode, isAsmStmt=false): PRope =

compiler/cgendata.nim

@@ -65,11 +65,13 @@ type
     prc*: PSym                # the Nimrod proc that this C proc belongs to
     beforeRetNeeded*: bool    # true iff 'BeforeRet' label for proc is needed
     threadVarAccessed*: bool  # true if the proc already accessed some threadvar
-    nestedTryStmts*: seq[PNode] # in how many nested try statements we are
+    nestedTryStmts*: seq[PNode]   # in how many nested try statements we are
-                                # (the vars must be volatile then)
+                                  # (the vars must be volatile then)
     inExceptBlock*: int       # are we currently inside an except block?
                               # leaving such scopes by raise or by return must
                               # execute any applicable finally blocks
+    finallySafePoints*: seq[PRope]  # For correctly cleaning up exceptions when
+                                    # using return in finally statements
     labels*: Natural          # for generating unique labels in the C proc
     blocks*: seq[TBlock]      # nested blocks
     breakIdx*: int            # the block that will be exited
@@ -142,6 +144,7 @@ proc newProc*(prc: PSym, module: BModule): BProc =
   else: result.options = gOptions
   newSeq(result.blocks, 1)
   result.nestedTryStmts = @[]
+  result.finallySafePoints = @[]
 
 iterator cgenModules*: var BModule =
   for i in 0..high(gModules):

tests/exception/tfinally4.nim

@@ -0,0 +1,40 @@
+discard """
+  file: "tfinally4.nim"
+  output: "B1\nA1\n1\nB1\nB2\ncatch\nA1\n1\nB1\nA1\nA2\n2\nB1\nB2\ncatch\nA1\nA2\n0\nB1\nA1\n1\nB1\nB2\nA1\n1\nB1\nA1\nA2\n2\nB1\nB2\nA1\nA2\n3"
+"""
+
+# More thorough test of return-in-finaly
+
+var raiseEx = true
+var returnA = true
+var returnB = false
+ 
+proc main: int = 
+  try: #A
+    try: #B
+      if raiseEx:
+        raise newException(EOS, "")
+      return 3
+    finally: #B
+      echo "B1"
+      if returnB:
+        return 2
+      echo "B2"
+  except EOS: #A
+    echo "catch"
+  finally: #A
+    echo "A1"
+    if returnA:
+      return 1
+    echo "A2"
+
+for x in [true, false]:
+  for y in [true, false]:
+    for z in [true, false]:
+      # echo "raiseEx: " & $x
+      # echo "returnA: " & $y
+      # echo "returnB: " & $z
+      raiseEx = x
+      returnA = y
+      returnB = z
+      echo main()

tests/exception/tnestedreturn.nim

@@ -0,0 +1,40 @@
+discard """
+  file: "tnestedreturn.nim"
+  output: "A\nB\nC\n"
+"""
+
+# Various tests of return nested in double try/except statements
+
+proc test1() =
+
+  finally: echo "A"
+
+  try:
+    raise newException(EOS, "Problem")
+  except EOS:
+    return
+
+test1()
+
+
+proc test2() =
+
+  finally: echo "B"
+
+  try:
+    return
+  except EOS:
+    discard
+
+test2()
+
+proc test3() =
+  try:
+    try:
+      raise newException(EOS, "Problem")
+    except EOS:
+      return
+  finally:
+    echo "C"
+
+test3()

tests/exception/tnestedreturn2.nim

@@ -0,0 +1,20 @@
+discard """
+  file: "tnestedreturn.nim"
+  outputsub: "Error: unhandled exception: Problem [EOS]"
+  exitcode: "1"
+"""
+
+proc test4() =
+  try:
+    try:
+      raise newException(EOS, "Problem")
+    except EOS:
+      return
+  finally:
+    discard
+
+# Should cause unhandled exception error,
+# but could cause segmentation fault if 
+# exceptions are not handled properly.
+test4()
+raise newException(EOS, "Problem")