Properly check __cpu_mask size [BZ #21696]

Message ID 20170701011435.GA24085@gmail.com
State New, archived
Headers

Commit Message

H.J. Lu July 1, 2017, 1:14 a.m. UTC
  posix/sched_cpucount.c assumes that size of __cpu_mask == size of long,
which is incorrect for x32.  This patch generates size of __cpu_mask and
uses it in posix/sched_cpucount.c.

Tested on i686, x86-64 and x32 with multi-arch disabled.

OK for master?

H.J.
	[BZ #21696]
	* posix/Makefile (gen-as-const-headers): New.
	* posix/cpu_mask.sym: New file.
	* posix/sched_cpucount.c: Include <cpu_mask.h>.
	(__sched_cpucount): Check CPU_MASK_SIZE instead of LONG_BIT.
	Replace the ul suffix with the ull suffix for 64-bit cpu_mask.
---
 posix/Makefile         |  2 ++
 posix/cpu_mask.sym     |  5 +++++
 posix/sched_cpucount.c | 15 ++++++++-------
 3 files changed, 15 insertions(+), 7 deletions(-)
 create mode 100644 posix/cpu_mask.sym
  

Comments

Florian Weimer July 1, 2017, 6:33 a.m. UTC | #1
On 07/01/2017 03:14 AM, H.J. Lu wrote:
> posix/sched_cpucount.c assumes that size of __cpu_mask == size of long,
> which is incorrect for x32.  This patch generates size of __cpu_mask and
> uses it in posix/sched_cpucount.c.
> 
> Tested on i686, x86-64 and x32 with multi-arch disabled.
> 
> OK for master?
> 
> H.J.
> 	[BZ #21696]
> 	* posix/Makefile (gen-as-const-headers): New.
> 	* posix/cpu_mask.sym: New file.
> 	* posix/sched_cpucount.c: Include <cpu_mask.h>.
> 	(__sched_cpucount): Check CPU_MASK_SIZE instead of LONG_BIT.
> 	Replace the ul suffix with the ull suffix for 64-bit cpu_mask.

Does one of the existing test cases fail reliably due to this?

> +gen-as-const-headers = cpu_mask.sym

This is not needed if you use a regular #if and not a preprocessor
conditional.

  _Static_assert (sizeof (l) == sizeof (unsigned int)
                 || sizeof (l) == sizeof (unsigned long)
		 || sizeof (l) == sizeof (unsigned long long),
                  "sizeof (__cpu_mask"));
  if (sizeof (__cpu_mask) == sizeof (unsigned int))
    s += __builtin_popcount (l);
  else if (sizeof (__cpu_mask) == sizeof (unsigned long))
    s += __builtin_popcountl (l);
  else
    s += __builtin_popcountll (l);

Untested, but __builtin_popcount… has fallback emulation GCC, so this
work.  (And GCC 4.8 already has it.)

Thanks,
Florian
  

Patch

diff --git a/posix/Makefile b/posix/Makefile
index 1c328b8..963cba8 100644
--- a/posix/Makefile
+++ b/posix/Makefile
@@ -144,6 +144,8 @@  tests-special += $(objpfx)bug-regex2-mem.out $(objpfx)bug-regex14-mem.out \
 xtests-special += $(objpfx)bug-ga2-mem.out
 endif
 
+gen-as-const-headers = cpu_mask.sym
+
 include ../Rules
 
 ifeq ($(run-built-tests),yes)
diff --git a/posix/cpu_mask.sym b/posix/cpu_mask.sym
new file mode 100644
index 0000000..33519e6
--- /dev/null
+++ b/posix/cpu_mask.sym
@@ -0,0 +1,5 @@ 
+#include <limits.h>
+#include <sched.h>
+
+--
+CPU_MASK_SIZE		sizeof (__cpu_mask)
diff --git a/posix/sched_cpucount.c b/posix/sched_cpucount.c
index eaddf61..e3622b8 100644
--- a/posix/sched_cpucount.c
+++ b/posix/sched_cpucount.c
@@ -17,6 +17,7 @@ 
 
 #include <limits.h>
 #include <sched.h>
+#include <cpu_mask.h>
 
 
 int
@@ -36,13 +37,13 @@  __sched_cpucount (size_t setsize, const cpu_set_t *setp)
       if (l == 0)
 	continue;
 
-# if LONG_BIT > 32
-      l = (l & 0x5555555555555555ul) + ((l >> 1) & 0x5555555555555555ul);
-      l = (l & 0x3333333333333333ul) + ((l >> 2) & 0x3333333333333333ul);
-      l = (l & 0x0f0f0f0f0f0f0f0ful) + ((l >> 4) & 0x0f0f0f0f0f0f0f0ful);
-      l = (l & 0x00ff00ff00ff00fful) + ((l >> 8) & 0x00ff00ff00ff00fful);
-      l = (l & 0x0000ffff0000fffful) + ((l >> 16) & 0x0000ffff0000fffful);
-      l = (l & 0x00000000fffffffful) + ((l >> 32) & 0x00000000fffffffful);
+# if CPU_MASK_SIZE > 4
+      l = (l & 0x5555555555555555ull) + ((l >> 1) & 0x5555555555555555ull);
+      l = (l & 0x3333333333333333ull) + ((l >> 2) & 0x3333333333333333ull);
+      l = (l & 0x0f0f0f0f0f0f0f0full) + ((l >> 4) & 0x0f0f0f0f0f0f0f0full);
+      l = (l & 0x00ff00ff00ff00ffull) + ((l >> 8) & 0x00ff00ff00ff00ffull);
+      l = (l & 0x0000ffff0000ffffull) + ((l >> 16) & 0x0000ffff0000ffffull);
+      l = (l & 0x00000000ffffffffull) + ((l >> 32) & 0x00000000ffffffffull);
 # else
       l = (l & 0x55555555ul) + ((l >> 1) & 0x55555555ul);
       l = (l & 0x33333333ul) + ((l >> 2) & 0x33333333ul);