drm_hwcomposer: Use clang thread-safety analysis in VSyncWorker
See https://clang.llvm.org/docs/ThreadSafetyAnalysis.html for details.
Define thread annotation macros according to the example provided in the
documentation, and use these to annotate VSyncWorker members that can be
accessed from multiple threads.
Thread safety analysis does not yet work with std::unique_lock, so
disable the warnings when it is used.
Change-Id: I8b3b503fbaf0923588fbe3d0180f6fe6ec228350
Signed-off-by: Drew Davenport <ddavenport@google.com>
diff --git a/.ci/Makefile b/.ci/Makefile
index 0dba6cc..d57ea86 100644
--- a/.ci/Makefile
+++ b/.ci/Makefile
@@ -9,7 +9,7 @@
SRC_DIR := .
CXXFLAGS := -Wall -Wextra -Werror -Wno-missing-designated-field-initializers
-CXXFLAGS += -D_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS
+CXXFLAGS += -D_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS -Wthread-safety
CXXFLAGS += -fvisibility-inlines-hidden -std=gnu++17 -DHWC2_USE_CPP11 -DHWC2_INCLUDE_STRINGIFICATION -fno-rtti
CXXARGS := $(shell cat $(BASE_DIR)/toolchain_wrapper/sharedlib.cppflags)
diff --git a/drm/VSyncWorker.cpp b/drm/VSyncWorker.cpp
index 6dd6b02..5234f6b 100644
--- a/drm/VSyncWorker.cpp
+++ b/drm/VSyncWorker.cpp
@@ -159,6 +159,9 @@
for (;;) {
{
std::unique_lock<std::mutex> lock(mutex_);
+ // Thread safety analysis doesn't understand std::unique_lock.
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wthread-safety-analysis"
if (thread_exit_)
break;
@@ -167,6 +170,7 @@
if (!enabled_)
continue;
+#pragma clang diagnostic pop
}
ret = -EAGAIN;
diff --git a/drm/VSyncWorker.h b/drm/VSyncWorker.h
index c76dd14..5b97328 100644
--- a/drm/VSyncWorker.h
+++ b/drm/VSyncWorker.h
@@ -23,6 +23,7 @@
#include <thread>
#include "DrmDevice.h"
+#include "utils/thread_annotations.h"
namespace android {
@@ -55,27 +56,26 @@
void ThreadFn();
- int64_t GetPhasedVSync(int64_t frame_ns, int64_t current) const;
+ int64_t GetPhasedVSync(int64_t frame_ns, int64_t current) const
+ REQUIRES(mutex_);
int SyntheticWaitVBlank(int64_t *timestamp);
- // Must hold the lock before calling these.
void UpdateVSyncControl();
- bool ShouldEnable() const;
+ bool ShouldEnable() const REQUIRES(mutex_);
SharedFd drm_fd_;
uint32_t high_crtc_ = 0;
- bool enabled_ = false;
- bool thread_exit_ = false;
- int64_t last_timestamp_ = -1;
+ bool enabled_ GUARDED_BY(mutex_) = false;
+ bool thread_exit_ GUARDED_BY(mutex_) = false;
+ int64_t last_timestamp_ GUARDED_BY(mutex_) = -1;
// Default to 60Hz refresh rate
static constexpr uint32_t kDefaultVSPeriodNs = 16666666;
- // Needs to be threadsafe.
- uint32_t vsync_period_ns_ = kDefaultVSPeriodNs;
- bool enable_vsync_timestamps_ = false;
- uint32_t last_vsync_timestamp_ = 0;
- std::optional<VsyncTimestampCallback> callback_;
+ uint32_t vsync_period_ns_ GUARDED_BY(mutex_) = kDefaultVSPeriodNs;
+ bool enable_vsync_timestamps_ GUARDED_BY(mutex_) = false;
+ uint32_t last_vsync_timestamp_ GUARDED_BY(mutex_) = 0;
+ std::optional<VsyncTimestampCallback> callback_ GUARDED_BY(mutex_);
std::condition_variable cv_;
std::thread vswt_;
diff --git a/utils/thread_annotations.h b/utils/thread_annotations.h
new file mode 100644
index 0000000..f3bd2e0
--- /dev/null
+++ b/utils/thread_annotations.h
@@ -0,0 +1,77 @@
+/*
+ * Copyright (C) 2025 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.
+ */
+
+#pragma once
+
+// Enable thread safety attributes only with clang.
+#if defined(__clang__)
+#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 RELEASE_GENERIC(...) \
+ THREAD_ANNOTATION_ATTRIBUTE__(release_generic_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)
\ No newline at end of file