From 3b74578c53647071ed22561f0f613b69d99e224c Mon Sep 17 00:00:00 2001 From: Lloyd Pique Date: Fri, 31 Aug 2018 17:34:40 -0700 Subject: Build libui and libgui with -std=c++1z This works around an issue reported with address sanitizer involving std::shared_ptr instances created by pre-C++17 compiled code, and destroyed by C++17 compiled SurfaceFlinger. The address sanitizer was complaining that there was a new/delete alignment mismatch, where the instances were allocated with default alignment, and destroyed with an 8-byte alignment requirement. Starting with C++17, new and delete now have versions that take an std::align_val_t argument that indicates the alignment of the allocation. The address sanitizer is verifying that the call to new matches the call to delete, and reports an error if it does not. The C++17 standard says that the compiler should behave in a way that is backwards compatible. In this case, FenceTime declares class-specific new and delete functions, and normally those would be used. Except that the current libc++ version of std::shared_ptr does not! It instead uses its own calls to allocate memory, and does a placement-new to actually create the FenceTime instance it shares. Unfortunately the version of new and delete called depends on whether it is compiled for C++17 or not. To make the address sanitizer happy, we can just build libui and libgui with -std=c++1z, which as a bonus allows the class-specific new to be removed. (Reproducing the failure requires a not-yet submitted CL which adds test coverage for composition calls, and another change to enable address sanitizer on the unit test) Bug: None Test: atest libsurfaceflinger_unittest Change-Id: I2321bbdbf64a8a068ba2b5ed73013ddd2fa6c32e --- libs/ui/FenceTime.cpp | 12 ------------ 1 file changed, 12 deletions(-) (limited to 'libs/ui/FenceTime.cpp') diff --git a/libs/ui/FenceTime.cpp b/libs/ui/FenceTime.cpp index 14147663de..340231d352 100644 --- a/libs/ui/FenceTime.cpp +++ b/libs/ui/FenceTime.cpp @@ -33,18 +33,6 @@ namespace android { const auto FenceTime::NO_FENCE = std::make_shared(Fence::NO_FENCE); -void* FenceTime::operator new(size_t byteCount) noexcept { - void *p = nullptr; - if (posix_memalign(&p, alignof(FenceTime), byteCount)) { - return nullptr; - } - return p; -} - -void FenceTime::operator delete(void *p) { - free(p); -} - FenceTime::FenceTime(const sp& fence) : mState(((fence.get() != nullptr) && fence->isValid()) ? State::VALID : State::INVALID), -- cgit v1.2.3-59-g8ed1b From 90055aa05059865793474dc2c5719a18cdaa3175 Mon Sep 17 00:00:00 2001 From: Ady Abraham Date: Wed, 15 May 2019 13:47:16 -0700 Subject: FenceTimeline: Fix a potential race condition FenceTimeline::updateSignalTimes checks if mQueue is empty without grabbing the lock. This can lead a situaltion that a threads calls pop() on an empty queue. Test: boot Bug: 132735340 Change-Id: I3007bfc1161797cb4d853506bb354e820bc9105d --- libs/ui/FenceTime.cpp | 2 +- libs/ui/include/ui/FenceTime.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) (limited to 'libs/ui/FenceTime.cpp') diff --git a/libs/ui/FenceTime.cpp b/libs/ui/FenceTime.cpp index 340231d352..bdfe04b0dd 100644 --- a/libs/ui/FenceTime.cpp +++ b/libs/ui/FenceTime.cpp @@ -279,8 +279,8 @@ void FenceTimeline::push(const std::shared_ptr& fence) { } void FenceTimeline::updateSignalTimes() { + std::lock_guard lock(mMutex); while (!mQueue.empty()) { - std::lock_guard lock(mMutex); std::shared_ptr fence = mQueue.front().lock(); if (!fence) { // The shared_ptr no longer exists and no one cares about the diff --git a/libs/ui/include/ui/FenceTime.h b/libs/ui/include/ui/FenceTime.h index a5a1fcbde7..ecba7f73e8 100644 --- a/libs/ui/include/ui/FenceTime.h +++ b/libs/ui/include/ui/FenceTime.h @@ -19,6 +19,7 @@ #include #include +#include #include #include @@ -159,7 +160,7 @@ public: private: mutable std::mutex mMutex; - std::queue> mQueue; + std::queue> mQueue GUARDED_BY(mMutex); }; // Used by test code to create or get FenceTimes for a given Fence. -- cgit v1.2.3-59-g8ed1b