This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PING: PATCH: Automatically test IFUNC implementations


On Tue, Oct 09, 2012 at 01:30:45PM -0700, Roland McGrath wrote:
> > +	* Rules (tests): Filter out $(tests-ifunc) if multi-arch isn't
> > +	enabled.
> > +	(xtests): Filter out $(xtests-ifunc) if multi-arch isn't enabled.
> > +	* include/libc-func.h: New file.
> > +	* misc/libc-func.c: Likewise.
> > +	* misc/Makefile (routines): Add libc-func if multi-arch is
> > +	enabled.
> 
> Be technically precise about conditionals:
> 
> 	* Rules [$(multi-arch) = no] (tests): Filter out $(tests-ifunc).
> 
> etc.
> 
> > +	* misc/Versions: Add __libc_supported_implementations to
> > +	GLIBC_PRIVATE
> 
> 	* misc/Versions (GLIBC_PRIVATE): Add __libc_supported_implementations.
> 
> > +	* string/test-string.h: Include <libc-func.h>.
> > +	(func_list): New.  Defined only if TEST_IFUNC and TEST_NAME are
> > +	defined.
> > +	(func_count): Likewise.
> > +	(impl_count): Likewise.
> > +	(impl_array): Likewise.
> 
> New what?!?!  And we have a convention for conditionals.  Use it!
> 
> 	[TEST_IFUNC && TEST_NAME] (func_list, func_count): New variables.
> 
> etc.
> 
> > +++ b/include/libc-func.h
> 
> This is a pretty poor name given the function's renaming.
> Since this purely for IFUNC cases, lets have ifunc in the name.
> How about libc-ifunc-impl-list.h, struct libc_ifunc_impl, and
> __libc_ifunc_impl_list (ifunc-impl-list.c)?
> 
> > +ifneq ($(multi-arch),no)
> > +routines   += libc-func
> > +endif
> 
> Let's just make it unconditional and let the trivial stub version be built
> when multiarch is not enabled.
> 
> > +/* Default __libc_supported_implementations.
> 
> /* Enumerate available IFUNC implementations of a function.  Stub version.
> 
> > +/* Fill ARRAY of MAX elements with IFUNC implementations for function
> > +   NAME supported on target machine and return the number of valid
> 
> "on this machine"
> 
> This whole scheme doesn't include any way to notice which or how many
> implementations you aren't testing because your machine is incompatbile.
> 
> I'm a bit inclined to make the struct include a "bool usable" field, have
> the array always filled with all function pointers, and let the testers
> skip the ones with usable==false.  Then the testers can emit output saying
> "there were N variants of foo I could not test".  Eventually we can feed
> this into some more full scheme to ensure that all the variants have really
> been tested well.
> > +	  for (impl = __start_impls; impl < __stop_impls; ++impl)	\
> > +	    impl_count++;						\
> 
> A loop is a lousy way to calculate a subtraction.
> 
> > +#if defined TEST_IFUNC && defined TEST_NAME
> > +# define SIZEOF_ARRAY(array) (sizeof array / sizeof array[0])
> > +  func_count = __libc_supported_implementations (TEST_NAME, func_list,
> > +						 SIZEOF_ARRAY (func_list));
> 
> Don't define a macro to use it only once.

Here is the updated patch.  OK to install?

Thanks.


H.J.
---
 ChangeLog                 | 16 +++++++++++++
 Rules                     |  5 ++++
 include/ifunc-impl-list.h | 61 +++++++++++++++++++++++++++++++++++++++++++++++
 misc/Makefile             |  2 +-
 misc/Versions             |  1 +
 misc/ifunc-impl-list.c    | 32 +++++++++++++++++++++++++
 string/test-string.h      | 58 +++++++++++++++++++++++++++++++++++++++++++-
 7 files changed, 173 insertions(+), 2 deletions(-)
 create mode 100644 include/ifunc-impl-list.h
 create mode 100644 misc/ifunc-impl-list.c

	* Rules [$(multi-arch) = no] (tests): Filter out $(tests-ifunc).
	[$(multi-arch) = no] (xtests): Filter out $(xtests-ifunc).
	* include/ifunc-impl-list.h: New file.
	* misc/ifunc-impl-list.c: Likewise.
	* misc/Makefile (routines): Add ifunc-impl-list.
	* misc/Versions (GLIBC_PRIVATE): __libc_ifunc_impl_list.
	* string/test-string.h: Include <ifunc-impl-list.h>.
	[TEST_IFUNC && TEST_NAME] (func_list, func_count, impl_count,
	impl_array): New variables.
	(FOR_EACH_IMPL): Support func_list if TEST_IFUNC and TEST_NAME
	are defined.
	(test_init): Call __libc_ifunc_impl_list to initialize
	func_list if TEST_IFUNC and TEST_NAME are defined.

diff --git a/Rules b/Rules
index 17d938e..5e33610 100644
--- a/Rules
+++ b/Rules
@@ -84,6 +84,11 @@ common-generated += dummy.o dummy.c
 # This makes all the auxiliary and test programs.
 
 .PHONY: others tests
+ifeq ($(multi-arch),no)
+tests := $(filter-out $(tests-ifunc), $(tests))
+xtests := $(filter-out $(xtests-ifunc), $(xtests))
+endif
+
 ifeq ($(build-programs),yes)
 others: $(addprefix $(objpfx),$(others) $(sysdep-others) $(extra-objs))
 else
diff --git a/include/ifunc-impl-list.h b/include/ifunc-impl-list.h
new file mode 100644
index 0000000..36444d3
--- /dev/null
+++ b/include/ifunc-impl-list.h
@@ -0,0 +1,56 @@
+/* Internal header file for __libc_supported_implementations.
+   Copyright (C) 2012 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef _IFUNC_IMPL_LIST_H
+#define _IFUNC_IMPL_LIST_H	1
+
+#include <stdbool.h>
+#include <stddef.h>
+
+struct libc_ifunc_impl
+{
+  /* The name of function to be tested.  */
+  const char *name;
+  /* The address of function to be tested.  */
+  void (*fn) (void);
+  /* True if this implementation is usable on this machine.  */
+  bool usable;
+};
+
+/* Add an IFUNC implementation, IMPL, for function FUNC, to ARRAY with
+   USABLE at index I and advance I by one.  */
+#define IFUNC_IMPL_ADD(array, i, func, usable, impl) \
+  extern __typeof (func) impl attribute_hidden; \
+  (array)[i++] = (struct libc_ifunc_impl) { #impl, (void (*) (void)) impl, (usable) };
+
+/* Return the number of IFUNC implementations, N, for function FUNC if
+   string NAME matches FUNC.  */
+#define IFUNC_IMPL(n, name, func, ...) \
+  if (strcmp (name, #func) == 0) \
+    { \
+      __VA_ARGS__; \
+      return n; \
+    }
+
+/* Fill ARRAY of MAX elements with IFUNC implementations for function
+   NAME and return the number of valid entries.  */
+extern size_t __libc_ifunc_impl_list (const char *name,
+				      struct libc_ifunc_impl *array,
+				      size_t max);
+
+#endif /* ifunc-impl-list.h */
diff --git a/misc/Makefile b/misc/Makefile
index e5e9b9f..ea68d26 100644
--- a/misc/Makefile
+++ b/misc/Makefile
@@ -65,7 +65,7 @@ routines := brk sbrk sstk ioctl \
 	    getloadavg getclktck \
 	    fgetxattr flistxattr fremovexattr fsetxattr getxattr \
 	    listxattr lgetxattr llistxattr lremovexattr lsetxattr \
-	    removexattr setxattr getauxval
+	    removexattr setxattr getauxval ifunc-impl-list
 
 generated := tst-error1.mtrace tst-error1-mem
 
diff --git a/misc/Versions b/misc/Versions
index b2a9147..64632f0 100644
--- a/misc/Versions
+++ b/misc/Versions
@@ -151,5 +151,6 @@ libc {
   }
   GLIBC_PRIVATE {
     __madvise;
+    __libc_ifunc_impl_list;
   }
 }
diff --git a/misc/ifunc-impl-list.c b/misc/ifunc-impl-list.c
new file mode 100644
index 0000000..c4b460d
--- /dev/null
+++ b/misc/ifunc-impl-list.c
@@ -0,0 +1,32 @@
+/* Enumerate available IFUNC implementations of a function.  Stub version.
+   Copyright (C) 2012 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <ifunc-impl-list.h>
+
+/* Fill ARRAY of MAX elements with IFUNC implementations for function
+   NAME supported on target machine and return the number of valid
+   entries.  */
+
+size_t
+__libc_ifunc_impl_list
+  (const char *name __attribute__ ((unused)),
+   struct libc_ifunc_impl *array __attribute__ ((unused)),
+   size_t max __attribute__ ((unused)))
+{
+  return 0;
+}
diff --git a/string/test-string.h b/string/test-string.h
index c94d822..5da11fb 100644
--- a/string/test-string.h
+++ b/string/test-string.h
@@ -50,6 +50,7 @@ extern impl_t __start_impls[], __stop_impls[];
 #include <error.h>
 #include <errno.h>
 #include <time.h>
+#include <ifunc-impl-list.h>
 #define GL(x) _##x
 #define GLRO(x) _##x
 #include <hp-timing.h>
@@ -106,9 +107,57 @@ size_t iterations = 100000;
 #define CALL(impl, ...)	\
   (* (proto_t) (impl)->fn) (__VA_ARGS__)
 
-#define FOR_EACH_IMPL(impl, notall) \
+#if defined TEST_IFUNC && defined TEST_NAME
+/* Increase size of FUNC_LIST if assert is triggered at run-time.  */
+static struct libc_ifunc_impl func_list[32];
+static int func_count;
+static int impl_count = -1;
+static impl_t *impl_array;
+
+# define FOR_EACH_IMPL(impl, notall) \
+  impl_t *impl;								\
+  int count;								\
+  if (impl_count == -1)							\
+    {									\
+      impl_count = 0;							\
+      if (func_count != 0)						\
+	{								\
+	  int f;							\
+	  impl_t *skip = NULL, *a;					\
+	  for (impl = __start_impls; impl < __stop_impls; ++impl)	\
+	    if (strcmp (impl->name, TEST_NAME) == 0)			\
+	      skip = impl;						\
+	    else							\
+	      impl_count++;						\
+	  a = impl_array = malloc ((impl_count + func_count) *		\
+				   sizeof (impl_t));			\
+	  for (impl = __start_impls; impl < __stop_impls; ++impl)	\
+	    if (impl != skip)						\
+	      *a++ = *impl;						\
+	  for (f = 0; f < func_count; f++)				\
+	    if (func_list[f].usable)					\
+	      {								\
+		a->name = func_list[f].name;				\
+		a->fn = func_list[f].fn;				\
+		a->test = 1;						\
+		a++;							\
+	      }								\
+	  impl_count = a - impl_array;					\
+	}								\
+      else								\
+        {								\
+	  impl_count = __stop_impls - __start_impls;			\
+	  impl_array = __start_impls;					\
+        }								\
+    }									\
+  impl = impl_array;							\
+  for (count = 0; count < impl_count; ++count, ++impl)			\
+    if (!notall || impl->test)
+#else
+# define FOR_EACH_IMPL(impl, notall) \
   for (impl_t *impl = __start_impls; impl < __stop_impls; ++impl)	\
     if (!notall || impl->test)
+#endif
 
 #define HP_TIMING_BEST(best_time, start, end)	\
   do									\
@@ -127,6 +176,13 @@ size_t iterations = 100000;
 static void
 test_init (void)
 {
+#if defined TEST_IFUNC && defined TEST_NAME
+# define SIZEOF_ARRAY(array) (sizeof array / sizeof array[0])
+  func_count = __libc_ifunc_impl_list (TEST_NAME, func_list,
+				       (sizeof func_list
+					/ sizeof func_list[0]));
+#endif
+
   page_size = 2 * getpagesize ();
 #ifdef MIN_PAGE_SIZE
   if (page_size < MIN_PAGE_SIZE)
-- 
1.7.11.4


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]