diff mbox

=?UTF-8?Q?Fix=20compiler=20crash=20with=20large=20Objective-C=20and=20?= =?UTF-8?Q?=20=5F=5FFLT=5FMAX=5F=5F=20(it=20may=20affect=20C++=20too)?=

Message ID 1293161553.252420281@192.168.4.58
State New
Headers show

Commit Message

Nicola Pero Dec. 24, 2010, 3:32 a.m. UTC 
This patch fixes a compiler segmentation fault when compiling a large Objective-C 
file and encountering __FLT_MAX__.  I found the bug while compiling NSView.m in 
gnustep-gui, which would crash GCC 4.6 with a segmentation fault.

The problem is that c-family/c-cppbuiltin.c, which is used by all the C family 
frontends, does lazy initialization of these macros (nice trick), using some 
variables that need GTY markers.

Unfortunately, the file was listed in the gtfiles list only for C, and not for
Objective-C, C++ and Objective-C++, which use it too ;-)

So, when using a C-family language different from C, if the source file is big 
enough to trigger garbage collection midway in the parsing, and if __FLT_MAX__ is 
then encountered later, GCC may crash with a segmentation fault because 
c-cppbuiltin.c is accessing variables that have been freed.  Exactly what
happens in Objective-C.

This patch simply adds c-family/c-cppbuiltin.c to the various gtfiles, fixing it.  
With this patch I can now compile gnustep-gui with GCC 4.6. :-)

At least, it fixes the bug for Objective-C.  I'd have expected to see a similar 
problem with Objective-C++ (and C++), but if I compile the same gnustep-gui
file with the ObjC++ compiler, I don't see a crash.  It most likely is due
to differences in how the garbage collector is invoked or used, but I haven't
investigated them.

I didn't really know how to produce a manageable testcase even for ObjC
but I later discovered that this had already been reported and fixed for the C 
language as PR bootstrap/44509 (pity the fix only fixed it for C, it would have 
saved me hours of debugging).  The fix (which consisted in adding the GTY markers 
and adding the file to gtfiles for C) did include a nice testcase, which I'm now 
recycling for ObjC.  It's a nice testcase that shows the compiler crash, and shows 
that the patch fixes it. :-)

I don't have testcases for C++ or ObjC++.  I haven't spent too much time on them, other than noticing that the file is missing from gtfiles, which suggests that there will be a problem under some conditions (if there's agreement on this, I recommend
applying the patch for all C-languages so that someone else will be spared the pain
of debugging it yet again from scratch for another language when it occurs).

Incidentally, I also noticed the missing dependency of c-cppbuiltin.c on its gt 
file, and added it.

Ok to commit to trunk ?

Thanks

Comments

Mike Stump Dec. 24, 2010, 10:33 p.m. UTC | #1 
On Dec 23, 2010, at 7:32 PM, "Nicola Pero" <nicola.pero@meta-innovation.com> wrote:
> This patch fixes a compiler segmentation fault when compiling a large Objective-C 
> file and encountering __FLT_MAX__.

Ok.
diff mbox

Patch

Index: objc/ChangeLog
===================================================================
--- objc/ChangeLog      (revision 168215)
+++ objc/ChangeLog      (working copy)
@@ -1,3 +1,7 @@ 
+2010-12-24  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       * config-lang.in (gtfiles): Added c-family/c-cppbuiltin.c.
+
 2010-12-22  Nathan Froyd  <froydnj@codesourcery.com>
 
        * objc-act.c (next_sjlj_build_enter_and_setjmp): Use prototype_p.
Index: objc/config-lang.in
===================================================================
--- objc/config-lang.in (revision 168215)
+++ objc/config-lang.in (working copy)
@@ -33,4 +33,4 @@ 
 # Most of the object files for cc1obj actually come from C.
 lang_requires="c"
 
