More SJLJ exception breakage

More SJLJ exception breakage

[Ulrich Weigand]

Hello,

it seems SJLJ exception handling is broken again (at least on SPU).  The
most obvious symptom is that libstdc++ "operator new" is miscompiled.

A reduced test case would be:

typedef long unsigned int size_t;
extern "C" void * malloc (size_t __size);

class bad_alloc
{
  public:
    bad_alloc() throw() { }
    virtual ~bad_alloc() throw();
    virtual const char* what() const throw();
};

void *
operator new (size_t sz) throw (bad_alloc)
{
  void *p;

  p = (void *) malloc (sz);
  if (p == 0)
    throw bad_alloc();

  return p;
}

Building this with -O2 results in:

        brsl    $lr,malloc      ;# 7    _call_value/2   [length = 4]
        brz     $3,.L8  ;# 10   *spu.md:3590    [length = 4]
#note result of malloc is in register $r3  (the branch goes to the throw case)

        ai      $3,$sp,48       ;# 87   addsi3/2        [length = 4]
        brsl    $lr,_Unwind_SjLj_Unregister     ;# 88   _call/2 [length = 4]
#which is clobbered here with the argument passed to _Unwind_SjLj_Unregister

        ai      $sp,$sp,176     ;# 160  addsi3/2        [length = 4]
        lqd     $lr,16($sp)     ;# 162  _movv4si/5      [length = 4]
        lqd     $127,-16($sp)   ;# 161  _movv4si/5      [length = 4]
        bi      $lr     ;# 164  _return [length = 4]
#and never restored on its way out

The RTL is broken already in ...133r.eh:

(code_label 32 31 41 10 1 "" [1 uses])

(note 41 32 33 10 [bb 10] NOTE_INSN_BASIC_BLOCK)

(insn 33 41 39 10 new_op.ii:23 (set (reg/i:SI 3 $3)
        (reg:SI 138 [ <result> ])) -1 (nil))

(insn 39 33 86 10 new_op.ii:23 (use (reg/i:SI 3 $3)) -1 (nil))

(insn 86 39 87 10 (set (reg:SI 186)
        (plus:SI (reg/f:SI 132 virtual-stack-vars)
            (const_int -112 [0xffffffffffffff90]))) -1 (nil))

(insn 87 86 88 10 (set (reg:SI 3 $3)
        (reg:SI 186)) -1 (nil))

(call_insn 88 87 0 10 (parallel [
            (call (mem:QI (symbol_ref:SI ("_Unwind_SjLj_Unregister") [flags 0x41]) [0 S1 A8])
                (const_int 0 [0x0]))
            (clobber (reg:SI 0 $lr))
            (clobber (reg:SI 130 hbr))
        ]) -1 (expr_list:REG_EH_REGION (const_int 0 [0x0])
        (nil))
    (expr_list:REG_DEP_TRUE (use (reg:SI 3 $3))
        (nil)))

It would appear the call to _Unwind_SjLj_Unregister is simply inserted
too late; it needs to take place *before* the result register is set up
(insn 33), not afterwards.


Any idea why this might happen or suggestions what else to check? 

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: More SJLJ exception breakage

[Richard Guenther]

