[PATCH] detect incompatible aliases (PR c/81854)

[PATCH] detect incompatible aliases (PR c/81854)

[Martin Sebor]

[-- Attachment #1: Type: text/plain, Size: 2465 bytes --]

Joseph, while looking into implementing enhancement your request
pr81824 I noticed that GCC silently accepts incompatible alias
declarations (pr81854) so as sort of a proof-concept for the
former I enhanced the checking already done for other kinds of
incompatibilities to also detect those mentioned in the latter
bug.  Attached is this patch, tested on x85_64-linux.

Jonathan, the patch requires suppressing the warning in libstdc++
compatibility symbol definitions in compatibility.cc.  I couldn't
find a way to do it without the suppression but I'd be happy to
try again if you have an idea for how.

As an example, the patch lets GCC detect mistakes like:

    size_t __attribute__ ((ifunc ("bar_resolver")))
    bar (void*, const void*, size_t);

    void* fast_bar (void *d, const void *s, size_t n) { ... }
    void* slow_bar (void *d, const void *s, size_t n) { ... }

    void* bar_resolver (void)
    {
       return fast ? &fast_bar : &slow_bar;
    }

By complaining that the ifunc resolver should return a function
pointer it makes the programmer change the declaration of the
resolver to one of:

    __typeof__ (bar)* bar_resolver (void) { ... }

or

    __typeof__ (fast_bar)* bar_resolver (void) { ... }

which then triggers either -Wincompatible-pointer-types or
-Wattributes, respectively.  (I used the latter warning in
my patch but maybe the former would be more appropriate).

Martin

PS A documentation-only patch to update the description of
attribute ifunc was posted separately here:
    https://gcc.gnu.org/ml/gcc-patches/2017-08/msg01095.html

PPS To make use of this checking (and compile without the new
warnings) Glibc needs the following patch:

diff --git a/include/libc-symbols.h b/include/libc-symbols.h
index fe3ab81..5413e56 100644
--- a/include/libc-symbols.h
+++ b/include/libc-symbols.h
@@ -790,7 +790,8 @@ for linking")

  /* Helper / base  macros for indirect function symbols.  */
  #define __ifunc_resolver(type_name, name, expr, arg, init, classifier) \
-  classifier inhibit_stack_protector void *name##_ifunc (arg) 
                        \
+  classifier inhibit_stack_protector                                   \
+  __typeof (type_name) *name##_ifunc (arg)                             \
    {                                                                    \
      init ();                                                           \
      __typeof (type_name) *res = expr;                                  \


[-- Attachment #2: gcc-81854.diff --]
[-- Type: text/x-patch, Size: 12313 bytes --]

PR c/81854 - weak alias of an incompatible symbol accepted

gcc/ChangeLog:

	PR c/81854
	* cgraphunit.c (handle_alias_pairs): Reject aliases between functions
	of incompatible types.

gcc/testsuite/ChangeLog:

	PR c/81854
	* gcc.dg/pr81854.c: New test.
	* g++.dg/ext/attr-ifunc-5.C: New test.
	* g++.dg/ext/attr-ifunc-1.C: Adjust.
	* g++.dg/ext/attr-ifunc-2.C: Same.
	* g++.dg/ext/attr-ifunc-3.C: Same.
	* g++.dg/ext/attr-ifunc-4.C: Same.
	* g++.old-deja/g++.abi/vtable2.C: Same.
	* gcc.dg/attr-ifunc-1.c: Same.

libstdc++-v3/ChangeLog:

	PR c/81854
	* src/c++98/compatibility.cc: Suppress -Wattributes.

diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index e8cc765..8d09a79 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -1352,6 +1352,63 @@ handle_alias_pairs (void)
       if (TREE_CODE (p->decl) == FUNCTION_DECL
           && target_node && is_a <cgraph_node *> (target_node))
 	{
+	  tree t1 = TREE_TYPE (p->decl);
+	  tree t2 = TREE_TYPE (target_node->decl);
+
+	  if (lookup_attribute ("ifunc", DECL_ATTRIBUTES (p->decl)))
+	    {
+	      t2 = TREE_TYPE (t2);
+	      if (POINTER_TYPE_P (t2))
+		{
+		  t2 = TREE_TYPE (t2);
+		  if (!FUNC_OR_METHOD_TYPE_P (t2))
+		    {
+		      if (warning_at (DECL_SOURCE_LOCATION (p->decl),
+				      OPT_Wattributes,
+				      "%q+D %<ifunc%> resolver should return "
+				      "a function pointer",
+				      p->decl))
+			inform (DECL_SOURCE_LOCATION (target_node->decl),
+				"resolver declaration here");
+
+		      t2 = NULL_TREE;
+		    }
+		}
+	      else
+		{
+		  /* Deal with static member function pointers.  */
+		  if (TREE_CODE (t2) == RECORD_TYPE
+		      && TYPE_FIELDS (t2)
+		      && TREE_CODE (TREE_TYPE (TYPE_FIELDS (t2))) == POINTER_TYPE
+		      && (TREE_CODE (TREE_TYPE (TREE_TYPE (TYPE_FIELDS (t2))))
+			  == METHOD_TYPE))
+		    t2 = TREE_TYPE (TREE_TYPE (TYPE_FIELDS (t2)));
+		  else
+		    {
+		      error ("%q+D %<ifunc%> resolver must return a function "
+			     "pointer",
+			     p->decl);
+		      inform (DECL_SOURCE_LOCATION (target_node->decl),
+			      "resolver declaration here");
+
+		      t2 = NULL_TREE;
+		    }
+		}
+	    }
+
+	  if (t2
+	      && (!FUNC_OR_METHOD_TYPE_P (t2)
+		  || (prototype_p (t1)
+		      && prototype_p (t2)
+		      && !types_compatible_p (t1, t2))))
+	    {
+	      if (warning_at (DECL_SOURCE_LOCATION (p->decl), OPT_Wattributes,
+			      "%q+D alias between functions of incompatible "
+			      "types %qT and %qT", p->decl, t1, t2))
+		inform (DECL_SOURCE_LOCATION (target_node->decl),
+			"aliased declaration here");
+	    }
+
 	  cgraph_node *src_node = cgraph_node::get (p->decl);
 	  if (src_node && src_node->definition)
 	    src_node->reset ();
@@ -1366,10 +1423,11 @@ handle_alias_pairs (void)
 	}
       else
 	{
-	  error ("%q+D alias in between function and variable is not supported",
+	  error ("%q+D alias between function and variable is not supported",
 		 p->decl);
-	  warning (0, "%q+D aliased declaration",
-		   target_node->decl);
+	  inform (DECL_SOURCE_LOCATION (target_node->decl),
+		  "aliased declaration here");
+
 	  alias_pairs->unordered_remove (i);
 	}
     }
diff --git a/gcc/testsuite/g++.dg/ext/attr-ifunc-1.C b/gcc/testsuite/g++.dg/ext/attr-ifunc-1.C
index d41fa7d..2c7bba1 100644
--- a/gcc/testsuite/g++.dg/ext/attr-ifunc-1.C
+++ b/gcc/testsuite/g++.dg/ext/attr-ifunc-1.C
@@ -2,33 +2,41 @@
 /* { dg-require-ifunc "" } */
 /* { dg-options "-Wno-pmf-conversions" } */
 
-#include <stdio.h>
-
 struct Klass
 {
   int implementation ();
   int magic ();
-  static void *resolver ();
+
+  typedef int (Klass::*MemFuncPtr)();
+
+  static MemFuncPtr resolver ();
 };
 
+Klass::MemFuncPtr p = &Klass::implementation;
+
 int Klass::implementation (void)
 {
-  printf ("'ere I am JH\n");
-  return 0;
+  __builtin_printf ("'ere I am JH\n");
+  return 1234;
 }
 
-void *Klass::resolver (void)
+
+Klass::MemFuncPtr Klass::resolver (void)
 {
-  int (Klass::*pmf) () = &Klass::implementation;
-  
-  return (void *)(int (*)(Klass *))(((Klass *)0)->*pmf);
+  return &Klass::implementation;
 }
 
+int f (void) __attribute__ ((ifunc ("foo")));
+
+typedef int (F)(void);
+extern "C" F* foo () { return 0; }
+
+
 int Klass::magic (void) __attribute__ ((ifunc ("_ZN5Klass8resolverEv")));
 
 int main ()
 {
   Klass obj;
-  
-  return obj.magic () != 0;
+
+  return !(obj.magic () == 1234);
 }
diff --git a/gcc/testsuite/g++.dg/ext/attr-ifunc-2.C b/gcc/testsuite/g++.dg/ext/attr-ifunc-2.C
index e205a2a..49872e0 100644
--- a/gcc/testsuite/g++.dg/ext/attr-ifunc-2.C
+++ b/gcc/testsuite/g++.dg/ext/attr-ifunc-2.C
@@ -8,7 +8,10 @@ struct Klass
 {
   int implementation ();
   int magic ();
-  static void *resolver ();
+
+  typedef int (Klass::*MemFuncPtr)();
+
+  static MemFuncPtr resolver ();
 };
 
 int Klass::implementation (void)
@@ -17,11 +20,9 @@ int Klass::implementation (void)
   return 0;
 }
 
-void *Klass::resolver (void)
+Klass::memFuncPtr Klass::resolver (void)
 {
-  int (Klass::*pmf) () = &Klass::implementation;
-  
-  return (void *)(int (*)(Klass *))(((Klass *)0)->*pmf);
+  return &Klass::implementation;
 }
 
 int Klass::magic (void) __attribute__ ((ifunc ("_ZN5Klass8resolverEv")));
@@ -33,6 +34,6 @@ struct Klassier : Klass
 int main ()
 {
   Klassier obj;
-  
+
   return obj.magic () != 0;
 }
diff --git a/gcc/testsuite/g++.dg/ext/attr-ifunc-3.C b/gcc/testsuite/g++.dg/ext/attr-ifunc-3.C
index ba65976..04206a1 100644
--- a/gcc/testsuite/g++.dg/ext/attr-ifunc-3.C
+++ b/gcc/testsuite/g++.dg/ext/attr-ifunc-3.C
@@ -8,7 +8,10 @@ struct Klass
 {
   int implementation ();
   int magic ();
-  static void *resolver ();
+
+  typedef int (Klass::*MemFuncPtr)();
+
+  static MemFuncPtr resolver ();
 };
 
 int Klass::implementation (void)
@@ -17,11 +20,9 @@ int Klass::implementation (void)
   return 0;
 }
 
-void *Klass::resolver (void)
+Klass::MemFuncPtr Klass::resolver (void)
 {
-  int (Klass::*pmf) () = &Klass::implementation;
-  
-  return (void *)(int (*)(Klass *))(((Klass *)0)->*pmf);
+  return &Klass::implementation;
 }
 
 int Klass::magic (void) __attribute__ ((ifunc ("_ZN5Klass8resolverEv")));
@@ -34,6 +35,6 @@ int Foo (Klass &obj, int (Klass::*pmf) ())
 int main ()
 {
   Klass obj;
-  
+
   return Foo (obj, &Klass::magic) != 0;
 }
diff --git a/gcc/testsuite/g++.dg/ext/attr-ifunc-4.C b/gcc/testsuite/g++.dg/ext/attr-ifunc-4.C
index 0cae410..b8d8e58 100644
--- a/gcc/testsuite/g++.dg/ext/attr-ifunc-4.C
+++ b/gcc/testsuite/g++.dg/ext/attr-ifunc-4.C
@@ -13,7 +13,10 @@ struct Klassier : Klass
 {
   int implementation ();
   int magic ();
-  static void *resolver ();
+
+  typedef int (Klass::*MemFuncPtr)();
+
+  static MemFuncPtr resolver ();
 };
 
 int Klassier::implementation (void)
@@ -22,11 +25,9 @@ int Klassier::implementation (void)
   return 0;
 }
 
