From 69c4010c1b3d43e2e179421e27379aaa5aeeb6d8 Mon Sep 17 00:00:00 2001 From: Ivan Maidanski Date: Fri, 20 Jan 2017 11:58:13 +0300 Subject: Fix test_atomic failure caused unaligned AO_double_t access on x86 (VC++) Passing an unaligned AO_double_t pointer to AO double-wide primitives results in an undefined behavior of the latter when running on x86 (or violation of the corresponding assertion on the alignment). MS VC++/x86 aligns AO_double_t values on a 4-byte boundary by default. For the proper alignment, __declspec(align(8)) should be applied to variables defined in the client code (which uses double-wide AO primitives). Unfortunately, the attribute cannot be added to AO_double_t definition itself because the compiler does not allow the attribute for function arguments. This patch introduces AO_DOUBLE_ALIGN attribute for use by clients of the double-wide AO primitives (and, thus, AO_stack clients). Matters only Visual Studio compiler for X86. The inner clients (atomic_ops_malloc, test_atomic, test_stack) are updated to use this attribute. * doc/README_win32.txt [x86] (AO_DOUBLE_ALIGN): Document. * src/atomic_ops/sysdeps/generic_pthread.h (AO_DOUBLE_ALIGN): Define (as empty). * src/atomic_ops/sysdeps/standard_ao_double_t.h [!_WIN64 && _WIN32 && !__GNUC__ && _MSC_VER] (AO_DOUBLE_ALIGN): Define as declspec align(8); document it. * src/atomic_ops/sysdeps/standard_ao_double_t.h [!AO_DOUBLE_ALIGN] (AO_DOUBLE_ALIGN): Define as empty (otherwise). * src/atomic_ops_stack.h [AO_USE_ALMOST_LOCK_FREE && !AO_DOUBLE_ALIGN] (AO_DOUBLE_ALIGN): Likewise. * src/atomic_ops_malloc.c (AO_free_list): Use AO_DOUBLE_ALIGN attribute. * tests/test_stack.c (the_list): Likewise. * src/atomic_ops_stack.h [!AO_USE_ALMOST_LOCK_FREE] (AO_stack_t): Document AO_DOUBLE_ALIGN usage (by clients). * tests/test_atomic_include.template (test_atomicXX): Use AO_DOUBLE_ALIGN attribute for old_w and w double-wide local variables (to avoid alignment assertion violation or AO primitives undefined behavior on x86 if the test code is compiled by VC++). --- src/atomic_ops/sysdeps/generic_pthread.h | 1 + src/atomic_ops/sysdeps/standard_ao_double_t.h | 14 ++++++++++++++ src/atomic_ops_malloc.c | 2 +- src/atomic_ops_stack.h | 6 ++++++ 4 files changed, 22 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/atomic_ops/sysdeps/generic_pthread.h b/src/atomic_ops/sysdeps/generic_pthread.h index 3c65624..2e70068 100644 --- a/src/atomic_ops/sysdeps/generic_pthread.h +++ b/src/atomic_ops/sysdeps/generic_pthread.h @@ -367,6 +367,7 @@ typedef struct { AO_t AO_val2; } AO_double_t; #define AO_HAVE_double_t +#define AO_DOUBLE_ALIGN /* empty */ #define AO_DOUBLE_T_INITIALIZER { (AO_t)0, (AO_t)0 } diff --git a/src/atomic_ops/sysdeps/standard_ao_double_t.h b/src/atomic_ops/sysdeps/standard_ao_double_t.h index 7d85c9d..636ba46 100644 --- a/src/atomic_ops/sysdeps/standard_ao_double_t.h +++ b/src/atomic_ops/sysdeps/standard_ao_double_t.h @@ -50,11 +50,25 @@ typedef __m128 double_ptr_storage; #elif defined(_WIN32) && !defined(__GNUC__) typedef unsigned __int64 double_ptr_storage; +# ifdef _MSC_VER + /* VC++/x86 does not align __int64 properly by default, thus, */ + /* causing an undefined behavior or assertions violation in */ + /* the double-wide atomic primitives. For the proper alignment, */ + /* all variables of AO_double_t type (in the client code) those */ + /* address is passed to an AO primitive should be defined with the */ + /* given attribute. Not a part of double_ptr_storage because the */ + /* attribute cannot be applied to function parameters. */ +# define AO_DOUBLE_ALIGN __declspec(align(8)) +# endif #else typedef unsigned long long double_ptr_storage; #endif # define AO_HAVE_DOUBLE_PTR_STORAGE +#ifndef AO_DOUBLE_ALIGN +# define AO_DOUBLE_ALIGN /* empty */ +#endif + typedef union { struct { AO_t AO_v1; AO_t AO_v2; } AO_parts; /* Note that AO_v1 corresponds to the low or the high part of */ diff --git a/src/atomic_ops_malloc.c b/src/atomic_ops_malloc.c index aae21d0..457d635 100644 --- a/src/atomic_ops_malloc.c +++ b/src/atomic_ops_malloc.c @@ -223,7 +223,7 @@ get_chunk(void) /* Object free lists. Ith entry corresponds to objects */ /* of total size 2**i bytes. */ -AO_stack_t AO_free_list[LOG_MAX_SIZE+1]; +AO_stack_t AO_DOUBLE_ALIGN AO_free_list[LOG_MAX_SIZE+1]; /* Break up the chunk, and add it to the object free list for */ /* the given size. We have exclusive access to chunk. */ diff --git a/src/atomic_ops_stack.h b/src/atomic_ops_stack.h index 1ca5f40..de41c6b 100644 --- a/src/atomic_ops_stack.h +++ b/src/atomic_ops_stack.h @@ -146,6 +146,11 @@ AO_INLINE void AO_stack_init(AO_stack_t *list) AO_stack_pop_explicit_aux_acquire(&((l)->AO_ptr), &((l)->AO_aux)) #define AO_HAVE_stack_pop_acquire +#ifndef AO_DOUBLE_ALIGN + /* For AO_stack clients only. */ +# define AO_DOUBLE_ALIGN /* empty */ +#endif + # else /* Use fully non-blocking data structure, wide CAS */ #ifndef AO_HAVE_double_t @@ -156,6 +161,7 @@ AO_INLINE void AO_stack_init(AO_stack_t *list) typedef volatile AO_double_t AO_stack_t; /* AO_val1 is version, AO_val2 is pointer. */ +/* AO_stack_t variables should have AO_DOUBLE_ALIGN attribute. */ #define AO_STACK_INITIALIZER AO_DOUBLE_T_INITIALIZER -- cgit v1.2.1