Skip to content

Fix thread-safety warnings in Mutex::lock/unlock for newer clang - #4

Open
mturnpaugh wants to merge 564 commits into
mainfrom
build/updateCmakeVersion
Open

Fix thread-safety warnings in Mutex::lock/unlock for newer clang#4
mturnpaugh wants to merge 564 commits into
mainfrom
build/updateCmakeVersion

Conversation

@mturnpaugh

Copy link
Copy Markdown

Problem

After upgrading to CMake 4.4.0 (which bundles AppleClang 17+), the pipeline started failing with:

src/mutex.h:79: error: mutex 'mut_' is still held at the end of function [-Werror,-Wthread-safety-analysis]
  void lock() ACQUIRE() { mut_.lock(); }
src/mutex.h:80: error: releasing mutex 'mut_' that was not held [-Werror,-Wthread-safety-analysis]
  void unlock() RELEASE() { mut_.unlock(); }

Root Cause

The Mutex class is declared as a CAPABILITY("mutex") wrapper. Its lock() and unlock() methods are annotated with ACQUIRE() and RELEASE() (no argument = acquires/releases *this).

The issue is that std::mutex::lock() and std::mutex::unlock() in libc++ are themselves annotated with clang thread-safety attributes (acquire_capability / release_capability). When Mutex::lock() calls mut_.lock(), clang's thread-safety analysis tracks mut_ (the inner std::mutex) as being acquired — but the outer function is annotated as acquiring *this (the Mutex capability), not mut_. Clang sees a std::mutex acquired and never released within lock(), producing the error.

Newer clang enforces this more strictly, which is why it worked before and breaks now.

Fix

Call pthread_mutex_lock / pthread_mutex_unlock directly via mut_.native_handle() instead of going through std::mutex::lock() / unlock(). The pthread_* functions have no clang thread-safety annotations, so the analysis does not track mut_ as a separate capability being acquired. The behavior is identical at runtime — std::mutex::native_handle() returns the underlying pthread_mutex_t*.

The ACQUIRE() / RELEASE() annotations on the public API are preserved, so callers of Mutex are still checked correctly by the thread-safety analysis.

Why other approaches didn't work

  • ACQUIRE(mut_) — wrong: annotates the function as acquiring the inner std::mutex object rather than the Mutex capability itself, which confuses callers.
  • NO_THREAD_SAFETY_ANALYSIS on the function — insufficient: clang's thread-safety analysis is function-scoped and still reports the held lock at end of function.
  • _Pragma("clang diagnostic ignored") — insufficient: suppresses the inline diagnostic but not the end-of-function analysis report.
  • Private helper methods with NO_THREAD_SAFETY_ANALYSIS — same problem; the inner std::mutex annotations still fire from any calling context.

dmah42 and others added 30 commits February 11, 2025 21:10
* [clang-tidy] fix warning about decaying array to pointer

* fix a different warning (old style cast)

* use string_view instead of old-style const char* strings

* ensure bazel windows is using c++17

* learn to use bazel

* and tests

* precommit fix

* more string_view creation and casting

* format

* format

