diff mbox

__libc_lock_define_initialized: Always initialize the lock

Message ID 5654DA50.5060002@redhat.com
State New, archived
Headers

Commit Message

Florian Weimer Nov. 24, 2015, 9:44 p.m. UTC 
  Since GCC 4.0, GCC will put zero-initialized variables into .bss.
This makes the macro suitable for automatic variables, too.

On x86_64-redhat-linux-gnu, all DSOs except libc.so.6 are unchanged.
The differences for libc.so.6 are harmless, the symbols still end up in
.bss:

  <https://gcc.gnu.org/ml/gcc-help/2015-11/msg00211.html>

I think this change is still worthwhile because it removes a nasty
source of errors, a macro with “initialized” in the name which does not
actually initialize anything.

Florian

Comments

Adhemerval Zanella Nov. 25, 2015, 6:43 p.m. UTC | #1 
On 24-11-2015 19:44, Florian Weimer wrote:
> Since GCC 4.0, GCC will put zero-initialized variables into .bss.
> This makes the macro suitable for automatic variables, too.
> 
> On x86_64-redhat-linux-gnu, all DSOs except libc.so.6 are unchanged.
> The differences for libc.so.6 are harmless, the symbols still end up in
> .bss:
> 
>   <https://gcc.gnu.org/ml/gcc-help/2015-11/msg00211.html>
> 
> I think this change is still worthwhile because it removes a nasty
> source of errors, a macro with “initialized” in the name which does not
> actually initialize anything.
> 
> Florian
> 

LGTM. Since you touching this, I think it is worthwhile to cleanup the
LLL_LOCK_INITIALIZER{_LOCKED,_WAITERS} definition (i386, sparc, and
x86_64 redefines it to same value).
Adhemerval Zanella Nov. 25, 2015, 7:26 p.m. UTC | #2 
On 25-11-2015 17:24, Florian Weimer wrote:
> On 11/25/2015 07:43 PM, Adhemerval Zanella wrote:
> 
>> LGTM. Since you touching this, I think it is worthwhile to cleanup the
>> LLL_LOCK_INITIALIZER{_LOCKED,_WAITERS} definition (i386, sparc, and
>> x86_64 redefines it to same value).
> 
> Do you suggest to remove the architecture-specific #defines?
 
Yes, since right now they are equal to default one at
sysdeps/nptl/lowlevellock.h, and I see not reason to have different
definition for each architecture.

> 
> Thanks,
> Florian
>
diff mbox

Patch

2015-11-24  Florian Weimer  <fweimer@redhat.com>

	* sysdeps/nptl/libc-lockP.h (__libc_lock_define_initialized):
	Always initialize the lock.

diff --git a/sysdeps/nptl/libc-lockP.h b/sysdeps/nptl/libc-lockP.h
index 3881b45..2a4809f 100644
--- a/sysdeps/nptl/libc-lockP.h
+++ b/sysdeps/nptl/libc-lockP.h
@@ -66,27 +66,15 @@  typedef pthread_key_t __libc_key_t;
 #define __rtld_lock_define_recursive(CLASS,NAME) \
   CLASS __rtld_lock_recursive_t NAME;
 
-/* Define an initialized lock variable NAME with storage class CLASS.
-
-   For the C library we take a deeper look at the initializer.  For
-   this implementation all fields are initialized to zero.  Therefore
-   we don't initialize the variable which allows putting it into the
-   BSS section.  (Except on PA-RISC and other odd architectures, where
-   initialized locks must be set to one due to the lack of normal
-   atomic operations.) */
+/* Define an initialized lock variable NAME with storage class CLASS.  */
 
 #define _LIBC_LOCK_INITIALIZER LLL_LOCK_INITIALIZER
 #if IS_IN (libc) || IS_IN (libpthread)
-# if LLL_LOCK_INITIALIZER == 0
-#  define __libc_lock_define_initialized(CLASS,NAME) \
-  CLASS __libc_lock_t NAME;
-# else
-#  define __libc_lock_define_initialized(CLASS,NAME) \
+# define __libc_lock_define_initialized(CLASS,NAME) \
   CLASS __libc_lock_t NAME = LLL_LOCK_INITIALIZER;
-# endif
 #else
 # define __libc_lock_define_initialized(CLASS,NAME) \
-  CLASS __libc_lock_t NAME;
+  CLASS __libc_lock_t NAME = PTHREAD_MUTEX_INITIALIZER;
 #endif
 
 #define __libc_rwlock_define_initialized(CLASS,NAME) \