On Thu, Apr 16, 2009 at 8:24 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> Hello,
>
> it seems SJLJ exception handling is broken again (at least on SPU).  The
> most obvious symptom is that libstdc++ "operator new" is miscompiled.
>
> A reduced test case would be:
>
> typedef long unsigned int size_t;
> extern "C" void * malloc (size_t __size);
>
> class bad_alloc
> {
>  public:
>    bad_alloc() throw() { }
>    virtual ~bad_alloc() throw();
>    virtual const char* what() const throw();
> };
>
> void *
> operator new (size_t sz) throw (bad_alloc)
> {
>  void *p;
>
>  p = (void *) malloc (sz);
>  if (p == 0)
>    throw bad_alloc();
>
>  return p;
> }
>
> Building this with -O2 results in:
>
>        brsl    $lr,malloc      ;# 7    _call_value/2   [length = 4]
>        brz     $3,.L8  ;# 10   *spu.md:3590    [length = 4]
> #note result of malloc is in register $r3  (the branch goes to the throw case)
>
>        ai      $3,$sp,48       ;# 87   addsi3/2        [length = 4]
>        brsl    $lr,_Unwind_SjLj_Unregister     ;# 88   _call/2 [length = 4]
> #which is clobbered here with the argument passed to _Unwind_SjLj_Unregister
>
>        ai      $sp,$sp,176     ;# 160  addsi3/2        [length = 4]
>        lqd     $lr,16($sp)     ;# 162  _movv4si/5      [length = 4]
>        lqd     $127,-16($sp)   ;# 161  _movv4si/5      [length = 4]
>        bi      $lr     ;# 164  _return [length = 4]
> #and never restored on its way out
>
> The RTL is broken already in ...133r.eh:
>
> (code_label 32 31 41 10 1 "" [1 uses])
>
> (note 41 32 33 10 [bb 10] NOTE_INSN_BASIC_BLOCK)
>
> (insn 33 41 39 10 new_op.ii:23 (set (reg/i:SI 3 $3)
>        (reg:SI 138 [ <result> ])) -1 (nil))
>
> (insn 39 33 86 10 new_op.ii:23 (use (reg/i:SI 3 $3)) -1 (nil))
>
> (insn 86 39 87 10 (set (reg:SI 186)
>        (plus:SI (reg/f:SI 132 virtual-stack-vars)
>            (const_int -112 [0xffffffffffffff90]))) -1 (nil))
>
> (insn 87 86 88 10 (set (reg:SI 3 $3)
>        (reg:SI 186)) -1 (nil))
>
> (call_insn 88 87 0 10 (parallel [
>            (call (mem:QI (symbol_ref:SI ("_Unwind_SjLj_Unregister") [flags 0x41]) [0 S1 A8])
>                (const_int 0 [0x0]))
>            (clobber (reg:SI 0 $lr))
>            (clobber (reg:SI 130 hbr))
>        ]) -1 (expr_list:REG_EH_REGION (const_int 0 [0x0])
>        (nil))
>    (expr_list:REG_DEP_TRUE (use (reg:SI 3 $3))
>        (nil)))
>
> It would appear the call to _Unwind_SjLj_Unregister is simply inserted
> too late; it needs to take place *before* the result register is set up
> (insn 33), not afterwards.
>
>
> Any idea why this might happen or suggestions what else to check?

Maybe fixed after http://gcc.gnu.org/ml/gcc-cvs/2009-04/msg00805.html?
Or needs a similar fix in the place wherever we handle SJLJ exceptions
in the middle-end?

Richard.

> Thanks,
> Ulrich
>
> --
>  Dr. Ulrich Weigand
>  GNU Toolchain for Linux on System z and Cell BE
>  Ulrich.Weigand@de.ibm.com
>

Re: More SJLJ exception breakage

[Ulrich Weigand]

Richard Guenther wrote:
> On Thu, Apr 16, 2009 at 8:24 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> > It would appear the call to _Unwind_SjLj_Unregister is simply inserted
> > too late; it needs to take place *before* the result register is set up
> > (insn 33), not afterwards.
> >
> >
> > Any idea why this might happen or suggestions what else to check?
> 
> Maybe fixed after http://gcc.gnu.org/ml/gcc-cvs/2009-04/msg00805.html?
> Or needs a similar fix in the place wherever we handle SJLJ exceptions
> in the middle-end?

Well, the _Unwind_SjLj_Unregister call is introduced only at the RTL
level; I'm not sure how gimple changes can affect this?

In any case, I now understand the problem a bit better.  Fundamentally,
this is about the assumption in sjlj_emit_function_exit that the place
where to insert the unregister call -that was announced (from function.c)
via the sjlj_emit_function_exit_after routine- is located within the
last basic block, i.e. the one with a fall-through edge to EXIT.

In GCC 4.4, this is true; sjlj_emit_function_exit_after installs the
code_label 32 as place to call the unregister function, and at the
time sjlj_emit_function_exit is called, the RTL looks like:

(code_label/s 32 31 41 7 1 "" [1 uses])

