> We can't instrument an IFUNC resolver nor its callees as it may require
> TLS which hasn't been set up yet when the dynamic linker is resolving
> IFUNC symbols.  Add an IFUNC resolver caller marker to symtab_node to
> avoid recursive checking.
> 
> gcc/ChangeLog:
> 
>       PR tree-optimization/114115
>       * cgraph.h (enum ifunc_caller): New.
>       (symtab_node): Add has_ifunc_caller.
Unless we have users outside of tree-profile, I think it is better to
avoid adding extra data to cgraph_node.  One can use node->get_uid() indexed 
hash
set to save the two bits needed for propagation.
>       * tree-profile.cc (check_ifunc_resolver): New.
>       (is_caller_ifunc_resolver): Likewise.
>       (tree_profiling): Don't instrument an IFUNC resolver nor its
>       callees.
> 
> gcc/testsuite/ChangeLog:
> 
>       PR tree-optimization/114115
>       * gcc.dg/pr114115.c: New test.

The problem with this approach is that tracking callees of ifunc
resolvers will stop on the translation unit boundary and also with
indirect call.  Also while ifunc resolver itself is called only once,
its callees may also be used from performance critical code.

So it is not really reliable fix (though I guess it will work a lot of
common code).  I wonder what would be alternatives.  In GCC generated
profling code we use TLS only for undirect call profiling (so there is
no need to turn off rest of profiling).  I wonder if there is any chance
to not make it seffault when it is done before TLS is set up?