-gtfiles="\$(srcdir)/objc/objc-act.h \$(srcdir)/c-parser.c \$(srcdir)/c-tree.h \$(srcdir)/c-decl.c \$(srcdir)/c-objc-common.c \$(srcdir)/c-family/c-common.c \$(srcdir)/c-family/c-common.h \$(srcdir)/c-family/c-objc.h \$(srcdir)/c-family/c-pragma.h \$(srcdir)/c-family/c-pragma.c \$(srcdir)/objc/objc-act.c"
+gtfiles="\$(srcdir)/objc/objc-act.h \$(srcdir)/c-parser.c \$(srcdir)/c-tree.h \$(srcdir)/c-decl.c \$(srcdir)/c-objc-common.c \$(srcdir)/c-family/c-common.c \$(srcdir)/c-family/c-common.h \$(srcdir)/c-family/c-objc.h \$(srcdir)/c-family/c-cppbuiltin.c \$(srcdir)/c-family/c-pragma.h \$(srcdir)/c-family/c-pragma.c \$(srcdir)/objc/objc-act.c"
Index: ChangeLog
===================================================================
--- ChangeLog   (revision 168215)
+++ ChangeLog   (working copy)
@@ -1,3 +1,8 @@ 
+2010-12-24  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       * Makefile.in (c-family/c-cppbuiltin.o): Depend on
+       gt-c-family-c-cppbuiltin.h.
+
 2010-12-23  Sebastian Pop  <sebastian.pop@amd.com>
            Richard Guenther  <rguenther@suse.de>
 
Index: testsuite/ChangeLog
===================================================================
--- testsuite/ChangeLog (revision 168215)
+++ testsuite/ChangeLog (working copy)
@@ -1,3 +1,9 @@ 
+2010-12-24  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       * objc.dg/pr44509.m: New.
+       
 2010-12-22  Sebastian Pop  <sebastian.pop@amd.com>
 
        PR tree-optimization/46758
Index: testsuite/objc.dg/pr44509.m
===================================================================
--- testsuite/objc.dg/pr44509.m (revision 0)
+++ testsuite/objc.dg/pr44509.m (revision 0)
@@ -0,0 +1,9 @@ 
+/* PR bootstrap/44509 */
+/* { dg-do compile } */
+/* { dg-options "--param ggc-min-expand=0 --param ggc-min-heapsize=0" } */
+
+double
+foo (void)
+{
+  return __DBL_MAX__ - __FLT_MAX__;
+}
Index: objcp/ChangeLog
===================================================================
--- objcp/ChangeLog     (revision 168215)
+++ objcp/ChangeLog     (working copy)
@@ -1,3 +1,7 @@ 
+2010-12-24  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       * config-lang.in (gtfiles): Added c-family/c-cppbuiltin.c.
+
 2010-12-06  Nicola Pero  <nicola.pero@meta-innovation.com>
 
        * config-lang.in (gtfiles): Added c-family/c-objc.h.
Index: objcp/config-lang.in
===================================================================
--- objcp/config-lang.in        (revision 168215)
+++ objcp/config-lang.in        (working copy)
@@ -37,4 +37,4 @@ 
 lang_requires="objc c++"
 subdir_requires="objc cp"
 
-gtfiles="\$(srcdir)/objcp/objcp-decl.c \$(srcdir)/objc/objc-act.c \$(srcdir)/objc/objc-act.h \$(srcdir)/cp/rtti.c \$(srcdir)/cp/mangle.c \$(srcdir)/cp/name-lookup.h \$(srcdir)/cp/name-lookup.c \$(srcdir)/cp/cp-tree.h \$(srcdir)/cp/decl.h \$(srcdir)/cp/call.c \$(srcdir)/cp/decl.c \$(srcdir)/cp/decl2.c \$(srcdir)/cp/pt.c \$(srcdir)/cp/repo.c \$(srcdir)/cp/semantics.c \$(srcdir)/cp/tree.c \$(srcdir)/cp/parser.c \$(srcdir)/cp/method.c \$(srcdir)/cp/typeck2.c \$(srcdir)/c-family/c-common.c \$(srcdir)/c-family/c-common.h \$(srcdir)/c-family/c-objc.h \$(srcdir)/c-family/c-lex.c \$(srcdir)/c-family/c-pragma.h \$(srcdir)/c-family/c-pragma.c \$(srcdir)/cp/cp-objcp-common.c"
+gtfiles="\$(srcdir)/objcp/objcp-decl.c \$(srcdir)/objc/objc-act.c \$(srcdir)/objc/objc-act.h \$(srcdir)/cp/rtti.c \$(srcdir)/cp/mangle.c \$(srcdir)/cp/name-lookup.h \$(srcdir)/cp/name-lookup.c \$(srcdir)/cp/cp-tree.h \$(srcdir)/cp/decl.h \$(srcdir)/cp/call.c \$(srcdir)/cp/decl.c \$(srcdir)/cp/decl2.c \$(srcdir)/cp/pt.c \$(srcdir)/cp/repo.c \$(srcdir)/cp/semantics.c \$(srcdir)/cp/tree.c \$(srcdir)/cp/parser.c \$(srcdir)/cp/method.c \$(srcdir)/cp/typeck2.c \$(srcdir)/c-family/c-common.c \$(srcdir)/c-family/c-common.h \$(srcdir)/c-family/c-objc.h \$(srcdir)/c-family/c-lex.c \$(srcdir)/c-family/c-cppbuiltin.c \$(srcdir)/c-family/c-pragma.h \$(srcdir)/c-family/c-pragma.c \$(srcdir)/cp/cp-objcp-common.c"
Index: cp/ChangeLog
===================================================================
--- cp/ChangeLog        (revision 168215)
+++ cp/ChangeLog        (working copy)
@@ -1,3 +1,7 @@ 
+2010-12-24  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       * config-lang.in (gtfiles): Added c-family/c-cppbuiltin.c.
+
 2010-12-22  Nathan Froyd  <froydnj@codesourcery.com>
 
        * decl.c (decls_match, duplicate_decls): Use prototype_p.
