bug#4643: two different definitions for handlerlist and catchlist in eval.c and alloc.c

bug#4643: two different definitions for handlerlist and catchlist in eval.c and alloc.c

[Dan Nicolaescu]

struct handlerlist and struct catchlist have different definitions in
eval.c and alloc.c.
This generates and error when using doing link time optimizations with
gcc. 

bug#4643: two different definitions for catchlist in eval.c and alloc.c

[Dan Nicolaescu]

Here's a patch that fixes this problem by moving the definition in a
separate file included in both places.

OK to check in?

Index: src/Makefile.in
===================================================================
RCS file: /cvsroot/emacs/emacs/src/Makefile.in,v
retrieving revision 1.450
diff -u -3 -p -r1.450 Makefile.in
--- src/Makefile.in	26 Sep 2009 19:49:17 -0000	1.450
+++ src/Makefile.in	7 Oct 2009 08:26:06 -0000
@@ -1231,14 +1231,14 @@ xsmfns.o: xsmfns.c $(config_h) systime.h
 /* The files of Lisp proper */
 
 alloc.o: alloc.c process.h frame.h window.h buffer.h  puresize.h syssignal.h keyboard.h \
- blockinput.h atimer.h systime.h character.h dispextern.h $(config_h) \
+ blockinput.h atimer.h systime.h character.h dispextern.h catchtag.h $(config_h) \
  $(INTERVALS_H)
 bytecode.o: bytecode.c buffer.h syntax.h character.h window.h dispextern.h \
   frame.h xterm.h $(config_h)
 data.o: data.c buffer.h puresize.h character.h syssignal.h keyboard.h frame.h \
    termhooks.h $(config_h)
 eval.o: eval.c commands.h keyboard.h blockinput.h atimer.h systime.h \
-  dispextern.h $(config_h)
+  dispextern.h catchtag.h $(config_h)
 floatfns.o: floatfns.c syssignal.h $(config_h)
 fns.o: fns.c commands.h $(config_h) frame.h buffer.h character.h keyboard.h \
  keymap.h frame.h window.h dispextern.h $(INTERVALS_H) coding.h md5.h \
Index: src/alloc.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/alloc.c,v
retrieving revision 1.449
diff -u -3 -p -r1.449 alloc.c
--- src/alloc.c	25 Aug 2009 06:03:09 -0000	1.449
+++ src/alloc.c	7 Oct 2009 08:26:08 -0000
@@ -56,6 +56,7 @@ along with GNU Emacs.  If not, see <http
 #include "syssignal.h"
 #include "termhooks.h"		/* For struct terminal.  */
 #include <setjmp.h>
+#include "catchtag.h"
 
 /* GC_MALLOC_CHECK defined means perform validity checks of malloc'd
    memory.  Can do this only if using gmalloc.c.  */
@@ -4939,13 +4940,6 @@ staticpro (varaddress)
     abort ();
 }
 
-struct catchtag
-{
-    Lisp_Object tag;
-    Lisp_Object val;
-    struct catchtag *next;
-};
-
 \f
 /***********************************************************************
 			  Protection from GC
Index: src/eval.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/eval.c,v
retrieving revision 1.315
diff -u -3 -p -r1.315 eval.c
--- src/eval.c	1 Oct 2009 17:47:44 -0000	1.315
+++ src/eval.c	7 Oct 2009 08:26:08 -0000
@@ -26,6 +26,7 @@ along with GNU Emacs.  If not, see <http
 #include "keyboard.h"
 #include "dispextern.h"
 #include <setjmp.h>
+#include "catchtag.h"
 
 #if HAVE_X_WINDOWS
 #include "xterm.h"
@@ -49,41 +50,6 @@ struct backtrace
 
 struct backtrace *backtrace_list;
 
-/* This structure helps implement the `catch' and `throw' control
-   structure.  A struct catchtag contains all the information needed
-   to restore the state of the interpreter after a non-local jump.
-
-   Handlers for error conditions (represented by `struct handler'
-   structures) just point to a catch tag to do the cleanup required
-   for their jumps.
-
-   catchtag structures are chained together in the C calling stack;
-   the `next' member points to the next outer catchtag.
-
-   A call like (throw TAG VAL) searches for a catchtag whose `tag'
-   member is TAG, and then unbinds to it.  The `val' member is used to
-   hold VAL while the stack is unwound; `val' is returned as the value
-   of the catch form.
-
-   All the other members are concerned with restoring the interpreter
-   state.  */
-
-struct catchtag
-{
-  Lisp_Object tag;
-  Lisp_Object val;
-  struct catchtag *next;
-  struct gcpro *gcpro;
-  jmp_buf jmp;
-  struct backtrace *backlist;
-  struct handler *handlerlist;
-  int lisp_eval_depth;
-  int pdlcount;
-  int poll_suppress_count;
-  int interrupt_input_blocked;
-  struct byte_stack *byte_stack;
-};
-
 struct catchtag *catchlist;
 
 #ifdef DEBUG_GCPRO
