Re: EH unwind bug

Re: EH unwind bug

[Jason Merrill]

>>>>> Thomas Weise <tom@zaphod.wh9.tu-dresden.de> writes:

> This is, the local object's destructor is executed twice, before the
> exception raiser's destructor and again after raising the exception.

This will be hard to fix.  The problem is that the return statement is
represented as a jump to the end of the function.  Jumps out of a scope run
any necessary cleanups; in this case, the destructors for obj and rais.
These cleanups are expanded at the point of the jump, which is inside the
EH regions for those same cleanups.

Fixing this will involve reworking fixup_gotos.

> Using -fsjlj-exceptions is fine. I hope this testcase comes in time and
> can be fixed before release.

-fsjlj-exceptions works for this testcase because it doesn't use EH regions
for those cleanups.

Jason

Re: EH unwind bug

[Thomas Weise]

> 
> > stackObj::~stackObj()
> > stackObj::~stackObj()
> > IOT trap/Abort
> 
> ./:
> Mon Dec  1 19:48:53 1997  Jason Merrill  <jason@yorick.cygnus.com>
> 
>         * except.c (expand_fixup_region_end): New fn.
>         * stmt.c (expand_cleanups): Use it to protect fixups.
> 
> cp/:
> Mon Dec  1 19:32:43 1997  Jason Merrill  <jason@yorick.cygnus.com>
> 
>         * exception.cc (__cp_pop_exception): Lose handler arg.
>         * except.c (do_pop_exception): Likewise.
>         (push_eh_cleanup): Let the cleanup mechanism supply the handler.
>         (expand_end_catch_block): Likewise.

[ http://www.cygnus.com/ml/egcs-bugs/1997-Dec/0008.html ]

I've assigned this patch to egcs971201 and it fixes the testcase.
Unfortunately I get a new internal error, please see the testcase below.

tom@hermes:/home/tom > g++ -v
Reading specs from
/usr/local/lib/gcc-lib/i486-pc-linux-gnulibc1/egcs-2.90.20/specs
gcc version egcs-2.90.20 971201 (gcc2-970802 experimental)
tom@hermes:/home/tom > g++ bug3.cc
bug3.cc: In method `void foo::op(unsigned char = false)':
bug3.cc:33: Internal compiler error.
bug3.cc:33: Please submit a full bug report to `egcs-bugs@cygnus.com'.

Re: EH unwind bug

[Jason Merrill]

>>>>> Thomas Weise <tom@zaphod.wh9.tu-dresden.de> writes:

> I've assigned this patch to egcs971201 and it fixes the testcase.
> Unfortunately I get a new internal error, please see the testcase below.

How about this instead?

Tue Dec  2 01:42:33 1997  Jason Merrill  <jason@yorick.cygnus.com>

	* except.c (expand_fixup_region_end): New fn.
	(expand_fixup_region_start): Likewise.
	(expand_eh_region_start_tree): Store cleanup into finalization here.
	* stmt.c (expand_cleanups): Use them to protect fixups.

Tue Dec  2 01:37:19 1997  Jason Merrill  <jason@yorick.cygnus.com>

	* exception.cc (__cp_pop_exception): Lose handler arg.
	* except.c (do_pop_exception): Likewise.
	(push_eh_cleanup): Let the cleanup mechanism supply the handler.
	(expand_end_catch_block): Likewise.

Index: except.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gcc/except.c,v
retrieving revision 1.33
diff -c -r1.33 except.c
*** except.c	1997/11/25 09:32:06	1.33
--- except.c	1997/12/02 09:59:15
***************
*** 1013,1018 ****
--- 1013,1019 ----
      }
  
    expand_eh_region_start_for_decl (decl);
+   ehstack.top->entry->finalization = cleanup;
  
    return 0;
  }
***************
*** 1138,1143 ****
--- 1139,1195 ----
      }
  }
  
+ /* End the EH region for a goto fixup.  We only need them in the region-based
+    EH scheme.  */
+ 
+ void
+ expand_fixup_region_start ()
+ {
+   if (! doing_eh (0) || exceptions_via_longjmp)
+     return;
+ 
+   expand_eh_region_start ();
+ }
+ 
+ /* End the EH region for a goto fixup.  CLEANUP is the cleanup we just
+    expanded; to avoid running it twice if it throws, we look through the
+    ehqueue for a matching region and rethrow from its outer_context.  */
+ 
+ void
+ expand_fixup_region_end (cleanup)
+      tree cleanup;
+ {
+   tree t;
+   struct eh_node *node;
+   int yes;
+ 
+   if (! doing_eh (0) || exceptions_via_longjmp)
+     return;
+ 
+   for (node = ehstack.top; node && node->entry->finalization != cleanup; )
+     node = node->chain;
+   if (node == 0)
+     for (node = ehqueue.head; node && node->entry->finalization != cleanup; )
+       node = node->chain;
+   if (node == 0)
+     abort ();
+ 
+   yes = suspend_momentary ();
+ 
+   t = build (RTL_EXPR, void_type_node, NULL_RTX, const0_rtx);
+   TREE_SIDE_EFFECTS (t) = 1;
+   do_pending_stack_adjust ();
+   start_sequence_for_rtl_expr (t);
+   expand_internal_throw (node->entry->outer_context);
+   do_pending_stack_adjust ();
+   RTL_EXPR_SEQUENCE (t) = get_insns ();
+   end_sequence ();
+ 
+   resume_momentary (yes);
+ 
+   expand_eh_region_end (t);
+ }
+ 
  /* If we are using the setjmp/longjmp EH codegen method, we emit a
     call to __sjthrow.
  
Index: stmt.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gcc/stmt.c,v
retrieving revision 1.131
diff -c -r1.131 stmt.c
*** stmt.c	1997/11/10 20:08:36	1.131
--- stmt.c	1997/12/02 04:11:35
***************
*** 4227,4233 ****
--- 4229,4242 ----
  		   the target.  Though the cleanups are expanded multiple
  		   times, the control paths are non-overlapping so the
  		   cleanups will not be executed twice.  */
+ 
+ 		/* We may need to protect fixups with rethrow regions.  */
+ 		int protect = (in_fixup && ! TREE_ADDRESSABLE (tail));
+ 		if (protect)
+ 		  expand_fixup_region_start ();
  		expand_expr (TREE_VALUE (tail), const0_rtx, VOIDmode, 0);
+ 		if (protect)
+ 		  expand_fixup_region_end (TREE_VALUE (tail));
  		free_temp_slots ();
  	      }
  	  }
Index: cp/exception.cc
===================================================================
RCS file: /cvs/cvsfiles/devo/gcc/cp/exception.cc,v
retrieving revision 1.10
diff -c -r1.10 exception.cc
*** cp/exception.cc	1997/11/25 06:15:39	1.10
--- cp/exception.cc	1997/12/02 10:04:24
***************
*** 126,142 ****
  
  /* Compiler hook to pop an exception that has been finalized.  Used by
     push_eh_cleanup().  P is the info for the exception caught by the
!    current catch block, and HANDLER determines if we've been called from
!    an exception handler; if so, we avoid destroying the object on rethrow.  */
  
  extern "C" void
