From 4e5b69134bbeab28bdfd18e1cc0cc88df302575d Mon Sep 17 00:00:00 2001 From: Siarhei Vishniakou Date: Wed, 12 Jul 2017 13:36:51 -0700 Subject: Add thread safety analysis annotations. Enable thread safety analysis annotations for clang. See https://clang.llvm.org/docs/ThreadSafetyAnalysis.html for instructions on using these in the source code. Bug: 28094863 Test: annotated frameworks/native/services/inputflinger/InputDispatcher.cpp and enabled '-Werror' and '-Wthread-safety' clang compiler flags in Android.bp for inputflinger. Observed compiler errors when accessing instance attributes without holding a lock. Also added a compile test Mutex_test.cpp, which can be build using m libutils_tests and run using /data/nativetest64/libutils_tests/libutils_tests Change-Id: I24ce111241cc339901bc45dda8b446df5299af4a --- libutils/include/utils/Mutex.h | 93 ++++++++++++++++++++++++++++++++---------- libutils/tests/Android.bp | 2 + libutils/tests/Mutex_test.cpp | 32 +++++++++++++++ 3 files changed, 106 insertions(+), 21 deletions(-) create mode 100644 libutils/tests/Mutex_test.cpp diff --git a/libutils/include/utils/Mutex.h b/libutils/include/utils/Mutex.h index d106185f0..af6076cf3 100644 --- a/libutils/include/utils/Mutex.h +++ b/libutils/include/utils/Mutex.h @@ -28,6 +28,53 @@ #include #include +// Enable thread safety attributes only with clang. +// The attributes can be safely erased when compiling with other compilers. +#if defined(__clang__) && (!defined(SWIG)) +#define THREAD_ANNOTATION_ATTRIBUTE__(x) __attribute__((x)) +#else +#define THREAD_ANNOTATION_ATTRIBUTE__(x) // no-op +#endif + +#define CAPABILITY(x) THREAD_ANNOTATION_ATTRIBUTE__(capability(x)) + +#define SCOPED_CAPABILITY THREAD_ANNOTATION_ATTRIBUTE__(scoped_lockable) + +#define GUARDED_BY(x) THREAD_ANNOTATION_ATTRIBUTE__(guarded_by(x)) + +#define PT_GUARDED_BY(x) THREAD_ANNOTATION_ATTRIBUTE__(pt_guarded_by(x)) + +#define ACQUIRED_BEFORE(...) THREAD_ANNOTATION_ATTRIBUTE__(acquired_before(__VA_ARGS__)) + +#define ACQUIRED_AFTER(...) THREAD_ANNOTATION_ATTRIBUTE__(acquired_after(__VA_ARGS__)) + +#define REQUIRES(...) THREAD_ANNOTATION_ATTRIBUTE__(requires_capability(__VA_ARGS__)) + +#define REQUIRES_SHARED(...) THREAD_ANNOTATION_ATTRIBUTE__(requires_shared_capability(__VA_ARGS__)) + +#define ACQUIRE(...) THREAD_ANNOTATION_ATTRIBUTE__(acquire_capability(__VA_ARGS__)) + +#define ACQUIRE_SHARED(...) THREAD_ANNOTATION_ATTRIBUTE__(acquire_shared_capability(__VA_ARGS__)) + +#define RELEASE(...) THREAD_ANNOTATION_ATTRIBUTE__(release_capability(__VA_ARGS__)) + +#define RELEASE_SHARED(...) THREAD_ANNOTATION_ATTRIBUTE__(release_shared_capability(__VA_ARGS__)) + +#define TRY_ACQUIRE(...) THREAD_ANNOTATION_ATTRIBUTE__(try_acquire_capability(__VA_ARGS__)) + +#define TRY_ACQUIRE_SHARED(...) \ + THREAD_ANNOTATION_ATTRIBUTE__(try_acquire_shared_capability(__VA_ARGS__)) + +#define EXCLUDES(...) THREAD_ANNOTATION_ATTRIBUTE__(locks_excluded(__VA_ARGS__)) + +#define ASSERT_CAPABILITY(x) THREAD_ANNOTATION_ATTRIBUTE__(assert_capability(x)) + +#define ASSERT_SHARED_CAPABILITY(x) THREAD_ANNOTATION_ATTRIBUTE__(assert_shared_capability(x)) + +#define RETURN_CAPABILITY(x) THREAD_ANNOTATION_ATTRIBUTE__(lock_returned(x)) + +#define NO_THREAD_SAFETY_ANALYSIS THREAD_ANNOTATION_ATTRIBUTE__(no_thread_safety_analysis) + // --------------------------------------------------------------------------- namespace android { // --------------------------------------------------------------------------- @@ -44,24 +91,24 @@ class Condition; * The mutex must be unlocked by the thread that locked it. They are not * recursive, i.e. the same thread can't lock it multiple times. */ -class Mutex { -public: +class CAPABILITY("mutex") Mutex { + public: enum { PRIVATE = 0, SHARED = 1 }; - Mutex(); - explicit Mutex(const char* name); - explicit Mutex(int type, const char* name = NULL); - ~Mutex(); + Mutex(); + explicit Mutex(const char* name); + explicit Mutex(int type, const char* name = NULL); + ~Mutex(); // lock or unlock the mutex - status_t lock(); - void unlock(); + status_t lock() ACQUIRE(); + void unlock() RELEASE(); // lock if possible; returns 0 on success, error otherwise - status_t tryLock(); + status_t tryLock() TRY_ACQUIRE(true); #if defined(__ANDROID__) // Lock the mutex, but don't wait longer than timeoutNs (relative time). @@ -75,32 +122,36 @@ public: // which is subject to NTP adjustments, and includes time during suspend, // so a timeout may occur even though no processes could run. // Not holding a partial wakelock may lead to a system suspend. - status_t timedLock(nsecs_t timeoutNs); + status_t timedLock(nsecs_t timeoutNs) TRY_ACQUIRE(true); #endif // Manages the mutex automatically. It'll be locked when Autolock is // constructed and released when Autolock goes out of scope. - class Autolock { - public: - inline explicit Autolock(Mutex& mutex) : mLock(mutex) { mLock.lock(); } - inline explicit Autolock(Mutex* mutex) : mLock(*mutex) { mLock.lock(); } - inline ~Autolock() { mLock.unlock(); } - private: + class SCOPED_CAPABILITY Autolock { + public: + inline explicit Autolock(Mutex& mutex) ACQUIRE(mutex) : mLock(mutex) { mLock.lock(); } + inline explicit Autolock(Mutex* mutex) ACQUIRE(mutex) : mLock(*mutex) { mLock.lock(); } + inline ~Autolock() RELEASE() { mLock.unlock(); } + + private: Mutex& mLock; + // Cannot be copied or moved - declarations only + Autolock(const Autolock&); + Autolock& operator=(const Autolock&); }; -private: + private: friend class Condition; // A mutex cannot be copied - Mutex(const Mutex&); - Mutex& operator = (const Mutex&); + Mutex(const Mutex&); + Mutex& operator=(const Mutex&); #if !defined(_WIN32) pthread_mutex_t mMutex; #else - void _init(); - void* mState; + void _init(); + void* mState; #endif }; diff --git a/libutils/tests/Android.bp b/libutils/tests/Android.bp index ea606a194..7cae13351 100644 --- a/libutils/tests/Android.bp +++ b/libutils/tests/Android.bp @@ -23,6 +23,7 @@ cc_test { srcs: [ "BitSet_test.cpp", "LruCache_test.cpp", + "Mutex_test.cpp", "Singleton_test.cpp", "String8_test.cpp", "StrongPointer_test.cpp", @@ -72,6 +73,7 @@ cc_test { "-Wall", "-Wextra", "-Werror", + "-Wthread-safety", ], } diff --git a/libutils/tests/Mutex_test.cpp b/libutils/tests/Mutex_test.cpp new file mode 100644 index 000000000..8a1805f51 --- /dev/null +++ b/libutils/tests/Mutex_test.cpp @@ -0,0 +1,32 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include + +static android::Mutex mLock; +static int i GUARDED_BY(mLock); + +void modifyLockedVariable() REQUIRES(mLock) { + i = 1; +} + +TEST(Mutex, compile) { + android::Mutex::Autolock _l(mLock); + i = 0; + modifyLockedVariable(); +} \ No newline at end of file -- cgit v1.2.3