Re: [PATCH] Make __get_cpu_features hidden

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Fri, Dec 14, 2012 at 10:40 AM, H.J. Lu <hongjiu.lu@intel.com> wrote:
>
> Hi,
>
> As shown in
>
> http://www.sourceware.org/bugzilla/show_bug.cgi?id=14955
>
> we can't call the external __get_cpu_features while relocating a DSO.
> This patch makes __get_cpu_features hidden and includes it all shared
> objects which use it.  Currently only libc and libm use __get_cpu_features.
> OK for 2.18?
>
>

The issue is similar to

http://sourceware.org/bugzilla/show_bug.cgi?id=13302

In this case, relocations in DSO are

elocation section '.rela.dyn' at offset 0x460 contains 9 entries:
  Offset          Info           Type           Sym. Value    Sym. Name + Addend
0000002007f0  000000000008 R_X86_64_RELATIVE                    6a0
0000002007f8  000000000008 R_X86_64_RELATIVE                    660
000000200808  000000000008 R_X86_64_RELATIVE                    200808
0000002009d0  000200000006 R_X86_64_GLOB_DAT 0000000000000000
_ITM_deregisterTMClone + 0
0000002009d8  000400000006 R_X86_64_GLOB_DAT 0000000000000000 __gmon_start__ + 0
0000002009e0  000500000006 R_X86_64_GLOB_DAT 0000000000000000
_Jv_RegisterClasses + 0
0000002009e8  000600000006 R_X86_64_GLOB_DAT 0000000000000000
_ITM_registerTMCloneTa + 0
0000002009f0  000700000006 R_X86_64_GLOB_DAT 0000000000000000 __cxa_finalize + 0
000000200a28  000000000025 R_X86_64_IRELATIV                    6dc

Relocation section '.rela.plt' at offset 0x538 contains 3 entries:
  Offset          Info           Type           Sym. Value    Sym. Name + Addend
000000200a10  000300000007 R_X86_64_JUMP_SLO 0000000000000000
__get_cpu_features + 0
000000200a20  000700000007 R_X86_64_JUMP_SLO 0000000000000000 __cxa_finalize + 0
000000200a18  000000000025 R_X86_64_IRELATIV                    6dc

We have an X86_64_IRELATIVE in .rela.dyn before PLT is set up by .rela.plt.
This updated patch adds static-init-arch.oS to libc_nonshared.a so that
__get_cpu_features can be used in other shared libraries.  OK to install?