(note 41 32 33 7 [bb 7] NOTE_INSN_BASIC_BLOCK)

(insn 33 41 39 7 new_op.ii:23 (set (reg/i:SI 3 $3)
        (reg:SI 138 [ <result> ])) -1 (nil))

(insn 39 33 0 7 new_op.ii:23 (use (reg/i:SI 3 $3)) -1 (nil))


In mainline GCC, however, sjlj_emit_function_exit_after also installs
code_label 32, but the RTL on entry to sjlj_emit_function_exit is:

(code_label 32 37 41 8 1 "" [1 uses])

(note 41 32 33 8 [bb 8] NOTE_INSN_BASIC_BLOCK)

(insn 33 41 38 8 new_op.ii:23 (set (reg/i:SI 3 $3)
        (reg:SI 138 [ <result> ])) -1 (nil))

(code_label 38 33 42 9 4 "" [1 uses])

(note 42 38 39 9 [bb 9] NOTE_INSN_BASIC_BLOCK)

(insn 39 42 0 9 new_op.ii:23 (use (reg/i:SI 3 $3)) -1 (nil))

So the last basic block is the one starting at code_label 38.


Now, the immediate cause for the difference is the following
patch by Honza:
http://gcc.gnu.org/ml/gcc-patches/2009-04/msg00375.html

With GCC 4.4, expand in fact generates the extra block at
code_label 38 as well, however, by the time sjlj_emit_function_exit
is called, the CFG has been cleaned up to remove it.  This cleanup
code was removed by the patch above.

In fact, the code generated by expand_function_end looks the same
for mainline and GCC 4.4:

;; Start of basic block () -> 7
(note 40 31 34 7 [bb 7] NOTE_INSN_BASIC_BLOCK)

(insn 34 40 35 7 new_op.ii:23 (clobber (reg/i:SI 3 $3)) -1 (nil))

(insn 35 34 36 7 new_op.ii:23 (clobber (reg:SI 138 [ <result> ])) -1 (nil))

(jump_insn 36 35 37 7 new_op.ii:23 (set (pc)
        (label_ref 38)) -1 (nil))
;; End of basic block 7 -> ( 9)

;; Succ edge  9 [100.0%]

(barrier 37 36 32)

;; Start of basic block ( 5) -> 8
;; Pred edge  5 [100.0%]
(code_label 32 37 41 8 1 "" [1 uses])

(note 41 32 33 8 [bb 8] NOTE_INSN_BASIC_BLOCK)

(insn 33 41 38 8 new_op.ii:23 (set (reg/i:SI 3 $3)
        (reg:SI 138 [ <result> ])) -1 (nil))
;; End of basic block 8 -> ( 9)

;; Succ edge  9 [100.0%]  (fallthru)

;; Start of basic block ( 7 8) -> 9
;; Pred edge  7 [100.0%]
;; Pred edge  8 [100.0%]  (fallthru)
(code_label 38 33 42 9 4 "" [1 uses])

(note 42 38 39 9 [bb 9] NOTE_INSN_BASIC_BLOCK)

(insn 39 42 0 9 new_op.ii:23 (use (reg/i:SI 3 $3)) -1 (nil))
;; End of basic block 9 -> ( 1)

;; Succ edge  EXIT [100.0%]  (fallthru)


In GCC 4.4, the delete_unreachable_block call in rest_of_handle_jump
then removes basic block 7; and the cleanup_cfg call in rest_of_handle_eh
then merges basic block 9 into 8.

The following patch, reverting those pieces of Honza's patch, fixes
the problem for me:

