We have also made a fix for this problem in ST. The idea of the fix is exactly the same, but some implementation details are different. I
reported it to Wu Yongchong, who suggested I submit it for general
review and approval.

Apologies: I am reporting a patch which I have only validated in the context of ST's sources, which have quite some modifications compared to the open64.net sources.

Basic fix (wgen-diffs-1.txt): We added a new function Maybe_Emit_Cleanups(), which does the work of maintaining the scope_cleanup_stack and emitting the cleanup. This function avoids duplicate code in WGEN_Expand_Goto and WGEN_Expand_Return. Also we used an STL stack to save the scope info information, rather than defining a new type.

We made another change at the same time (wgen-diffs-2.txt): since the only remaining calls to Maybe_Emit_Cleanup() are made when generating handlers, the from_handler parameter is no longer required, so we
removed it and renamed the function.

Regards,
Steve.

Wu Yongchong wrote:
Hi, all
Can a gatekeeper help review this patch

It is fix of open64.net bug 828.

The change of version 3360 is introduced to resolve the issue about
cleanup code of GOTO to outer block and RETURN statements. It
manipulate the scope-cleanup-stack, so that cleanup code of each
cleanup would only contain cleanups not finished yet.

The issue of this patch is that the cleanup code generation process
may push and pop some items on the scope-clean-up stack, so that the
elements beyond the top of stack would be destroyed. To avoid this
issue, the new change save and restore these elements on the stack.




--- wgen_stmt.cxx.orig  2011-09-07 16:36:30.378726000 +0100
+++ wgen_stmt.cxx.1     2011-09-07 16:53:56.868216000 +0100
@@ -354,6 +354,43 @@
   }
 }
 
