This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] pr61324 pr 63649 - fix crash in ipa_comdats


> From: Trevor Saunders <tsaunders@mozilla.com>
> 
> Hi,
> 
> the interesting symbol in the test case for pr61324 is __GLOBAL__sub_I_s.  It
> refers to nothing, and is called by nothing, however it is kept (I believe
> because of -fkeep-inline-functions).  That means ipa_comdats never tries to put

Aha, that explans why it is around.

> it in a comdat, and so it never ends up in the hash table.  It seems like the
> simplest solution is to just check if symbol is not in the map before trying to
> get the comdat it should go in, but another approach might be to keep separate
> hash maps for comdat functions and functions that can't be in any comdat, and
> then iterate over only the functions that belong in a comdat.

Well, -fkeep-inline-functions promise you that you can call any inline function from debugger.
I suppose in this case you also want to be able to call static functions. Comdat pass may
bundle the function into comdat that is later optimized away by linker, so I would say we
just want to disable the whole comdat pass when -fkeep-inline-functions is used?

Patch for that is preapproved.
Honza
> 
> boottstrapped + regtested x86_64-unknown-linux-gnu, ok?
> 
> Trev
> 
> gcc/
> 
> 	* ipa-comdats.c (ipa_commdats): check if map contains symbol before
> 	trying to put symbol in a comdat.
> 
> diff --git a/gcc/ipa-comdats.c b/gcc/ipa-comdats.c
> index af2aef8..8695a7e 100644
> --- a/gcc/ipa-comdats.c
> +++ b/gcc/ipa-comdats.c
> @@ -327,18 +327,18 @@ ipa_comdats (void)
>  	  && !symbol->alias
>  	  && symbol->real_symbol_p ())
>  	{
> -	  tree group = *map.get (symbol);
> +	  tree *group = map.get (symbol);
>  
> -	  if (group == error_mark_node)
> +	  if (!group || *group == error_mark_node)
>  	    continue;
>  	  if (dump_file)
>  	    {
>  	      fprintf (dump_file, "Localizing symbol\n");
>  	      symbol->dump (dump_file);
> -	      fprintf (dump_file, "To group: %s\n", IDENTIFIER_POINTER (group));
> +	      fprintf (dump_file, "To group: %s\n", IDENTIFIER_POINTER (*group));
>  	    }
>  	  symbol->call_for_symbol_and_aliases (set_comdat_group,
> -					     *comdat_head_map.get (group),
> +					     *comdat_head_map.get (*group),
>  					     true);
>  	}
>      }
> diff --git a/gcc/testsuite/g++.dg/pr61324.C b/gcc/testsuite/g++.dg/pr61324.C
> new file mode 100644
> index 0000000..6102574
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/pr61324.C
> @@ -0,0 +1,13 @@
> +// { dg-do compile }
> +// { dg-options "-O -fkeep-inline-functions -fno-use-cxa-atexit" }
> +void foo ();
> +
> +struct S
> +{
> +  ~S ()
> +  {
> +    foo ();
> +  }
> +};
> +
> +S s;
> -- 
> 2.1.3


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]