Index: except.c
===================================================================
*** except.c    (revision 146246)
--- except.c    (working copy)
*************** gate_handle_eh (void)
*** 4267,4272 ****
--- 4267,4273 ----
  static unsigned int
  rest_of_handle_eh (void)
  {
+   cleanup_cfg (CLEANUP_NO_INSN_DEL);
    finish_eh_generation ();
    cleanup_cfg (CLEANUP_NO_INSN_DEL);
    return 0;
Index: cfgcleanup.c
===================================================================
*** cfgcleanup.c        (revision 146246)
--- cfgcleanup.c        (working copy)
*************** cleanup_cfg (int mode)
*** 2198,2203 ****
--- 2198,2205 ----
  static unsigned int
  rest_of_handle_jump (void)
  {
+   delete_unreachable_blocks ();
+
    if (crtl->tail_call_emit)
      fixup_tail_calls ();
    return 0;


However, I'm wondering why expand_function_end deliberately generates an
unreachable basic block, which (in 4.4) was subsequently *always* deleted
again in the immediately following pass ...

This is done by the following piece of code:

  /* Emit the actual code to clobber return register.  */
  {
    rtx seq;

    start_sequence ();
    clobber_return_register ();
    expand_naked_return ();
    seq = get_insns ();
    end_sequence ();

    emit_insn_after (seq, clobber_after);
  }

  /* Output the label for the naked return from the function.  */
  emit_label (naked_return_label);

I'm not quite sure whether this is really still necessary (in particular
with the new DF and IRA infrastructure) ...


Any suggestions how to best fix this?

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

[PATCH] Re: More SJLJ exception breakage

[Ulrich Weigand]

I wrote:

> In any case, I now understand the problem a bit better.  Fundamentally,
> this is about the assumption in sjlj_emit_function_exit that the place
> where to insert the unregister call -that was announced (from function.c)
> via the sjlj_emit_function_exit_after routine- is located within the
> last basic block, i.e. the one with a fall-through edge to EXIT.

The patch below fixes the problem for me.  It changes sjlj_emit_function_exit
to always respect the location announced via sjlj_emit_function_exit_after.

The current code seems to be intended to cope with the case where that
location is "after" the last basic block.  However, expand_function_end
will in fact always announce the location of the "return_label" ... I
don't quite see how a label (which starts a new basic block anyway) can
ever be outside a basic block.  (In particular as the various CFG cleanup
before EH processing is no longer performed now ...)

Thus it seems to me it should be OK to simply always insert the call to
the unregister function at that location, and remove the "insert on edge"
code path.

This fixes nearly all instances of exception-related problems on SPU.
However, there is one case where it appears a pre-existing problem is
exposed: in the case of a return by fall-through at the end of a 
function without explict return statement, expand_function_end
generates a branch bypassing the "return_label" and going directly
to the "naked_return_label".

Now that sjlj_emit_function_exit in fact always places the unregister
call at the return_label, this causes that call to now by bypassed as
well in this case.

However, I don't quite understand why the return label is bypassed
anyway; in the semantically identical situation where a "return;"
is the last statement of the function, the return_label is *not*
bypassed.

The patch below therefore changes expand_function_end to omit that
extra branch, and have the fall-through function end case also go
through the return_label.  (The only remaining case where the
naked_return_label is used is __builtin_return.)

The patch fixes all C++ test suite regressions, without introducing
any new regressions on the SPU.

OK for mainline?

Bye,
Ulrich


ChangeLog:

	* function.c (expand_function_end): Do not emit a jump to the "naked"
	return label for fall-through returns.
	* except.c (sjlj_emit_function_exit): Always place the call to the
	unregister function at the location installed by expand_function_end.


Index: gcc/function.c
===================================================================
*** gcc/function.c	(revision 146246)
--- gcc/function.c	(working copy)
*************** expand_function_end (void)
*** 4804,4810 ****
  
      start_sequence ();
      clobber_return_register ();
-     expand_naked_return ();
      seq = get_insns ();
      end_sequence ();
  
--- 4804,4809 ----
*************** expand_function_end (void)
*** 4812,4818 ****
    }
  
    /* Output the label for the naked return from the function.  */
!   emit_label (naked_return_label);
  
    /* @@@ This is a kludge.  We want to ensure that instructions that
       may trap are not moved into the epilogue by scheduling, because
--- 4811,4818 ----
    }
  
    /* Output the label for the naked return from the function.  */
