in the file wgen-diffs-2.txt line 62 and line 71
- Maybe_Emit_Cleanup (j, TRUE);
+ Maybe_Handler_Emit_Cleanup (j);
you replay Maybe_Emit_Cleanup with Maybe_Handler_Emit_Cleanup. Should
it be Maybe_Emit_Handler_cleanup ?
On Fri, Sep 9, 2011 at 6:08 PM, Stephen Clarke <stephen.cla...@st.com> wrote:
> 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;
> }
> }
>
>
--
yongchong
------------------------------------------------------------------------------
BlackBerry® DevCon Americas, Oct. 18-20, San Francisco, CA
http://p.sf.net/sfu/rim-devcon-copy2
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel
