[RFA] exceptions.h: Fix definition of TRY_CATCH

[RFA] exceptions.h: Fix definition of TRY_CATCH

[Corinna Vinschen]

Hi,


this patch

  http://sourceware.org/ml/gdb/2005-01/msg00064.html

unfortunately breaks Cygwin.

The reason is the way how TRY_CATCH is defined as a macro:

  #define TRY_CATCH(EXCEPTION,MASK) \
     for (EXCEPTIONS_SIGSETJMP \
             (*exceptions_state_mc_init (uiout, &(EXCEPTION), (MASK))); \
          exceptions_state_mc_action_iter () ; ) \
      while (exceptions_state_mc_action_iter_1 ())

it calls EXCEPTIONS_SIGSETJMP (aka sigsetjmp on systems supporting it),
without bothering how sigsetjmp is defined on the host system.

Cygwin's (as well as RTEMS') sigsetjmp is defined in newlib's machine/setjmp.h
as a macro, too:

  #define sigsetjmp(env, savemask) ((env)[_SAVEMASK] = savemask,\
               sigprocmask (SIG_SETMASK, 0, (sigset_t *) ((env) + _SIGMASK)),\
               setjmp (env))

If you inspect it, you'll see that the first parameter `env' is evaluated
three times.  Since the first parameter is a call to the function
exceptions_state_mc_init(), the result is that three new catcher structures
are created.  But only one of them, the last one which has been created,
is correctly iniitialized by the call to exceptions_state_mc_action_iter(). 
I guess you see what will happen at one point.  A non-initialized catcher
is current when the function exceptions_state_mc_action_iter_1() is called
==> internal_error.

Note that this is *not* a flaw in newlib or Cygwin.  It's perfectly fine
that setjmp and sigsetjmp are defined as macros as above, see
http://www.opengroup.org/onlinepubs/009695399/functions/setjmp.html and
http://www.opengroup.org/onlinepubs/009695399/functions/sigsetjmp.html,
so the usual macro expansion restrictions apply.

Conclusion:  The actual problem is that the sigjmp_buf is created in a
function call which is the first parameter to sigsetjmp.

Solution: Don't do it. :-)

A patch is below.  Tested on Cygwin and Linux.


Ok to check in?


Corinna


	* exceptions.h (TRY_CATCH): Define setjmp/sigsetjmp macro save.

Index: exceptions.h
===================================================================
RCS file: /cvs/src/src/gdb/exceptions.h,v
retrieving revision 1.11
diff -u -p -r1.11 exceptions.h
--- exceptions.h	8 Feb 2005 23:44:06 -0000	1.11
+++ exceptions.h	9 Mar 2005 15:14:41 -0000
@@ -115,10 +115,13 @@ int exceptions_state_mc_action_iter_1 (v
   */
 
 #define TRY_CATCH(EXCEPTION,MASK) \
-    for (EXCEPTIONS_SIGSETJMP \
-           (*exceptions_state_mc_init (uiout, &(EXCEPTION), (MASK))); \
-         exceptions_state_mc_action_iter (); ) \
-      while (exceptions_state_mc_action_iter_1 ())
+     { \
+       EXCEPTIONS_SIGJMP_BUF *buf = \
+	 exceptions_state_mc_init (uiout, &(EXCEPTION), (MASK)); \
+       EXCEPTIONS_SIGSETJMP (*buf); \
+     } \
+     while (exceptions_state_mc_action_iter ()) \
+       while (exceptions_state_mc_action_iter_1 ())
 
 /* *INDENT-ON* */
 

-- 
Corinna Vinschen
Cygwin Project Co-Leader
Red Hat, Inc.

Re: [RFA] exceptions.h: Fix definition of TRY_CATCH

[Daniel Jacobowitz]

On Wed, Mar 09, 2005 at 05:48:03PM +0100, Corinna Vinschen wrote:
> If you inspect it, you'll see that the first parameter `env' is evaluated
> three times.  Since the first parameter is a call to the function
> exceptions_state_mc_init(), the result is that three new catcher structures
> are created.  But only one of them, the last one which has been created,
> is correctly iniitialized by the call to exceptions_state_mc_action_iter(). 
> I guess you see what will happen at one point.  A non-initialized catcher
> is current when the function exceptions_state_mc_action_iter_1() is called
> ==> internal_error.
> 
> Note that this is *not* a flaw in newlib or Cygwin.  It's perfectly fine
> that setjmp and sigsetjmp are defined as macros as above, see
> http://www.opengroup.org/onlinepubs/009695399/functions/setjmp.html and
> http://www.opengroup.org/onlinepubs/009695399/functions/sigsetjmp.html,
> so the usual macro expansion restrictions apply.
> 
> Conclusion:  The actual problem is that the sigjmp_buf is created in a
> function call which is the first parameter to sigsetjmp.

Nice catch!

> Solution: Don't do it. :-)
> 
> A patch is below.  Tested on Cygwin and Linux.
> 
> 
> Ok to check in?
> 
> 
> Corinna
> 
> 
> 	* exceptions.h (TRY_CATCH): Define setjmp/sigsetjmp macro save.

Yes, this is OK (save -> safe in the changelog).

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: [RFA] exceptions.h: Fix definition of TRY_CATCH

[Corinna Vinschen]

On Mar  9 12:40, Daniel Jacobowitz wrote:
> On Wed, Mar 09, 2005 at 05:48:03PM +0100, Corinna Vinschen wrote:
> > Conclusion:  The actual problem is that the sigjmp_buf is created in a
> > function call which is the first parameter to sigsetjmp.
> 
> Nice catch!

Thanks :-)

> > 	* exceptions.h (TRY_CATCH): Define setjmp/sigsetjmp macro save.
> 
> Yes, this is OK (save -> safe in the changelog).

Applied.  I've also took the freedom to fix the date in Joel's
ChangeLog entry.


Corinna

-- 
Corinna Vinschen
Cygwin Project Co-Leader
Red Hat, Inc.