--- /dev/null	2009-08-21 19:23:04.524086726 -0700
+++ src/catchtag.h	2009-10-07 01:11:39.784950000 -0700
@@ -0,0 +1,57 @@
+/* Structure for implementing the `catch' and `throw' control structure.
+   Copyright (C) 2009 Free Software Foundation, Inc.
+
+This file is part of GNU Emacs.
+
+GNU Emacs is free software: you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation, either version 3 of the License, or
+(at your option) any later version.
+
+GNU Emacs is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef CATCHTAG_H
+#define CATCHTAG_H
+
+/* This structure helps implement the `catch' and `throw' control
+   structure.  A struct catchtag contains all the information needed
+   to restore the state of the interpreter after a non-local jump.
+
+   Handlers for error conditions (represented by `struct handler'
+   structures) just point to a catch tag to do the cleanup required
+   for their jumps.
+
+   catchtag structures are chained together in the C calling stack;
+   the `next' member points to the next outer catchtag.
+
+   A call like (throw TAG VAL) searches for a catchtag whose `tag'
+   member is TAG, and then unbinds to it.  The `val' member is used to
+   hold VAL while the stack is unwound; `val' is returned as the value
+   of the catch form.
+
+   All the other members are concerned with restoring the interpreter
+   state.  */
+
+struct catchtag
+{
+  Lisp_Object tag;
+  Lisp_Object val;
+  struct catchtag *next;
+  struct gcpro *gcpro;
+  jmp_buf jmp;
+  struct backtrace *backlist;
+  struct handler *handlerlist;
+  int lisp_eval_depth;
+  int pdlcount;
+  int poll_suppress_count;
+  int interrupt_input_blocked;
+  struct byte_stack *byte_stack;
+};
+
+#endif /* CATCHTAG_H */

bug#4643: two different definitions for catchlist in eval.c and alloc.c

[Dan Nicolaescu]

Dan Nicolaescu <dann@ics.uci.edu> writes:

  > Here's a patch that fixes this problem by moving the definition in a
  > separate file included in both places.
  > 
  > OK to check in?

Ping.

