PATCH: Always create a new language function for nested functions

[Meador Inge <meadori at codesourcery dot com>]

Hi All,

Last week I went to build a mips-gnu-linux toolchain and the built compiler
seg faulted while building glibc.  Bisecting the change pointed to r187757.  The
problem really goes back to r178692, which added support for
-Wunused-local-typedefs.  r187757 just made it easier to hit by enabling
the warning more aggressively.

The problem is that in r187757 the following changes were applied:

@@ -8455,9 +8479,11 @@
 void
 c_push_function_context (void)
 {
-  struct language_function *p;
-  p = ggc_alloc_language_function ();
-  cfun->language = p;
+  struct language_function *p = cfun->language;
+  /* cfun->language might have been already allocated by the use of
+     -Wunused-local-typedefs.  In that case, just re-use it.  */
+  if (p == NULL)
+    cfun->language = p = ggc_alloc_cleared_language_function ();

   p->base.x_stmt_tree = c_stmt_tree;
   c_stmt_tree.x_cur_stmt_list
@@ -8483,7 +8509,11 @@

   pop_function_context ();
   p = cfun->language;
-  cfun->language = NULL;
+  /* When -Wunused-local-typedefs is in effect, cfun->languages is
+     used to store data throughout the life time of the current cfun,
+     So don't deallocate it.  */
+  if (!warn_unused_local_typedefs)
+    cfun->language = NULL;


This causes problems with nested functions because the following scenario
might happen (and did happen in the cause of building glibc for MIPS):

   1. A nested function is found while compiling with -Wunused-local-typedefs.
   2. c_push_function_context reuses the outer functions cfun->language
      instance.
   3. c_push_function_context saves a reference to the current functions
      statement tree:
          p->base.x_stmt_tree = c_stmt_tree;
   4. c_pop_function_context is executed after processing the nested function.
      Note the c_stmt_tree value is still saved per step (3).
   5. The outer function continues to be parsed and upon encountering more
      statements the statement tree is resized.  This puts the original
      statement tree memory back in the free pool.  Therefore
      cfun->language->base.x_stmt_tree is pointing to free memory.
   6. The memory that was previously associated with the statement tree gets
      allocated to something else and written.
   7. finish_function is called for the outer function and the garbage collector
      is invoked.  The garbage collector crashes trying to walk the memory
      associated with the x_stmt_tree field, which is now owned by something
      else.

The attached patch fixes the problem by always allocating a new language
function when going into a new function context (it reverts back to the
original code).  I suppose another option would be to clear all the saved
fields in c_pop_function_context, but that seems like more trouble than
it is worth.

I wasn't able to devise a simplified reproduction case for this problem.  I
could only reproduce it by building glibc.  The patch was tested by building
mips-linux-gnu and bootstrapping and running the full test suite for
i686-pc-linux-gnu.

OK?

P.S.  If it is OK, then can someone commit for me (I don't have write access)?


2012-05-29  Meador Inge  <meadori@codesourcery.com>

	* c-decl.c (c_push_function_context): Always create a new language
	function.
	(c_pop_function_context): Clear the language function created in
	c_push_function_context.

-- 
Meador Inge
CodeSourcery / Mentor Embedded
http://www.mentor.com/embedded-software

Index: gcc/c-decl.c
===================================================================
--- gcc/c-decl.c	(revision 187923)
+++ gcc/c-decl.c	(working copy)
@@ -8579,11 +8579,9 @@ check_for_loop_decls (location_t loc, bo
 void
 c_push_function_context (void)
 {
-  struct language_function *p = cfun->language;
-  /* cfun->language might have been already allocated by the use of
-     -Wunused-local-typedefs.  In that case, just re-use it.  */
-  if (p == NULL)
-    cfun->language = p = ggc_alloc_cleared_language_function ();
+  struct language_function *p;
+  p = ggc_alloc_language_function ();
+  cfun->language = p;
 
   p->base.x_stmt_tree = c_stmt_tree;
   c_stmt_tree.x_cur_stmt_list
@@ -8609,11 +8607,7 @@ c_pop_function_context (void)
 
   pop_function_context ();
   p = cfun->language;
-  /* When -Wunused-local-typedefs is in effect, cfun->languages is
-     used to store data throughout the life time of the current cfun,
-     So don't deallocate it.  */
-  if (!warn_unused_local_typedefs)
-    cfun->language = NULL;
+  cfun->language = NULL;
 
   if (DECL_STRUCT_FUNCTION (current_function_decl) == 0
       && DECL_SAVED_TREE (current_function_decl) == NULL_TREE)