!   if (naked_return_label)
!     emit_label (naked_return_label);
  
    /* @@@ This is a kludge.  We want to ensure that instructions that
       may trap are not moved into the epilogue by scheduling, because
Index: gcc/except.c
===================================================================
*** gcc/except.c	(revision 146246)
--- gcc/except.c	(working copy)
*************** sjlj_emit_function_exit_after (rtx after
*** 2072,2080 ****
  static void
  sjlj_emit_function_exit (void)
  {
!   rtx seq;
!   edge e;
!   edge_iterator ei;
  
    start_sequence ();
  
--- 2072,2078 ----
  static void
  sjlj_emit_function_exit (void)
  {
!   rtx seq, insn;
  
    start_sequence ();
  
*************** sjlj_emit_function_exit (void)
*** 2088,2118 ****
       post-dominates all can_throw_internal instructions.  This is
       the last possible moment.  */
  
!   FOR_EACH_EDGE (e, ei, EXIT_BLOCK_PTR->preds)
!     if (e->flags & EDGE_FALLTHRU)
!       break;
!   if (e)
!     {
!       rtx insn;
  
!       /* Figure out whether the place we are supposed to insert libcall
!          is inside the last basic block or after it.  In the other case
!          we need to emit to edge.  */
!       gcc_assert (e->src->next_bb == EXIT_BLOCK_PTR);
!       for (insn = BB_HEAD (e->src); ; insn = NEXT_INSN (insn))
! 	{
! 	  if (insn == crtl->eh.sjlj_exit_after)
! 	    {
! 	      if (LABEL_P (insn))
! 		insn = NEXT_INSN (insn);
! 	      emit_insn_after (seq, insn);
! 	      return;
! 	    }
! 	  if (insn == BB_END (e->src))
! 	    break;
! 	}
!       insert_insn_on_edge (seq, e);
!     }
  }
  
  static void
--- 2086,2096 ----
       post-dominates all can_throw_internal instructions.  This is
       the last possible moment.  */
  
!   insn = crtl->eh.sjlj_exit_after;
!   if (LABEL_P (insn))
!     insn = NEXT_INSN (insn);
  
!   emit_insn_after (seq, insn);
  }
  
  static void


-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [PATCH] Re: More SJLJ exception breakage

[Richard Guenther]

On Tue, Apr 21, 2009 at 2:34 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> I wrote:
>
>> In any case, I now understand the problem a bit better.  Fundamentally,
>> this is about the assumption in sjlj_emit_function_exit that the place
>> where to insert the unregister call -that was announced (from function.c)
>> via the sjlj_emit_function_exit_after routine- is located within the
>> last basic block, i.e. the one with a fall-through edge to EXIT.
>
> The patch below fixes the problem for me.  It changes sjlj_emit_function_exit
> to always respect the location announced via sjlj_emit_function_exit_after.
>
> The current code seems to be intended to cope with the case where that
> location is "after" the last basic block.  However, expand_function_end
> will in fact always announce the location of the "return_label" ... I
> don't quite see how a label (which starts a new basic block anyway) can
> ever be outside a basic block.  (In particular as the various CFG cleanup
> before EH processing is no longer performed now ...)
>
> Thus it seems to me it should be OK to simply always insert the call to
> the unregister function at that location, and remove the "insert on edge"
> code path.
>
> This fixes nearly all instances of exception-related problems on SPU.
> However, there is one case where it appears a pre-existing problem is
> exposed: in the case of a return by fall-through at the end of a
> function without explict return statement, expand_function_end
> generates a branch bypassing the "return_label" and going directly
> to the "naked_return_label".
>
> Now that sjlj_emit_function_exit in fact always places the unregister
> call at the return_label, this causes that call to now by bypassed as
> well in this case.
>
> However, I don't quite understand why the return label is bypassed
> anyway; in the semantically identical situation where a "return;"
> is the last statement of the function, the return_label is *not*
> bypassed.
>
> The patch below therefore changes expand_function_end to omit that
> extra branch, and have the fall-through function end case also go
> through the return_label.  (The only remaining case where the
> naked_return_label is used is __builtin_return.)
>
> The patch fixes all C++ test suite regressions, without introducing
> any new regressions on the SPU.
>
> OK for mainline?

