* commit '9c4d0941f985e2ab9ec5fca579dc98d4ad26fec2': Fix bug for recursive/errorcheck mutex on 32-bit devices.
This commit is contained in:
		@@ -392,6 +392,30 @@ static inline __always_inline int __recursive_increment(pthread_mutex_internal_t
 | 
			
		||||
    return 0;
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
static inline __always_inline int __recursive_or_errorcheck_mutex_wait(
 | 
			
		||||
                                                      pthread_mutex_internal_t* mutex,
 | 
			
		||||
                                                      uint16_t shared,
 | 
			
		||||
                                                      uint16_t old_state,
 | 
			
		||||
                                                      const timespec* rel_timeout) {
 | 
			
		||||
// __futex_wait always waits on a 32-bit value. But state is 16-bit. For a normal mutex, the owner_tid
 | 
			
		||||
// field in mutex is not used. On 64-bit devices, the __pad field in mutex is not used.
 | 
			
		||||
// But when a recursive or errorcheck mutex is used on 32-bit devices, we need to add the
 | 
			
		||||
// owner_tid value in the value argument for __futex_wait, otherwise we may always get EAGAIN error.
 | 
			
		||||
 | 
			
		||||
#if defined(__LP64__)
 | 
			
		||||
  return __futex_wait_ex(&mutex->state, shared, old_state, rel_timeout);
 | 
			
		||||
 | 
			
		||||
#else
 | 
			
		||||
  // This implementation works only when the layout of pthread_mutex_internal_t matches below expectation.
 | 
			
		||||
  // And it is based on the assumption that Android is always in little-endian devices.
 | 
			
		||||
  static_assert(offsetof(pthread_mutex_internal_t, state) == 0, "");
 | 
			
		||||
  static_assert(offsetof(pthread_mutex_internal_t, owner_tid) == 2, "");
 | 
			
		||||
 | 
			
		||||
  uint32_t owner_tid = atomic_load_explicit(&mutex->owner_tid, memory_order_relaxed);
 | 
			
		||||
  return __futex_wait_ex(&mutex->state, shared, (owner_tid << 16) | old_state, rel_timeout);
 | 
			
		||||
#endif
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
static int __pthread_mutex_lock_with_timeout(pthread_mutex_internal_t* mutex,
 | 
			
		||||
                                           const timespec* abs_timeout_or_null, clockid_t clock) {
 | 
			
		||||
    uint16_t old_state = atomic_load_explicit(&mutex->state, memory_order_relaxed);
 | 
			
		||||
@@ -469,7 +493,7 @@ static int __pthread_mutex_lock_with_timeout(pthread_mutex_internal_t* mutex,
 | 
			
		||||
                return ETIMEDOUT;
 | 
			
		||||
            }
 | 
			
		||||
        }
 | 
			
		||||
        if (__futex_wait_ex(&mutex->state, shared, old_state, rel_timeout) == -ETIMEDOUT) {
 | 
			
		||||
        if (__recursive_or_errorcheck_mutex_wait(mutex, shared, old_state, rel_timeout) == -ETIMEDOUT) {
 | 
			
		||||
            return ETIMEDOUT;
 | 
			
		||||
        }
 | 
			
		||||
        old_state = atomic_load_explicit(&mutex->state, memory_order_relaxed);
 | 
			
		||||
 
 | 
			
		||||
@@ -128,6 +128,9 @@ libBionicStandardTests_c_includes := \
 | 
			
		||||
    bionic/libc \
 | 
			
		||||
    external/tinyxml2 \
 | 
			
		||||
 | 
			
		||||
libBionicStandardTests_static_libraries := \
 | 
			
		||||
    libbase \
 | 
			
		||||
 | 
			
		||||
libBionicStandardTests_ldlibs_host := \
 | 
			
		||||
    -lrt \
 | 
			
		||||
 | 
			
		||||
@@ -257,6 +260,7 @@ bionic-unit-tests_whole_static_libraries := \
 | 
			
		||||
bionic-unit-tests_static_libraries := \
 | 
			
		||||
    libtinyxml2 \
 | 
			
		||||
    liblog \
 | 
			
		||||
    libbase \
 | 
			
		||||
 | 
			
		||||
# TODO: Include __cxa_thread_atexit_test.cpp to glibc tests once it is upgraded (glibc 2.18+)
 | 
			
		||||
bionic-unit-tests_src_files := \
 | 
			
		||||
@@ -317,6 +321,7 @@ bionic-unit-tests-static_static_libraries := \
 | 
			
		||||
    libdl \
 | 
			
		||||
    libtinyxml2 \
 | 
			
		||||
    liblog \
 | 
			
		||||
    libbase \
 | 
			
		||||
 | 
			
		||||
bionic-unit-tests-static_force_static_executable := true
 | 
			
		||||
 | 
			
		||||
@@ -355,6 +360,11 @@ bionic-unit-tests-glibc_whole_static_libraries := \
 | 
			
		||||
    libBionicGtestMain \
 | 
			
		||||
    $(fortify_libs) \
 | 
			
		||||
 | 
			
		||||
bionic-unit-tests-glibc_static_libraries := \
 | 
			
		||||
    libbase \
 | 
			
		||||
    liblog \
 | 
			
		||||
    libcutils \
 | 
			
		||||
 | 
			
		||||
bionic-unit-tests-glibc_ldlibs := \
 | 
			
		||||
    -lrt -ldl -lutil \
 | 
			
		||||
 | 
			
		||||
 
 | 
			
		||||
@@ -29,13 +29,19 @@
 | 
			
		||||
#include <unistd.h>
 | 
			
		||||
 | 
			
		||||
#include <atomic>
 | 
			
		||||
#include <regex>
 | 
			
		||||
#include <vector>
 | 
			
		||||
 | 
			
		||||
#include <base/file.h>
 | 
			
		||||
#include <base/stringprintf.h>
 | 
			
		||||
 | 
			
		||||
#include "private/bionic_macros.h"
 | 
			
		||||
#include "private/ScopeGuard.h"
 | 
			
		||||
#include "BionicDeathTest.h"
 | 
			
		||||
#include "ScopedSignalHandler.h"
 | 
			
		||||
 | 
			
		||||
extern "C" pid_t gettid();
 | 
			
		||||
 | 
			
		||||
TEST(pthread, pthread_key_create) {
 | 
			
		||||
  pthread_key_t key;
 | 
			
		||||
  ASSERT_EQ(0, pthread_key_create(&key, NULL));
 | 
			
		||||
@@ -704,6 +710,23 @@ TEST(pthread, pthread_rwlock_smoke) {
 | 
			
		||||
  ASSERT_EQ(0, pthread_rwlock_destroy(&l));
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
static void WaitUntilThreadSleep(std::atomic<pid_t>& pid) {
 | 
			
		||||
  while (pid == 0) {
 | 
			
		||||
    usleep(1000);
 | 
			
		||||
  }
 | 
			
		||||
  std::string filename = android::base::StringPrintf("/proc/%d/stat", pid.load());
 | 
			
		||||
  std::regex regex {R"(\s+S\s+)"};
 | 
			
		||||
 | 
			
		||||
  while (true) {
 | 
			
		||||
    std::string content;
 | 
			
		||||
    ASSERT_TRUE(android::base::ReadFileToString(filename, &content));
 | 
			
		||||
    if (std::regex_search(content, regex)) {
 | 
			
		||||
      break;
 | 
			
		||||
    }
 | 
			
		||||
    usleep(1000);
 | 
			
		||||
  }
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
struct RwlockWakeupHelperArg {
 | 
			
		||||
  pthread_rwlock_t lock;
 | 
			
		||||
  enum Progress {
 | 
			
		||||
@@ -713,9 +736,11 @@ struct RwlockWakeupHelperArg {
 | 
			
		||||
    LOCK_ACCESSED
 | 
			
		||||
  };
 | 
			
		||||
  std::atomic<Progress> progress;
 | 
			
		||||
  std::atomic<pid_t> tid;
 | 
			
		||||
};
 | 
			
		||||
 | 
			
		||||
static void pthread_rwlock_reader_wakeup_writer_helper(RwlockWakeupHelperArg* arg) {
 | 
			
		||||
  arg->tid = gettid();
 | 
			
		||||
  ASSERT_EQ(RwlockWakeupHelperArg::LOCK_INITIALIZED, arg->progress);
 | 
			
		||||
  arg->progress = RwlockWakeupHelperArg::LOCK_WAITING;
 | 
			
		||||
 | 
			
		||||
@@ -732,14 +757,14 @@ TEST(pthread, pthread_rwlock_reader_wakeup_writer) {
 | 
			
		||||
  ASSERT_EQ(0, pthread_rwlock_init(&wakeup_arg.lock, NULL));
 | 
			
		||||
  ASSERT_EQ(0, pthread_rwlock_rdlock(&wakeup_arg.lock));
 | 
			
		||||
  wakeup_arg.progress = RwlockWakeupHelperArg::LOCK_INITIALIZED;
 | 
			
		||||
  wakeup_arg.tid = 0;
 | 
			
		||||
 | 
			
		||||
  pthread_t thread;
 | 
			
		||||
  ASSERT_EQ(0, pthread_create(&thread, NULL,
 | 
			
		||||
    reinterpret_cast<void* (*)(void*)>(pthread_rwlock_reader_wakeup_writer_helper), &wakeup_arg));
 | 
			
		||||
  while (wakeup_arg.progress != RwlockWakeupHelperArg::LOCK_WAITING) {
 | 
			
		||||
    usleep(5000);
 | 
			
		||||
  }
 | 
			
		||||
  usleep(5000);
 | 
			
		||||
  WaitUntilThreadSleep(wakeup_arg.tid);
 | 
			
		||||
  ASSERT_EQ(RwlockWakeupHelperArg::LOCK_WAITING, wakeup_arg.progress);
 | 
			
		||||
 | 
			
		||||
  wakeup_arg.progress = RwlockWakeupHelperArg::LOCK_RELEASED;
 | 
			
		||||
  ASSERT_EQ(0, pthread_rwlock_unlock(&wakeup_arg.lock));
 | 
			
		||||
 | 
			
		||||
@@ -749,6 +774,7 @@ TEST(pthread, pthread_rwlock_reader_wakeup_writer) {
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
static void pthread_rwlock_writer_wakeup_reader_helper(RwlockWakeupHelperArg* arg) {
 | 
			
		||||
  arg->tid = gettid();
 | 
			
		||||
  ASSERT_EQ(RwlockWakeupHelperArg::LOCK_INITIALIZED, arg->progress);
 | 
			
		||||
  arg->progress = RwlockWakeupHelperArg::LOCK_WAITING;
 | 
			
		||||
 | 
			
		||||
@@ -765,14 +791,14 @@ TEST(pthread, pthread_rwlock_writer_wakeup_reader) {
 | 
			
		||||
  ASSERT_EQ(0, pthread_rwlock_init(&wakeup_arg.lock, NULL));
 | 
			
		||||
  ASSERT_EQ(0, pthread_rwlock_wrlock(&wakeup_arg.lock));
 | 
			
		||||
  wakeup_arg.progress = RwlockWakeupHelperArg::LOCK_INITIALIZED;
 | 
			
		||||
  wakeup_arg.tid = 0;
 | 
			
		||||
 | 
			
		||||
  pthread_t thread;
 | 
			
		||||
  ASSERT_EQ(0, pthread_create(&thread, NULL,
 | 
			
		||||
    reinterpret_cast<void* (*)(void*)>(pthread_rwlock_writer_wakeup_reader_helper), &wakeup_arg));
 | 
			
		||||
  while (wakeup_arg.progress != RwlockWakeupHelperArg::LOCK_WAITING) {
 | 
			
		||||
    usleep(5000);
 | 
			
		||||
  }
 | 
			
		||||
  usleep(5000);
 | 
			
		||||
  WaitUntilThreadSleep(wakeup_arg.tid);
 | 
			
		||||
  ASSERT_EQ(RwlockWakeupHelperArg::LOCK_WAITING, wakeup_arg.progress);
 | 
			
		||||
 | 
			
		||||
  wakeup_arg.progress = RwlockWakeupHelperArg::LOCK_RELEASED;
 | 
			
		||||
  ASSERT_EQ(0, pthread_rwlock_unlock(&wakeup_arg.lock));
 | 
			
		||||
 | 
			
		||||
@@ -1263,7 +1289,6 @@ TEST(pthread, pthread_mutex_init_same_as_static_initializers) {
 | 
			
		||||
  ASSERT_EQ(0, memcmp(&lock_recursive, &m3.lock, sizeof(pthread_mutex_t)));
 | 
			
		||||
  ASSERT_EQ(0, pthread_mutex_destroy(&lock_recursive));
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
class MutexWakeupHelper {
 | 
			
		||||
 private:
 | 
			
		||||
  PthreadMutex m;
 | 
			
		||||
@@ -1274,8 +1299,10 @@ class MutexWakeupHelper {
 | 
			
		||||
    LOCK_ACCESSED
 | 
			
		||||
  };
 | 
			
		||||
  std::atomic<Progress> progress;
 | 
			
		||||
  std::atomic<pid_t> tid;
 | 
			
		||||
 | 
			
		||||
  static void thread_fn(MutexWakeupHelper* helper) {
 | 
			
		||||
    helper->tid = gettid();
 | 
			
		||||
    ASSERT_EQ(LOCK_INITIALIZED, helper->progress);
 | 
			
		||||
    helper->progress = LOCK_WAITING;
 | 
			
		||||
 | 
			
		||||
@@ -1293,15 +1320,15 @@ class MutexWakeupHelper {
 | 
			
		||||
  void test() {
 | 
			
		||||
    ASSERT_EQ(0, pthread_mutex_lock(&m.lock));
 | 
			
		||||
    progress = LOCK_INITIALIZED;
 | 
			
		||||
    tid = 0;
 | 
			
		||||
 | 
			
		||||
    pthread_t thread;
 | 
			
		||||
    ASSERT_EQ(0, pthread_create(&thread, NULL,
 | 
			
		||||
      reinterpret_cast<void* (*)(void*)>(MutexWakeupHelper::thread_fn), this));
 | 
			
		||||
 | 
			
		||||
    while (progress != LOCK_WAITING) {
 | 
			
		||||
      usleep(5000);
 | 
			
		||||
    }
 | 
			
		||||
    usleep(5000);
 | 
			
		||||
    WaitUntilThreadSleep(tid);
 | 
			
		||||
    ASSERT_EQ(LOCK_WAITING, progress);
 | 
			
		||||
 | 
			
		||||
    progress = LOCK_RELEASED;
 | 
			
		||||
    ASSERT_EQ(0, pthread_mutex_unlock(&m.lock));
 | 
			
		||||
 | 
			
		||||
 
 | 
			
		||||
@@ -24,14 +24,10 @@
 | 
			
		||||
#include <pthread.h>
 | 
			
		||||
#include <stdint.h>
 | 
			
		||||
#include <stdio.h>
 | 
			
		||||
#include <sys/syscall.h>
 | 
			
		||||
#include <unistd.h>
 | 
			
		||||
#include <set>
 | 
			
		||||
 | 
			
		||||
#if defined(__GLIBC__)
 | 
			
		||||
// glibc doesn't expose gettid(2).
 | 
			
		||||
pid_t gettid() { return syscall(__NR_gettid); }
 | 
			
		||||
#endif // __GLIBC__
 | 
			
		||||
extern "C" pid_t gettid();
 | 
			
		||||
 | 
			
		||||
// For x86, bionic and glibc have per-thread stack guard values (all identical).
 | 
			
		||||
#if defined(__i386__)
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user