Without a change like this emacs cannot be compiled using gcc-4.5 -flto 


  > Index: src/Makefile.in
  > ===================================================================
  > RCS file: /cvsroot/emacs/emacs/src/Makefile.in,v
  > retrieving revision 1.450
  > diff -u -3 -p -r1.450 Makefile.in
  > --- src/Makefile.in	26 Sep 2009 19:49:17 -0000	1.450
  > +++ src/Makefile.in	7 Oct 2009 08:26:06 -0000
  > @@ -1231,14 +1231,14 @@ xsmfns.o: xsmfns.c $(config_h) systime.h
  >  /* The files of Lisp proper */
  >  
  >  alloc.o: alloc.c process.h frame.h window.h buffer.h  puresize.h syssignal.h keyboard.h \
  > - blockinput.h atimer.h systime.h character.h dispextern.h $(config_h) \
  > + blockinput.h atimer.h systime.h character.h dispextern.h catchtag.h $(config_h) \
  >   $(INTERVALS_H)
  >  bytecode.o: bytecode.c buffer.h syntax.h character.h window.h dispextern.h \
  >    frame.h xterm.h $(config_h)
  >  data.o: data.c buffer.h puresize.h character.h syssignal.h keyboard.h frame.h \
  >     termhooks.h $(config_h)
  >  eval.o: eval.c commands.h keyboard.h blockinput.h atimer.h systime.h \
  > -  dispextern.h $(config_h)
  > +  dispextern.h catchtag.h $(config_h)
  >  floatfns.o: floatfns.c syssignal.h $(config_h)
  >  fns.o: fns.c commands.h $(config_h) frame.h buffer.h character.h keyboard.h \
  >   keymap.h frame.h window.h dispextern.h $(INTERVALS_H) coding.h md5.h \
  > Index: src/alloc.c
  > ===================================================================
  > RCS file: /cvsroot/emacs/emacs/src/alloc.c,v
  > retrieving revision 1.449
  > diff -u -3 -p -r1.449 alloc.c
  > --- src/alloc.c	25 Aug 2009 06:03:09 -0000	1.449
  > +++ src/alloc.c	7 Oct 2009 08:26:08 -0000
  > @@ -56,6 +56,7 @@ along with GNU Emacs.  If not, see <http
  >  #include "syssignal.h"
  >  #include "termhooks.h"		/* For struct terminal.  */
  >  #include <setjmp.h>
  > +#include "catchtag.h"
  >  
  >  /* GC_MALLOC_CHECK defined means perform validity checks of malloc'd
  >     memory.  Can do this only if using gmalloc.c.  */
  > @@ -4939,13 +4940,6 @@ staticpro (varaddress)
  >      abort ();
  >  }
  >  
  > -struct catchtag
  > -{
  > -    Lisp_Object tag;
  > -    Lisp_Object val;
  > -    struct catchtag *next;
  > -};
  > -
  >  \f
  >  /***********************************************************************
  >  			  Protection from GC
  > Index: src/eval.c
  > ===================================================================
  > RCS file: /cvsroot/emacs/emacs/src/eval.c,v
  > retrieving revision 1.315
  > diff -u -3 -p -r1.315 eval.c
  > --- src/eval.c	1 Oct 2009 17:47:44 -0000	1.315
  > +++ src/eval.c	7 Oct 2009 08:26:08 -0000
  > @@ -26,6 +26,7 @@ along with GNU Emacs.  If not, see <http
  >  #include "keyboard.h"
  >  #include "dispextern.h"
  >  #include <setjmp.h>
  > +#include "catchtag.h"
  >  
  >  #if HAVE_X_WINDOWS
  >  #include "xterm.h"
  > @@ -49,41 +50,6 @@ struct backtrace
  >  
  >  struct backtrace *backtrace_list;
  >  
  > -/* This structure helps implement the `catch' and `throw' control
  > -   structure.  A struct catchtag contains all the information needed
  > -   to restore the state of the interpreter after a non-local jump.
  > -
  > -   Handlers for error conditions (represented by `struct handler'
  > -   structures) just point to a catch tag to do the cleanup required
  > -   for their jumps.
  > -
  > -   catchtag structures are chained together in the C calling stack;
  > -   the `next' member points to the next outer catchtag.
  > -
  > -   A call like (throw TAG VAL) searches for a catchtag whose `tag'
  > -   member is TAG, and then unbinds to it.  The `val' member is used to
  > -   hold VAL while the stack is unwound; `val' is returned as the value
  > -   of the catch form.
  > -
  > -   All the other members are concerned with restoring the interpreter
  > -   state.  */
  > -
  > -struct catchtag
  > -{
  > -  Lisp_Object tag;
  > -  Lisp_Object val;
  > -  struct catchtag *next;
  > -  struct gcpro *gcpro;
  > -  jmp_buf jmp;
  > -  struct backtrace *backlist;
  > -  struct handler *handlerlist;
  > -  int lisp_eval_depth;
  > -  int pdlcount;
  > -  int poll_suppress_count;
  > -  int interrupt_input_blocked;
  > -  struct byte_stack *byte_stack;
  > -};
  > -
  >  struct catchtag *catchlist;
  >  
  >  #ifdef DEBUG_GCPRO
  > --- /dev/null	2009-08-21 19:23:04.524086726 -0700
  > +++ src/catchtag.h	2009-10-07 01:11:39.784950000 -0700
  > @@ -0,0 +1,57 @@
  > +/* Structure for implementing the `catch' and `throw' control structure.
  > +   Copyright (C) 2009 Free Software Foundation, Inc.
  > +
  > +This file is part of GNU Emacs.
  > +
  > +GNU Emacs is free software: you can redistribute it and/or modify
  > +it under the terms of the GNU General Public License as published by
  > +the Free Software Foundation, either version 3 of the License, or
  > +(at your option) any later version.
  > +
  > +GNU Emacs is distributed in the hope that it will be useful,
  > +but WITHOUT ANY WARRANTY; without even the implied warranty of
  > +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  > +GNU General Public License for more details.
  > +
  > +You should have received a copy of the GNU General Public License
  > +along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
  > +
  > +#ifndef CATCHTAG_H
  > +#define CATCHTAG_H
  > +
  > +/* This structure helps implement the `catch' and `throw' control
  > +   structure.  A struct catchtag contains all the information needed
  > +   to restore the state of the interpreter after a non-local jump.
  > +
  > +   Handlers for error conditions (represented by `struct handler'
  > +   structures) just point to a catch tag to do the cleanup required
  > +   for their jumps.
  > +
  > +   catchtag structures are chained together in the C calling stack;
  > +   the `next' member points to the next outer catchtag.
  > +
  > +   A call like (throw TAG VAL) searches for a catchtag whose `tag'
  > +   member is TAG, and then unbinds to it.  The `val' member is used to
  > +   hold VAL while the stack is unwound; `val' is returned as the value
  > +   of the catch form.
  > +
  > +   All the other members are concerned with restoring the interpreter
  > +   state.  */
  > +
  > +struct catchtag
  > +{
  > +  Lisp_Object tag;
  > +  Lisp_Object val;
  > +  struct catchtag *next;
  > +  struct gcpro *gcpro;
  > +  jmp_buf jmp;
  > +  struct backtrace *backlist;
  > +  struct handler *handlerlist;
  > +  int lisp_eval_depth;
  > +  int pdlcount;
  > +  int poll_suppress_count;
  > +  int interrupt_input_blocked;
  > +  struct byte_stack *byte_stack;
  > +};
  > +
  > +#endif /* CATCHTAG_H */