I don't know the history of that code too well, thus I defer to Ian
for approving the change.  But, can you give the patch full
bootstrapping and regtesting on a primary target?

Thanks,
Richard.

> Bye,
> Ulrich
>
>
> ChangeLog:
>
>        * function.c (expand_function_end): Do not emit a jump to the "naked"
>        return label for fall-through returns.
>        * except.c (sjlj_emit_function_exit): Always place the call to the
>        unregister function at the location installed by expand_function_end.
>
>
> Index: gcc/function.c
> ===================================================================
> *** gcc/function.c      (revision 146246)
> --- gcc/function.c      (working copy)
> *************** expand_function_end (void)
> *** 4804,4810 ****
>
>      start_sequence ();
>      clobber_return_register ();
> -     expand_naked_return ();
>      seq = get_insns ();
>      end_sequence ();
>
> --- 4804,4809 ----
> *************** expand_function_end (void)
> *** 4812,4818 ****
>    }
>
>    /* Output the label for the naked return from the function.  */
> !   emit_label (naked_return_label);
>
>    /* @@@ This is a kludge.  We want to ensure that instructions that
>       may trap are not moved into the epilogue by scheduling, because
> --- 4811,4818 ----
>    }
>
>    /* Output the label for the naked return from the function.  */
> !   if (naked_return_label)
> !     emit_label (naked_return_label);
>
>    /* @@@ This is a kludge.  We want to ensure that instructions that
>       may trap are not moved into the epilogue by scheduling, because
> Index: gcc/except.c
> ===================================================================
> *** gcc/except.c        (revision 146246)
> --- gcc/except.c        (working copy)
> *************** sjlj_emit_function_exit_after (rtx after
> *** 2072,2080 ****
>  static void
>  sjlj_emit_function_exit (void)
>  {
> !   rtx seq;
> !   edge e;
> !   edge_iterator ei;
>
>    start_sequence ();
>
> --- 2072,2078 ----
>  static void
>  sjlj_emit_function_exit (void)
>  {
> !   rtx seq, insn;
>
>    start_sequence ();
>
> *************** sjlj_emit_function_exit (void)
> *** 2088,2118 ****
>       post-dominates all can_throw_internal instructions.  This is
>       the last possible moment.  */
>
> !   FOR_EACH_EDGE (e, ei, EXIT_BLOCK_PTR->preds)
> !     if (e->flags & EDGE_FALLTHRU)
> !       break;
> !   if (e)
> !     {
> !       rtx insn;
>
> !       /* Figure out whether the place we are supposed to insert libcall
> !          is inside the last basic block or after it.  In the other case
> !          we need to emit to edge.  */
> !       gcc_assert (e->src->next_bb == EXIT_BLOCK_PTR);
> !       for (insn = BB_HEAD (e->src); ; insn = NEXT_INSN (insn))
> !       {
> !         if (insn == crtl->eh.sjlj_exit_after)
> !           {
> !             if (LABEL_P (insn))
> !               insn = NEXT_INSN (insn);
> !             emit_insn_after (seq, insn);
> !             return;
> !           }
> !         if (insn == BB_END (e->src))
> !           break;
> !       }
> !       insert_insn_on_edge (seq, e);
> !     }
>  }
>
>  static void
> --- 2086,2096 ----
>       post-dominates all can_throw_internal instructions.  This is
>       the last possible moment.  */
>
> !   insn = crtl->eh.sjlj_exit_after;
> !   if (LABEL_P (insn))
> !     insn = NEXT_INSN (insn);
>
> !   emit_insn_after (seq, insn);
>  }
>
>  static void
>
>
> --
>  Dr. Ulrich Weigand
>  GNU Toolchain for Linux on System z and Cell BE
>  Ulrich.Weigand@de.ibm.com
>

Re: [PATCH] Re: More SJLJ exception breakage

[Ian Lance Taylor]

Richard Guenther <richard.guenther@gmail.com> writes:

> On Tue, Apr 21, 2009 at 2:34 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:

>> However, I don't quite understand why the return label is bypassed
>> anyway; in the semantically identical situation where a "return;"
>> is the last statement of the function, the return_label is *not*
>> bypassed.

