[patch] Inlining bug

[patch] Inlining bug

[Bruno Haible]

Hi,

Two months ago I reported the following bug:

The following program compiles fine with g++-2.7.2, but crashes egcs-971225
on i486-linux.

================================ foo.cc ==================================
struct A {
	A ();
	~A ();
};
struct C {
	C ();
	void memberfn ()
	{
		A localvar1;
		typedef struct _INNER1 {
			A slot1;
			_INNER1 () {}
		} INNER1;
		typedef struct _INNER2 {
			INNER1 slot2;
			_INNER2 (A& dummy) {}
		} INNER2;
		INNER2 localvar2 (localvar1);
	}
};
int main ()
{
	C c;
	c.memberfn();
}
==========================================================================

$ g++-971225 -O -S -fno-exceptions buginline.cc 
buginline.cc: In method `void C::memberfn()':
buginline.cc:25: Internal compiler error.
buginline.cc:25: Please submit a full bug report to `bug-g++@prep.ai.mit.edu'.

The following happens: The constructors and destructors for _INNER1 and
_INNER2 are output first, then memberfn() should be output but is deferred,
then main() is compiled, and finally memberfn() is output. In this last
step, an insn

========================== from foo.cc.rtl =========================
(call_insn/i 29 28 30 (set (reg:SI 0 %eax)
        (call (mem:QI (symbol_ref:SI ("__1A")))
            (const_int 4))) -1 (nil)
    (nil)
    (nil))

gets changed to

=========================== from foo.cc.jump =======================
(call_insn/i 29 28 30 (set (UnKnown Unknown)
        (call (mem:QI (symbol_ref:SI ("__1A")))
            (const_int 4))) -1 (nil)
    (nil)
    (nil))

Since the SET's destination gets overwritten by zeroes inside main()'s
compilation (more precisely by bzero(), called from init_alias_analysis()),
this looks like the RTL of memberfn() has already been [partially?] freed.
I don't know where this happens. Maybe the best thing would be to keep all
the RTL of memberfn() and its nested functions alive forever. But an easy
workaround is below.

This workaround works because memberfn() gets DECL_DEFER_OUTPUT set to 1
by cp/decl.c, and later current_function_contains_functions is set to 1.
Inside rest_of_compilation, inlinable = 0 and decl_function_context(decl) = 0.

Can you please put in (or improve :-)) this patch, and also add the above
snippet to the testsuite? Thanks.

                             Bruno


Sat Jan  3 19:59:17 1998  Bruno Haible  <bruno@linuix.mathematik.uni-karlsruhe.de>

        * toplev.c (rest_of_compilation): Force immediate compilation if
          current_function_contains_functions.

*** egcs-971225/gcc/toplev.c.bak	Thu Jan  1 13:47:46 1998
--- egcs-971225/gcc/toplev.c	Sat Jan  3 20:26:13 1998
***************
*** 3098,3105 ****
  
        /* If we can, defer compiling inlines until EOF.
  	 save_for_inline_copying can be extremely expensive.  */
-       if (inlinable && ! decl_function_context (decl))
- 	DECL_DEFER_OUTPUT (decl) = 1;
  
        /* If function is inline, and we don't yet know whether to
  	 compile it by itself, defer decision till end of compilation.
--- 3098,3103 ----
***************
*** 3119,3124 ****
--- 3117,3130 ----
  	    purge_addressof (insns);
  	  else
  	    DECL_DEFER_OUTPUT (decl) = 1;
+ 	}
+       else
+ 	{
+ 	  /* If this function contains and calls some nested inline
+ 	     functions, we cannot defer it, even if the front end
+ 	     requests this.  */
+ 	  if (current_function_contains_functions)
+ 	    DECL_DEFER_OUTPUT (decl) = 0;
  	}
  
        if (! current_function_contains_functions

Re: [patch] Inlining bug

[Jim Wilson]

I suspect that this is a bug in the C++ front end's handling of obstacks for
nested functions.  In the C front end, start_function does not call
temporary_allocation if this is a nested function.  The C++ front end does.
If I change the C++ front end to be more like the C front end, then your
testcase works.  I am not sure what this will do to memory usage in the
C++ compiler though.

Tue Feb 24 20:04:43 1998  Jim Wilson  <wilson@cygnus.com>

	* decl.c (start_function): Don't call temporary_allocation for a
	nested function.

Index: decl.c
===================================================================
RCS file: /egcs/carton/cvsfiles/egcs/gcc/cp/decl.c,v
retrieving revision 1.64
diff -p -r1.64 decl.c
*** decl.c	1998/02/23 05:11:54	1.64
--- decl.c	1998/02/25 04:04:37
*************** start_function (declspecs, declarator, a
*** 11754,11761 ****
        TREE_THIS_VOLATILE (DECL_RESULT (decl1)) = TYPE_VOLATILE (restype);
      }
  
!     /* Allocate further tree nodes temporarily during compilation
!        of this function only.  Tiemann moved up here from bottom of fn.  */
      temporary_allocation ();
  
    if (processing_template_decl)
--- 11754,11764 ----
        TREE_THIS_VOLATILE (DECL_RESULT (decl1)) = TYPE_VOLATILE (restype);
      }
  
!   /* Allocate further tree nodes temporarily during compilation
!      of this function only.  Tiemann moved up here from bottom of fn.  */
!   /* If this is a nested function, then we must continue to allocate RTL
!      on the permanent obstack in case we need to inline it later.  */
!   if (! hack_decl_function_context (decl1))
      temporary_allocation ();
  
    if (processing_template_decl)

Re: [patch] Inlining bug

[Bruno Haible]

> Tue Feb 24 20:04:43 1998  Jim Wilson  <wilson@cygnus.com>
> 
> 	* decl.c (start_function): Don't call temporary_allocation for a
> 	nested function.

Excellent! This fixes not only the sample program I submitted, but
also a larger one which my tentative patch failed to compile.

I'd like to see your fix in egcs 1.0.2.

Re: [patch] Inlining bug

[Jeffrey A Law]

  In message <199802261355.OAA03872@halles.ilog.fr>you write:
  > 
  > > Tue Feb 24 20:04:43 1998  Jim Wilson  <wilson@cygnus.com>
  > > 
  > > 	* decl.c (start_function): Don't call temporary_allocation for a
  > > 	nested function.
  > 
  > Excellent! This fixes not only the sample program I submitted, but
  > also a larger one which my tentative patch failed to compile.
  > 
  > I'd like to see your fix in egcs 1.0.2.
I've installed this patch on the release branch too.

jeff