bug#4643: two different definitions for catchlist in eval.c and alloc.c

[Stefan Monnier]

>> Here's a patch that fixes this problem by moving the definition in a
>> separate file included in both places.
>> 
>> OK to check in?

> Ping.

I agree on the idea, but I don't want a new file just for that
declaration, so please move it to some existing file instead.


        Stefan

bug#4643: two different definitions for catchlist in eval.c and alloc.c

[Dan Nicolaescu]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

  > >> Here's a patch that fixes this problem by moving the definition in a
  > >> separate file included in both places.
  > >> 
  > >> OK to check in?
  > 
  > > Ping.
  > 
  > I agree on the idea, but I don't want a new file just for that
  > declaration, so please move it to some existing file instead.

The catchtag structure definition needs a #include <setjmp.h> to precede it.
We don't include setjmp.h in many places, so it will need to be
added... 
I don't see any existing header that would be a good match (other than lisp.h).
So please suggest where to add it.

bug#4643: two different definitions for catchlist in eval.c and alloc.c

[Stefan Monnier]

>> >> Here's a patch that fixes this problem by moving the definition in a
>> >> separate file included in both places.
>> >> 
>> >> OK to check in?
>> 
>> > Ping.
>> 
>> I agree on the idea, but I don't want a new file just for that
>> declaration, so please move it to some existing file instead.

> The catchtag structure definition needs a #include <setjmp.h> to
> precede it.  We don't include setjmp.h in many places, so it will need
> to be added...  I don't see any existing header that would be a good
> match (other than lisp.h).  So please suggest where to add it.

Huh.... lisp.h?


        Stefan