! __cp_pop_exception (cp_eh_info *p, bool handler)
  {
    cp_eh_info **q = &__eh_info;
  
    --p->handlers;
  
!   if (p->handlers > 0 || (handler && p == *q))
      return;
  
    for (; *q; q = &((*q)->next))
--- 126,145 ----
  
  /* Compiler hook to pop an exception that has been finalized.  Used by
     push_eh_cleanup().  P is the info for the exception caught by the
!    current catch block.  */
  
  extern "C" void
! __cp_pop_exception (cp_eh_info *p)
  {
    cp_eh_info **q = &__eh_info;
  
    --p->handlers;
  
!   /* Don't really pop if there are still active handlers for our exception,
!      or if our exception is being rethrown (i.e. if the active exception is
!      our exception and it is uncaught).  */
!   if (p->handlers != 0
!       || (p == *q && !p->caught))
      return;
  
    for (; *q; q = &((*q)->next))
Index: cp/except.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gcc/cp/except.c,v
retrieving revision 1.138
diff -c -r1.138 except.c
*** cp/except.c	1997/11/27 20:55:03	1.138
--- cp/except.c	1997/12/02 03:27:09
***************
*** 440,447 ****
     if it is, it avoids destroying the object on rethrow.  */
  
  static tree
! do_pop_exception (handler)
!      tree handler;
  {
    tree fn, cleanup;
    fn = get_identifier ("__cp_pop_exception");
--- 440,446 ----
     if it is, it avoids destroying the object on rethrow.  */
  
  static tree
! do_pop_exception ()
  {
    tree fn, cleanup;
    fn = get_identifier ("__cp_pop_exception");
***************
*** 456,464 ****
        fn = build_lang_decl
  	(FUNCTION_DECL, fn,
  	 build_function_type (void_type_node, tree_cons
! 			      (NULL_TREE, ptr_type_node, tree_cons
! 			       (NULL_TREE, boolean_type_node,
! 				void_list_node))));
        DECL_EXTERNAL (fn) = 1;
        TREE_PUBLIC (fn) = 1;
        DECL_ARTIFICIAL (fn) = 1;
--- 455,461 ----
        fn = build_lang_decl
  	(FUNCTION_DECL, fn,
  	 build_function_type (void_type_node, tree_cons
! 			      (NULL_TREE, ptr_type_node, void_list_node)));
        DECL_EXTERNAL (fn) = 1;
        TREE_PUBLIC (fn) = 1;
        DECL_ARTIFICIAL (fn) = 1;
***************
*** 471,478 ****
    /* Arrange to do a dynamically scoped cleanup upon exit from this region.  */
    cleanup = lookup_name (get_identifier ("__exception_info"), 0);
    cleanup = build_function_call (fn, expr_tree_cons
! 				 (NULL_TREE, cleanup, expr_tree_cons
! 				  (NULL_TREE, handler, NULL_TREE)));
    return cleanup;
  }
  
--- 468,474 ----
    /* Arrange to do a dynamically scoped cleanup upon exit from this region.  */
    cleanup = lookup_name (get_identifier ("__exception_info"), 0);
    cleanup = build_function_call (fn, expr_tree_cons
! 				 (NULL_TREE, cleanup, NULL_TREE));
    return cleanup;
  }
  
***************
*** 481,497 ****
  static void
  push_eh_cleanup ()
  {
!   /* All cleanups must last longer than normal.  */
!   int yes = suspend_momentary ();
!   expand_decl_cleanup_no_eh (NULL_TREE, do_pop_exception (boolean_false_node));
!   resume_momentary (yes);
  
    expand_expr (build_unary_op (PREINCREMENT_EXPR, get_eh_handlers (), 1),
  	       const0_rtx, VOIDmode, EXPAND_NORMAL);
  
!   /* We don't destroy the exception object on rethrow, so we can't use
!      the normal cleanup mechanism for it.  */
!   expand_eh_region_start ();
  }
  
  /* call this to start a catch block. Typename is the typename, and identifier
--- 477,491 ----
  static void
  push_eh_cleanup ()
  {
!   int yes;
  
    expand_expr (build_unary_op (PREINCREMENT_EXPR, get_eh_handlers (), 1),
  	       const0_rtx, VOIDmode, EXPAND_NORMAL);
  
!   yes = suspend_momentary ();
!   /* All cleanups must last longer than normal.  */
!   expand_decl_cleanup (NULL_TREE, do_pop_exception ());
!   resume_momentary (yes);
  }
  
  /* call this to start a catch block. Typename is the typename, and identifier
***************
*** 657,665 ****
    expand_end_bindings (getdecls (), kept_level_p (), 0);
    poplevel (kept_level_p (), 1, 0);
        
-   /* Matches push_eh_cleanup.  */
-   expand_eh_region_end (do_pop_exception (boolean_true_node));
- 
    /* Cleanup the EH object.  */
    expand_end_bindings (getdecls (), kept_level_p (), 0);
    poplevel (kept_level_p (), 1, 0);
--- 651,656 ----

Re: EH unwind bug

[Thomas Weise]

Jason Merrill wrote:
> 
> >>>>> Thomas Weise <tom@zaphod.wh9.tu-dresden.de> writes:
> 
> > I've assigned this patch to egcs971201 and it fixes the testcase.
> > Unfortunately I get a new internal error, please see the testcase below.
> 
> How about this instead?

[ http://www.cygnus.com/ml/egcs-bugs/1997-Dec/0020.html ]

Looks great! No further compilation or runtime problems. Below are the
g++ test results (egcs971201 + EH unwind patch). Will this patch get
into the "release tarballs"?

Thanks for all the brilliant work,

Tom.

Test Run By tom on Wed Dec  3 13:02:38 1997
Native configuration is i486-pc-linux-gnulibc1

                === g++ tests ===

Running target unix
Using /usr/local/bin/../share/dejagnu/baseboards/unix.exp as board
description file for target.
Using /usr/local/bin/../share/dejagnu/config/unix.exp as generic
interface file for target.
Using /home/tom/egcs-971201/gcc/testsuite/config/default.exp as
tool-and-target-specific interface file.
Running /home/tom/egcs-971201/gcc/testsuite/g++.old-deja/old-deja.exp
...
XPASS: g++.jason/destruct3.C - (test for bogus messages, line 38)
XPASS: g++.mike/dyncast1.C  Execution test
XPASS: g++.mike/dyncast2.C  Execution test

                === g++ Summary ===

# of expected passes            3400
# of unexpected successes       3
# of expected failures          80
# of untested testcases         6
/home/tom/egcs-971201/gcc/testsuite/../xgcc version egcs-2.90.20 971201
(gcc2-970802 experimental)

-- 
Thomas Weise, http://www.inf.tu-dresden.de/~tw4
Dresden University of Technology, Department of Computer Science

Re: EH unwind bug

[Jeffrey A Law]

  In message < 348564CC.7721@zaphod.wh9.tu-dresden.de >you write:
  > Looks great! No further compilation or runtime problems. Below are the
  > g++ test results (egcs971201 + EH unwind patch). Will this patch get
  > into the "release tarballs"?
No.  The release is frozen; the unwind patch will go into the mailine
sources and will appear in the next snapshot (which we expect to be
done from the mainline tree instead of the release branch.

jeff

EH unwind bug

[Thomas Weise]

egcs-2.90.18, i486-pc-linux-gnulibc1, libc-5.4.39

After latest EH fixes I've finally managed to provide a testcase for the
EH bug I've already reported. Building and runnings attached program
gives following:

tom@hermes:/home/tom > g++ -v
Reading specs from
/usr/local/lib/gcc-lib/i486-pc-linux-gnulibc1/egcs-2.90.18/specs
gcc version egcs-2.90.18 971122 (gcc2-970802 experimental)
tom@hermes:/home/tom > g++ bug.cc
tom@hermes:/home/tom > a.out
stackObj::~stackObj()
stackObj::~stackObj()
IOT trap/Abort

This is, the local object's destructor is executed twice, before the
exception raiser's destructor and again after raising the exception.
Using -fsjlj-exceptions is fine. I hope this testcase comes in time and
can be fixed before release.

Tom.

-- 
Thomas Weise, http://www.inf.tu-dresden.de/~tw4
Dresden University of Technology, Department of Computer Science


#include <iostream.h>

class myExc
{
};


class myExcRaiser
{
public:
   ~myExcRaiser();
};


class stackObj
{
public:
   ~stackObj() { cout << "stackObj::~stackObj()" << endl; };
};


myExcRaiser::~myExcRaiser()
{
   throw myExc();
}


int test()
{
   myExcRaiser rais;
   stackObj obj;
   return 0;
}


int main()
{
  try {
      test();
  }
  catch (myExc &)
  {
      //cout << "caught myExc as expected\n";
  }
  cout << "SUCCESS" << endl;
  return 0;
}

EH unwind bug

[Thomas Weise]

egcs-2.90.18, i486-pc-linux-gnulibc1, libc-5.4.39

After latest EH fixes I've finally managed to provide a testcase for the
EH bug I've already reported. Building and runnings attached program
gives following:

tom@hermes:/home/tom > g++ -v
Reading specs from
/usr/local/lib/gcc-lib/i486-pc-linux-gnulibc1/egcs-2.90.18/specs
gcc version egcs-2.90.18 971122 (gcc2-970802 experimental)
tom@hermes:/home/tom > g++ bug.cc
tom@hermes:/home/tom > a.out
stackObj::~stackObj()
stackObj::~stackObj()
IOT trap/Abort

This is, the local object's destructor is executed twice, before the
exception raiser's destructor and again after raising the exception.
I hope it comes in time and can be fixed before release.

Tom.

-- 
Thomas Weise, http://www.inf.tu-dresden.de/~tw4
Dresden University of Technology, Department of Computer Science


#include <iostream.h>

class myExc
{
};


class myExcRaiser
{
public:
   ~myExcRaiser();
};


class stackObj
{
public:
   ~stackObj() { cout << "stackObj::~stackObj()" << endl; };
};


myExcRaiser::~myExcRaiser()
{
   throw myExc();
}


int test()
{
   myExcRaiser rais;
   stackObj obj;
   return 0;
}


int main()
{
  try {
      test();
  }
  catch (myExc &)
  {
      //cout << "caught myExc as expected\n";
  }
  cout << "SUCCESS" << endl;
  return 0;
}