From: Ivan Maidanski <ivmai@mail.ru>
Date: Tue, 24 Jan 2017 19:46:57 +0000 (+0300)
Subject: Fix test_atomic failure caused unaligned AO_double_t access on x86
X-Git-Tag: v7.2h~26
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=77e241c597f8a601fa0a373f3c19ae27d9c7b210;p=libatomic_ops

Fix test_atomic failure caused unaligned AO_double_t access on x86
(Cherry-pick commit c386add from 'release-7_4' branch.)

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.template (test_atomicXX): Define w
local variable as static (as otherwise, e.g., it could have 4-byte
alignment on x86); add comment.
* tests/test_atomic_include.h: Regenerate.
---

diff --git a/doc/README.txt b/doc/README.txt
index 9fab3bc..8812273 100644
--- a/doc/README.txt
+++ b/doc/README.txt
@@ -133,6 +133,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 95803f2..3a9b00f 100644
--- a/src/atomic_ops/sysdeps/standard_ao_double_t.h
+++ b/src/atomic_ops/sysdeps/standard_ao_double_t.h
@@ -23,5 +23,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.                              */
+
 #define AO_val1 AO_parts.AO_v1
 #define AO_val2 AO_parts.AO_v2
diff --git a/src/atomic_ops_stack.h b/src/atomic_ops_stack.h
index 6c8b5bb..58a9b23 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 {0}
 
diff --git a/tests/test_atomic.template b/tests/test_atomic.template
index a20edd7..db60267 100644
--- a/tests/test_atomic.template
+++ b/tests/test_atomic.template
@@ -26,7 +26,7 @@ void test_atomicXX(void)
     AO_TS_t z = AO_TS_INITIALIZER;
 # endif
 # if defined(AO_HAVE_double_t)
-    AO_double_t w;
+    static AO_double_t w; /* static to avoid misalignment */
     w.AO_val1 = 0;
     w.AO_val2 = 0;
 # endif
diff --git a/tests/test_atomic_include.h b/tests/test_atomic_include.h
index 2ab5a93..4a300e3 100644
--- a/tests/test_atomic_include.h
+++ b/tests/test_atomic_include.h
@@ -26,7 +26,7 @@ void test_atomic(void)
     AO_TS_t z = AO_TS_INITIALIZER;
 # endif
 # if defined(AO_HAVE_double_t)
-    AO_double_t w;
+    static AO_double_t w; /* static to avoid misalignment */
     w.AO_val1 = 0;
     w.AO_val2 = 0;
 # endif
@@ -230,7 +230,7 @@ void test_atomic_release(void)
     AO_TS_t z = AO_TS_INITIALIZER;
 # endif
 # if defined(AO_HAVE_double_t)
-    AO_double_t w;
+    static AO_double_t w; /* static to avoid misalignment */
     w.AO_val1 = 0;
     w.AO_val2 = 0;
 # endif
@@ -434,7 +434,7 @@ void test_atomic_acquire(void)
     AO_TS_t z = AO_TS_INITIALIZER;
 # endif
 # if defined(AO_HAVE_double_t)
-    AO_double_t w;
+    static AO_double_t w; /* static to avoid misalignment */
     w.AO_val1 = 0;
     w.AO_val2 = 0;
 # endif
@@ -638,7 +638,7 @@ void test_atomic_read(void)
     AO_TS_t z = AO_TS_INITIALIZER;
 # endif
 # if defined(AO_HAVE_double_t)
-    AO_double_t w;
+    static AO_double_t w; /* static to avoid misalignment */
     w.AO_val1 = 0;
     w.AO_val2 = 0;
 # endif
@@ -842,7 +842,7 @@ void test_atomic_write(void)
     AO_TS_t z = AO_TS_INITIALIZER;
 # endif
 # if defined(AO_HAVE_double_t)
-    AO_double_t w;
+    static AO_double_t w; /* static to avoid misalignment */
     w.AO_val1 = 0;
     w.AO_val2 = 0;
 # endif
@@ -1046,7 +1046,7 @@ void test_atomic_full(void)
     AO_TS_t z = AO_TS_INITIALIZER;
 # endif
 # if defined(AO_HAVE_double_t)
-    AO_double_t w;
+    static AO_double_t w; /* static to avoid misalignment */
     w.AO_val1 = 0;
     w.AO_val2 = 0;
 # endif
@@ -1250,7 +1250,7 @@ void test_atomic_release_write(void)
     AO_TS_t z = AO_TS_INITIALIZER;
 # endif
 # if defined(AO_HAVE_double_t)
-    AO_double_t w;
+    static AO_double_t w; /* static to avoid misalignment */
     w.AO_val1 = 0;
     w.AO_val2 = 0;
 # endif
@@ -1454,7 +1454,7 @@ void test_atomic_acquire_read(void)
     AO_TS_t z = AO_TS_INITIALIZER;
 # endif
 # if defined(AO_HAVE_double_t)
-    AO_double_t w;
+    static AO_double_t w; /* static to avoid misalignment */
     w.AO_val1 = 0;
     w.AO_val2 = 0;
 # endif