Merge "Add test for pthread types alignment check."

libc/bionic/pthread_cond.cpp

@@ -120,9 +120,15 @@ struct pthread_cond_internal_t {
 #endif
 };
 
+static_assert(sizeof(pthread_cond_t) == sizeof(pthread_cond_internal_t),
+              "pthread_cond_t should actually be pthread_cond_internal_t in implementation.");
+
+// For binary compatibility with old version of pthread_cond_t, we can't use more strict alignment
+// than 4-byte alignment.
+static_assert(alignof(pthread_cond_t) == 4,
+              "pthread_cond_t should fulfill the alignment requirement of pthread_cond_internal_t.");
+
 static pthread_cond_internal_t* __get_internal_cond(pthread_cond_t* cond_interface) {
-  static_assert(sizeof(pthread_cond_t) == sizeof(pthread_cond_internal_t),
-                "pthread_cond_t should actually be pthread_cond_internal_t in implementation.");
   return reinterpret_cast<pthread_cond_internal_t*>(cond_interface);
 }
 

libc/bionic/pthread_rwlock.cpp

@@ -107,9 +107,15 @@ struct pthread_rwlock_internal_t {
 #endif
 };
 
+static_assert(sizeof(pthread_rwlock_t) == sizeof(pthread_rwlock_internal_t),
+              "pthread_rwlock_t should actually be pthread_rwlock_internal_t in implementation.");
+
+// For binary compatibility with old version of pthread_rwlock_t, we can't use more strict
+// alignment than 4-byte alignment.
+static_assert(alignof(pthread_rwlock_t) == 4,
+             "pthread_rwlock_t should fulfill the alignment requirement of pthread_rwlock_internal_t.");
+
 static inline pthread_rwlock_internal_t* __get_internal_rwlock(pthread_rwlock_t* rwlock_interface) {
-  static_assert(sizeof(pthread_rwlock_t) == sizeof(pthread_rwlock_internal_t),
-                "pthread_rwlock_t should actually be pthread_rwlock_internal_t in implementation.");
   return reinterpret_cast<pthread_rwlock_internal_t*>(rwlock_interface);
 }
 

libc/include/pthread.h

@@ -78,7 +78,7 @@ typedef struct {
 #else
   char __private[4];
 #endif
-} pthread_cond_t __attribute__((aligned(sizeof(long))));
+} pthread_cond_t __attribute__((aligned(4)));
 
 #define PTHREAD_COND_INITIALIZER  { { 0 } }
 
@@ -93,7 +93,7 @@ typedef struct {
 #else
   char __private[40];
 #endif
-} pthread_rwlock_t __attribute__((aligned(8)));
+} pthread_rwlock_t __attribute__((aligned(4)));
 
 #define PTHREAD_RWLOCK_INITIALIZER  { { 0 } }
 

tests/pthread_test.cpp

@@ -33,6 +33,7 @@
 #include <unistd.h>
 
 #include <atomic>
+#include <vector>
 
 TEST(pthread, pthread_key_create) {
   pthread_key_t key;
@@ -1303,3 +1304,60 @@ TEST(pthread, pthread_mutex_owner_tid_limit) {
   // Change the implementation if we need to support higher value than 65535.
   ASSERT_LE(pid_max, 65536);
 }
+
+class StrictAlignmentAllocator {
+ public:
+  void* allocate(size_t size, size_t alignment) {
+    char* p = new char[size + alignment * 2];
+    allocated_array.push_back(p);
+    while (!is_strict_aligned(p, alignment)) {
+      ++p;
+    }
+    return p;
+  }
+
+  ~StrictAlignmentAllocator() {
+    for (auto& p : allocated_array) {
+      delete [] p;
+    }
+  }
+
+ private:
+  bool is_strict_aligned(char* p, size_t alignment) {
+    return (reinterpret_cast<uintptr_t>(p) % (alignment * 2)) == alignment;
+  }
+
+  std::vector<char*> allocated_array;
+};
+
+TEST(pthread, pthread_types_allow_four_bytes_alignment) {
+#if defined(__BIONIC__)
+  // For binary compatibility with old version, we need to allow 4-byte aligned data for pthread types.
+  StrictAlignmentAllocator allocator;
+  pthread_mutex_t* mutex = reinterpret_cast<pthread_mutex_t*>(
+                             allocator.allocate(sizeof(pthread_mutex_t), 4));
+  ASSERT_EQ(0, pthread_mutex_init(mutex, NULL));
+  ASSERT_EQ(0, pthread_mutex_lock(mutex));
+  ASSERT_EQ(0, pthread_mutex_unlock(mutex));
+  ASSERT_EQ(0, pthread_mutex_destroy(mutex));
+
+  pthread_cond_t* cond = reinterpret_cast<pthread_cond_t*>(
+                           allocator.allocate(sizeof(pthread_cond_t), 4));
+  ASSERT_EQ(0, pthread_cond_init(cond, NULL));
+  ASSERT_EQ(0, pthread_cond_signal(cond));
+  ASSERT_EQ(0, pthread_cond_broadcast(cond));
+  ASSERT_EQ(0, pthread_cond_destroy(cond));
+
+  pthread_rwlock_t* rwlock = reinterpret_cast<pthread_rwlock_t*>(
+                               allocator.allocate(sizeof(pthread_rwlock_t), 4));
+  ASSERT_EQ(0, pthread_rwlock_init(rwlock, NULL));
+  ASSERT_EQ(0, pthread_rwlock_rdlock(rwlock));
+  ASSERT_EQ(0, pthread_rwlock_unlock(rwlock));
+  ASSERT_EQ(0, pthread_rwlock_wrlock(rwlock));
+  ASSERT_EQ(0, pthread_rwlock_unlock(rwlock));
+  ASSERT_EQ(0, pthread_rwlock_destroy(rwlock));
+
+#else
+  GTEST_LOG_(INFO) << "This test tests bionic implementation details.";
+#endif
+}