From f9a05ebb21f6c0dca0555000f0eb3635c336ab97 Mon Sep 17 00:00:00 2001 From: Dennis Felsing Date: Fri, 27 May 2022 00:57:45 +0200 Subject: [PATCH 1/2] Add back compiler warnings on macOS (oops) --- CMakeLists.txt | 54 ++++++++++++++++++++++++++------------------------ 1 file changed, 28 insertions(+), 26 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 78252f227..cea9411b3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -255,34 +255,36 @@ if(CMAKE_GENERATOR STREQUAL "Ninja") add_cxx_compiler_flag_if_supported(OUR_FLAGS -fcolor-diagnostics) endif() -if(NOT MSVC AND NOT HAIKU AND NOT TARGET_OS STREQUAL "mac") - if(NOT FUSE_LD STREQUAL OFF) - add_linker_flag_if_supported(OUR_FLAGS_LINK -fuse-ld=${FUSE_LD}) - if(FLAG_SUPPORTED_fuse_ld_${FUSE_LD}) - message(STATUS "Using ${FUSE_LD} linker") - endif() - else() - if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang" OR NOT ENABLE_IPO) - # GCC+LTO: pthread_create has failed: Resource temporarily unavailable - add_linker_flag_if_supported(OUR_FLAGS_LINK -fuse-ld=mold) - if(FLAG_SUPPORTED_fuse_ld_mold) - message(STATUS "Using mold linker") - else() - # Does not support GCC+LTO - add_linker_flag_if_supported(OUR_FLAGS_LINK -fuse-ld=lld) - if(FLAG_SUPPORTED_fuse_ld_lld) - message(STATUS "Using lld linker") - else() - add_linker_flag_if_supported(OUR_FLAGS_LINK -fuse-ld=gold) - if(FLAG_SUPPORTED_fuse_ld_gold) - message(STATUS "Using gold linker") - endif() - endif() +if(NOT MSVC AND NOT HAIKU) + IF(NOT TARGET_OS STREQUAL "mac") + if(NOT FUSE_LD STREQUAL OFF) + add_linker_flag_if_supported(OUR_FLAGS_LINK -fuse-ld=${FUSE_LD}) + if(FLAG_SUPPORTED_fuse_ld_${FUSE_LD}) + message(STATUS "Using ${FUSE_LD} linker") endif() else() - add_linker_flag_if_supported(OUR_FLAGS_LINK -fuse-ld=gold) - if(FLAG_SUPPORTED_fuse_ld_gold) - message(STATUS "Using gold linker") + if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang" OR NOT ENABLE_IPO) + # GCC+LTO: pthread_create has failed: Resource temporarily unavailable + add_linker_flag_if_supported(OUR_FLAGS_LINK -fuse-ld=mold) + if(FLAG_SUPPORTED_fuse_ld_mold) + message(STATUS "Using mold linker") + else() + # Does not support GCC+LTO + add_linker_flag_if_supported(OUR_FLAGS_LINK -fuse-ld=lld) + if(FLAG_SUPPORTED_fuse_ld_lld) + message(STATUS "Using lld linker") + else() + add_linker_flag_if_supported(OUR_FLAGS_LINK -fuse-ld=gold) + if(FLAG_SUPPORTED_fuse_ld_gold) + message(STATUS "Using gold linker") + endif() + endif() + endif() + else() + add_linker_flag_if_supported(OUR_FLAGS_LINK -fuse-ld=gold) + if(FLAG_SUPPORTED_fuse_ld_gold) + message(STATUS "Using gold linker") + endif() endif() endif() endif() From 174b855774f035e9514afc467e898405fc9f16e2 Mon Sep 17 00:00:00 2001 From: Dennis Felsing Date: Sat, 28 May 2022 02:14:46 +0200 Subject: [PATCH 2/2] Enable -Wthread-safety-negative (as suggested by Chairn) --- CMakeLists.txt | 1 + src/base/tl/threading.h | 4 ++-- src/engine/client/serverbrowser_http.cpp | 4 ++-- src/engine/client/updater.h | 8 ++++---- src/engine/client/video.cpp | 4 ++-- src/engine/client/video.h | 8 ++++---- src/engine/shared/jobs.h | 4 ++-- 7 files changed, 17 insertions(+), 16 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index cea9411b3..827027aad 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -344,6 +344,7 @@ if(NOT MSVC AND NOT HAIKU) add_cxx_compiler_flag_if_supported(OUR_FLAGS_OWN -Wshadow-all) # clang add_cxx_compiler_flag_if_supported(OUR_FLAGS_OWN -Wshadow=global) # gcc add_cxx_compiler_flag_if_supported(OUR_FLAGS_OWN -Wthread-safety) + add_cxx_compiler_flag_if_supported(OUR_FLAGS_OWN -Wthread-safety-negative) add_cxx_compiler_flag_if_supported(OUR_FLAGS_OWN -Wsuggest-override) add_linker_flag_if_supported(OUR_FLAGS_LINK -Wno-alloc-size-larger-than) # save.cpp with LTO # add_cxx_compiler_flag_if_supported(OUR_FLAGS_OWN -Wdouble-promotion) # Many occurences diff --git a/src/base/tl/threading.h b/src/base/tl/threading.h index 7c3b3cc5e..84ed6361a 100644 --- a/src/base/tl/threading.h +++ b/src/base/tl/threading.h @@ -45,8 +45,8 @@ public: CLock(const CLock &) = delete; - void Take() ACQUIRE(m_Lock) { lock_wait(m_Lock); } - void Release() RELEASE() { lock_unlock(m_Lock); } + void Take() ACQUIRE(m_Lock) REQUIRES(!m_Lock) { lock_wait(m_Lock); } + void Release() RELEASE() REQUIRES(m_Lock) { lock_unlock(m_Lock); } }; class CScopeLock diff --git a/src/engine/client/serverbrowser_http.cpp b/src/engine/client/serverbrowser_http.cpp index beeb74673..6850c19b1 100644 --- a/src/engine/client/serverbrowser_http.cpp +++ b/src/engine/client/serverbrowser_http.cpp @@ -54,13 +54,13 @@ private: std::shared_ptr m_pData; std::unique_ptr m_pHead PT_GUARDED_BY(m_Lock); std::unique_ptr m_pGet PT_GUARDED_BY(m_Lock); - void Run() override; + void Run() override REQUIRES(!m_Lock); public: CJob(std::shared_ptr pData) : m_pData(std::move(pData)) { m_Lock = lock_create(); } virtual ~CJob() { lock_destroy(m_Lock); } - void Abort(); + void Abort() REQUIRES(!m_Lock); }; IEngine *m_pEngine; diff --git a/src/engine/client/updater.h b/src/engine/client/updater.h index 788327489..591605fef 100644 --- a/src/engine/client/updater.h +++ b/src/engine/client/updater.h @@ -65,15 +65,15 @@ class CUpdater : public IUpdater bool ReplaceClient(); bool ReplaceServer(); - void SetCurrentState(int NewState); + void SetCurrentState(int NewState) REQUIRES(!m_Lock); public: CUpdater(); ~CUpdater(); - int GetCurrentState() override; - void GetCurrentFile(char *pBuf, int BufSize) override; - int GetCurrentPercent() override; + int GetCurrentState() override REQUIRES(!m_Lock); + void GetCurrentFile(char *pBuf, int BufSize) override REQUIRES(!m_Lock); + int GetCurrentPercent() override REQUIRES(!m_Lock); void InitiateUpdate() override; void Init(); diff --git a/src/engine/client/video.cpp b/src/engine/client/video.cpp index 235545229..9789f1fcd 100644 --- a/src/engine/client/video.cpp +++ b/src/engine/client/video.cpp @@ -152,7 +152,7 @@ void CVideo::Start() for(size_t i = 0; i < m_VideoThreads; ++i) { std::unique_lock Lock(m_vVideoThreads[i]->m_Mutex); - m_vVideoThreads[i]->m_Thread = std::thread([this, i]() { RunVideoThread(i == 0 ? (m_VideoThreads - 1) : (i - 1), i); }); + m_vVideoThreads[i]->m_Thread = std::thread([this, i]() REQUIRES(!g_WriteLock) { RunVideoThread(i == 0 ? (m_VideoThreads - 1) : (i - 1), i); }); m_vVideoThreads[i]->m_Cond.wait(Lock, [this, i]() -> bool { return m_vVideoThreads[i]->m_Started; }); } @@ -164,7 +164,7 @@ void CVideo::Start() for(size_t i = 0; i < m_AudioThreads; ++i) { std::unique_lock Lock(m_vAudioThreads[i]->m_Mutex); - m_vAudioThreads[i]->m_Thread = std::thread([this, i]() { RunAudioThread(i == 0 ? (m_AudioThreads - 1) : (i - 1), i); }); + m_vAudioThreads[i]->m_Thread = std::thread([this, i]() REQUIRES(!g_WriteLock) { RunAudioThread(i == 0 ? (m_AudioThreads - 1) : (i - 1), i); }); m_vAudioThreads[i]->m_Cond.wait(Lock, [this, i]() -> bool { return m_vAudioThreads[i]->m_Started; }); } diff --git a/src/engine/client/video.h b/src/engine/client/video.h index b326ebf70..e16f1ecb1 100644 --- a/src/engine/client/video.h +++ b/src/engine/client/video.h @@ -50,7 +50,7 @@ public: CVideo(class CGraphics_Threaded *pGraphics, class ISound *pSound, class IStorage *pStorage, class IConsole *pConsole, int width, int height, const char *name); ~CVideo(); - void Start() override; + void Start() override REQUIRES(!g_WriteLock); void Stop() override; void Pause(bool Pause) override; bool IsRecording() override { return m_Recording; } @@ -66,11 +66,11 @@ public: static void Init() { av_log_set_level(AV_LOG_DEBUG); } private: - void RunVideoThread(size_t ParentThreadIndex, size_t ThreadIndex); - void FillVideoFrame(size_t ThreadIndex); + void RunVideoThread(size_t ParentThreadIndex, size_t ThreadIndex) REQUIRES(!g_WriteLock); + void FillVideoFrame(size_t ThreadIndex) REQUIRES(!g_WriteLock); void ReadRGBFromGL(size_t ThreadIndex); - void RunAudioThread(size_t ParentThreadIndex, size_t ThreadIndex); + void RunAudioThread(size_t ParentThreadIndex, size_t ThreadIndex) REQUIRES(!g_WriteLock); void FillAudioFrame(size_t ThreadIndex); bool OpenVideo(); diff --git a/src/engine/shared/jobs.h b/src/engine/shared/jobs.h index b28b2476a..a6378df86 100644 --- a/src/engine/shared/jobs.h +++ b/src/engine/shared/jobs.h @@ -51,7 +51,7 @@ class CJobPool std::shared_ptr m_pFirstJob GUARDED_BY(m_Lock); std::shared_ptr m_pLastJob GUARDED_BY(m_Lock); - static void WorkerThread(void *pUser); + static void WorkerThread(void *pUser) NO_THREAD_SAFETY_ANALYSIS; public: CJobPool(); @@ -59,7 +59,7 @@ public: void Init(int NumThreads); void Destroy(); - void Add(std::shared_ptr pJob); + void Add(std::shared_ptr pJob) REQUIRES(!m_Lock); static void RunBlocking(IJob *pJob); }; #endif