]> granicus.if.org Git - libatomic_ops/commitdiff
Fix test_atomic failure caused unaligned AO_double_t access on x86
authorIvan Maidanski <ivmai@mail.ru>
Tue, 24 Jan 2017 19:46:57 +0000 (22:46 +0300)
committerIvan Maidanski <ivmai@mail.ru>
Mon, 6 Feb 2017 06:31:18 +0000 (09:31 +0300)
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
src/atomic_ops/sysdeps/standard_ao_double_t.h
src/atomic_ops_stack.h
tests/test_atomic_include.template

index b467a53145b2afc44ec229d1c455db970a056986..2b96cd27e751ebecaa8e2003a918bb8b234807dd 100644 (file)
@@ -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
index bf76979a19c6992c9e2a109aff5ce29c6ffc353a..6814ad70b19ada960eaa122d8581ac1c05a5d1d7 100644 (file)
@@ -51,6 +51,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];
index 1ca5f406e58dd10e00d1d925bb8aca319462c794..a71b61b275a01cd9a686935d265714a37aa43c0e 100644 (file)
@@ -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
 
index 06894cd9a2d83c15aa0ad531d1c964a4bffd3999..de7a666d689fcf6b10aca11e701b3cdcf4a9205d 100644 (file)
@@ -28,13 +28,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