This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] pr61324 pr 63649 - fix crash in ipa_comdats
- From: Jan Hubicka <hubicka at ucw dot cz>
- To: tsaunders at mozilla dot com
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Tue, 25 Nov 2014 21:59:46 +0100
- Subject: Re: [PATCH] pr61324 pr 63649 - fix crash in ipa_comdats
- Authentication-results: sourceware.org; auth=none
- References: <1416769157-19316-1-git-send-email-tsaunders at mozilla dot com>
> 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