[PATCH] Fix bogus use of cfun in gen_subprogram_die and premark_used_types

[Martin Jambor <mjambor at suse dot cz>]

Hi,

while looking into how to remove push/pop_cfun from dwarf2out.c, I
have noticed the following wrong use of cfun in premark_used_types,
which is the first thing called by gen_subprogram_die.

What happens is that:

1. early inliner calls dwarf2out_abstract_function, cfun corresponds
   to the function being inlined to, argument decl is the function
   being inlined.

2. dwarf2out_abstract_function calls gen_type_die_for_member to
   generate an "in-class declaration DIE."  It does this before
   changing cfun.

3. gen_type_die_for_member calls gen_type_die_for_member because
   member is a function decl.

4. gen_subprogram_die calls premark_used_types to mark DIEs of all
   types in cfun->used_types_hash as perennial.  But cfun does not
   correspond to the decl it is supposed to be emitting a DIE for,
   instead, used_types of the function decl is being inlined to are
   being marked as perennial.

Similarly, when dealing with nested functions, gen_subprogram_die can
call itself, just with a different decl parameter but unchanged cfun
through decls_for_scope.

I was not able to produce a failing testcase similar to
gcc.dg/20060410.c, mainly because dwarf2out_abstract_function then
changes cfun and indirectly invokes gen_subprogram_die again but still
I believe the intention was to use DECL_STRUCT_FUNCTION(decl) rather
than cfun in premark_used_types and everywhere in gen_subprogram_die.
The patch below does exactly that and as far as my experiments go,
seems to work.

This patch also removes push/pop cfun from dwarf2out_abstract_function
and only leaves the change of current_function_decl.  Richi suggested
that we push NULL cfun at this point but my goal is to enforce that
cfun and current_function_decl match at each push_cfun and since
dwarf2out_abstract_function can call itself, that is not the case.
Nevertheless, I also bootstrapped, tested and compiled Firefox with a
version in which I do push_cfun(NULL) when cfun is not already NULL
and there were no problems.

Bootstrapped and tested on x86_64-linux.  OK for trunk?

Thanks,

Martin


2012-08-30  Martin Jambor  <mjambor@suse.cz>

	* dwarf2out.c (dwarf2out_abstract_function): Do not change cfun.
	(premark_used_types): New parameter fun, use it instead of cfun.
	(gen_subprogram_die): Use DECL_STRUCT_FUNCTION (decl) instead of cfun,
	also pass it to premark_used_types.


*** /tmp/3GCMxa_dwarf2out.c	Tue Sep  4 15:10:23 2012
--- gcc/dwarf2out.c	Mon Sep  3 14:48:02 2012
*************** dwarf2out_abstract_function (tree decl)
*** 16765,16771 ****
    /* Pretend we've just finished compiling this function.  */
    save_fn = current_function_decl;
    current_function_decl = decl;
-   push_cfun (DECL_STRUCT_FUNCTION (decl));
  
    was_abstract = DECL_ABSTRACT (decl);
    set_decl_abstract_flags (decl, 1);
--- 16765,16770 ----
*************** dwarf2out_abstract_function (tree decl)
*** 16779,16785 ****
    call_arg_locations = old_call_arg_locations;
    call_site_count = old_call_site_count;
    tail_call_site_count = old_tail_call_site_count;
-   pop_cfun ();
  }
  
  /* Helper function of premark_used_types() which gets called through
--- 16778,16783 ----
*************** premark_types_used_by_global_vars_helper
*** 16838,16847 ****
  /* Mark all members of used_types_hash as perennial.  */
  
  static void
! premark_used_types (void)
  {
!   if (cfun && cfun->used_types_hash)
!     htab_traverse (cfun->used_types_hash, premark_used_types_helper, NULL);
  }
  
  /* Mark all members of types_used_by_vars_entry as perennial.  */
--- 16836,16845 ----
  /* Mark all members of used_types_hash as perennial.  */
  
  static void
! premark_used_types (struct function *fun)
  {
!   if (fun && fun->used_types_hash)
!     htab_traverse (fun->used_types_hash, premark_used_types_helper, NULL);
  }
  
  /* Mark all members of types_used_by_vars_entry as perennial.  */
*************** gen_subprogram_die (tree decl, dw_die_re
*** 16904,16910 ****
    int declaration = (current_function_decl != decl
  		     || class_or_namespace_scope_p (context_die));
  
!   premark_used_types ();
  
    /* It is possible to have both DECL_ABSTRACT and DECLARATION be true if we
       started to generate the abstract instance of an inline, decided to output
--- 16902,16908 ----
    int declaration = (current_function_decl != decl
  		     || class_or_namespace_scope_p (context_die));
  
!   premark_used_types (DECL_STRUCT_FUNCTION (decl));
  
    /* It is possible to have both DECL_ABSTRACT and DECLARATION be true if we
       started to generate the abstract instance of an inline, decided to output
*************** gen_subprogram_die (tree decl, dw_die_re
*** 17067,17079 ****
    else if (!DECL_EXTERNAL (decl))
      {
        HOST_WIDE_INT cfa_fb_offset;
  
        if (!old_die || !get_AT (old_die, DW_AT_inline))
  	equate_decl_number_to_die (decl, subr_die);
  
        if (!flag_reorder_blocks_and_partition)
  	{
! 	  dw_fde_ref fde = cfun->fde;
  	  if (fde->dw_fde_begin)
  	    {
  	      /* We have already generated the labels.  */
--- 17065,17079 ----
    else if (!DECL_EXTERNAL (decl))
      {
        HOST_WIDE_INT cfa_fb_offset;
+       struct function *fun = DECL_STRUCT_FUNCTION (decl);
  
        if (!old_die || !get_AT (old_die, DW_AT_inline))
  	equate_decl_number_to_die (decl, subr_die);
  
+       gcc_checking_assert (fun);
        if (!flag_reorder_blocks_and_partition)
  	{
! 	  dw_fde_ref fde = fun->fde;
  	  if (fde->dw_fde_begin)
  	    {
  	      /* We have already generated the labels.  */
*************** gen_subprogram_die (tree decl, dw_die_re
*** 17119,17125 ****
        else
  	{
  	  /* Generate pubnames entries for the split function code ranges.  */
! 	  dw_fde_ref fde = cfun->fde;
  
  	  if (fde->dw_fde_second_begin)
  	    {
--- 17119,17125 ----
        else
  	{
  	  /* Generate pubnames entries for the split function code ranges.  */
! 	  dw_fde_ref fde = fun->fde;
  
  	  if (fde->dw_fde_second_begin)
  	    {
*************** gen_subprogram_die (tree decl, dw_die_re
*** 17219,17227 ****
  	 by this displacement.  */
        compute_frame_pointer_to_fb_displacement (cfa_fb_offset);
  
!       if (cfun->static_chain_decl)
  	add_AT_location_description (subr_die, DW_AT_static_link,
! 		 loc_list_from_tree (cfun->static_chain_decl, 2));
      }
  
    /* Generate child dies for template paramaters.  */
--- 17219,17227 ----
  	 by this displacement.  */
        compute_frame_pointer_to_fb_displacement (cfa_fb_offset);
  
!       if (fun->static_chain_decl)
  	add_AT_location_description (subr_die, DW_AT_static_link,
! 		 loc_list_from_tree (fun->static_chain_decl, 2));
      }
  
    /* Generate child dies for template paramaters.  */