Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 4e5b6913 authored by Siarhei Vishniakou's avatar Siarhei Vishniakou
Browse files

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
parent f42d0283
Loading
Loading
Loading
Loading
+72 −21
Original line number Original line Diff line number Diff line
@@ -28,6 +28,53 @@
#include <utils/Errors.h>
#include <utils/Errors.h>
#include <utils/Timers.h>
#include <utils/Timers.h>


// 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 {
namespace android {
// ---------------------------------------------------------------------------
// ---------------------------------------------------------------------------
@@ -44,7 +91,7 @@ class Condition;
 * The mutex must be unlocked by the thread that locked it.  They are not
 * 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.
 * recursive, i.e. the same thread can't lock it multiple times.
 */
 */
class Mutex {
class CAPABILITY("mutex") Mutex {
  public:
  public:
    enum {
    enum {
        PRIVATE = 0,
        PRIVATE = 0,
@@ -57,11 +104,11 @@ public:
    ~Mutex();
    ~Mutex();


    // lock or unlock the mutex
    // lock or unlock the mutex
    status_t    lock();
    status_t lock() ACQUIRE();
    void        unlock();
    void unlock() RELEASE();


    // lock if possible; returns 0 on success, error otherwise
    // lock if possible; returns 0 on success, error otherwise
    status_t    tryLock();
    status_t tryLock() TRY_ACQUIRE(true);


#if defined(__ANDROID__)
#if defined(__ANDROID__)
    // Lock the mutex, but don't wait longer than timeoutNs (relative time).
    // Lock the mutex, but don't wait longer than timeoutNs (relative time).
@@ -75,18 +122,22 @@ public:
    // which is subject to NTP adjustments, and includes time during suspend,
    // which is subject to NTP adjustments, and includes time during suspend,
    // so a timeout may occur even though no processes could run.
    // so a timeout may occur even though no processes could run.
    // Not holding a partial wakelock may lead to a system suspend.
    // 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
#endif


    // Manages the mutex automatically. It'll be locked when Autolock is
    // Manages the mutex automatically. It'll be locked when Autolock is
    // constructed and released when Autolock goes out of scope.
    // constructed and released when Autolock goes out of scope.
    class Autolock {
    class SCOPED_CAPABILITY Autolock {
      public:
      public:
        inline explicit Autolock(Mutex& mutex) : mLock(mutex)  { mLock.lock(); }
        inline explicit Autolock(Mutex& mutex) ACQUIRE(mutex) : mLock(mutex) { mLock.lock(); }
        inline explicit Autolock(Mutex* mutex) : mLock(*mutex) { mLock.lock(); }
        inline explicit Autolock(Mutex* mutex) ACQUIRE(mutex) : mLock(*mutex) { mLock.lock(); }
        inline ~Autolock() { mLock.unlock(); }
        inline ~Autolock() RELEASE() { mLock.unlock(); }

      private:
      private:
        Mutex& mLock;
        Mutex& mLock;
        // Cannot be copied or moved - declarations only
        Autolock(const Autolock&);
        Autolock& operator=(const Autolock&);
    };
    };


  private:
  private:
+2 −0
Original line number Original line Diff line number Diff line
@@ -23,6 +23,7 @@ cc_test {
    srcs: [
    srcs: [
        "BitSet_test.cpp",
        "BitSet_test.cpp",
        "LruCache_test.cpp",
        "LruCache_test.cpp",
        "Mutex_test.cpp",
        "Singleton_test.cpp",
        "Singleton_test.cpp",
        "String8_test.cpp",
        "String8_test.cpp",
        "StrongPointer_test.cpp",
        "StrongPointer_test.cpp",
@@ -72,6 +73,7 @@ cc_test {
        "-Wall",
        "-Wall",
        "-Wextra",
        "-Wextra",
        "-Werror",
        "-Werror",
        "-Wthread-safety",
    ],
    ],
}
}


+32 −0
Original line number Original line Diff line number Diff line
/*
 * 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 <utils/Mutex.h>

#include <gtest/gtest.h>

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