[Gnash-commit] /srv/bzr/gnash/trunk r9507: Fix to try/catch/finally with function returns. The fix is suspiciously

[Benjamin Wolsey]

------------------------------------------------------------
revno: 9507
committer: Benjamin Wolsey <address@hidden>
branch nick: trunk
timestamp: Sun 2008-07-20 16:46:31 +0200
message:
  Fix to try/catch/finally with function returns. The fix is suspiciously
  simple, but has no more unexpected failures. There's more to do on
  try/catch.
modified:
  server/vm/ActionExec.cpp
  testsuite/misc-mtasc.all/exception.as
    ------------------------------------------------------------
    revno: 9502.1.7
    committer: Benjamin Wolsey <address@hidden>
    branch nick: workingcopy
    timestamp: Sun 2008-07-20 16:40:18 +0200
    message:
      Go to straight to finally on function return in try. Fixes the swfdec
      crash-0.5.4-13379-catch-in-register.swf test (at last) and the new
      misc-mtasc.all/exception.as tests. 
    modified:
      server/vm/ActionExec.cpp
      testsuite/misc-mtasc.all/exception.as
    ------------------------------------------------------------
    revno: 9502.1.8
    committer: Benjamin Wolsey <address@hidden>
    branch nick: workingcopy
    timestamp: Sun 2008-07-20 16:45:37 +0200
    message:
      One more test to check catch / finally returns. (Gnash passes already).
    modified:
      testsuite/misc-mtasc.all/exception.as

=== modified file 'server/vm/ActionExec.cpp'
--- a/server/vm/ActionExec.cpp  2008-07-01 02:17:03 +0000
+++ b/server/vm/ActionExec.cpp  2008-07-20 14:40:18 +0000
@@ -52,7 +52,7 @@
 # define STACK_DUMP_LIMIT 32
 
 // Define to get debugging messages for try / catch
-//#define GNASH_DEBUG_TRY 1
+#define GNASH_DEBUG_TRY 1
 
 #endif
 
@@ -379,15 +379,16 @@
 // catch { }
 // finally { };
 //
-// The catch and finally blocks are both optional.
-// 1. the catch block is *always* executed, even when no exception is thrown. 
This
-//    may be different when the catch variable is typed ( "catch (e:Error) 
{};" )
-// 2. the finally block is *always* executed, even when an exception is thrown.
-// 3. execution is interrupted if there are no catchers left and an exception
-//    is still on the stack.
+// For functions:
+//
+// 1. The catch block is only executed when an exception is thrown.
+// 2. The finally block is *always* executed, even when there is no exception
+//    *and* a return in try!
+// 3. Unhandled executions are handled the same.
 // 4. If an exception is thrown in a function and not caught within that 
function,
 //    the return value is the exception, unless the 'finally' block has its own
 //    return. (In that case, the exception is never handled).
+
 bool
 ActionExec::processExceptions(TryBlock& t)
 {
@@ -424,9 +425,17 @@
 #ifdef GNASH_DEBUG_TRY 
                 log_debug("TRY block: No exception, continuing as normal.");
 #endif
-                // All code up to the end of the TryBlock should be
-                // executed.
-                stop_pc = t._afterTriedOffset;
+
+                // No exception, so the try block should be executed,
+                // then finally (even if a function return is in try). There
+                // should be an action jump to finally at the end of try; if
+                // this is missing, catch must be executed as well.
+
+                // If there is a return in the try block, go straight to 
+                // finally.
+                if (_returning) pc = t._finallyOffset;
+                else stop_pc = t._finallyOffset;
+
                 t._tryState = TryBlock::TRY_FINALLY;
             }
             break;
@@ -438,8 +447,8 @@
 #ifdef GNASH_DEBUG_TRY
             log_debug("CATCH: TryBlock name = %s", t._name); 
 #endif               
-            // Process exceptions. The code in catch { } will 
-            // be executed whether this block is reached or not.
+            // If we are here, there should have been an exception
+            // in 'try'. 
             
             if (env.stack_size() && env.top(0).is_exception())
             {

=== modified file 'testsuite/misc-mtasc.all/exception.as'
--- a/testsuite/misc-mtasc.all/exception.as     2008-07-20 08:29:20 +0000
+++ b/testsuite/misc-mtasc.all/exception.as     2008-07-20 14:45:37 +0000
@@ -134,6 +134,26 @@
                }
        }
 
+       function returnInCatchAndFinally(o)
+       {
+               try {
+                       throw 'try'
+                       note ("After throw in try");
+                       o.num += 1;
+               }
+               catch (e) {
+                   note ("Catch after throw in try");
+                       return 'catch';
+                   o.num += 1;
+               }
+               finally {
+                   note ("Finally after returns in catch");
+                   o.num += 1;
+                   return 'finally';
+               }
+       }
+
+
        function test_all()
        {
                var res = 'string';
@@ -203,17 +223,21 @@
         try {
             ret = returnInTryAndCatch(o);
         }
-        xcheck_equals(ret, "try");
-        xcheck_equals(o.num, 5);
+        check_equals(ret, "try");
+        check_equals(o.num, 5);
 
         ret = returnInTryCatchAndFinally(o);
-        xcheck_equals(ret, "finally");
-        xcheck_equals(o.num, 6);
+        check_equals(ret, "finally");
+        check_equals(o.num, 6);
         
         o.sequence = "";
         tryCatchAndFinally(o);
         check_equals(o.sequence, "tryfinally");
-        xcheck_equals(o.num, 8);
+        check_equals(o.num, 8);
+
+        ret = returnInCatchAndFinally(o);
+        check_equals(o.num, 9);
+        check_equals(ret, "finally");
 
                try {
                        throwNested();
@@ -230,7 +254,7 @@
                var myTest = new Test;
                myTest.test_all();
 
-        check_totals(21);
+        check_totals(23);
         Dejagnu.done();
        }