bug#4643: two different definitions for catchlist in eval.c and alloc.c

[Dan Nicolaescu]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

  > >> >> Here's a patch that fixes this problem by moving the definition in a
  > >> >> separate file included in both places.
  > >> >> 
  > >> >> OK to check in?
  > >> 
  > >> > Ping.
  > >> 
  > >> I agree on the idea, but I don't want a new file just for that
  > >> declaration, so please move it to some existing file instead.
  > 
  > > The catchtag structure definition needs a #include <setjmp.h> to
  > > precede it.  We don't include setjmp.h in many places, so it will need
  > > to be added...  I don't see any existing header that would be a good
  > > match (other than lisp.h).  So please suggest where to add it.
  > 
  > Huh.... lisp.h?

If you don't mind setjmp.h being included everywhere, fine with me.

bug#4643: two different definitions for catchlist in eval.c and alloc.c

[Stefan Monnier]

> If you don't mind setjmp.h being included everywhere, fine with me.

Indeed, I don't mind.


        Stefan

bug#4643: marked as done (two different definitions for handlerlist and catchlist in eval.c and alloc.c)

[Emacs bug Tracking System]

[-- Attachment #1: Type: text/plain, Size: 960 bytes --]

Your message dated Sun, 18 Oct 2009 21:30:15 -0700 (PDT)
with message-id <200910190430.n9J4UFNE012243@godzilla.ics.uci.edu>
and subject line Re: bug#4643: two different definitions for catchlist in eval.c and alloc.c
has caused the Emacs bug report #4643,
regarding two different definitions for handlerlist and catchlist in eval.c and alloc.c
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@emacsbugs.donarmstrong.com
immediately.)


-- 
4643: http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=4643
Emacs Bug Tracking System
Contact owner@emacsbugs.donarmstrong.com with problems

[-- Attachment #2: Type: message/rfc822, Size: 2727 bytes --]

From: Dan Nicolaescu <dann@ics.uci.edu>
To: bug-gnu-emacs <bug-gnu-emacs@gnu.org>
Subject: two different definitions for handlerlist and catchlist in eval.c and alloc.c
Date: Mon, 5 Oct 2009 09:18:33 -0700 (PDT)
Message-ID: <200910051618.n95GIX0i012545@godzilla.ics.uci.edu>


struct handlerlist and struct catchlist have different definitions in
eval.c and alloc.c.
This generates and error when using doing link time optimizations with
gcc. 




[-- Attachment #3: Type: message/rfc822, Size: 2495 bytes --]

From: Dan Nicolaescu <dann@ics.uci.edu>
To: Stefan Monnier <monnier@iro.umontreal.ca>
Cc: 4643-done@emacsbugs.donarmstrong.com
Subject: Re: bug#4643: two different definitions for catchlist in eval.c and alloc.c
Date: Sun, 18 Oct 2009 21:30:15 -0700 (PDT)
Message-ID: <200910190430.n9J4UFNE012243@godzilla.ics.uci.edu>

Stefan Monnier <monnier@iro.umontreal.ca> writes:

  > > If you don't mind setjmp.h being included everywhere, fine with me.
  > 
  > Indeed, I don't mind.

Thanks. Done.

bug#4643: two different definitions for catchlist in eval.c and alloc.c

[Adrian Robert]

Just out of curiosity, is this new #include definitely not needed in  
the ns*.m files?  I don't understand the actual issue, but it seems  
like every other source file has been touched.  While gcc-4.5 will  
probably never be used on OS X (moving to clang), it might be in  
GNUstep compilations.

bug#4643: two different definitions for catchlist in eval.c and alloc.c

[Dan Nicolaescu]

Adrian Robert <adrian.b.robert@gmail.com> writes:

  > Just out of curiosity, is this new #include definitely not needed in
  > the ns*.m files?  I don't understand the actual issue, but it seems

I think I grepped for *.[ch], so *.m where accidentally left out.
Fixed now.

  > like every other source file has been touched.  While gcc-4.5 will
  > probably never be used on OS X (moving to clang), it might be in
  > GNUstep compilations.

This would affect any compiler that does IPA.