This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Do not allow to inline ifunc resolvers (PR ipa/81128).
On 06/27/2017 05:26 PM, Jan Hubicka wrote:
>> diff --git a/gcc/ipa-visibility.c b/gcc/ipa-visibility.c
>> index d5a3ae56c46..69e6e295d55 100644
>> --- a/gcc/ipa-visibility.c
>> +++ b/gcc/ipa-visibility.c
>> @@ -97,7 +97,8 @@ non_local_p (struct cgraph_node *node, void *data ATTRIBUTE_UNUSED)
>> && !DECL_EXTERNAL (node->decl)
>> && !node->externally_visible
>> && !node->used_from_other_partition
>> - && !node->in_other_partition);
>> + && !node->in_other_partition
>> + && !lookup_attribute ("ifunc", DECL_ATTRIBUTES (node->decl)));
>> }
>>
>> /* Return true when function can be marked local. */
>>
>> It's questionable if local.local can be true for ifunc function? If can, one would need to
>> move check for ifunc aerly in:
>> /* Return function availability. See cgraph.h for description of individual
>> return values. */
>> enum availability
>> cgraph_node::get_availability (symtab_node *ref)
>> {
>> if (ref)
>> {
>> cgraph_node *cref = dyn_cast <cgraph_node *> (ref);
>> if (cref)
>> ref = cref->global.inlined_to;
>> }
>> enum availability avail;
>> if (!analyzed)
>> avail = AVAIL_NOT_AVAILABLE;
>> else if (local.local)
>> avail = AVAIL_LOCAL;
>> else if (global.inlined_to)
>> avail = AVAIL_AVAILABLE;
>> else if (transparent_alias)
>> ultimate_alias_target (&avail, ref);
>> else if (lookup_attribute ("ifunc", DECL_ATTRIBUTES (decl)))
>> avail = AVAIL_INTERPOSABLE;
>>
>> ...
>>
>> What solution do you prefer Honza?
>
> Probably just update non_local_p to also check that availability is at least AVAIL_AVAILABLE.
> Then we will have one place to collect such a side cases where !EXTERNAL function can be
> interposed.
Good, this is tested version of patch I'm going to install.
Martin
>
> Honza
>>
>> Thanks,
>> Martin
>>
>>>
>>> Honza
>>>> Martin
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>> 2017-06-22 Martin Liska <mliska@suse.cz>
>>>>
>>>> * ipa-inline.c (can_inline_edge_p): Return false for ifunc fns.
>>>> * ipa-visibility.c (can_replace_by_local_alias): Likewise.
>>>>
>>>> gcc/c-family/ChangeLog:
>>>>
>>>> 2017-06-22 Martin Liska <mliska@suse.cz>
>>>>
>>>> * c-attribs.c (handle_alias_ifunc_attribute): Append ifunc alias
>>>> to a function declaration.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>> 2017-06-22 Martin Liska <mliska@suse.cz>
>>>>
>>>> * gcc.target/i386/pr81128.c: New test.
>>>> ---
>>>> gcc/c-family/c-attribs.c | 11 ++++--
>>>> gcc/ipa-inline.c | 2 +
>>>> gcc/ipa-visibility.c | 3 +-
>>>> gcc/testsuite/gcc.target/i386/pr81128.c | 65 +++++++++++++++++++++++++++++++++
>>>> 4 files changed, 77 insertions(+), 4 deletions(-)
>>>> create mode 100644 gcc/testsuite/gcc.target/i386/pr81128.c
>>>>
>>>>
>>>
>>>> diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
>>>> index 2b6845f2cbd..626ffa1cde7 100644
>>>> --- a/gcc/c-family/c-attribs.c
>>>> +++ b/gcc/c-family/c-attribs.c
>>>> @@ -1846,9 +1846,14 @@ handle_alias_ifunc_attribute (bool is_alias, tree *node, tree name, tree args,
>>>> TREE_STATIC (decl) = 1;
>>>>
>>>> if (!is_alias)
>>>> - /* ifuncs are also aliases, so set that attribute too. */
>>>> - DECL_ATTRIBUTES (decl)
>>>> - = tree_cons (get_identifier ("alias"), args, DECL_ATTRIBUTES (decl));
>>>> + {
>>>> + /* ifuncs are also aliases, so set that attribute too. */
>>>> + DECL_ATTRIBUTES (decl)
>>>> + = tree_cons (get_identifier ("alias"), args,
>>>> + DECL_ATTRIBUTES (decl));
>>>> + DECL_ATTRIBUTES (decl) = tree_cons (get_identifier ("ifunc"),
>>>> + NULL, DECL_ATTRIBUTES (decl));
>>>> + }
>>>> }
>>>> else
>>>> {
>>>> diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c
>>>> index fb20d3723cc..588fa9c41e4 100644
>>>> --- a/gcc/ipa-inline.c
>>>> +++ b/gcc/ipa-inline.c
>>>> @@ -370,6 +370,8 @@ can_inline_edge_p (struct cgraph_edge *e, bool report,
>>>> e->inline_failed = CIF_ATTRIBUTE_MISMATCH;
>>>> inlinable = false;
>>>> }
>>>> + else if (lookup_attribute ("ifunc", DECL_ATTRIBUTES (e->callee->decl)))
>>>> + inlinable = false;
>>>> /* Check if caller growth allows the inlining. */
>>>> else if (!DECL_DISREGARD_INLINE_LIMITS (callee->decl)
>>>> && !disregard_limits
>>>> diff --git a/gcc/ipa-visibility.c b/gcc/ipa-visibility.c
>>>> index d5a3ae56c46..79d05b41085 100644
>>>> --- a/gcc/ipa-visibility.c
>>>> +++ b/gcc/ipa-visibility.c
>>>> @@ -345,7 +345,8 @@ can_replace_by_local_alias (symtab_node *node)
>>>>
>>>> return (node->get_availability () > AVAIL_INTERPOSABLE
>>>> && !decl_binds_to_current_def_p (node->decl)
>>>> - && !node->can_be_discarded_p ());
>>>> + && !node->can_be_discarded_p ()
>>>> + && !lookup_attribute ("ifunc", DECL_ATTRIBUTES (node->decl)));
>>>> }
>>>>
>>>> /* Return true if we can replace reference to NODE by local alias
>>>> diff --git a/gcc/testsuite/gcc.target/i386/pr81128.c b/gcc/testsuite/gcc.target/i386/pr81128.c
>>>> new file mode 100644
>>>> index 00000000000..90a567ad690
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.target/i386/pr81128.c
>>>> @@ -0,0 +1,65 @@
>>>> +/* PR ipa/81128 */
>>>> +/* { dg-do run } */
>>>> +/* { dg-options "-O3" } */
>>>> +/* { dg-require-ifunc "" } */
>>>> +
>>>> +
>>>> +#include <stdio.h>
>>>> +#include <stdlib.h>
>>>> +#include <time.h>
>>>> +
>>>> +int resolver_fn = 0;
>>>> +int resolved_fn = 0;
>>>> +
>>>> +static inline void
>>>> +do_it_right_at_runtime_A ()
>>>> +{
>>>> + resolved_fn++;
>>>> +}
>>>> +
>>>> +static inline void
>>>> +do_it_right_at_runtime_B ()
>>>> +{
>>>> + resolved_fn++;
>>>> +}
>>>> +
>>>> +static inline void do_it_right_at_runtime (void);
>>>> +
>>>> +void do_it_right_at_runtime (void)
>>>> + __attribute__ ((ifunc ("resolve_do_it_right_at_runtime")));
>>>> +
>>>> +static void (*resolve_do_it_right_at_runtime (void)) (void)
>>>> +{
>>>> + srand (time (NULL));
>>>> + int r = rand ();
>>>> + resolver_fn++;
>>>> +
>>>> + /* Use intermediate variable to get a warning for non-matching
>>>> + * prototype. */
>>>> + typeof(do_it_right_at_runtime) *func;
>>>> + if (r & 1)
>>>> + func = do_it_right_at_runtime_A;
>>>> + else
>>>> + func = do_it_right_at_runtime_B;
>>>> +
>>>> + return (void *) func;
>>>> +}
>>>> +
>>>> +int
>>>> +main (void)
>>>> +{
>>>> + const unsigned int ITERS = 10;
>>>> +
>>>> + for (int i = ITERS; i > 0; i--)
>>>> + {
>>>> + do_it_right_at_runtime ();
>>>> + }
>>>> +
>>>> + if (resolver_fn != 1)
>>>> + __builtin_abort ();
>>>> +
>>>> + if (resolved_fn != 10)
>>>> + __builtin_abort ();
>>>> +
>>>> + return 0;
>>>> +}
>>>>
>>>
>From f1fcd618d424fe617cd75190d89e85941e9d7b98 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Thu, 22 Jun 2017 09:19:31 +0200
Subject: [PATCH] Do not allow to inline ifunc resolvers (PR ipa/81128).
gcc/ChangeLog:
2017-06-22 Martin Liska <mliska@suse.cz>
* ipa-visibility.c (non_local_p): Handle visibility.
gcc/c-family/ChangeLog:
2017-06-22 Martin Liska <mliska@suse.cz>
* c-attribs.c (handle_alias_ifunc_attribute): Append ifunc alias
to a function declaration.
gcc/testsuite/ChangeLog:
2017-06-22 Martin Liska <mliska@suse.cz>
* gcc.target/i386/pr81128.c: New test.
---
gcc/c-family/c-attribs.c | 11 ++++--
gcc/ipa-visibility.c | 3 +-
gcc/testsuite/gcc.target/i386/pr81128.c | 65 +++++++++++++++++++++++++++++++++
3 files changed, 75 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/i386/pr81128.c
diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 2b6845f2cbd..626ffa1cde7 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -1846,9 +1846,14 @@ handle_alias_ifunc_attribute (bool is_alias, tree *node, tree name, tree args,
TREE_STATIC (decl) = 1;
if (!is_alias)
- /* ifuncs are also aliases, so set that attribute too. */
- DECL_ATTRIBUTES (decl)
- = tree_cons (get_identifier ("alias"), args, DECL_ATTRIBUTES (decl));
+ {
+ /* ifuncs are also aliases, so set that attribute too. */
+ DECL_ATTRIBUTES (decl)
+ = tree_cons (get_identifier ("alias"), args,
+ DECL_ATTRIBUTES (decl));
+ DECL_ATTRIBUTES (decl) = tree_cons (get_identifier ("ifunc"),
+ NULL, DECL_ATTRIBUTES (decl));
+ }
}
else
{
diff --git a/gcc/ipa-visibility.c b/gcc/ipa-visibility.c
index d5a3ae56c46..da4a22e7329 100644
--- a/gcc/ipa-visibility.c
+++ b/gcc/ipa-visibility.c
@@ -97,7 +97,8 @@ non_local_p (struct cgraph_node *node, void *data ATTRIBUTE_UNUSED)
&& !DECL_EXTERNAL (node->decl)
&& !node->externally_visible
&& !node->used_from_other_partition
- && !node->in_other_partition);
+ && !node->in_other_partition
+ && node->get_availability () >= AVAIL_AVAILABLE);
}
/* Return true when function can be marked local. */
diff --git a/gcc/testsuite/gcc.target/i386/pr81128.c b/gcc/testsuite/gcc.target/i386/pr81128.c
new file mode 100644
index 00000000000..90a567ad690
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr81128.c
@@ -0,0 +1,65 @@
+/* PR ipa/81128 */
+/* { dg-do run } */
+/* { dg-options "-O3" } */
+/* { dg-require-ifunc "" } */
+
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <time.h>
+
+int resolver_fn = 0;
+int resolved_fn = 0;
+
+static inline void
+do_it_right_at_runtime_A ()
+{
+ resolved_fn++;
+}
+
+static inline void
+do_it_right_at_runtime_B ()
+{
+ resolved_fn++;
+}
+
+static inline void do_it_right_at_runtime (void);
+
+void do_it_right_at_runtime (void)
+ __attribute__ ((ifunc ("resolve_do_it_right_at_runtime")));
+
+static void (*resolve_do_it_right_at_runtime (void)) (void)
+{
+ srand (time (NULL));
+ int r = rand ();
+ resolver_fn++;
+
+ /* Use intermediate variable to get a warning for non-matching
+ * prototype. */
+ typeof(do_it_right_at_runtime) *func;
+ if (r & 1)
+ func = do_it_right_at_runtime_A;
+ else
+ func = do_it_right_at_runtime_B;
+
+ return (void *) func;
+}
+
+int
+main (void)
+{
+ const unsigned int ITERS = 10;
+
+ for (int i = ITERS; i > 0; i--)
+ {
+ do_it_right_at_runtime ();
+ }
+
+ if (resolver_fn != 1)
+ __builtin_abort ();
+
+ if (resolved_fn != 10)
+ __builtin_abort ();
+
+ return 0;
+}
--
2.13.1