-- 
H.J.
---
2013-01-03  H.J. Lu  <hongjiu.lu@intel.com>

	[BZ #14955]
	* include/libc-symbols.h (libc_ifunc): Remove INIT_ARCH.
	(libm_ifunc): Defined to libc_ifunc.
	* sysdeps/i386/i686/multiarch/Makefile (sysdep_routines): Add
	static-init-arch.
	(static-only-routines): Likewise.
	* sysdeps/x86_64/multiarch/Makefile (sysdep_routines): Likewise.
	(static-only-routines): Likewise.
	* sysdeps/i386/i686/multiarch/Versions: Removed.
	* sysdeps/x86_64/multiarch/Versions: Likewise.
	* sysdeps/i386/i686/multiarch/static-init-arch.c: New file.
	* sysdeps/x86_64/multiarch/static-init-arch.c: Likewise.
	* sysdeps/x86/Makefile (tests): Add tst-ifunc-cpu1-main.
	(modules-names): Add tst-ifunc-cpu1-mod.
	(tst-ifunc-cpu1-mod.so-no-z-defs): New macro.
	($(objpfx)tst-ifunc-cpu1-main): New dependency.
	* sysdeps/x86/tst-ifunc-cpu1-main.c: New file.
	* sysdeps/x86/tst-ifunc-cpu1-mod.c: Likewise.
	* sysdeps/x86_64/multiarch/init-arch.c (__always_inline): Defined
	to empty.
	(__get_cpu_features): Removed.
	* sysdeps/x86_64/multiarch/init-arch.h (INIT_ARCH): Removed.
	(__get_cpu_features): Defined as an inline function.

diff --git a/include/libc-symbols.h b/include/libc-symbols.h
index 0608320..3e4c5ab 100644
--- a/include/libc-symbols.h
+++ b/include/libc-symbols.h
@@ -751,22 +751,12 @@ for linking")
   extern void *name##_ifunc (void) __asm__ (#name);			\
   void *name##_ifunc (void)						\
   {									\
-    INIT_ARCH ();							\
     __typeof (name) *res = expr;					\
     return res;								\
   }									\
   __asm__ (".type " #name ", %gnu_indirect_function");

-/* The body of the function is supposed to use __get_cpu_features
-   which will, if necessary, initialize the data first.  */
-#define libm_ifunc(name, expr)						\
-  extern void *name##_ifunc (void) __asm__ (#name);			\
-  void *name##_ifunc (void)						\
-  {									\
-    __typeof (name) *res = expr;					\
-    return res;								\
-  }									\
-  __asm__ (".type " #name ", %gnu_indirect_function");
+#define libm_ifunc(name, expr) libc_ifunc (name, (expr))

 #ifdef HAVE_ASM_SET_DIRECTIVE
 # define libc_ifunc_hidden_def1(local, name)				\
diff --git a/sysdeps/i386/i686/multiarch/Makefile
b/sysdeps/i386/i686/multiarch/Makefile
index 8946bfa..e3c9724 100644
--- a/sysdeps/i386/i686/multiarch/Makefile
+++ b/sysdeps/i386/i686/multiarch/Makefile
@@ -1,5 +1,7 @@
 ifeq ($(subdir),csu)
 aux += init-arch
+sysdep_routines += static-init-arch
+static-only-routines += static-init-arch
 tests += test-multiarch
 gen-as-const-headers += ifunc-defines.sym
 endif
diff --git a/sysdeps/i386/i686/multiarch/Versions
b/sysdeps/i386/i686/multiarch/Versions
deleted file mode 100644
index 59b185a..0000000
--- a/sysdeps/i386/i686/multiarch/Versions
+++ /dev/null
@@ -1,5 +0,0 @@
-libc {
-  GLIBC_PRIVATE {
-    __get_cpu_features;
-  }
-}
diff --git a/sysdeps/i386/i686/multiarch/static-init-arch.c
b/sysdeps/i386/i686/multiarch/static-init-arch.c
new file mode 100644
index 0000000..6d8100d
--- /dev/null
+++ b/sysdeps/i386/i686/multiarch/static-init-arch.c
@@ -0,0 +1 @@
+#include <sysdeps/x86_64/multiarch/static-init-arch.c>
diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
index 3e87a77..79a4406 100644
--- a/sysdeps/x86/Makefile
+++ b/sysdeps/x86/Makefile
@@ -6,4 +6,13 @@ tests: $(objpfx)tst-xmmymm.out
 $(objpfx)tst-xmmymm.out: ../sysdeps/x86/tst-xmmymm.sh $(objpfx)ld.so
 	@echo "Checking ld.so for SSE register use.  This will take a few seconds..."
 	$(SHELL) $< $(objpfx) '$(NM)' '$(OBJDUMP)' '$(READELF)' > $@
+
+ifeq (yesyes,$(build-shared)$(multi-arch))
+tests += tst-ifunc-cpu1-main
+modules-names += tst-ifunc-cpu1-mod
+
+tst-ifunc-cpu1-mod.so-no-z-defs = yes
+
+$(objpfx)tst-ifunc-cpu1-main: $(objpfx)tst-ifunc-cpu1-mod.so
+endif
 endif
diff --git a/sysdeps/x86/tst-ifunc-cpu1-main.c
b/sysdeps/x86/tst-ifunc-cpu1-main.c
new file mode 100644
index 0000000..37c16e2
--- /dev/null
+++ b/sysdeps/x86/tst-ifunc-cpu1-main.c
@@ -0,0 +1,10 @@
+/* Test function pointer to local STT_GNU_IFUNC function.  */
+
+extern void (*foo_ptr) (void);
+
+int
+main (void)
+{
+  foo_ptr ();
+  return 0;
+}
diff --git a/sysdeps/x86/tst-ifunc-cpu1-mod.c b/sysdeps/x86/tst-ifunc-cpu1-mod.c
new file mode 100644
index 0000000..08ff30f
--- /dev/null
+++ b/sysdeps/x86/tst-ifunc-cpu1-mod.c
@@ -0,0 +1,24 @@
+#include <init-arch.h>
+
+static void
+one (void)
+{
+}
+
+static void
+two (void)
+{
+}
+
+void * foo_ifunc (void) __asm__ ("foo") attribute_hidden;
+__asm__(".type foo, %gnu_indirect_function");
+
+void *
+foo_ifunc (void)
+{
+  const struct cpu_features * cpu = __get_cpu_features ();
+  return cpu->max_cpuid > 1 ? two : one;
+}
+
+extern void foo (void) attribute_hidden;
+void (*foo_ptr) (void) = foo;
diff --git a/sysdeps/x86_64/multiarch/Makefile
b/sysdeps/x86_64/multiarch/Makefile
index dd6c27d..5a91a39 100644
--- a/sysdeps/x86_64/multiarch/Makefile
+++ b/sysdeps/x86_64/multiarch/Makefile
@@ -1,5 +1,7 @@
 ifeq ($(subdir),csu)
 aux += init-arch
+sysdep_routines += static-init-arch
+static-only-routines += static-init-arch
 tests += test-multiarch
 gen-as-const-headers += ifunc-defines.sym
 endif
diff --git a/sysdeps/x86_64/multiarch/Versions
b/sysdeps/x86_64/multiarch/Versions
deleted file mode 100644
index 59b185a..0000000
--- a/sysdeps/x86_64/multiarch/Versions
+++ /dev/null
@@ -1,5 +0,0 @@
-libc {
-  GLIBC_PRIVATE {
-    __get_cpu_features;
-  }
-}
diff --git a/sysdeps/x86_64/multiarch/init-arch.c
b/sysdeps/x86_64/multiarch/init-arch.c
index 992cbfb..9ee968e 100644
--- a/sysdeps/x86_64/multiarch/init-arch.c
+++ b/sysdeps/x86_64/multiarch/init-arch.c
@@ -19,6 +19,9 @@

 #include <atomic.h>
 #include <cpuid.h>
+/* Define __get_cpu_features.  */
+#undef __always_inline
+#define __always_inline
 #include "init-arch.h"


@@ -177,14 +180,3 @@ __init_cpu_features (void)
   atomic_write_barrier ();
   __cpu_features.kind = kind;
 }
-
-#undef __get_cpu_features
-
-const struct cpu_features *
-__get_cpu_features (void)
-{
-  if (__cpu_features.kind == arch_kind_unknown)
-    __init_cpu_features ();
-
-  return &__cpu_features;
-}
diff --git a/sysdeps/x86_64/multiarch/init-arch.h
b/sysdeps/x86_64/multiarch/init-arch.h
index 0aece18..cba10cf 100644
--- a/sysdeps/x86_64/multiarch/init-arch.h
+++ b/sysdeps/x86_64/multiarch/init-arch.h
@@ -109,19 +109,18 @@ extern struct cpu_features


 extern void __init_cpu_features (void) attribute_hidden;
-# define INIT_ARCH() \
-  do							\
-    if (__cpu_features.kind == arch_kind_unknown)	\
-      __init_cpu_features ();				\
-  while (0)
-
-/* Used from outside libc.so to get access to the CPU features structure.  */
-extern const struct cpu_features *__get_cpu_features (void)
-     __attribute__ ((const));
-
-# ifndef NOT_IN_libc
-#  define __get_cpu_features()	(&__cpu_features)
-# endif
+
+/* Implicitly call __init_cpu_features before accessing the CPU features
+   structure.  */
+
+extern __always_inline attribute_hidden
+const struct cpu_features *
+__get_cpu_features (void)
+{
+  if (__cpu_features.kind == arch_kind_unknown)
+    __init_cpu_features ();
+  return &__cpu_features;
+}

 # define HAS_CPU_FEATURE(idx, reg, bit) \
   ((__get_cpu_features ()->cpuid[idx].reg & (bit)) != 0)
diff --git a/sysdeps/x86_64/multiarch/static-init-arch.c
b/sysdeps/x86_64/multiarch/static-init-arch.c
new file mode 100644
index 0000000..a82d978
--- /dev/null
+++ b/sysdeps/x86_64/multiarch/static-init-arch.c
@@ -0,0 +1,3 @@
+#ifdef LIBC_NONSHARED
+# include "init-arch.c"
+#endif
--