On Wed, 13 Mar 2019, Jakub Jelinek wrote:
> Hi!
>
> create_dispatcher_calls iterates over ipa_ref *s referring the current node
> and wants to remove them all and create new ones. The problem is
> that if there are any ipa_ref objects where two or more of them are from the
> same cgraph node to the current node (in the testcases there are both cases
> where there are two or more foo -> foo self-references, or two or more
> foo.avx -> foo references), when we ref->remove_reference () one of them,
> that method will actually move some other ipa_ref that was last in
> references vector over to that and thus invalidate what some other ipa_ref
> pointers point to (the pointers still point to some elements of some
> references vector, but could either point past the last one, or could point
> to an element that was overwritten with something else).
>
> The following patch fixes it by remove_reference all references immediately,
> but push into the vector their content first; besides that
> ref->remove_reference we are just accessing a couple of fields from that,
> not calling any other methods.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
LGTM.
Richard.
> 2019-03-13 Jakub Jelinek <ja...@redhat.com>
>
> PR ipa/89684
> * multiple_target.c (create_dispatcher_calls): Change
> references_to_redirect from vector of ipa_ref * to vector of ipa_ref.
> In the node->iterate_referring loop, push *ref rather than ref, call
> ref->remove_reference () and always pass 0 to iterate_referring.
>
> --- gcc/multiple_target.c.jj 2019-03-13 09:23:35.345567298 +0100
> +++ gcc/multiple_target.c 2019-03-13 10:12:49.071371927 +0100
> @@ -103,10 +103,16 @@ create_dispatcher_calls (struct cgraph_n
> inode->resolve_alias (cgraph_node::get (resolver_decl));
>
> auto_vec<cgraph_edge *> edges_to_redirect;
> - auto_vec<ipa_ref *> references_to_redirect;
> + /* We need to capture the references by value rather than just pointers to
> them
> + and remove them right away, as removing them later would invalidate what
> + some other reference pointers point to. */
> + auto_vec<ipa_ref> references_to_redirect;
>
> - for (unsigned i = 0; node->iterate_referring (i, ref); i++)
> - references_to_redirect.safe_push (ref);
> + while (node->iterate_referring (0, ref))
> + {
> + references_to_redirect.safe_push (*ref);
> + ref->remove_reference ();
> + }
>
> /* We need to remember NEXT_CALLER as it could be modified in the loop. */
> for (cgraph_edge *e = node->callers; e ; e = e->next_caller)
> @@ -146,13 +152,11 @@ create_dispatcher_calls (struct cgraph_n
> }
>
> symtab_node *source = ref->referring;
> - ref->remove_reference ();
> source->create_reference (inode, IPA_REF_ADDR);
> }
> else if (ref->use == IPA_REF_ALIAS)
> {
> symtab_node *source = ref->referring;
> - ref->remove_reference ();
> source->create_reference (inode, IPA_REF_ALIAS);
> source->add_to_same_comdat_group (inode);
> }
> --- gcc/testsuite/gcc.target/i386/pr89684.c.jj 2019-03-13
> 10:12:05.677080120 +0100
> +++ gcc/testsuite/gcc.target/i386/pr89684.c 2019-03-13 10:11:56.390231682
> +0100
> @@ -0,0 +1,23 @@
> +/* PR ipa/89684 */
> +/* { dg-do compile } */
> +/* { dg-require-ifunc "" } */
> +
> +void bar (int, void (*) (void));
> +
> +__attribute__((target_clones ("default", "avx")))
> +void foo (void)
> +{
> + bar (0, foo);
> + bar (0, foo);
> +}
> +
> +__attribute__((target_clones ("default", "avx", "avx2")))
> +void baz (void)
> +{
> + bar (0, foo);
> + bar (0, foo);
> + bar (0, foo);
> + bar (0, foo);
> + bar (0, foo);
> + bar (0, foo);
> +}
>
> Jakub
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