Honza
> ---
>  gcc/cgraph.h                    | 18 +++++++
>  gcc/testsuite/gcc.dg/pr114115.c | 24 +++++++++
>  gcc/tree-profile.cc             | 92 +++++++++++++++++++++++++++++++++
>  3 files changed, 134 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.dg/pr114115.c
> 
> diff --git a/gcc/cgraph.h b/gcc/cgraph.h
> index 47f35e8078d..ce99f4a5114 100644
> --- a/gcc/cgraph.h
> +++ b/gcc/cgraph.h
> @@ -100,6 +100,21 @@ enum symbol_partitioning_class
>     SYMBOL_DUPLICATE
>  };
>  
> +/* Classification whether a function has any IFUNC resolver caller.  */
> +enum ifunc_caller
> +{
> +  /* It is unknown if this function has any IFUNC resolver caller.  */
> +  IFUNC_CALLER_UNKNOWN,
> +  /* Work in progress to check if this function has any IFUNC resolver
> +     caller.  */
> +  IFUNC_CALLER_WIP,
> +  /* This function has at least an IFUNC resolver caller, including
> +     itself.  */
> +  IFUNC_CALLER_TRUE,
> +  /* This function doesn't have any IFUNC resolver caller.  */
> +  IFUNC_CALLER_FALSE
> +};
> +
>  /* Base of all entries in the symbol table.
>     The symtab_node is inherited by cgraph and varpol nodes.  */
>  struct GTY((desc ("%h.type"), tag ("SYMTAB_SYMBOL"),
> @@ -121,6 +136,7 @@ public:
>        used_from_other_partition (false), in_other_partition (false),
>        address_taken (false), in_init_priority_hash (false),
>        need_lto_streaming (false), offloadable (false), ifunc_resolver 
> (false),
> +      has_ifunc_caller (IFUNC_CALLER_UNKNOWN),
>        order (false), next_sharing_asm_name (NULL),
>        previous_sharing_asm_name (NULL), same_comdat_group (NULL), ref_list 
> (),
>        alias_target (NULL), lto_file_data (NULL), aux (NULL),
> @@ -595,6 +611,8 @@ public:
>    /* Set when symbol is an IFUNC resolver.  */
>    unsigned ifunc_resolver : 1;
>  
> +  /* Classification whether a function has any IFUNC resolver caller.  */
> +  ENUM_BITFIELD (ifunc_caller) has_ifunc_caller : 2;
>  
>    /* Ordering of all symtab entries.  */
>    int order;
> diff --git a/gcc/testsuite/gcc.dg/pr114115.c b/gcc/testsuite/gcc.dg/pr114115.c
> new file mode 100644
> index 00000000000..2629f591877
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr114115.c
> @@ -0,0 +1,24 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O0 -fprofile-generate -fdump-tree-optimized" } */
> +/* { dg-require-profiling "-fprofile-generate" } */
> +/* { dg-require-ifunc "" } */
> +
> +void *foo_ifunc2() __attribute__((ifunc("foo_resolver")));
> +
> +void bar(void)
> +{
> +}
> +
> +static int f3()
> +{
> +  bar ();
> +  return 5;
> +}
> +
> +void (*foo_resolver(void))(void)
> +{
> +  f3();
> +  return bar;
> +}
> +
> +/* { dg-final { scan-tree-dump-not "__gcov_indirect_call_profiler_v" 
> "optimized" } } */
> diff --git a/gcc/tree-profile.cc b/gcc/tree-profile.cc
> index aed13e2b1bc..46478648b32 100644
> --- a/gcc/tree-profile.cc
> +++ b/gcc/tree-profile.cc
> @@ -738,6 +738,72 @@ include_source_file_for_profile (const char *filename)
>    return false;
>  }
>  
> +/* Return true and set *DATA to true if NODE is an ifunc resolver.  */
> +
> +static bool
> +check_ifunc_resolver (cgraph_node *node, void *data)
> +{
> +  if (node->ifunc_resolver)
> +    {
> +      bool *is_ifunc_resolver = (bool *) data;
> +      *is_ifunc_resolver = true;
> +      return true;
> +    }
> +  return false;
> +}
> +
> +/* Return true if any caller of NODE is an ifunc resolver.  */
> +
> +static bool
> +is_caller_ifunc_resolver (cgraph_node *node)
> +{
> +  if (node->has_ifunc_caller == IFUNC_CALLER_WIP)
> +    gcc_unreachable ();
> +
> +  node->has_ifunc_caller = IFUNC_CALLER_WIP;
> +  bool is_ifunc_resolver = false;
> +
> +  for (cgraph_edge *e = node->callers; e; e = e->next_caller)
> +    {
> +      /* Check for recursive call.  */
> +      if (e->caller == node)
> +     continue;
> +
> +      switch (e->caller->has_ifunc_caller)
> +     {
> +     case IFUNC_CALLER_UNKNOWN:
> +       e->caller->call_for_symbol_and_aliases (check_ifunc_resolver,
> +                                               &is_ifunc_resolver,
> +                                               true);
> +       if (is_ifunc_resolver)
> +         {
> +           e->caller->has_ifunc_caller = IFUNC_CALLER_TRUE;
> +           return true;
> +         }
> +       break;
> +     case IFUNC_CALLER_TRUE:
> +       return true;
> +     case IFUNC_CALLER_FALSE:
> +       /* This caller doesn't have any IFUNC resolver call.  Check
> +          the next caller.  */
> +       continue;
> +
> +     case IFUNC_CALLER_WIP:
> +       continue;
> +     }
> +
> +      if (is_caller_ifunc_resolver (e->caller))
> +     {
> +       e->caller->has_ifunc_caller = IFUNC_CALLER_TRUE;
> +       return true;
> +     }
> +      else
> +     e->caller->has_ifunc_caller = IFUNC_CALLER_FALSE;
> +    }
> +
> +  return false;
> +}
> +
>  #ifndef HAVE_sync_compare_and_swapsi
>  #define HAVE_sync_compare_and_swapsi 0
>  #endif
> @@ -848,6 +914,32 @@ tree_profiling (void)
>           }
>       }
>  
> +      /* Do not instrument an IFUNC resolver nor its callees.  */
> +      bool is_ifunc_resolver = false;
> +      switch (node->has_ifunc_caller)
> +     {
> +     case IFUNC_CALLER_UNKNOWN:
> +       node->call_for_symbol_and_aliases (check_ifunc_resolver,
> +                                          &is_ifunc_resolver, true);
> +       if (is_ifunc_resolver || is_caller_ifunc_resolver (node))
> +         {
> +           node->has_ifunc_caller = IFUNC_CALLER_TRUE;
> +           continue;
> +         }
> +       else
> +         node->has_ifunc_caller = IFUNC_CALLER_FALSE;
> +       break;
> +
> +     case IFUNC_CALLER_TRUE:
> +       continue;
> +
> +     case IFUNC_CALLER_FALSE:
> +       break;
> +
> +     case IFUNC_CALLER_WIP:
> +       gcc_unreachable ();
> +     }
> +
>        push_cfun (DECL_STRUCT_FUNCTION (node->decl));
>  
>        if (dump_file)
> -- 
> 2.43.2
> 

Reply via email to