* [clang-tidy] use unique_ptr for benchmark registration (google#1927)

* use unique_ptr for benchmark registration
* [clang-tidy] fix missing braces
* [clang-tidy] autofix cppcoreguidelines

* better than automation maybe
* [clang-tidy] autofix readability issues

* more modern clang format
…gle#1937)

The changes are an autofix added in Buildifier 8.0.1, designed to future-proof
Bazel projects against the eventual removal of these rules from the native
Bazel namespace.
No new functionality, just an update to the newest nanobind.

Co-authored-by: dominic <510002+dmah42@users.noreply.github.com>
* use smart pointers
* use vectors
* size_t
* [clang-tidy] resolve some global clang-tidy issues

* more nolint and some global fixes
Change type of callbacks to take `std::function`
There is no upstream PFM cmake package config file to use, so this has
to be installed for the benchmark cmake package config file to work.

Bug: https://bugs.gentoo.org/950573
See-Also: c214639

Signed-off-by: Alfred Wingate <parona@protonmail.com>
* fix memory manager result bug

* change is_valid to memory_iterations

* fix test

* some fixes

* fix test

...for msvc

* fix test

* fix test

add the correct explicitly casts

* fix msvc failure

* some fixes

* remove unnecessary include
... because that is what the MSan is built against,
and mixing them clearly causes issues.
This ends up being *much* faster, noticeably speeding up these jobs.
…google#1864)

* update version of gtest to v1.15.2 (latest) and also the cmake configuration to avoid deprecation warnings

* `cmake/GoogleTest.cmake.in`: do a shallow clone of google test

---------

Co-authored-by: dominic <510002+dmah42@users.noreply.github.com>
Co-authored-by: Roman Lebedev <lebedev.ri@gmail.com>
* `CMakeLists.txt`: drop hopefully obsolete code

* README.md: update

* Unbreak `BENCHMARK_HAS_CXX11` macro

835365f stopped defining it,
but didn't un-conditionalize the code guarded under it...

* Drop `BENCHMARK_HAS_NO_VARIADIC_REGISTER_BENCHMARK`

We no longer support such an old gcc version

* `docs/user_guide.md`: proofread

* Add a test to ensure that `benchmark.h` remains C++14 header

* Revert `[[maybe_unused]]` changes - it requires C++17

* Also support C++11 standard for using the library

I don't think we want to support C++03 though,
but i suppose C++11 is palatable, at least right now.
* ThreadManager::WaitForAllThreads removed

* WaitForAllThreads was only called either in single threaded
  environments or just before all threads are joined anyway. As this
  doesn't add a useful synchronization point, it's removed.

* Formatting issue

* Thread Sanitizer satisfied

* More formatting issues
Refactor the multi-threading api to support
using custom user-provided thread factory
instead of always spawning POSIX Threads.
Builds bindings with nanobind v2.7.0, which contains a few bug fixes and improvements.
getloadavg returns with -1 if cannot obtain load average, but the
current source casts the return value to size_t right away. The cast
result for such a case is probably maximum unsigned long int, so the
resizing of the res vector is certainly going to fail.

This change keeps the original return type of getloadavg and casts it
just before the resizing of the res vector.
dependabot Bot and others added 23 commits June 22, 2026 12:01
Bumps [scipy](https://github.com/scipy/scipy) from 1.17.1 to 1.18.0.
- [Release notes](https://github.com/scipy/scipy/releases)
- [Commits](scipy/scipy@v1.17.1...v1.18.0)

---
updated-dependencies:
- dependency-name: scipy
  dependency-version: 1.18.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [lukka/get-cmake](https://github.com/lukka/get-cmake) from 4.3.3 to 4.3.4.
- [Release notes](https://github.com/lukka/get-cmake/releases)
- [Changelog](https://github.com/lukka/get-cmake/blob/main/RELEASE_PROCESS.md)
- [Commits](lukka/get-cmake@591817e...f5b8fbb)

---
updated-dependencies:
- dependency-name: lukka/get-cmake
  dependency-version: 4.3.4
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: dominic <510002+dmah42@users.noreply.github.com>
Bumps [numpy](https://github.com/numpy/numpy) from 2.4.6 to 2.5.0.
- [Release notes](https://github.com/numpy/numpy/releases)
- [Changelog](https://github.com/numpy/numpy/blob/main/doc/RELEASE_WALKTHROUGH.rst)
- [Commits](numpy/numpy@v2.4.6...v2.5.0)

---
updated-dependencies:
- dependency-name: numpy
  dependency-version: 2.5.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [actions/cache](https://github.com/actions/cache) from 5.0.5 to 6.0.0.
- [Release notes](https://github.com/actions/cache/releases)
- [Changelog](https://github.com/actions/cache/blob/main/RELEASES.md)
- [Commits](actions/cache@27d5ce7...2c8a9bd)

---
updated-dependencies:
- dependency-name: actions/cache
  dependency-version: 6.0.0
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [actions/setup-python](https://github.com/actions/setup-python) from 6.2.0 to 6.3.0.
- [Release notes](https://github.com/actions/setup-python/releases)
- [Commits](actions/setup-python@a309ff8...ece7cb0)

---
updated-dependencies:
- dependency-name: actions/setup-python
  dependency-version: 6.3.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [actions/cache](https://github.com/actions/cache) from 6.0.0 to 6.1.0.
- [Release notes](https://github.com/actions/cache/releases)
- [Changelog](https://github.com/actions/cache/blob/main/RELEASES.md)
- [Commits](actions/cache@2c8a9bd...55cc834)

---
updated-dependencies:
- dependency-name: actions/cache
  dependency-version: 6.1.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…2237)

The Rust binding's initialize() creates temporary CString buffers that are freed on return, but benchmark::Initialize() stores argv[0] as a raw pointer in BenchmarkReporter::Context::executable_name. Later reporter calls dereference freed heap memory (use-after-free).

Copy argv[0] into static storage in the C++ bridge before calling benchmark::Initialize(), ensuring the retained pointer remains valid for the process lifetime.

AI assistance disclosure: AI tooling was used to assist with auditing, reproducer design, patch drafting, and PR text preparation.
* docs: document Bazel integration

* fix(docs): avoid pinning current benchmark version
Zizmor is going to be automatically installed in this repo shortly. To
avoid any churn, this is a pre-run to fix up any potential
vulnerabilities identified by the tool.
* fix(linux): prevent infinite re-exec loop under AppArmor

The `MaybeReenterWithoutASLR()` function in `src/benchmark.cc` caused infinite execv() loops when running benchmarks under AppArmor-enabled LSMs. The existing fix from google#1985 only checked whether personality(ADDR_NO_RANDOMIZE) succeeded in the current process before calling execv(). However, some LSMs like AppArmor can silently reset personality flags during the execve() system call transition, even though the flag was successfully set in the parent process.
Additionally, ensure that we actually report that ASLR is still on in case we fail to unset it.

Signed-off-by: anish <anishesg@users.noreply.github.com>
Co-authored-by: anish <anishesg@users.noreply.github.com>
Co-authored-by: Roman Lebedev <lebedev.ri@gmail.com>
Bumps [astral-sh/setup-uv](https://github.com/astral-sh/setup-uv) from 8.2.0 to 8.3.0.
- [Release notes](https://github.com/astral-sh/setup-uv/releases)
- [Commits](astral-sh/setup-uv@fac544c...d31148d)

---
updated-dependencies:
- dependency-name: astral-sh/setup-uv
  dependency-version: 8.3.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [numpy](https://github.com/numpy/numpy) from 2.5.0 to 2.5.1.
- [Release notes](https://github.com/numpy/numpy/releases)
- [Changelog](https://github.com/numpy/numpy/blob/main/doc/RELEASE_WALKTHROUGH.rst)
- [Commits](numpy/numpy@v2.5.0...v2.5.1)

---
updated-dependencies:
- dependency-name: numpy
  dependency-version: 2.5.1
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: dominic <510002+dmah42@users.noreply.github.com>
Bumps [astral-sh/setup-uv](https://github.com/astral-sh/setup-uv) from 8.3.0 to 8.3.1.
- [Release notes](https://github.com/astral-sh/setup-uv/releases)
- [Commits](astral-sh/setup-uv@d31148d...f98e069)

---
updated-dependencies:
- dependency-name: astral-sh/setup-uv
  dependency-version: 8.3.1
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [astral-sh/setup-uv](https://github.com/astral-sh/setup-uv) from 8.3.1 to 8.3.2.
- [Release notes](https://github.com/astral-sh/setup-uv/releases)
- [Commits](astral-sh/setup-uv@f98e069...11f9893)

---
updated-dependencies:
- dependency-name: astral-sh/setup-uv
  dependency-version: 8.3.2
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Make --benchmark_list_tests respect --benchmark_format via reporter List() (google#1642)

* Export FindBenchmarksInternal for shared-library test link; fix include order
This sysctl override is only used on some OS'es
so it causes compile error with latest LLVM.
Bumps [lukka/get-cmake](https://github.com/lukka/get-cmake) from 4.3.4 to 4.4.0.
- [Release notes](https://github.com/lukka/get-cmake/releases)
- [Changelog](https://github.com/lukka/get-cmake/blob/main/RELEASE_PROCESS.md)
- [Commits](lukka/get-cmake@f5b8fbb...e690607)

---
updated-dependencies:
- dependency-name: lukka/get-cmake
  dependency-version: 4.4.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: dominic <510002+dmah42@users.noreply.github.com>
Bumps [actions/setup-python](https://github.com/actions/setup-python) from 6.3.0 to 7.0.0.
- [Release notes](https://github.com/actions/setup-python/releases)
- [Commits](actions/setup-python@ece7cb0...5fda3b9)

---
updated-dependencies:
- dependency-name: actions/setup-python
  dependency-version: 7.0.0
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: dominic <510002+dmah42@users.noreply.github.com>
Bumps [actions/checkout](https://github.com/actions/checkout) from 7.0.0 to 7.0.1.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@9c091bb...3d3c42e)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-version: 7.0.1
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: dominic <510002+dmah42@users.noreply.github.com>
Bumps [pypa/gh-action-pypi-publish](https://github.com/pypa/gh-action-pypi-publish) from 1.14.0 to 1.14.1.
- [Release notes](https://github.com/pypa/gh-action-pypi-publish/releases)
- [Commits](pypa/gh-action-pypi-publish@cef2210...ba38be9)

---
updated-dependencies:
- dependency-name: pypa/gh-action-pypi-publish
  dependency-version: 1.14.1
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: dominic <510002+dmah42@users.noreply.github.com>
@mturnpaugh
mturnpaugh force-pushed the build/updateCmakeVersion branch 4 times, most recently from 0cf42df to 35b4bdf Compare July 28, 2026 20:33
AppleClang 17+ with -Wthread-safety-analysis errors on Mutex::lock/unlock
because std::mutex::lock() is annotated with acquire_capability in libc++.
Clang sees mut_ acquired inside lock() (annotated ACQUIRE() for *this),
causing a held-at-end-of-function error.

Fix by calling pthread_mutex_lock/unlock directly under __clang__, bypassing
std::mutex's clang thread-safety annotations. Behavior is identical at runtime
— std::mutex::native_handle() returns the underlying pthread_mutex_t*.

MSVC does not support -Wthread-safety and does not have pthread.h, so it
continues to use std::mutex::lock/unlock unchanged.
@mturnpaugh
mturnpaugh force-pushed the build/updateCmakeVersion branch from 35b4bdf to fffeac7 Compare July 28, 2026 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.