Index: cp/config-lang.in
===================================================================
--- cp/config-lang.in   (revision 168215)
+++ cp/config-lang.in   (working copy)
@@ -30,4 +30,4 @@ 
 
 target_libs="target-libstdc++-v3"
 
-gtfiles="\$(srcdir)/cp/rtti.c \$(srcdir)/cp/mangle.c \$(srcdir)/cp/name-lookup.h \$(srcdir)/cp/name-lookup.c \$(srcdir)/cp/cp-tree.h \$(srcdir)/cp/decl.h \$(srcdir)/cp/call.c \$(srcdir)/cp/decl.c \$(srcdir)/cp/decl2.c \$(srcdir)/cp/pt.c \$(srcdir)/cp/repo.c \$(srcdir)/cp/semantics.c \$(srcdir)/cp/tree.c \$(srcdir)/cp/parser.c \$(srcdir)/cp/method.c \$(srcdir)/cp/typeck2.c \$(srcdir)/c-family/c-common.c \$(srcdir)/c-family/c-common.h \$(srcdir)/c-family/c-objc.h \$(srcdir)/c-family/c-lex.c \$(srcdir)/c-family/c-pragma.h \$(srcdir)/c-family/c-pragma.c \$(srcdir)/cp/class.c \$(srcdir)/cp/cp-objcp-common.c \$(srcdir)/cp/cp-lang.c"
+gtfiles="\$(srcdir)/cp/rtti.c \$(srcdir)/cp/mangle.c \$(srcdir)/cp/name-lookup.h \$(srcdir)/cp/name-lookup.c \$(srcdir)/cp/cp-tree.h \$(srcdir)/cp/decl.h \$(srcdir)/cp/call.c \$(srcdir)/cp/decl.c \$(srcdir)/cp/decl2.c \$(srcdir)/cp/pt.c \$(srcdir)/cp/repo.c \$(srcdir)/cp/semantics.c \$(srcdir)/cp/tree.c \$(srcdir)/cp/parser.c \$(srcdir)/cp/method.c \$(srcdir)/cp/typeck2.c \$(srcdir)/c-family/c-common.c \$(srcdir)/c-family/c-common.h \$(srcdir)/c-family/c-objc.h \$(srcdir)/c-family/c-lex.c \$(srcdir)/c-family/c-cppbuiltin.c \$(srcdir)/c-family/c-pragma.h \$(srcdir)/c-family/c-pragma.c \$(srcdir)/cp/class.c \$(srcdir)/cp/cp-objcp-common.c \$(srcdir)/cp/cp-lang.c"
Index: Makefile.in
===================================================================
--- Makefile.in (revision 168215)
+++ Makefile.in (working copy)
@@ -2107,7 +2107,8 @@ 
 c-family/c-cppbuiltin.o : c-family/c-cppbuiltin.c $(CONFIG_H) $(SYSTEM_H) \
        coretypes.h $(TM_H) $(TREE_H) version.h $(C_COMMON_H) $(C_PRAGMA_H) \
        $(FLAGS_H) output.h $(TREE_H) $(TARGET_H) \
-       $(TM_P_H) debug.h $(CPP_ID_DATA_H) cppbuiltin.h
+       $(TM_P_H) debug.h $(CPP_ID_DATA_H) cppbuiltin.h \
+       gt-c-family-c-cppbuiltin.h
        $(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) \
                $< $(OUTPUT_OPTION)