+static INT32
+Maybe_Emit_Cleanups (gs_t sentinel)
+{
+  // Emit all the cleanups required at a transfer of control out of
+  // the current scope, e.g. a goto or return.   Note, after this function,
+  // we continue in the same scope, so we should not actually pop the
+  // cleanups, but for correct processing we must temporarily pop them as we
+  // emit them.
+  // Therefore, pop them to a temporary stack, then restore them again
+  // afterwards.
+  INT32 result;
+  std::stack<SCOPE_CLEANUP_INFO> saved_cleanups;
+
+  while (scope_cleanup_i >= 0 &&
+         scope_cleanup_stack[scope_cleanup_i].stmt != sentinel) {
+    SCOPE_CLEANUP_INFO scope_cleanup = scope_cleanup_stack [scope_cleanup_i];
+    --scope_cleanup_i;
+    saved_cleanups.push(scope_cleanup);
+    gs_t stmt = scope_cleanup.stmt;
+    if (gs_tree_code(stmt) == GS_CLEANUP_STMT &&
+        !scope_cleanup.cleanup_eh_only)
+      WGEN_One_Stmt_Cleanup (gs_cleanup_expr(stmt));
+    else if (gs_tree_code(stmt) == GS_TRY_FINALLY_EXPR)
+      WGEN_One_Stmt_Cleanup (gs_tree_operand(stmt, 1));
+  }
+
+  result = scope_cleanup_i;
+
+  while (! saved_cleanups.empty ()) {
+    scope_cleanup_stack[++scope_cleanup_i] = saved_cleanups.top ();
+    saved_cleanups.pop ();
+  }
+
+  return result;
+}
+
+
 static void
 Emit_Cleanup(gs_t cleanup)
 {
@@ -2133,16 +2170,7 @@
        if (*li != *ci) break;
       if (ci!=Current_scope_nest.rend())
       {
-       int scope_cleanup_i_save = scope_cleanup_i;
-       i = scope_cleanup_i;
-        Is_True(i != -1, ("WGEN_Expand_Goto: scope_cleanup_stack empty"));
-        while ((i >= 0) && (scope_cleanup_stack [i].stmt != *ci))
-        {
-          scope_cleanup_i --;
-           Maybe_Emit_Cleanup (i, FALSE);
-          --i;
-        }
-        scope_cleanup_i = scope_cleanup_i_save;
+        i = Maybe_Emit_Cleanups (*ci);
         if (i == -1)
         {
 #ifdef FE_GNU_4_2_0
@@ -2293,20 +2321,8 @@
     }
 #endif
 
-    int i = scope_cleanup_i;
-    int scope_cleanup_i_save = scope_cleanup_i;
-    while (i != -1) {
-#ifdef KEY
-      scope_cleanup_i--;
-      Maybe_Emit_Cleanup (i, FALSE);
-#else
-      if (gs_tree_code(scope_cleanup_stack [i].stmt) == GS_CLEANUP_STMT)
-        WGEN_One_Stmt_Cleanup (gs_cleanup_expr(scope_cleanup_stack [i].stmt));
-#endif
-      --i;
-    }
+    Maybe_Emit_Cleanups (NULL);
 #ifdef KEY
-    scope_cleanup_i = scope_cleanup_i_save;
     if (emit_exceptions && processing_handler) {
        HANDLER_INFO hi = handler_stack.top();
        FmtAssert (hi.scope, ("NULL scope"));
@@ -2402,20 +2418,8 @@
     }
 #endif
 
-    int i = scope_cleanup_i;
-    int scope_cleanup_i_save = scope_cleanup_i;
-    while (i != -1) {
-#ifdef KEY
-      scope_cleanup_i--;
-      Maybe_Emit_Cleanup (i, FALSE);
-#else
-      if (gs_tree_code(scope_cleanup_stack [i].stmt) == GS_CLEANUP_STMT)
-        WGEN_One_Stmt_Cleanup (gs_cleanup_expr(scope_cleanup_stack [i].stmt));
-#endif
-      --i;
-    }
+    Maybe_Emit_Cleanups (NULL);
 #ifdef KEY
-    scope_cleanup_i = scope_cleanup_i_save;
     if (emit_exceptions && processing_handler) {
        HANDLER_INFO hi = handler_stack.top();
        FmtAssert (hi.scope, ("NULL scope"));
--- wgen_stmt.cxx.1     2011-09-07 16:53:56.868216000 +0100
+++ wgen_stmt.cxx.2     2011-09-07 17:04:36.141813000 +0100
@@ -324,34 +324,22 @@
 }
 
 // May be emit cleanup in I-th entry of stack depending on the statement
-// in the entry. FROM_HANDLER determines which stack to use. This function
+// in the entry. Called when we are emitting a handler. This function
 // is called when the decision whether to emit a cleanup and how to emit it
 // depends on the type of statement.
 // Wgen TODO: This function probably needs to be called from a few other
 // places also, they will be added as the need arises.
 static inline void
-Maybe_Emit_Cleanup(INT i, BOOL from_handler)
+Maybe_Emit_Handler_Cleanup(INT i)
 {
-  if (!from_handler)
-  {
-    gs_t stmt = scope_cleanup_stack [i].stmt;
-    if (gs_tree_code(stmt) == GS_CLEANUP_STMT &&
-        !scope_cleanup_stack[i].cleanup_eh_only)
-      WGEN_One_Stmt_Cleanup (gs_cleanup_expr(stmt));
-    else if (gs_tree_code(stmt) == GS_TRY_FINALLY_EXPR)
-      WGEN_One_Stmt_Cleanup (gs_tree_operand(stmt, 1));
-  }
-  else
-  {
-    HANDLER_INFO hi = handler_stack.top();
-    gs_t stmt = (*hi.scope) [i].stmt;
-
-    if (gs_tree_code(stmt) == GS_CLEANUP_STMT &&
-        !(*hi.scope) [i].cleanup_eh_only)
-      WGEN_One_Stmt_Cleanup (gs_cleanup_expr(stmt));
-    else if (gs_tree_code(stmt) == GS_TRY_FINALLY_EXPR)
-      WGEN_One_Stmt_Cleanup (gs_tree_operand(stmt, 1));
-  }
+  HANDLER_INFO hi = handler_stack.top();
+  gs_t stmt = (*hi.scope) [i].stmt;
+  
+  if (gs_tree_code(stmt) == GS_CLEANUP_STMT &&
+      !(*hi.scope) [i].cleanup_eh_only)
+    WGEN_One_Stmt_Cleanup (gs_cleanup_expr(stmt));
+  else if (gs_tree_code(stmt) == GS_TRY_FINALLY_EXPR)
+    WGEN_One_Stmt_Cleanup (gs_tree_operand(stmt, 1));
 }
 
 static INT32
@@ -2195,7 +2183,7 @@
     Is_True(i != -1, ("WGEN_Expand_Goto: scope_cleanup_stack empty inside 
handler"));
     while ((i >= 0) && ((*hi.scope) [i].stmt != *ci))
     {
-      Maybe_Emit_Cleanup (i, TRUE);
+      Maybe_Emit_Handler_Cleanup (i);
       --i;
     }
   }
@@ -2328,7 +2316,7 @@
        FmtAssert (hi.scope, ("NULL scope"));
        int j = hi.scope->size()-1;
        while (j != -1) {
-           Maybe_Emit_Cleanup (j, TRUE);
+           Maybe_Handler_Emit_Cleanup (j);
            --j;
        }
     }
@@ -2425,7 +2413,7 @@
        FmtAssert (hi.scope, ("NULL scope"));
        int j = hi.scope->size()-1;
        while (j != -1) {
-           Maybe_Emit_Cleanup (j, TRUE);
+           Maybe_Handler_Emit_Cleanup (j);
            --j;
        }
     }
------------------------------------------------------------------------------
Why Cloud-Based Security and Archiving Make Sense
Osterman Research conducted this study that outlines how and why cloud
computing security and archiving is rapidly being adopted across the IT 
space for its ease of implementation, lower cost, and increased 
reliability. Learn more. http://www.accelacomm.com/jaw/sfnl/114/51425301/
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel

Reply via email to