Re: V5 [PATCH 2/2] x86: Add a LD_PRELOAD IFUNC resolver test for CPU_FEATURE_USABLE

[Florian Weimer <fweimer at redhat dot com>]

* H. J. Lu:

> +static bool
> +ifunc_check_sse2 (void)
> +{
> +  return CPU_FEATURE_USABLE (SSE2);
> +}
> +
> +static bool
> +(*get_check_sse2 (void)) (void)
> +{
> +  return ifunc_check_sse2;
> +}
> +
> +bool check_sse2 (void) __attribute__((ifunc("get_check_sse2")));

The IFUNC resolver does not call CPU_FEATURE_USABLE, so this is not the
test I had in mind.

The patch below fixes this, and adds something that actually violates
the ordering constraints.  With these changes, I get this when building
glibc with --enable-bind-now:

elf/tst-x86-platform-4: Relink `./libc.so.6' with `elf/tst-x86-platform-preload-4.so' for IFUNC symbol `free'
Segmentation fault (core dumped)

I think using CPU_FEATURE_USABLE in a malloc implementation is something
that could be quite natural, so I think this really should be fixed in
some way.

Thanks,
Florian

diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
index 1e759d3efc..773b9137b3 100644
--- a/sysdeps/x86/Makefile
+++ b/sysdeps/x86/Makefile
@@ -22,6 +22,9 @@ $(objpfx)tst-x86-platform-mod-4.so: $(libsupport)
 $(objpfx)tst-x86-platform-4: $(objpfx)tst-x86-platform-mod-4.so
 $(objpfx)tst-x86-platform-4.out: $(objpfx)tst-x86-platform-preload-4.so
 tst-x86-platform-4-ENV = LD_PRELOAD=$(objpfx)tst-x86-platform-preload-4.so
+LDFLAGS-tst-x86-platform-preload-4.so = -Wl,-z,now
+LDFLAGS-tst-x86-platform-4.so = -Wl,-z,now
+LDFLAGS-tst-x86-platform-4 = -Wl,-z,now
 endif
 
 ifeq ($(subdir),misc)
diff --git a/sysdeps/x86/tst-x86-platform-4.c b/sysdeps/x86/tst-x86-platform-4.c
index fe7bd59b82..fb4e86b566 100644
--- a/sysdeps/x86/tst-x86-platform-4.c
+++ b/sysdeps/x86/tst-x86-platform-4.c
@@ -16,14 +16,18 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#include <stdlib.h>
-#include <stdio.h>
 #include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <support/check.h>
+#include <support/xstdio.h>
 #include <sys/platform/x86.h>
 
 extern bool check_sse2 (void);
 extern bool check_avx (void);
 extern bool check_avx2 (void);
+extern int get_expected_free_variant (void);
+extern volatile int free_variant_called;
 
 static int
 do_test (void)
@@ -48,6 +52,22 @@ do_test (void)
       failed = true;
     }
 
+  /* Determine if the correct free function was selected.  */
+  int expected_variant = get_expected_free_variant ();
+  free (NULL);
+  TEST_COMPARE (expected_variant, free_variant_called);
+
+  /* Same for a free within libc.so.6.  */
+  {
+    FILE * fp = xfopen ("/dev/null", "r");
+    free_variant_called = 0;
+    xfclose (fp);
+    if (free_variant_called == 0)
+      puts ("warning: fclose did not call free");
+    else
+      TEST_COMPARE (expected_variant, free_variant_called);
+  }
+
   return failed ? EXIT_FAILURE : EXIT_SUCCESS;
 }
 
diff --git a/sysdeps/x86/tst-x86-platform-mod-4.c b/sysdeps/x86/tst-x86-platform-mod-4.c
index 789389ce3e..031634c615 100644
--- a/sysdeps/x86/tst-x86-platform-mod-4.c
+++ b/sysdeps/x86/tst-x86-platform-mod-4.c
@@ -35,3 +35,11 @@ check_avx2 (void)
 {
   return false;
 }
