bug-glibc
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Fix for THREAD_GETMEM (et al) madness in useldt.h


From: M.E. O'Neill
Subject: Fix for THREAD_GETMEM (et al) madness in useldt.h
Date: Thu, 23 Nov 2000 20:16:23 -0800

Currently, in useldt.h, the macros THREAD_GETMEM, THREAD_GETMEM_NC,
THREAD_SETMEM, and THREAD_SETMEM_NC check at compile-time whether they
are called correctly.  If they are used incorrectly, the compiler emits
an unconditional call to abort().  In other words, although the
*compiler* detects incorrect use, it won't be discovered until *runtime*
when the code in question executes.

Since the test can be done at compile, I've replaced it with a compile-time
assertion (via a macro STATIC_ASSERT).  Making this change produces
better code, because gcc generates better PIC code for leaf functions
(with the current code, the ``potential'' call to abort() makes functions
such as pthread_self() non-leaf functions).

You'll probably want to move STATIC_ASSERT somewhere more general, and
perhaps rename it.  You might also want to recode it, since I just
invented it off the top of my head (I was actually expecting to find a
compile-time assertion macro in the code already, but didn't find one).
The way it is coded right now, it works on gcc and Sun Workshop compilers
(and I think it should be valid on any ANSI compiler, but I don't have
any others to hand, and gcc compatibility was all that mattered for the
code in question).

While I'm on the topic, why is useldt.h only enabled
   (a) On i686 platforms (when every IA32 processor has both the GS register
       and LDT support)
   (b) On 2.4.x kernels  (it appears to work fine on 2.2.16 and 2.2.18pre20).

Also, linuxthreads/sysdeps/i386/i686/pt-machine.h works just fine on
everything higher than a 386 (it can't run on an i386 because it uses
cmpxchg).  Although Red Hat only ships glibc for i386 and i686, other
vendors and individuals may compile i486 and i586 versions, and thus it
seems capricious to hamstring i486- and i586-specific versions with
unnecessary i386 compatibility code.

FWIW, I'm running Red Hat's glibc-2.2 with the LDT code and the ``i686''
pt-machine.h with 2.2.18pre20 on a dual Pentium 133 machine without
problems.  (I also compiled it with -momit-leaf-frame-pointer).

    M.E.O.

Enc.

--- glibc-2.2/linuxthreads/sysdeps/i386/useldt.h~       Wed Aug 16 05:38:12 2000
+++ glibc-2.2/linuxthreads/sysdeps/i386/useldt.h        Wed Nov 22 20:00:24 2000
@@ -44,6 +44,9 @@
 /* System call to set LDT entry.  */
 extern int __modify_ldt (int, struct modify_ldt_ldt_s *, size_t);

+#define STATIC_ASSERT(_bool) \
+    do {typedef char __compile_time_assert_failed[1-2*(!(_bool))];} while(0)
+ /* ^-- uses a nested typedef so no memory is actually allocated */

 /* Return the thread descriptor for the current thread.

@@ -82,6 +85,8 @@
 #define THREAD_GETMEM(descr, member) \
 ({                                                                           \
   __typeof__ (descr->member) __value;                                        \
+  /* There should not be any value with a size other than 1 or 4.  */        \
+  STATIC_ASSERT(sizeof (__value) == 4 || sizeof (__value) == 1);             \
   if (sizeof (__value) == 1)                                                 \
     __asm__ __volatile__ ("movb %%gs:%P2,%b0"                                \
                          : "=q" (__value)                                    \
@@ -89,16 +94,10 @@
                            "i" (offsetof (struct _pthread_descr_struct,      \
                                           member)));                         \
   else                                                                       \
-    {                                                                        \
-      if (sizeof (__value) != 4)                                             \
-       /* There should not be any value with a size other than 1 or 4.  */   \
-       abort ();                                                             \
-                                                                             \
       __asm__ __volatile__ ("movl %%gs:%P1,%0"                               \
                            : "=r" (__value)                                  \
                            : "i" (offsetof (struct _pthread_descr_struct,    \
                                             member)));                       \
-    }                                                                        \
   __value;                                                                   \
 })

@@ -106,6 +105,8 @@
 #define THREAD_GETMEM_NC(descr, member) \
 ({                                                                           \
   __typeof__ (descr->member) __value;                                        \
+  /* There should not be any value with a size other than 1 or 4.  */        \
+  STATIC_ASSERT(sizeof (__value) == 4 || sizeof (__value) == 1);             \
   if (sizeof (__value) == 1)                                                 \
     __asm__ __volatile__ ("movb %%gs:(%2),%b0"                               \
                          : "=q" (__value)                                    \
@@ -113,16 +114,10 @@
                            "r" (offsetof (struct _pthread_descr_struct,      \
                                           member)));                         \
   else                                                                       \
-    {                                                                        \
-      if (sizeof (__value) != 4)                                             \
-       /* There should not be any value with a size other than 1 or 4.  */   \
-       abort ();                                                             \
-                                                                             \
       __asm__ __volatile__ ("movl %%gs:(%1),%0"                                
      \
                            : "=r" (__value)                                  \
                            : "r" (offsetof (struct _pthread_descr_struct,    \
                                             member)));                       \
-    }                                                                        \
   __value;                                                                   \
 })

@@ -130,44 +125,36 @@
 #define THREAD_SETMEM(descr, member, value) \
 ({                                                                           \
   __typeof__ (descr->member) __value = (value);                                
      \
+  /* There should not be any value with a size other than 1 or 4.  */        \
+  STATIC_ASSERT(sizeof (__value) == 4 || sizeof (__value) == 1);             \
   if (sizeof (__value) == 1)                                                 \
     __asm__ __volatile__ ("movb %0,%%gs:%P1" :                               \
                          : "q" (__value),                                    \
                            "i" (offsetof (struct _pthread_descr_struct,      \
                                           member)));                         \
   else                                                                       \
-    {                                                                        \
-      if (sizeof (__value) != 4)                                             \
-       /* There should not be any value with a size other than 1 or 4.  */   \
-       abort ();                                                             \
-                                                                             \
       __asm__ __volatile__ ("movl %0,%%gs:%P1" :                             \
                            : "r" (__value),                                  \
                              "i" (offsetof (struct _pthread_descr_struct,    \
                                             member)));                       \
-    }                                                                        \
 })

 /* Set member of the thread descriptor directly.  */
 #define THREAD_SETMEM_NC(descr, member, value) \
 ({                                                                           \
   __typeof__ (descr->member) __value = (value);                                
      \
+  /* There should not be any value with a size other than 1 or 4.  */        \
+  STATIC_ASSERT(sizeof (__value) == 4 || sizeof (__value) == 1);             \
   if (sizeof (__value) == 1)                                                 \
     __asm__ __volatile__ ("movb %0,%%gs:(%1)" :                                
      \
                          : "q" (__value),                                    \
                            "r" (offsetof (struct _pthread_descr_struct,      \
                                           member)));                         \
   else                                                                       \
-    {                                                                        \
-      if (sizeof (__value) != 4)                                             \
-       /* There should not be any value with a size other than 1 or 4.  */   \
-       abort ();                                                             \
-                                                                             \
       __asm__ __volatile__ ("movl %0,%%gs:(%1)" :                            \
                            : "r" (__value),                                  \
                              "r" (offsetof (struct _pthread_descr_struct,    \
                                             member)));                       \
-    }                                                                        \
 })

 /* We want the OS to assign stack addresses.  */



reply via email to

[Prev in Thread] Current Thread [Next in Thread]