-void *Klassier::resolver (void)
+Klassier::MemFuncPtr Klassier::resolver (void)
 {
-  int (Klassier::*pmf) () = &Klassier::implementation;
-  
-  return (void *)(int (*)(Klassier *))(((Klassier *)0)->*pmf);
+  return &Klassier::implementation;
 }
 
 int Klassier::magic (void) __attribute__ ((ifunc ("_ZN8Klassier8resolverEv")));
@@ -39,6 +40,6 @@ int __attribute__ ((weak)) Foo (Klass &base)
 int main ()
 {
   Klassier obj;
-  
+
   return Foo (obj) != 0;
 }
diff --git a/gcc/testsuite/g++.dg/ext/attr-ifunc-5.C b/gcc/testsuite/g++.dg/ext/attr-ifunc-5.C
new file mode 100644
index 0000000..05855dd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/attr-ifunc-5.C
@@ -0,0 +1,29 @@
+// PR c/81854 - weak alias of an incompatible symbol accepted
+// { dg-do compile }
+// { dg-require-ifunc "" } */
+
+struct Klass
+{
+  int implementation ();
+  const char* magic ();
+
+  typedef int (Klass::*MemFuncPtr)();
+
+  static MemFuncPtr resolver ();
+};
+
+int Klass::implementation (void)
+{
+  return 0;
+}
+
+const char* __attribute__ ((ifunc ("_ZN5Klass8resolverEv")))
+  Klass::magic ();   // { dg-warning "alias between functions of incompatible types" }
+
+
+
+Klass::MemFuncPtr
+Klass::resolver (void) // { dg-message "aliased declaration here" }
+{
+  return &Klass::implementation;
+}
diff --git a/gcc/testsuite/g++.old-deja/g++.abi/vtable2.C b/gcc/testsuite/g++.old-deja/g++.abi/vtable2.C
index 3022875..2c88a95 100644
--- a/gcc/testsuite/g++.old-deja/g++.abi/vtable2.C
+++ b/gcc/testsuite/g++.old-deja/g++.abi/vtable2.C
@@ -1,5 +1,5 @@
 // { dg-do run  }
-// { dg-options "-fno-strict-aliasing" }
+// { dg-options "-Wno-attributes -fno-strict-aliasing" }
 // Origin: Mark Mitchell <mark@codesourcery.com>
 
 #if defined (__GXX_ABI_VERSION) && __GXX_ABI_VERSION >= 100
@@ -126,7 +126,8 @@ void S4::s1 ()
 extern "C" {
   /* We can use weakref here without dg-require-weak, because we know
      the symbols are defined, so we don't actually issue the .weak
-     directives.  */
+     directives.  The references to the incompatible virtual S3::s3()
+     and S4::s1() trigger -Wattributes.  */
   static void S3_s3 () __attribute__((__weakref__ ("_ZN2S32s3Ev")));
   static void S4_s1 () __attribute__((__weakref__ ("_ZN2S42s1Ev")));
 }
diff --git a/gcc/testsuite/gcc.dg/attr-ifunc-1.c b/gcc/testsuite/gcc.dg/attr-ifunc-1.c
index f9c6482..3f7450e 100644
--- a/gcc/testsuite/gcc.dg/attr-ifunc-1.c
+++ b/gcc/testsuite/gcc.dg/attr-ifunc-1.c
@@ -2,17 +2,17 @@
 /* { dg-require-ifunc "" } */
 /* { dg-options "" } */
 
-#include <stdio.h>
+typedef int F (void);
 
 static int implementation (void)
 {
-  printf ("'ere I am JH\n");
+  __builtin_printf ("'ere I am JH\n");
   return 0;
 }
 
-static void *resolver (void)
+static F* resolver (void)
 {
-  return (void *)implementation;
+  return implementation;
 }
 
 extern int magic (void) __attribute__ ((ifunc ("resolver")));
diff --git a/gcc/testsuite/gcc.dg/pr81854.c b/gcc/testsuite/gcc.dg/pr81854.c
new file mode 100644
index 0000000..4b0f4da
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr81854.c
@@ -0,0 +1,63 @@
+/* PR c/81854 - weak alias of an incompatible symbol accepted
+   { dg-do compile } */
+
+const char* __attribute__ ((weak, alias ("f0_target")))
+f0 (void);          /* { dg-error "alias between function and variable" } */
+
+int f0_target;      /* { dg-message "aliased declaration here" } */
+
+
+const char* __attribute__ ((weak, alias ("f1_target")))
+f1 (void);          /* { dg-warning "alias between functions of incompatible types" } */
+
+void f1_target (int *p)   /* { dg-message "aliased declaration here" } */
+{
+  *p = 0;
+}
+
+
+const char* __attribute__ ((alias ("f2_target")))
+f2 (void*);   /* { dg-warning "alias between functions of incompatible types" } */
+
+const char* f2_target (int i)   /* { dg-message "aliased declaration here" } */
+{
+  (void)&i;
+  return 0;
+}
+
+
+int __attribute__ ((ifunc ("f3_resolver")))
+f3 (void);          /* { dg-error ".ifunc. resolver must return a function pointer" } */
+
+int f3_resolver (void)   /* { dg-message "resolver declaration here" } */
+{
+  return 0;
+}
+
+
+int __attribute__ ((ifunc ("f4_resolver")))
+f4 (void);          /* { dg-warning ".ifunc. resolver should return a function pointer" } */
+
+void* f4_resolver (void) /* { dg-message "resolver declaration here" } */
+{
+  return 0;
+}
+
+
+int __attribute__ ((ifunc ("f5_resolver")))
+f5 (void);          /* { dg-warning "alias between functions of incompatible types" } */
+
+typedef void F5 (void);
+F5* f5_resolver (void) /* { dg-message "aliased declaration here" } */
+{
+  return 0;
+}
+
+const char* __attribute__ ((ifunc ("f6_resolver")))
+f6 (void);
+
+typedef const char* F6 (void);
+F6* f6_resolver (void)
+{
+  return 0;
+}
diff --git a/libstdc++-v3/src/c++98/compatibility.cc b/libstdc++-v3/src/c++98/compatibility.cc
index 381f4c4..5f56b9e 100644
--- a/libstdc++-v3/src/c++98/compatibility.cc
+++ b/libstdc++-v3/src/c++98/compatibility.cc
@@ -213,6 +213,11 @@ _ZNSt19istreambuf_iteratorIcSt11char_traitsIcEEppEv
 _ZNSt19istreambuf_iteratorIwSt11char_traitsIwEEppEv
  */
 