The return label is bypassed because the return label is for code which
copies the return value pseudo-register into the return hard register.
This is for cases like this:

int 
f(int i)
{
  if (i)
    my_exit_function();
  else
    return i;
}

Here let's assume that my_exit_function() does not have the noreturn
attribute and that the user is ignoring warnings about not returning a
value.  Without the naked return label the copy of the uninitialized
pseudo-register into the return register would trigger inappropriate
warnings and perhaps even break the old flow code.

It's entirely possible that these issues don't matter any more.  You do
need to confirm that code like the above does not issue an inappropriate
warning about an uninitialized value.  You may need to check that in a
few different languages.

Ian

Re: [PATCH] Re: More SJLJ exception breakage

[Ulrich Weigand]

Ian Lance Taylor wrote:

> The return label is bypassed because the return label is for code which
> copies the return value pseudo-register into the return hard register.
> This is for cases like this:
> 
> int 
> f(int i)
> {
>   if (i)
>     my_exit_function();
>   else
>     return i;
> }
> 
> Here let's assume that my_exit_function() does not have the noreturn
> attribute and that the user is ignoring warnings about not returning a
> value.  Without the naked return label the copy of the uninitialized
> pseudo-register into the return register would trigger inappropriate
> warnings and perhaps even break the old flow code.
> 
> It's entirely possible that these issues don't matter any more.  You do
> need to confirm that code like the above does not issue an inappropriate
> warning about an uninitialized value.  You may need to check that in a
> few different languages.

I've experimented with various versions of the above, and I never get
any inappropriate warning.  In fact, the RTL contains an explicit
clobber of the result *pseudo*-register -- that's probably why it
would have worked even with the old flow code.

What we now have is

  --- fall-through ---

  clobbers generated by clobber_return_register (both result pseudo
  and hard return registers are clobbered)

return_label:

  copy result pseudo into hard return register

naked_return_label:

  use return register


On the other hand, even without my patch, very similar sequences are
already being generated; the above example, *without* the patch,
results in:

  call my_exit_function

  clobbers generated by clobber_return_register

  goto return_label

[...]

return_label:

  copy result

naked_return_label:

  use return register

This is because the tree before expand contains a naked return statment,
which gets expanded via expand_null_return, which basically does a
clobber_return_register followed by goto return_label.

So it seems to me that the patch does not actually introduce any significantly
different situations from what we already today have and handle ...


In addition, as requested by Richard, I've now successfully completed a full
bootstrap and test run (all languages except Ada) on i686-linux with no
regressions.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [PATCH] Re: More SJLJ exception breakage

[Ian Lance Taylor]

"Ulrich Weigand" <uweigand@de.ibm.com> writes:

> In addition, as requested by Richard, I've now successfully completed a full
> bootstrap and test run (all languages except Ada) on i686-linux with no
> regressions.

The patch is fine by me if Richi is OK with it.

Thanks.

Ian

Re: [PATCH] Re: More SJLJ exception breakage

[Richard Guenther]

On Tue, Apr 21, 2009 at 11:21 PM, Ian Lance Taylor <iant@google.com> wrote:
> "Ulrich Weigand" <uweigand@de.ibm.com> writes:
>
>> In addition, as requested by Richard, I've now successfully completed a full
>> bootstrap and test run (all languages except Ada) on i686-linux with no
>> regressions.
>
> The patch is fine by me if Richi is OK with it.

Yes, go ahead.

Thanks,
Richard.

Re: [PATCH] Re: More SJLJ exception breakage

[Ulrich Weigand]

Richard Guenter wrote:
> On Tue, Apr 21, 2009 at 11:21 PM, Ian Lance Taylor <iant@google.com> wrote:
> > "Ulrich Weigand" <uweigand@de.ibm.com> writes:
> >
> >> In addition, as requested by Richard, I've now successfully completed a full
> >> bootstrap and test run (all languages except Ada) on i686-linux with no
> >> regressions.
> >
> > The patch is fine by me if Richi is OK with it.
> 
> Yes, go ahead.

Thanks, Richand and Ian!  I've checked this in now.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com