+
+int
+get_expected_free_variant (void)
+{
+  return -1;
+}
+
+volatile int free_variant_called;
diff --git a/sysdeps/x86/tst-x86-platform-preload-4.c b/sysdeps/x86/tst-x86-platform-preload-4.c
index d1216b90e9..6cb5b82377 100644
--- a/sysdeps/x86/tst-x86-platform-preload-4.c
+++ b/sysdeps/x86/tst-x86-platform-preload-4.c
@@ -18,45 +18,118 @@
 
 #include <stdbool.h>
 #include <sys/platform/x86.h>
+#include <stdlib.h>
 
 static bool
-ifunc_check_sse2 (void)
+true_function (void)
 {
-  return CPU_FEATURE_USABLE (SSE2);
+  return true;
 }
 
 static bool
-(*get_check_sse2 (void)) (void)
+false_function (void)
 {
-  return ifunc_check_sse2;
+  return false;
 }
 
-bool check_sse2 (void) __attribute__((ifunc("get_check_sse2")));
-
 static bool
-ifunc_check_avx (void)
+(*get_check_sse2 (void)) (void)
 {
-  return CPU_FEATURE_USABLE (AVX);
+  if (CPU_FEATURE_USABLE (SSE2))
+    return true_function;
+  else
+    return false_function;
 }
 
+bool check_sse2 (void) __attribute__ ((ifunc ("get_check_sse2")));
+
 static bool
 (*get_check_avx (void)) (void)
 {
-  return ifunc_check_avx;
+  if (CPU_FEATURE_USABLE (AVX))
+    return true_function;
+  else
+    return false_function;
 }
 
-bool check_avx (void) __attribute__((ifunc("get_check_avx")));
+bool check_avx (void) __attribute__ ((ifunc ("get_check_avx")));
 
 static bool
-ifunc_check_avx2 (void)
+(*get_check_avx2 (void)) (void)
 {
-  return CPU_FEATURE_USABLE (AVX2);
+  if (CPU_FEATURE_USABLE (AVX2))
+    return true_function;
+  else
+    return false_function;
 }
 
-static bool
-(*get_check_avx2 (void)) (void)
+bool check_avx2 (void) __attribute__ ((ifunc ("get_check_avx2")));
+
+/* The following is used to check what happens when the free function
+   in libc.so.6 is interposed.  This implementation simply does not
+   free any memory, to avoid the need for a complete malloc.  It
+   records the variant called in free_variant_called, so that it is
+   possible to check the selected implementation by calling the free
+   function.  */
+
+volatile int free_variant_called;
+
+void
+free_fallback (void *ignored)
+{
+  free_variant_called = 1;
+}
+
+void
+free_with_see (void *ignored)
+{
+  free_variant_called = 2;
+}
+
+void
+free_with_avx (void *ignored)
+{
+  free_variant_called = 3;
+}
+
+void
+free_with_avx2 (void *ignored)
+{
+  free_variant_called = 4;
+}
+
+void
+free_with_rtm (void *ignored)
+{
+  free_variant_called = 5;
+}
+
+int
+get_expected_free_variant (void)
+{
+  if (CPU_FEATURE_USABLE (RTM))
+    return 5;
+  if (CPU_FEATURE_USABLE (AVX2))
+    return 4;
+  if (CPU_FEATURE_USABLE (AVX))
+    return 3;
+  if (CPU_FEATURE_USABLE (SSE))
+    return 2;
+  return 1;
+}
+
+static __typeof__ (free) *
+get_free (void)
 {
-  return ifunc_check_avx2;
+  if (CPU_FEATURE_USABLE (RTM))
+    return free_with_rtm;
+  if (CPU_FEATURE_USABLE (AVX2))
+    return free_with_avx2;
+  if (CPU_FEATURE_USABLE (AVX))
+    return free_with_avx;
+  if (CPU_FEATURE_USABLE (SSE))
+    return free_with_see;
+  return free_fallback;
 }
 
-bool check_avx2 (void) __attribute__((ifunc("get_check_avx2")));
+void free (void *) __attribute__ ((ifunc ("get_free")));