+// Disable warning about declaring aliases between functions with
+// incompatible types.
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wattributes"
+
 namespace std _GLIBCXX_VISIBILITY(default)
 {
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
@@ -509,6 +514,9 @@ _GLIBCXX_MATHL_WRAPPER1 (tan, GLIBCXX_3.4);
 #endif
 #endif // _GLIBCXX_LONG_DOUBLE_COMPAT
 
+// Restore disable -Wattributes
+#pragma GCC diagnostic pop
+
 #endif
 
 #ifdef _GLIBCXX_LONG_DOUBLE_COMPAT

Re: [PATCH] detect incompatible aliases (PR c/81854)

[Joseph Myers]

On Thu, 17 Aug 2017, Martin Sebor wrote:

> PPS To make use of this checking (and compile without the new
> warnings) Glibc needs the following patch:

This should be submitted to libc-alpha (independently of the GCC patch, 
presuming existing GCC versions accept the correct types).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] detect incompatible aliases (PR c/81854)

[Jonathan Wakely]

On 17/08/17 21:21 -0600, Martin Sebor wrote:
>Joseph, while looking into implementing enhancement your request
>pr81824 I noticed that GCC silently accepts incompatible alias
>declarations (pr81854) so as sort of a proof-concept for the
>former I enhanced the checking already done for other kinds of
>incompatibilities to also detect those mentioned in the latter
>bug.  Attached is this patch, tested on x85_64-linux.
>
>Jonathan, the patch requires suppressing the warning in libstdc++
>compatibility symbol definitions in compatibility.cc.  I couldn't
>find a way to do it without the suppression but I'd be happy to
>try again if you have an idea for how.

Doing it that way is fine, but ...

>diff --git a/libstdc++-v3/src/c++98/compatibility.cc b/libstdc++-v3/src/c++98/compatibility.cc
>index 381f4c4..5f56b9e 100644
>--- a/libstdc++-v3/src/c++98/compatibility.cc
>+++ b/libstdc++-v3/src/c++98/compatibility.cc
>@@ -213,6 +213,11 @@ _ZNSt19istreambuf_iteratorIcSt11char_traitsIcEEppEv
> _ZNSt19istreambuf_iteratorIwSt11char_traitsIwEEppEv
>  */
> 
>+// Disable warning about declaring aliases between functions with
>+// incompatible types.
>+#pragma GCC diagnostic push
>+#pragma GCC diagnostic ignored "-Wattributes"
>+

Could this be moved closer to the point where it's needed?

It's not needed until after line 361, right?

