From d1ad2daa7e195c82e3e5423e31af90fb01f7307d Mon Sep 17 00:00:00 2001 From: Ivan Maidanski Date: Tue, 24 Jan 2017 22:46:57 +0300 Subject: [PATCH] Fix test_atomic failure caused unaligned AO_double_t access on x86 The failure is caused by violation of an assertion that checks AO_double_t variable is 8-byte aligned on x86. * doc/README.txt (AO_double_t): Add note about required alignment. * src/atomic_ops/sysdeps/standard_ao_double_t.h (AO_double_t): Add comment about alignment. * src/atomic_ops_stack.h (AO_stack_t): Likewise. * tests/test_atomic_include.template (test_atomicXX): Define old_w, w local variables as static (as otherwise, e.g., they could have 4-byte alignment on x86); add comment. --- doc/README.txt | 10 ++++++++++ src/atomic_ops/sysdeps/standard_ao_double_t.h | 5 +++++ src/atomic_ops_stack.h | 3 +++ tests/test_atomic_include.template | 4 ++-- 4 files changed, 20 insertions(+), 2 deletions(-) diff --git a/doc/README.txt b/doc/README.txt index 043ea0d..df124ab 100644 --- a/doc/README.txt +++ b/doc/README.txt @@ -140,6 +140,16 @@ both of type AO_t. For compare_and_swap_double, we compare against the val1 field. AO_double_t exists only if AO_HAVE_double_t is defined. +Please note that AO_double_t (and AO_stack_t) variables should be properly +aligned (8-byte alignment on 32-bit targets, 16-byte alignment on 64-bit ones) +otherwise the behavior of a double-wide atomic primitive might be undefined +(or an assertion violation might occur) if such a misaligned variable is +passed (as a reference) to the primitive. Global and static variables should +already have proper alignment automatically but automatic variables (i.e. +located on the stack) might be misaligned because the stack might be +word-aligned (e.g. 4-byte stack alignment is the default one for x86). +Luckily, stack-allocated AO variables is a rare case in practice. + ORDERING CONSTRAINTS: Each operation name also includes a suffix that specifies the associated diff --git a/src/atomic_ops/sysdeps/standard_ao_double_t.h b/src/atomic_ops/sysdeps/standard_ao_double_t.h index 7d85c9d..6a652b1 100644 --- a/src/atomic_ops/sysdeps/standard_ao_double_t.h +++ b/src/atomic_ops/sysdeps/standard_ao_double_t.h @@ -67,6 +67,11 @@ typedef union { } AO_double_t; #define AO_HAVE_double_t +/* Note: AO_double_t volatile variables are not intended to be local */ +/* ones (at least those which are passed to AO double-wide primitives */ +/* as the first argument), otherwise it is the client responsibility to */ +/* ensure they have double-word alignment. */ + /* Dummy declaration as a compile-time assertion for AO_double_t size. */ struct AO_double_t_size_static_assert { char dummy[sizeof(AO_double_t) == 2 * sizeof(AO_t) ? 1 : -1]; diff --git a/src/atomic_ops_stack.h b/src/atomic_ops_stack.h index 1ca5f40..a71b61b 100644 --- a/src/atomic_ops_stack.h +++ b/src/atomic_ops_stack.h @@ -156,6 +156,9 @@ 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. */ +/* Note: AO_stack_t variables are not intended to be local ones, */ +/* otherwise it is the client responsibility to ensure they have */ +/* double-word alignment. */ #define AO_STACK_INITIALIZER AO_DOUBLE_T_INITIALIZER diff --git a/tests/test_atomic_include.template b/tests/test_atomic_include.template index a9e1895..12cceed 100644 --- a/tests/test_atomic_include.template +++ b/tests/test_atomic_include.template @@ -36,13 +36,13 @@ void test_atomicXX(void) # if defined(AO_HAVE_double_compare_and_swapXX) \ || defined(AO_HAVE_double_loadXX) \ || defined(AO_HAVE_double_storeXX) - AO_double_t old_w; + static AO_double_t old_w; /* static to avoid misalignment */ AO_double_t new_w; # endif # if defined(AO_HAVE_compare_and_swap_doubleXX) \ || defined(AO_HAVE_compare_double_and_swap_doubleXX) \ || defined(AO_HAVE_double_compare_and_swapXX) - AO_double_t w; + static AO_double_t w; /* static to avoid misalignment */ w.AO_val1 = 0; w.AO_val2 = 0; # endif -- 2.40.0