Detect thread safety attribute support in mutex.h (#2263)

[devtejasx]

The thread safety annotations were only enabled when the surrounding build system defined HAVE_THREAD_SAFETY_ATTRIBUTES, but nothing in the tree ever defines it. Build systems that turn on -Wthread-safety by default -- Bazel does on macOS -- therefore analyzed Mutex/MutexLock with every annotation erased. Where the standard library annotates std::mutex itself (libc++), that makes the analysis conclude that Mutex::lock() leaks the lock and that Mutex::unlock() releases a lock that was never held, which is a hard error under -Werror:

src/mutex.h:79:40: error: mutex 'mut_' is still held at the end of function
src/mutex.h:80:34: error: releasing mutex 'mut_' that was not held

Enable the annotations whenever the compiler actually supports them, probed with __has_attribute(capability), so the wrappers are analyzed with the contract they are meant to have. An explicit HAVE_THREAD_SAFETY_ATTRIBUTES from the build system still takes precedence, and
BENCHMARK_DISABLE_THREAD_SAFETY_ATTRIBUTES forces the previous no-op behaviour back.

Verified with clang 21 + libc++ (reproduces before, clean after), clang 21 + libstdc++, gcc 15 and MSVC; the full library builds clean under -Wall -Wextra -Wthread-safety -Werror with clang.

---

Per AGENTS.md: AI-assisted — the patch was drafted with AI assistance
(Claude) and then reviewed, tested, and understood by me. I take full
responsibility for it.

[dmah42]

but nothing has been changed in the build system so this is not "instead of" is it?

[devtejasx]

Correct — nothing in the build system changes, and the wording was wrong. Reworded to state the situation plainly rather than describe it as a replacement:

HAVE_THREAD_SAFETY_ATTRIBUTES is left to whoever compiles the library, and nothing in this tree defines it. That is fine as long as -Wthread-safety is off. Bazel turns it on by default, and there the wrappers get analysed with their own annotations erased: Mutex::lock() is then seen acquiring mut_ and never releasing it, which is an error wherever the standard library annotates std::mutex itself (libc++, hence macOS in #2263).

[dmah42]

i'm a bit confused. if Bazel turns it on, then the macro HAVE_THREAD_SAFETY_ATTRIBUTES should be enabled. or is this trying to suggest that Bazel turns it on but doesn't enable the macro?

either way, i think this whole comment should be focused on why we need to do this check here (and not, say, in the build system: if we know Bazel enables it then why not set the cc_defines in the build script?)

[LebedevRI]

(I think something like this overall diff should be done.)

Is there any compiler other than clang that supports TSA anyway?
Can we just check for clang, and perhaps do a version check?

[LebedevRI]

(to be more specific, i strongly suspect that __has_attribute(capability) is bogus/unsufficient,
that only covers CAPABILITY(), what about others? Can we just assume they are implied by it?)

[devtejasx]

Both addressed.

@LebedevRI — dropped __has_attribute(capability); you're right that probing one attribute says nothing about the other twenty, and clang is the only compiler that implements any of them. It is now just:

#if !defined(HAVE_THREAD_SAFETY_ATTRIBUTES) && defined(__clang__)
#define HAVE_THREAD_SAFETY_ATTRIBUTES
#endif

No version check: the attributes used here have all been in clang since 3.x, well below anything this project supports.

@dmah42 — on the confusion: yes, Bazel turns on the warning without defining the macro. Its default C++ toolchain adds -Wthread-safety whenever the compiler accepts it (rules_cc), and nothing tells it about HAVE_THREAD_SAFETY_ATTRIBUTES. So the wrappers get analysed with their annotations erased, which is only an error where the standard library annotates std::mutex itself — libc++, hence macOS and not Linux.

On setting it from BUILD.bazel instead: that would fix the Bazel build, but the same BUILD.bazel also has to serve gcc, which rejects the attributes, so it would need the compiler condition anyway. Since it is a property of the compiler rather than of the build, keeping the decision next to the attributes means every build system gets it right without knowing about the macro. The comment now says that instead of what it said before.

Verified against clang 21 + libc++ (reproduces before, clean after), clang 21 + libstdc++, gcc 15 and MSVC; the library still builds clean under -Wall -Wextra -Wthread-safety -Werror.

[dmah42]

i would have thought https://bazel.build/docs/configurable-attributes would be the right way to solve it for compiler.

[devtejasx]

That works for the Bazel build, and if it is what you want I will send it — @rules_cc//cc/compiler:clang gives the config_setting, and it would be a select() on local_defines.

The reason I did not: -Wthread-safety is not Bazel-specific. Any consumer can turn it on through any build system, and CMake has the same hole today — cmake -DCMAKE_CXX_FLAGS=-Wthread-safety with clang reproduces #2263 with no Bazel involved. Fixing it per build file means fixing it once per build system and keeping each one in step with the compiler check, whereas whether the annotations are valid is a property of the compiler alone. That is also what @LebedevRI suggested above ("Can we just check for clang").

So the trade-off as I see it: one line in the header that every build system inherits, versus a select() that fixes the reported case and leaves the others. Happy to switch — or to add the select() on top, if you would rather Bazel state it explicitly as well. Which would you prefer?