> namespace std _GLIBCXX_VISIBILITY(default)
> {
> _GLIBCXX_BEGIN_NAMESPACE_VERSION
>@@ -509,6 +514,9 @@ _GLIBCXX_MATHL_WRAPPER1 (tan, GLIBCXX_3.4);
> #endif
> #endif // _GLIBCXX_LONG_DOUBLE_COMPAT
> 
>+// Restore disable -Wattributes
>+#pragma GCC diagnostic pop
>+
> #endif
> 
> #ifdef _GLIBCXX_LONG_DOUBLE_COMPAT
>

Re: [PATCH] detect incompatible aliases (PR c/81854)

[Martin Sebor]

On 08/18/2017 07:10 AM, Jonathan Wakely wrote:
> On 17/08/17 21:21 -0600, Martin Sebor wrote:
>> Joseph, while looking into implementing enhancement your request
>> pr81824 I noticed that GCC silently accepts incompatible alias
>> declarations (pr81854) so as sort of a proof-concept for the
>> former I enhanced the checking already done for other kinds of
>> incompatibilities to also detect those mentioned in the latter
>> bug.  Attached is this patch, tested on x85_64-linux.
>>
>> Jonathan, the patch requires suppressing the warning in libstdc++
>> compatibility symbol definitions in compatibility.cc.  I couldn't
>> find a way to do it without the suppression but I'd be happy to
>> try again if you have an idea for how.
>
> Doing it that way is fine, but ...
>
>> diff --git a/libstdc++-v3/src/c++98/compatibility.cc
>> b/libstdc++-v3/src/c++98/compatibility.cc
>> index 381f4c4..5f56b9e 100644
>> --- a/libstdc++-v3/src/c++98/compatibility.cc
>> +++ b/libstdc++-v3/src/c++98/compatibility.cc
>> @@ -213,6 +213,11 @@ _ZNSt19istreambuf_iteratorIcSt11char_traitsIcEEppEv
>> _ZNSt19istreambuf_iteratorIwSt11char_traitsIwEEppEv
>>  */
>>
>> +// Disable warning about declaring aliases between functions with
>> +// incompatible types.
>> +#pragma GCC diagnostic push
>> +#pragma GCC diagnostic ignored "-Wattributes"
>> +
>
> Could this be moved closer to the point where it's needed?
>
> It's not needed until after line 361, right?

Sure.  The other possibility that I forgot to mention is to
declare the alias without a prototype, which in C++ looks
like this:

   void foo (...);

The patch would then look like this.  Do you have a preference
between these two approaches?

Martin

diff --git a/libstdc++-v3/src/c++98/compatibility.cc 
b/libstdc++-v3/src/c++98/compatibility.cc
index 381f4c4..b49a5ca 100644
--- a/libstdc++-v3/src/c++98/compatibility.cc
+++ b/libstdc++-v3/src/c++98/compatibility.cc
@@ -367,13 +367,13 @@ _GLIBCXX_END_NAMESPACE_VERSION

  #define _GLIBCXX_3_4_SYMVER(XXname, name) \
     extern "C" void \
-   _X##name() \
+   _X##name(...) \
     __attribute__ ((alias(#XXname))); \
     asm (".symver " "_X" #name "," #name "@GLIBCXX_3.4");

  #define _GLIBCXX_3_4_5_SYMVER(XXname, name) \
     extern "C" void \
-   _Y##name() \
+   _Y##name(...) \
     __attribute__ ((alias(#XXname))); \
     asm (".symver " "_Y" #name  "," #name "@@GLIBCXX_3.4.5");

Re: [PATCH] detect incompatible aliases (PR c/81854)

[Jonathan Wakely]

On 18/08/17 08:54 -0600, Martin Sebor wrote:
>On 08/18/2017 07:10 AM, Jonathan Wakely wrote:
>>On 17/08/17 21:21 -0600, Martin Sebor wrote:
>>>Joseph, while looking into implementing enhancement your request
>>>pr81824 I noticed that GCC silently accepts incompatible alias
>>>declarations (pr81854) so as sort of a proof-concept for the
>>>former I enhanced the checking already done for other kinds of
>>>incompatibilities to also detect those mentioned in the latter
>>>bug.  Attached is this patch, tested on x85_64-linux.
>>>
>>>Jonathan, the patch requires suppressing the warning in libstdc++
>>>compatibility symbol definitions in compatibility.cc.  I couldn't
>>>find a way to do it without the suppression but I'd be happy to
>>>try again if you have an idea for how.
>>
>>Doing it that way is fine, but ...
>>
>>>diff --git a/libstdc++-v3/src/c++98/compatibility.cc
>>>b/libstdc++-v3/src/c++98/compatibility.cc
>>>index 381f4c4..5f56b9e 100644
>>>--- a/libstdc++-v3/src/c++98/compatibility.cc
>>>+++ b/libstdc++-v3/src/c++98/compatibility.cc
>>>@@ -213,6 +213,11 @@ _ZNSt19istreambuf_iteratorIcSt11char_traitsIcEEppEv
>>>_ZNSt19istreambuf_iteratorIwSt11char_traitsIwEEppEv
>>> */
>>>
>>>+// Disable warning about declaring aliases between functions with
>>>+// incompatible types.
>>>+#pragma GCC diagnostic push
>>>+#pragma GCC diagnostic ignored "-Wattributes"
>>>+
>>
>>Could this be moved closer to the point where it's needed?
>>
>>It's not needed until after line 361, right?
>
>Sure.  The other possibility that I forgot to mention is to
>declare the alias without a prototype, which in C++ looks
>like this:
>
>  void foo (...);
>
>The patch would then look like this.  Do you have a preference
>between these two approaches?

If this doesn't change the generated code, but avoids the warnings
then I think I prefer this. i.e. fix the code, not just suppress the
warnings.


>Martin
>
>diff --git a/libstdc++-v3/src/c++98/compatibility.cc 
>b/libstdc++-v3/src/c++98/compatibility.cc
>index 381f4c4..b49a5ca 100644
>--- a/libstdc++-v3/src/c++98/compatibility.cc
>+++ b/libstdc++-v3/src/c++98/compatibility.cc
>@@ -367,13 +367,13 @@ _GLIBCXX_END_NAMESPACE_VERSION
>
> #define _GLIBCXX_3_4_SYMVER(XXname, name) \
>    extern "C" void \
>-   _X##name() \
>+   _X##name(...) \
>    __attribute__ ((alias(#XXname))); \
>    asm (".symver " "_X" #name "," #name "@GLIBCXX_3.4");
>
> #define _GLIBCXX_3_4_5_SYMVER(XXname, name) \
>    extern "C" void \
>-   _Y##name() \
>+   _Y##name(...) \
>    __attribute__ ((alias(#XXname))); \
>    asm (".symver " "_Y" #name  "," #name "@@GLIBCXX_3.4.5");
>
>

Re: [PATCH] detect incompatible aliases (PR c/81854)

[Martin Sebor]

On 08/18/2017 10:48 AM, Jonathan Wakely wrote:
> On 18/08/17 08:54 -0600, Martin Sebor wrote:
>> On 08/18/2017 07:10 AM, Jonathan Wakely wrote:
>>> On 17/08/17 21:21 -0600, Martin Sebor wrote:
>>>> Joseph, while looking into implementing enhancement your request
>>>> pr81824 I noticed that GCC silently accepts incompatible alias
>>>> declarations (pr81854) so as sort of a proof-concept for the
>>>> former I enhanced the checking already done for other kinds of
>>>> incompatibilities to also detect those mentioned in the latter
>>>> bug.  Attached is this patch, tested on x85_64-linux.
>>>>
>>>> Jonathan, the patch requires suppressing the warning in libstdc++
>>>> compatibility symbol definitions in compatibility.cc.  I couldn't
>>>> find a way to do it without the suppression but I'd be happy to
>>>> try again if you have an idea for how.
>>>
>>> Doing it that way is fine, but ...
>>>
>>>> diff --git a/libstdc++-v3/src/c++98/compatibility.cc
>>>> b/libstdc++-v3/src/c++98/compatibility.cc
>>>> index 381f4c4..5f56b9e 100644
>>>> --- a/libstdc++-v3/src/c++98/compatibility.cc
>>>> +++ b/libstdc++-v3/src/c++98/compatibility.cc
>>>> @@ -213,6 +213,11 @@
>>>> _ZNSt19istreambuf_iteratorIcSt11char_traitsIcEEppEv
>>>> _ZNSt19istreambuf_iteratorIwSt11char_traitsIwEEppEv
>>>> */
>>>>
>>>> +// Disable warning about declaring aliases between functions with
>>>> +// incompatible types.
>>>> +#pragma GCC diagnostic push
>>>> +#pragma GCC diagnostic ignored "-Wattributes"
>>>> +
>>>
>>> Could this be moved closer to the point where it's needed?
>>>
>>> It's not needed until after line 361, right?
>>
>> Sure.  The other possibility that I forgot to mention is to
>> declare the alias without a prototype, which in C++ looks
>> like this:
>>
>>  void foo (...);
>>
>> The patch would then look like this.  Do you have a preference
>> between these two approaches?
>
> If this doesn't change the generated code, but avoids the warnings
> then I think I prefer this. i.e. fix the code, not just suppress the
> warnings.

It's the same as calling a function without a prototype in C.
I rebuilt libstdc++ with this change and reran the test suite
with no unexpected failures so I'll go ahead and commit this
change instead.

To be clear, though, this is just a suppression mechanism not
unlike a cast.  The ideal solution would be to declare the
aliases to have the right type, e.g., using __typeof__ or
decltype.  I just couldn't find a way to make it work with
the macros.

Martin

>
>
>> Martin
>>
>> diff --git a/libstdc++-v3/src/c++98/compatibility.cc
>> b/libstdc++-v3/src/c++98/compatibility.cc
>> index 381f4c4..b49a5ca 100644
>> --- a/libstdc++-v3/src/c++98/compatibility.cc
>> +++ b/libstdc++-v3/src/c++98/compatibility.cc
>> @@ -367,13 +367,13 @@ _GLIBCXX_END_NAMESPACE_VERSION
>>
>> #define _GLIBCXX_3_4_SYMVER(XXname, name) \
>>    extern "C" void \
>> -   _X##name() \
>> +   _X##name(...) \
>>    __attribute__ ((alias(#XXname))); \
>>    asm (".symver " "_X" #name "," #name "@GLIBCXX_3.4");
>>
>> #define _GLIBCXX_3_4_5_SYMVER(XXname, name) \
>>    extern "C" void \
>> -   _Y##name() \
>> +   _Y##name(...) \
>>    __attribute__ ((alias(#XXname))); \
>>    asm (".symver " "_Y" #name  "," #name "@@GLIBCXX_3.4.5");
>>
>>

[PING] [PATCH] detect incompatible aliases (PR c/81854)

[Martin Sebor]

Jeff, this is the other attribute patch I mentioned the other
day:

   https://gcc.gnu.org/ml/gcc-patches/2017-08/msg01103.html

The Glibc and libstdc++ changes have already been committed:
   https://sourceware.org/ml/glibc-cvs/2017-q3/msg00587.html
   https://gcc.gnu.org/ml/gcc-cvs/2017-08/msg00459.html

Thanks
Martin

On 08/17/2017 09:21 PM, Martin Sebor wrote:
> Joseph, while looking into implementing enhancement your request
> pr81824 I noticed that GCC silently accepts incompatible alias
> declarations (pr81854) so as sort of a proof-concept for the
> former I enhanced the checking already done for other kinds of
> incompatibilities to also detect those mentioned in the latter
> bug.  Attached is this patch, tested on x85_64-linux.
>
> Jonathan, the patch requires suppressing the warning in libstdc++
> compatibility symbol definitions in compatibility.cc.  I couldn't
> find a way to do it without the suppression but I'd be happy to
> try again if you have an idea for how.
>
> As an example, the patch lets GCC detect mistakes like:
>
>    size_t __attribute__ ((ifunc ("bar_resolver")))
>    bar (void*, const void*, size_t);
>
>    void* fast_bar (void *d, const void *s, size_t n) { ... }
>    void* slow_bar (void *d, const void *s, size_t n) { ... }
>
>    void* bar_resolver (void)
>    {
>       return fast ? &fast_bar : &slow_bar;
>    }
>
> By complaining that the ifunc resolver should return a function
> pointer it makes the programmer change the declaration of the
> resolver to one of:
>
>    __typeof__ (bar)* bar_resolver (void) { ... }
>
> or
>
>    __typeof__ (fast_bar)* bar_resolver (void) { ... }
>
> which then triggers either -Wincompatible-pointer-types or
> -Wattributes, respectively.  (I used the latter warning in
> my patch but maybe the former would be more appropriate).
>
> Martin
>
> PS A documentation-only patch to update the description of
> attribute ifunc was posted separately here:
>    https://gcc.gnu.org/ml/gcc-patches/2017-08/msg01095.html
>
> PPS To make use of this checking (and compile without the new
> warnings) Glibc needs the following patch:
>
> diff --git a/include/libc-symbols.h b/include/libc-symbols.h
> index fe3ab81..5413e56 100644
> --- a/include/libc-symbols.h
> +++ b/include/libc-symbols.h
> @@ -790,7 +790,8 @@ for linking")
>
>  /* Helper / base  macros for indirect function symbols.  */
>  #define __ifunc_resolver(type_name, name, expr, arg, init, classifier) \
> -  classifier inhibit_stack_protector void *name##_ifunc (arg)
>                        \
> +  classifier inhibit_stack_protector                                   \
> +  __typeof (type_name) *name##_ifunc (arg)                             \
>    {                                                                    \
>      init ();                                                           \
>      __typeof (type_name) *res = expr;                                  \
>

Re: [PATCH] detect incompatible aliases (PR c/81854)

[Joseph Myers]

On Thu, 17 Aug 2017, Martin Sebor wrote:

> +		  || (prototype_p (t1)
> +		      && prototype_p (t2)
> +		      && !types_compatible_p (t1, t2))))

Why the restriction to prototyped types?  I'd expect a warning for an 
alias between unprototyped functions of types int () and void (), for 
example.  Or for an alias between void () and void (char), as a function 
with a char argument is not compatible with a non-prototype function in C.  
Is this an issue with the problem being diagnosed at a point where the 
langhooks for language-specific type compatibility rules aren't available?  
If that's preventing diagnosing incompatibility involving unprototyped 
functions, then the patch is OK.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] detect incompatible aliases (PR c/81854)

[Martin Sebor]

On 09/12/2017 10:17 AM, Joseph Myers wrote:
> On Thu, 17 Aug 2017, Martin Sebor wrote:
>
>> +		  || (prototype_p (t1)
>> +		      && prototype_p (t2)
>> +		      && !types_compatible_p (t1, t2))))
>
> Why the restriction to prototyped types?  I'd expect a warning for an
> alias between unprototyped functions of types int () and void (), for
> example.  Or for an alias between void () and void (char), as a function
> with a char argument is not compatible with a non-prototype function in C.
> Is this an issue with the problem being diagnosed at a point where the
> langhooks for language-specific type compatibility rules aren't available?
> If that's preventing diagnosing incompatibility involving unprototyped
> functions, then the patch is OK.

It's meant as an escape hatch.  It allows declaring compatibility
symbols, for example by the libstdc++ _GLIBCXX_3_4_SYMVER macro
defined in libstdc++-v3/src/c++98/compatibility.cc.  The macro is
used to declare compatibility functions of all sorts of incompatible
types.  The originally posted patch had libstdc++ disable the warning
for the file with the symbols but Jonathan preferred this solution.

It could perhaps be tightened up to detect some of the cases on your
list but I'm not sure it's worth the effort and added complexity.
Let me know if you feel differently (or have a different suggestion),
otherwise I will go ahead and commit the patch as is.

Thanks
Martin

Re: [PATCH] detect incompatible aliases (PR c/81854)

[Joseph Myers]

On Mon, 18 Sep 2017, Martin Sebor wrote:

> It's meant as an escape hatch.  It allows declaring compatibility
> symbols, for example by the libstdc++ _GLIBCXX_3_4_SYMVER macro
> defined in libstdc++-v3/src/c++98/compatibility.cc.  The macro is
> used to declare compatibility functions of all sorts of incompatible
> types.  The originally posted patch had libstdc++ disable the warning
> for the file with the symbols but Jonathan preferred this solution.
> 
> It could perhaps be tightened up to detect some of the cases on your
> list but I'm not sure it's worth the effort and added complexity.
> Let me know if you feel differently (or have a different suggestion),
> otherwise I will go ahead and commit the patch as is.

Please add a comment explaining this reasoning and commit the patch.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] detect incompatible aliases (PR c/81854)

[Martin Sebor]

On 09/18/2017 03:44 PM, Joseph Myers wrote:
> On Mon, 18 Sep 2017, Martin Sebor wrote:
>
>> It's meant as an escape hatch.  It allows declaring compatibility
>> symbols, for example by the libstdc++ _GLIBCXX_3_4_SYMVER macro
>> defined in libstdc++-v3/src/c++98/compatibility.cc.  The macro is
>> used to declare compatibility functions of all sorts of incompatible
>> types.  The originally posted patch had libstdc++ disable the warning
>> for the file with the symbols but Jonathan preferred this solution.
>>
>> It could perhaps be tightened up to detect some of the cases on your
>> list but I'm not sure it's worth the effort and added complexity.
>> Let me know if you feel differently (or have a different suggestion),
>> otherwise I will go ahead and commit the patch as is.
>
> Please add a comment explaining this reasoning and commit the patch.

Done in r252976.

Thanks
Martin

Re: [PATCH] detect incompatible aliases (PR c/81854)

[Steve Ellcey]

On Tue, 2017-09-19 at 09:16 -0600, Martin Sebor wrote:
> On 09/18/2017 03:44 PM, Joseph Myers wrote:
> > 
> > On Mon, 18 Sep 2017, Martin Sebor wrote:
> > 
> > > 
> > > It's meant as an escape hatch.  It allows declaring compatibility
> > > symbols, for example by the libstdc++ _GLIBCXX_3_4_SYMVER macro
> > > defined in libstdc++-v3/src/c++98/compatibility.cc.  The macro is
> > > used to declare compatibility functions of all sorts of
> > > incompatible
> > > types.  The originally posted patch had libstdc++ disable the
> > > warning
> > > for the file with the symbols but Jonathan preferred this
> > > solution.
> > > 
> > > It could perhaps be tightened up to detect some of the cases on
> > > your
> > > list but I'm not sure it's worth the effort and added complexity.
> > > Let me know if you feel differently (or have a different
> > > suggestion),
> > > otherwise I will go ahead and commit the patch as is.
> > Please add a comment explaining this reasoning and commit the
> > patch.
> Done in r252976.
> 
> Thanks
> Martin

This patch is causing my gcc/glibc ToT build to fail on aarch64.  I am
not sure if everything should be working at this point or not or if
there is more that needs to be done.  The problem is with the memcpy,
memmove, etc. ifuncs on aarch64.

Steve Ellcey
sellcey@cavium.com


In file included from <command-line>:0:0:
../sysdeps/aarch64/multiarch/memmove.c:38:31: error: ‘memmove’ alias
between functions of incompatible types ‘void *(void *, const void *,
size_t) {aka void *(void *, const void *, long unsigned int)}’ and
‘void * (*(void))(void *, const void *, size_t) {aka void *
(*(void))(void *, const void *, long unsigned int)}’ [-
Werror=attributes]
 strong_alias (__libc_memmove, memmove);
                               ^~~~~~~
./../include/libc-symbols.h:135:26: note: in definition of macro
‘_strong_alias’
   extern __typeof (name) aliasname __attribute__ ((alias (#name)));
                          ^~~~~~~~~
../sysdeps/aarch64/multiarch/memmove.c:38:1: note: in expansion of
macro ‘strong_alias’
 strong_alias (__libc_memmove, memmove);
 ^~~~~~~~~~~~
In file included from <command-line>:0:0:
../sysdeps/aarch64/multiarch/memmove.c:34:13: note: aliased declaration
here
 libc_ifunc (__libc_memmove,
             ^~~~~~~~~~~~~~
./../include/libc-symbols.h:800:25: note: in definition of macro
‘__ifunc_resolver’

   __typeof (type_name) *name##_ifunc (arg)    \
                         ^~~~
./../include/libc-symbols.h:916:32: note: in expansion of macro
‘__ifunc’
 #define libc_ifunc(name, expr) __ifunc (name, name, expr, void,
INIT_ARCH)
                                ^~~~~~~
../sysdeps/aarch64/multiarch/memmove.c:34:1: note: in expansion of
macro ‘libc_ifunc’
 libc_ifunc (__libc_memmove,
 ^~~~~~~~~~
cc1: all warnings being treated as errors

Re: [PATCH] detect incompatible aliases (PR c/81854)

[Martin Sebor]

On 09/20/2017 09:37 AM, Steve Ellcey wrote:
> On Tue, 2017-09-19 at 09:16 -0600, Martin Sebor wrote:
>> On 09/18/2017 03:44 PM, Joseph Myers wrote:
>>>
>>> On Mon, 18 Sep 2017, Martin Sebor wrote:
>>>
>>>>
>>>> It's meant as an escape hatch.  It allows declaring compatibility
>>>> symbols, for example by the libstdc++ _GLIBCXX_3_4_SYMVER macro
>>>> defined in libstdc++-v3/src/c++98/compatibility.cc.  The macro is
>>>> used to declare compatibility functions of all sorts of
>>>> incompatible
>>>> types.  The originally posted patch had libstdc++ disable the
>>>> warning
>>>> for the file with the symbols but Jonathan preferred this
>>>> solution.
>>>>
>>>> It could perhaps be tightened up to detect some of the cases on
>>>> your
>>>> list but I'm not sure it's worth the effort and added complexity.
>>>> Let me know if you feel differently (or have a different
>>>> suggestion),
>>>> otherwise I will go ahead and commit the patch as is.
>>> Please add a comment explaining this reasoning and commit the
>>> patch.
>> Done in r252976.
>>
>> Thanks
>> Martin
>
> This patch is causing my gcc/glibc ToT build to fail on aarch64.  I am
> not sure if everything should be working at this point or not or if
> there is more that needs to be done.  The problem is with the memcpy,
> memmove, etc. ifuncs on aarch64.

The GCC bits should be working correctly.  The Glibc patches were
checked some time ago so it should build as well, and does on x86_64
and I believe also powerpc64le.  I didn't test aarch64 or multiarch
builds but the error below indicates a type mismatch between the alias
and the target (one is a memcpy-like function and the other a function
that returns a pointer to a memcpy-like function).  That's correct for
ifunc where the resolver returns such a pointer but not for the alias
which is expected to have the same type.

The comment copied below from sysdeps/aarch64/multiarch/memmove.c
suggests this might be deliberate:

/* Redefine memmove so that the compiler won't complain about the type
    mismatch with the IFUNC selector in strong_alias, below.  */

so my guess is that the improved type checking has just exposed
this mismatch.  Can you please post the translation unit so we
can confirm it?

Thanks
Martin

>
> Steve Ellcey
> sellcey@cavium.com
>
>
> In file included from <command-line>:0:0:
> ../sysdeps/aarch64/multiarch/memmove.c:38:31: error: ‘memmove’ alias
> between functions of incompatible types ‘void *(void *, const void *,
> size_t) {aka void *(void *, const void *, long unsigned int)}’ and
> ‘void * (*(void))(void *, const void *, size_t) {aka void *
> (*(void))(void *, const void *, long unsigned int)}’ [-
> Werror=attributes]
>  strong_alias (__libc_memmove, memmove);
>                                ^~~~~~~
> ./../include/libc-symbols.h:135:26: note: in definition of macro
> ‘_strong_alias’
>    extern __typeof (name) aliasname __attribute__ ((alias (#name)));
>                           ^~~~~~~~~
> ../sysdeps/aarch64/multiarch/memmove.c:38:1: note: in expansion of
> macro ‘strong_alias’
>  strong_alias (__libc_memmove, memmove);
>  ^~~~~~~~~~~~
> In file included from <command-line>:0:0:
> ../sysdeps/aarch64/multiarch/memmove.c:34:13: note: aliased declaration
> here
>  libc_ifunc (__libc_memmove,
>              ^~~~~~~~~~~~~~
> ./../include/libc-symbols.h:800:25: note: in definition of macro
> ‘__ifunc_resolver’
>
>    __typeof (type_name) *name##_ifunc (arg)    \
>                          ^~~~
> ./../include/libc-symbols.h:916:32: note: in expansion of macro
> ‘__ifunc’
>  #define libc_ifunc(name, expr) __ifunc (name, name, expr, void,
> INIT_ARCH)
>                                 ^~~~~~~
> ../sysdeps/aarch64/multiarch/memmove.c:34:1: note: in expansion of
> macro ‘libc_ifunc’
>  libc_ifunc (__libc_memmove,
>  ^~~~~~~~~~
> cc1: all warnings being treated as errors
>

Re: [PATCH] detect incompatible aliases (PR c/81854)

[Steve Ellcey]

On Wed, 2017-09-20 at 10:17 -0600, Martin Sebor wrote:
> 
> The comment copied below from sysdeps/aarch64/multiarch/memmove.c
> suggests this might be deliberate:
> 
> /* Redefine memmove so that the compiler won't complain about the
> type
>     mismatch with the IFUNC selector in strong_alias, below.  */
> 
> so my guess is that the improved type checking has just exposed
> this mismatch.  Can you please post the translation unit so we
> can confirm it?
> 
> Thanks
> Martin

The translation unit is sysdeps/aarch64/multiarch/memmove.c.  If I
preprocess that and cut out the unneeded parts I get:

extern int i;
typedef long unsigned int size_t;
extern void *__redirect_memmove (void *__dest, const void *__src,
size_t __n)
     __attribute__ ((__nothrow__ )) ;
extern __typeof (__redirect_memmove) __libc_memmove;
extern __typeof (__redirect_memmove) __memmove_generic ;
extern __typeof (__redirect_memmove) __memmove_thunderx ;
extern __typeof (__libc_memmove) __libc_memmove;
__typeof (__libc_memmove) *__libc_memmove_ifunc (void) __asm__
("__libc_memmove");
__attribute__ ((__optimize__ ("-fno-stack-protector"))) __typeof
(__libc_memmove) *__libc_memmove_ifunc (void)
        {
                __typeof (__libc_memmove) *res = i ? __memmove_thunderx
: __memmove_generic; return res; }

__asm__ (".type " "__libc_memmove" ", %gnu_indirect_function");
extern __typeof (__libc_memmove) memmove __attribute__ ((alias
("__libc_memmove")));;


Which, when compiled gives me:


% install/bin/gcc -c x.c
x.c:15:34: warning: ‘memmove’ alias between functions of incompatible
types ‘void *(void *, const void *, size_t) {aka void *(void *, const
void *, long unsigned int)}’ and ‘void * (*(void))(void *, const void
*, size_t) {aka void * (*(void))(void *, const void *, long unsigned
int)}’ [-Wattributes]
 extern __typeof (__libc_memmove) memmove __attribute__ ((alias
("__libc_memmove")));;
                                  ^~~~~~~
x.c:10:84: note: aliased declaration here
 (__optimize__ ("-fno-stack-protector"))) __typeof (__libc_memmove)
*__libc_memmove_ifunc (void)
                                                                     ^~
~~~~~~~~~~~~~~~~~~

Re: [PATCH] detect incompatible aliases (PR c/81854)

[Steve Ellcey]

Interesting new data point.  aarch64 GCC does not enable the
resolver/indirect function feature by default (--enable-gnu-indirect-
function is not set by default).  If I enable this on the GCC build
then GCC can build glibc.  Without it, the glibc build fails.

Steve Ellcey
sellcey@cavium.com


On Wed, 2017-09-20 at 09:38 -0700, Steve Ellcey wrote:
> On Wed, 2017-09-20 at 10:17 -0600, Martin Sebor wrote:
> > 
> >  
> > The comment copied below from sysdeps/aarch64/multiarch/memmove.c
> > suggests this might be deliberate:
> > 
> > /* Redefine memmove so that the compiler won't complain about the
> > type
> >     mismatch with the IFUNC selector in strong_alias, below.  */
> > 
> > so my guess is that the improved type checking has just exposed
> > this mismatch.  Can you please post the translation unit so we
> > can confirm it?
> > 
> > Thanks
> > Martin
> The translation unit is sysdeps/aarch64/multiarch/memmove.c.  If I
> preprocess that and cut out the unneeded parts I get:
> 
> extern int i;
> typedef long unsigned int size_t;
> extern void *__redirect_memmove (void *__dest, const void *__src,
> size_t __n)
>      __attribute__ ((__nothrow__ )) ;
> extern __typeof (__redirect_memmove) __libc_memmove;
> extern __typeof (__redirect_memmove) __memmove_generic ;
> extern __typeof (__redirect_memmove) __memmove_thunderx ;
> extern __typeof (__libc_memmove) __libc_memmove;
> __typeof (__libc_memmove) *__libc_memmove_ifunc (void) __asm__
> ("__libc_memmove");
> __attribute__ ((__optimize__ ("-fno-stack-protector"))) __typeof
> (__libc_memmove) *__libc_memmove_ifunc (void)
>         {
>                 __typeof (__libc_memmove) *res = i ?
> __memmove_thunderx
> : __memmove_generic; return res; }
> 
> __asm__ (".type " "__libc_memmove" ", %gnu_indirect_function");
> extern __typeof (__libc_memmove) memmove __attribute__ ((alias
> ("__libc_memmove")));;
> 
> 
> Which, when compiled gives me:
> 
> 
> % install/bin/gcc -c x.c
> x.c:15:34: warning: ‘memmove’ alias between functions of incompatible
> types ‘void *(void *, const void *, size_t) {aka void *(void *, const
> void *, long unsigned int)}’ and ‘void * (*(void))(void *, const void
> *, size_t) {aka void * (*(void))(void *, const void *, long unsigned
> int)}’ [-Wattributes]
>  extern __typeof (__libc_memmove) memmove __attribute__ ((alias
> ("__libc_memmove")));;
>                                   ^~~~~~~
> x.c:10:84: note: aliased declaration here
>  (__optimize__ ("-fno-stack-protector"))) __typeof (__libc_memmove)
> *__libc_memmove_ifunc (void)
>                                                                      
> ^~
> ~~~~~~~~~~~~~~~~~~

Re: [PATCH] detect incompatible aliases (PR c/81854)

[Martin Sebor]

On 09/20/2017 10:38 AM, Steve Ellcey wrote:
> On Wed, 2017-09-20 at 10:17 -0600, Martin Sebor wrote:
>>
>> The comment copied below from sysdeps/aarch64/multiarch/memmove.c
>> suggests this might be deliberate:
>>
>> /* Redefine memmove so that the compiler won't complain about the
>> type
>>     mismatch with the IFUNC selector in strong_alias, below.  */
>>
>> so my guess is that the improved type checking has just exposed
>> this mismatch.  Can you please post the translation unit so we
>> can confirm it?
>>
>> Thanks
>> Martin
>
> The translation unit is sysdeps/aarch64/multiarch/memmove.c.  If I
> preprocess that and cut out the unneeded parts I get:
>
> extern int i;
> typedef long unsigned int size_t;
> extern void *__redirect_memmove (void *__dest, const void *__src,
> size_t __n)
>      __attribute__ ((__nothrow__ )) ;
> extern __typeof (__redirect_memmove) __libc_memmove;
> extern __typeof (__redirect_memmove) __memmove_generic ;
> extern __typeof (__redirect_memmove) __memmove_thunderx ;
> extern __typeof (__libc_memmove) __libc_memmove;
> __typeof (__libc_memmove) *__libc_memmove_ifunc (void) __asm__
> ("__libc_memmove");
> __attribute__ ((__optimize__ ("-fno-stack-protector"))) __typeof
> (__libc_memmove) *__libc_memmove_ifunc (void)
>         {
>                 __typeof (__libc_memmove) *res = i ? __memmove_thunderx
> : __memmove_generic; return res; }
>
> __asm__ (".type " "__libc_memmove" ", %gnu_indirect_function");
> extern __typeof (__libc_memmove) memmove __attribute__ ((alias
> ("__libc_memmove")));;
>
>
> Which, when compiled gives me:
>
>
> % install/bin/gcc -c x.c
> x.c:15:34: warning: ‘memmove’ alias between functions of incompatible
> types ‘void *(void *, const void *, size_t) {aka void *(void *, const
> void *, long unsigned int)}’ and ‘void * (*(void))(void *, const void
> *, size_t) {aka void * (*(void))(void *, const void *, long unsigned
> int)}’ [-Wattributes]
>  extern __typeof (__libc_memmove) memmove __attribute__ ((alias
> ("__libc_memmove")));;
>                                   ^~~~~~~
> x.c:10:84: note: aliased declaration here
>  (__optimize__ ("-fno-stack-protector"))) __typeof (__libc_memmove)
> *__libc_memmove_ifunc (void)
>                                                                      ^~
> ~~~~~~~~~~~~~~~~~~

Thanks.  I think the warning is working correctly, but I'm not
sure I completely understand the circuitous definitions or how
they are supposed to work.  They do appear to play tricks that
aren't type safe.

It looks to me like the file declares the __libc_memmove symbol
to have the same type as memmove.  It then defines the
__libc_memmove_ifunc function as an ifunc resolver for memmove
(i.e., a function that returns a pointer to memmove).  But it
also defines __libc_memmove as an alias for __libc_memmove_ifunc
and then memmove as an alias for the redefined __libc_memmove
with __libc_memmove_ifunc's type, and that's what GCC sees and
complains about.

I'm not intimately familiar with the Glibc ifunc infrastructure
to suggest a good solution here, so assuming this works my only
idea is to suppress the warning for these builds.

Joseph, do you have a better suggestion?

Martin

Re: [PATCH] detect incompatible aliases (PR c/81854)

[Steve Ellcey]

> I'm not intimately familiar with the Glibc ifunc infrastructure
> to suggest a good solution here, so assuming this works my only
> idea is to suppress the warning for these builds.
> 
> Joseph, do you have a better suggestion?
> 
> Martin

Now that I know building GCC with '--enable-gnu-indirect-function' lets
me build GLIBC with IFUNCs maybe it is time for GLIBC to require this
instead of just recommending it.  Though I guess that is a discussion
to take over to the libc-alpha mailing list.

From glibc configure.ac:

if test x"$libc_cv_gcc_indirect_function" != xyes &&
   test x"$multi_arch" = xyes; then
  AC_MSG_WARN([--enable-multi-arch support recommends a gcc with gnu-
indirect-function support.
Please use a gcc which supports it by default or configure gcc with --
enable-gnu-indirect-function])
fi



Steve Ellcey
sellcey@cavium.com

Re: [PATCH] detect incompatible aliases (PR c/81854)

[Joseph Myers]

On Wed, 20 Sep 2017, Martin Sebor wrote:

> I'm not intimately familiar with the Glibc ifunc infrastructure
> to suggest a good solution here, so assuming this works my only
> idea is to suppress the warning for these builds.
> 
> Joseph, do you have a better suggestion?

Is the warning because of a declaration of memmove as aliasing 
__libc_memmove being compared with the type of the __libc_memmove_ifunc 
declaration (asm name __libc_memmove) rather than the __libc_memmove 
declaration?  If so, maybe in the non-HAVE_GCC_IFUNC case the alias should 
be declared with a different type?  Or should be defined inside asm in 
this case (presumably with new ifunc-related macros)?

(It may nevertheless be a good idea to set 
default_gnu_indirect_function=yes for AArch64 configurations in GCC that 
use glibc.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] detect incompatible aliases (PR c/81854)

[Joseph Myers]

[-- Attachment #1: Type: text/plain, Size: 1172 bytes --]

On Wed, 20 Sep 2017, Steve Ellcey wrote:

> > I'm not intimately familiar with the Glibc ifunc infrastructure
> > to suggest a good solution here, so assuming this works my only
> > idea is to suppress the warning for these builds.
> > 
> > Joseph, do you have a better suggestion?
> > 
> > Martin
> 
> Now that I know building GCC with '--enable-gnu-indirect-function' lets
> me build GLIBC with IFUNCs maybe it is time for GLIBC to require this
> instead of just recommending it.  Though I guess that is a discussion
> to take over to the libc-alpha mailing list.

We should not break builds with default GCC configurations, for any GCC 
version supported for building glibc.  The point to require that option is 
the point where all GCC versions supported for building glibc default to 
that option on all architectures for which IFUNCs are used in glibc (and 
even then you'd have to allow for the case of --enable-multi-arch 
defaulting to on because new binutils has IFUNC support and not break 
builds with GCC predating that binutils version - really, the 
architecture-specific default in GCC is very questionable).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] detect incompatible aliases (PR c/81854)

[Steve Ellcey]

On Wed, 2017-09-20 at 18:01 +0000, Joseph Myers wrote:
> 
> (It may nevertheless be a good idea to set 
> default_gnu_indirect_function=yes for AArch64 configurations in GCC
> that 
> use glibc.)
> 

I have submitted a patch for that.

https://gcc.gnu.org/ml/gcc-patches/2017-09/msg00285.html

Steve Ellcey
sellcey@cavium.com

Re: [PATCH] detect incompatible aliases (PR c/81854)

[Joseph Myers]

[-- Attachment #1: Type: text/plain, Size: 1493 bytes --]

On Wed, 20 Sep 2017, Steve Ellcey wrote:

> On Wed, 2017-09-20 at 18:01 +0000, Joseph Myers wrote:
> > 
> > (It may nevertheless be a good idea to set 
> > default_gnu_indirect_function=yes for AArch64 configurations in GCC
> > that 
> > use glibc.)
> > 
> 
> I have submitted a patch for that.
> 
> https://gcc.gnu.org/ml/gcc-patches/2017-09/msg00285.html

I'm not entirely convinced the architecture conditions make sense there.

The problem case is: binutils gains support for IFUNCs for an architecture 
that didn't previously have them.  glibc detects the support, but cannot 
use IFUNC attributes with any existing GCC version for that architecture 
because GCC didn't know that binutils would gain that support.  That means 
existing GCC versions unnecessarily cannot build glibc with new binutils 
for that architecture.  (You could make the multi-arch support test check 
for IFUNC attribute support so multi-arch just gets disabled in that case, 
but it would be better to be able to build with existing GCC versions.)

I.e. what I think would be better is: compiler support depends on the 
target OS, and make sure the linker gives a clear error (even better, that 
the assembler does so) if you try to build something with IFUNCs on an 
architecture without support.  (If the linker / assembler don't already 
give such errors you could choose to keep architecture conditionals when 
building GCC with older binutils versions.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] detect incompatible aliases (PR c/81854)

[Joseph Myers]

[-- Attachment #1: Type: text/plain, Size: 890 bytes --]

On Wed, 20 Sep 2017, Steve Ellcey wrote:

> On Wed, 2017-09-20 at 18:01 +0000, Joseph Myers wrote:
> > 
> > (It may nevertheless be a good idea to set 
> > default_gnu_indirect_function=yes for AArch64 configurations in GCC
> > that 
> > use glibc.)
> > 
> 
> I have submitted a patch for that.
> 
> https://gcc.gnu.org/ml/gcc-patches/2017-09/msg00285.html

SPARC glibc build is also broken by the GCC patch.  Presumably SPARC 
should also be added to the architectures with this feature enabled by 
default (in addition to any fix in glibc to allow it to build without 
ifunc attributes).

x86_64 x32 glibc build is also broken by the GCC patch.  In that case, 
it's gettimeofday generated from syscalls.list.  The ifunc support in 
make-syscalls.sh appears only to support the old way of generating IFUNCs, 
not the use of attributes.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] detect incompatible aliases (PR c/81854)

[Martin Sebor]

On 09/20/2017 12:01 PM, Joseph Myers wrote:
> On Wed, 20 Sep 2017, Martin Sebor wrote:
>
>> I'm not intimately familiar with the Glibc ifunc infrastructure
>> to suggest a good solution here, so assuming this works my only
>> idea is to suppress the warning for these builds.
>>
>> Joseph, do you have a better suggestion?
>
> Is the warning because of a declaration of memmove as aliasing
> __libc_memmove being compared with the type of the __libc_memmove_ifunc
> declaration (asm name __libc_memmove) rather than the __libc_memmove
> declaration?  If so, maybe in the non-HAVE_GCC_IFUNC case the alias should
> be declared with a different type?  Or should be defined inside asm in
> this case (presumably with new ifunc-related macros)?

The warning is because in the declaration:

   extern __typeof (__libc_memmove)
   memmove __attribute__ ((alias ("__libc_memmove")));

__typeof (__libc_memmove) is the same as the type of memmove
as it should be but the type of the alias ("__libc_memmove") is
that of __libc_memmove_ifunc, presumably because of the following:

   __typeof (__libc_memmove)*
   __libc_memmove_ifunc (void) __asm__ ("__libc_memmove");

I think this is because the type of "__libc_memmove" in the alias
is looked up "late" after the asm above has associated its type
with __libc_memmove_ifunc.

Besides suppressing the warning using a pragma it can also be
suppressed by declaring memove with no prototype (which would
mean changing the definition of the strong_alias macro for this
kind of builds as you suggest).  But this would just paper over
the type mismatch.  I'm afraid I don't know enough about this
machinery to tell what might be an appropriate fix (i.e., what
would declare memove as an alias for __libc_memmove_ifunc but
with the correct type), or even if there is one.

Martin

Re: [PATCH] detect incompatible aliases (PR c/81854)

[Joseph Myers]

On Wed, 20 Sep 2017, Martin Sebor wrote:

> The warning is because in the declaration:
> 
>   extern __typeof (__libc_memmove)
>   memmove __attribute__ ((alias ("__libc_memmove")));
> 
> __typeof (__libc_memmove) is the same as the type of memmove
> as it should be but the type of the alias ("__libc_memmove") is
> that of __libc_memmove_ifunc, presumably because of the following:
> 
>   __typeof (__libc_memmove)*
>   __libc_memmove_ifunc (void) __asm__ ("__libc_memmove");
> 
> I think this is because the type of "__libc_memmove" in the alias
> is looked up "late" after the asm above has associated its type
> with __libc_memmove_ifunc.
> 
> Besides suppressing the warning using a pragma it can also be
> suppressed by declaring memove with no prototype (which would
> mean changing the definition of the strong_alias macro for this
> kind of builds as you suggest).  But this would just paper over
> the type mismatch.  I'm afraid I don't know enough about this
> machinery to tell what might be an appropriate fix (i.e., what
> would declare memove as an alias for __libc_memmove_ifunc but
> with the correct type), or even if there is one.

I think in the case of IFUNCs defined without the attribute, a symbol 
inherently has two incompatible types, one at C level and one at assembler 
level.  So disabling this particular warning for the combination of 
(multi-arch, no IFUNC attribute) may make sense - but maybe it would be 
better to enable the IFUNC attribute for all GNU/Linux targets (without 
regard to whether the assembler and linker support the feature, but 
arranging as needed for the assembler to give an error if support is 
missing).  We do *not* however want to disable any other warnings relating 
to attributes.

The x32 case should be fixed by using the common ifunc macros in 
make-syscalls.sh.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] detect incompatible aliases (PR c/81854)

[Steve Ellcey]

On Wed, 2017-09-20 at 08:37 -0700, Steve Ellcey wrote:
> On Tue, 2017-09-19 at 09:16 -0600, Martin Sebor wrote:
> > On 09/18/2017 03:44 PM, Joseph Myers wrote:
> > > On Mon, 18 Sep 2017, Martin Sebor wrote:
> > > > It's meant as an escape hatch.  It allows declaring
> > > > compatibility
> > > > symbols, for example by the libstdc++ _GLIBCXX_3_4_SYMVER macro
> > > > defined in libstdc++-v3/src/c++98/compatibility.cc.  The macro
> > > > is
> > > > used to declare compatibility functions of all sorts of
> > > > incompatible
> > > > types.  The originally posted patch had libstdc++ disable the
> > > > warning
> > > > for the file with the symbols but Jonathan preferred this
> > > > solution.
> > > > 
> > > > It could perhaps be tightened up to detect some of the cases on
> > > > your
> > > > list but I'm not sure it's worth the effort and added
> > > > complexity.
> > > > Let me know if you feel differently (or have a different
> > > > suggestion),
> > > > otherwise I will go ahead and commit the patch as is.
> > > Please add a comment explaining this reasoning and commit the
> > > patch.
> > Done in r252976.
> > 
> > Thanks
> > Martin
> This patch is causing my gcc/glibc ToT build to fail on aarch64.  I am
> not sure if everything should be working at this point or not or if
> there is more that needs to be done.  The problem is with the memcpy,
> memmove, etc. ifuncs on aarch64.
> 
> Steve Ellcey
> sellcey@cavium.com

Martin,

I think there is more fallout from this patch.  The libatomic library
can use ifuncs and right now it is not working on aarch64 (testing a
proposed patch I sent) because the ifunc check fails due to the new
warnings.  I believe this can be reproduced with ToT on x86 or arm as
they use ifuncs in the checked in sources but I have not tried that
yet.  Note that the build does not fail, it is just that the check for
ifunc support fails and thus the library is built without them.

Steve Ellcey
sellcey@cavium.com

configure:14698: checking whether the target supports the ifunc attribute
configure:14720: /home/sellcey/gcc-libatomic/obj/gcc/./gcc/xgcc
-B/home/sellcey/gcc-libatomic/obj/gcc/./gcc/ -B/home/sellcey/gcc-
libatomic/install/aarch64-unknown-linux-gnu/bin/ -B/home/sellcey/gcc-
libatomic/install/aarch64-unknown-linux-gnu/lib/ -isystem
/home/sellcey/gcc-libatomic/install/aarch64-unknown-linux-gnu/include
-isystem /home/sellcey/gcc-libatomic/install/aarch64-unknown-linux-
gnu/sys-include    -o conftest -g -O2  -pthread
-Werror   conftest.c  >&5
conftest.c:68:9: error: 'foo' 'ifunc' resolver should return a function
pointer [-Werror=attributes]
     int foo(void) __attribute__((ifunc("foo_sel")));
         ^~~
conftest.c:67:11: note: resolver declaration here
     void *foo_sel(void) { return foo_alt; }
           ^~~~~~~
cc1: all warnings being treated as errors
configure:14720: $? = 1

Re: [PATCH] detect incompatible aliases (PR c/81854)

[Joseph Myers]

[-- Attachment #1: Type: text/plain, Size: 908 bytes --]

On Mon, 2 Oct 2017, Steve Ellcey wrote:

> I think there is more fallout from this patch.  The libatomic library
> can use ifuncs and right now it is not working on aarch64 (testing a
> proposed patch I sent) because the ifunc check fails due to the new
> warnings.  I believe this can be reproduced with ToT on x86 or arm as
> they use ifuncs in the checked in sources but I have not tried that
> yet.  Note that the build does not fail, it is just that the check for
> ifunc support fails and thus the library is built without them.

In the ARM case we don't enable ifunc attribute support by default.  I 
think we *should* add arm*-* to that case statement to enable it by 
default (for GNU/Linux targets); it just so happens the absence for ARM 
doesn't break building glibc because all IFUNCs in glibc on ARM are 
defined in .S files, not in C code.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] detect incompatible aliases (PR c/81854)

[Martin Sebor]

On 10/02/2017 01:49 PM, Steve Ellcey wrote:
> On Wed, 2017-09-20 at 08:37 -0700, Steve Ellcey wrote:
>> On Tue, 2017-09-19 at 09:16 -0600, Martin Sebor wrote:
>>> On 09/18/2017 03:44 PM, Joseph Myers wrote:
>>>> On Mon, 18 Sep 2017, Martin Sebor wrote:
>>>>> It's meant as an escape hatch.  It allows declaring
>>>>> compatibility
>>>>> symbols, for example by the libstdc++ _GLIBCXX_3_4_SYMVER macro
>>>>> defined in libstdc++-v3/src/c++98/compatibility.cc.  The macro
>>>>> is
>>>>> used to declare compatibility functions of all sorts of
>>>>> incompatible
>>>>> types.  The originally posted patch had libstdc++ disable the
>>>>> warning
>>>>> for the file with the symbols but Jonathan preferred this
>>>>> solution.
>>>>>
>>>>> It could perhaps be tightened up to detect some of the cases on
>>>>> your
>>>>> list but I'm not sure it's worth the effort and added
>>>>> complexity.
>>>>> Let me know if you feel differently (or have a different
>>>>> suggestion),
>>>>> otherwise I will go ahead and commit the patch as is.
>>>> Please add a comment explaining this reasoning and commit the
>>>> patch.
>>> Done in r252976.
>>>
>>> Thanks
>>> Martin
>> This patch is causing my gcc/glibc ToT build to fail on aarch64.  I am
>> not sure if everything should be working at this point or not or if
>> there is more that needs to be done.  The problem is with the memcpy,
>> memmove, etc. ifuncs on aarch64.
>>
>> Steve Ellcey
>> sellcey@cavium.com
>
> Martin,
>
> I think there is more fallout from this patch.  The libatomic library
> can use ifuncs and right now it is not working on aarch64 (testing a
> proposed patch I sent) because the ifunc check fails due to the new
> warnings.  I believe this can be reproduced with ToT on x86 or arm as
> they use ifuncs in the checked in sources but I have not tried that
> yet.  Note that the build does not fail, it is just that the check for
> ifunc support fails and thus the library is built without them.
>
> Steve Ellcey
> sellcey@cavium.com
>
> configure:14698: checking whether the target supports the ifunc attribute
> configure:14720: /home/sellcey/gcc-libatomic/obj/gcc/./gcc/xgcc
> -B/home/sellcey/gcc-libatomic/obj/gcc/./gcc/ -B/home/sellcey/gcc-
> libatomic/install/aarch64-unknown-linux-gnu/bin/ -B/home/sellcey/gcc-
> libatomic/install/aarch64-unknown-linux-gnu/lib/ -isystem
> /home/sellcey/gcc-libatomic/install/aarch64-unknown-linux-gnu/include
> -isystem /home/sellcey/gcc-libatomic/install/aarch64-unknown-linux-
> gnu/sys-include    -o conftest -g -O2  -pthread
> -Werror   conftest.c  >&5
> conftest.c:68:9: error: 'foo' 'ifunc' resolver should return a function
> pointer [-Werror=attributes]
>      int foo(void) __attribute__((ifunc("foo_sel")));
>          ^~~
> conftest.c:67:11: note: resolver declaration here
>      void *foo_sel(void) { return foo_alt; }
>            ^~~~~~~
> cc1: all warnings being treated as errors
> configure:14720: $? = 1

Looks like the libatomic/configure script needs to be updated
to have the ifunc test either declare the resolver to return
a function pointer or avoid using -Werror.  I assume the below
is the preferred fix but I need to get the right version of
autoconf etc. to regenerate configure.  Let me work on that
and submit a patch.

Martin

diff --git a/libatomic/acinclude.m4 b/libatomic/acinclude.m4
index 485d731..383218f 100644
--- a/libatomic/acinclude.m4
+++ b/libatomic/acinclude.m4
@@ -195,7 +195,8 @@ AC_DEFUN([LIBAT_CHECK_IFUNC], [
    CFLAGS="$CFLAGS -Werror"
    AC_TRY_LINK([
      int foo_alt(void) { return 0; }
-    void *foo_sel(void) { return foo_alt; }
+    typedef int F (void);
+    F *foo_sel(void) { return foo_alt; }
      int foo(void) __attribute__((ifunc("foo_sel")));],
      [return foo();], libat_cv_have_ifunc=yes, libat_cv_have_ifunc=no)])
    LIBAT_DEFINE_YESNO([HAVE_IFUNC], [$libat_cv_have_ifunc],


Martin

Re: [PATCH] detect incompatible aliases (PR c/81854)

[Andreas Schwab]

On Aug 17 2017, Martin Sebor <msebor@gmail.com> wrote:

> 	* g++.dg/ext/attr-ifunc-1.C: Adjust.
> 	* g++.dg/ext/attr-ifunc-2.C: Same.
> 	* g++.dg/ext/attr-ifunc-3.C: Same.
> 	* g++.dg/ext/attr-ifunc-4.C: Same.

All the tests are now crashing on powerpc.  Klass::resolver doesn't look
like a valid ifunc resolver.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] detect incompatible aliases (PR c/81854)

[Martin Sebor]

On 10/03/2017 03:41 PM, Andreas Schwab wrote:
> On Aug 17 2017, Martin Sebor <msebor@gmail.com> wrote:
>
>> 	* g++.dg/ext/attr-ifunc-1.C: Adjust.
>> 	* g++.dg/ext/attr-ifunc-2.C: Same.
>> 	* g++.dg/ext/attr-ifunc-3.C: Same.
>> 	* g++.dg/ext/attr-ifunc-4.C: Same.
>
> All the tests are now crashing on powerpc.  Klass::resolver doesn't look
> like a valid ifunc resolver.

This was reported in bug 82301.  Please check that bug for the status
of the patch.

Martin