[Clang] Warn when `std::atomic_thread_fence` is used with `fsanitize=thread` #166542

BStott6 · 2025-11-05T12:09:58Z

ThreadSanitizer currently does not support std::atomic_thread_fence, leading to false positives: ThreadSanitizer: false report about data race #52942.
GCC produces a warning when std::atomic_thread_fence is used with -fsanitize=thread while Clang doesn't.
This PR introduces a matching warning in Clang to avoid confusion as in the linked issue.

…thread`

github-actions · 2025-11-05T12:10:24Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2025-11-05T12:10:56Z

@llvm/pr-subscribers-clang

Author: None (BStott6)

Changes

ThreadSanitizer currently does not support std::atomic_thread_fence, leading to false positives: ThreadSanitizer: false report about data race #52942.
GCC produces a warning when std::atomic_thread_fence is used with -fsanitize=thread while Clang doesn't.
This PR introduces a matching warning in Clang to avoid confusion as in the linked issue.

Full diff: https://github.com/llvm/llvm-project/pull/166542.diff

5 Files Affected:

(modified) clang/include/clang/Basic/DiagnosticGroups.td (+2)
(modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+3)
(modified) clang/include/clang/Sema/Sema.h (+2)
(modified) clang/lib/Sema/SemaChecking.cpp (+34)
(added) clang/test/SemaCXX/warn-tsan-atomic-fence.cpp (+11)

diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 8aa3489a2a62b..489b9f5ec552b 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -1769,3 +1769,5 @@ def ExplicitSpecializationStorageClass : DiagGroup<"explicit-specialization-stor
 
 // A warning for options that enable a feature that is not yet complete
 def ExperimentalOption : DiagGroup<"experimental-option">;
+
+def TSan : DiagGroup<"tsan">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 4e369be0bbb92..928b52b3d41f0 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -112,6 +112,9 @@ def warn_max_unsigned_zero : Warning<
   "%select{a value and unsigned zero|unsigned zero and a value}0 "
   "is always equal to the other value">,
   InGroup<MaxUnsignedZero>;
+def warn_atomic_thread_fence_with_tsan : Warning<
+  "`std::atomic_thread_fence` is not supported with `-fsanitize=thread`">,
+  InGroup<TSan>;
 def note_remove_max_call : Note<
   "remove call to max function and unsigned zero argument">;
 
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index c67ed99b1f49e..99e486179dd2e 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3033,6 +3033,8 @@ class Sema final : public SemaBase {
 
   void CheckMaxUnsignedZero(const CallExpr *Call, const FunctionDecl *FDecl);
 
+  void CheckUseOfAtomicThreadFenceWithTSan(const CallExpr *Call, const FunctionDecl *FDecl);
+
   /// Check for dangerous or invalid arguments to memset().
   ///
   /// This issues warnings on known problematic, dangerous or unspecified
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index ad2c2e4a97bb9..13b61ea453f80 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -45,6 +45,7 @@
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/LangOptions.h"
+#include "clang/Basic/NoSanitizeList.h"
 #include "clang/Basic/OpenCLOptions.h"
 #include "clang/Basic/OperatorKinds.h"
 #include "clang/Basic/PartialDiagnostic.h"
@@ -4100,6 +4101,7 @@ bool Sema::CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall,
   CheckAbsoluteValueFunction(TheCall, FDecl);
   CheckMaxUnsignedZero(TheCall, FDecl);
   CheckInfNaNFunction(TheCall, FDecl);
+  CheckUseOfAtomicThreadFenceWithTSan(TheCall, FDecl);
 
   if (getLangOpts().ObjC)
     ObjC().DiagnoseCStringFormatDirectiveInCFAPI(FDecl, Args, NumArgs);
@@ -9822,6 +9824,38 @@ void Sema::CheckMaxUnsignedZero(const CallExpr *Call,
         << FixItHint::CreateRemoval(RemovalRange);
 }
 
+//===--- CHECK: Warn on use of `std::atomic_thread_fence` with TSan. ------===//
+void Sema::CheckUseOfAtomicThreadFenceWithTSan(const CallExpr *Call,
+                                               const FunctionDecl *FDecl) {
+  // Thread sanitizer currently does not support `std::atomic_thread_fence`,
+  // leading to false positive. Example issue:
+  // https://github.com/llvm/llvm-project/issues/52942
+
+  if (!Call || !FDecl)
+    return;
+
+  if (!IsStdFunction(FDecl, "atomic_thread_fence"))
+    return;
+
+  // See if TSan is enabled in this function
+  const auto EnabledTSanMask =
+      Context.getLangOpts().Sanitize.Mask & (SanitizerKind::Thread);
+  if (!EnabledTSanMask)
+    return;
+
+  const auto &NoSanitizeList = Context.getNoSanitizeList();
+  if (NoSanitizeList.containsLocation(EnabledTSanMask,
+                                      Call->getSourceRange().getBegin()))
+    // File is excluded
+    return;
+  if (NoSanitizeList.containsFunction(EnabledTSanMask,
+                                      FDecl->getQualifiedNameAsString()))
+    // Function is excluded
+    return;
+
+  Diag(Call->getExprLoc(), diag::warn_atomic_thread_fence_with_tsan);
+}
+
 //===--- CHECK: Standard memory functions ---------------------------------===//
 
 /// Takes the expression passed to the size_t parameter of functions
diff --git a/clang/test/SemaCXX/warn-tsan-atomic-fence.cpp b/clang/test/SemaCXX/warn-tsan-atomic-fence.cpp
new file mode 100644
index 0000000000000..b8720260fe1c1
--- /dev/null
+++ b/clang/test/SemaCXX/warn-tsan-atomic-fence.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang -std=c++17 %s 2>&1 | FileCheck %s --check-prefix=NO-TSAN --allow-empty
+// RUN: %clang -std=c++17 -fsanitize=thread %s 2>&1 | FileCheck %s --check-prefix=WITH-TSAN
+
+// WITH-TSAN: `std::atomic_thread_fence` is not supported with `-fsanitize=thread`
+// NO-TSAN-NOT: `std::atomic_thread_fence` is not supported with `-fsanitize=thread`
+
+#include <atomic>
+
+int main() {
+    std::atomic_thread_fence(std::memory_order::memory_order_relaxed);
+}

gbMattN · 2025-11-05T14:15:53Z

clang/include/clang/Basic/DiagnosticGroups.td

 // A warning for options that enable a feature that is not yet complete
 def ExperimentalOption : DiagGroup<"experimental-option">;
+
+def TSan : DiagGroup<"tsan">;


Perhaps this could be renamed to be more descriptive?
Address sanitizer on line 1599 has

// AddressSanitizer frontend instrumentation remarks. def SanitizeAddressRemarks : DiagGroup<"sanitize-address">;

The string is "tsan" to match GCC, I agree about renaming the record though
How about ThreadSanitizerWarnings?

Hmm, unless you can think of any reason not to, I think copying the ASan style exactly, and maybe moving your change to be next to it to logically group them together would be better. Keeping consistency with llvm over gcc seems like it makes sense.

Given that this is only needed for a single diagnostic, I would skip making a group entirely and just use an inline group in DiagnosticSemaKinds.td. As for the string we expose for the group name, I think consistency with GCC is more useful; the sanitize-address group is for controlling remarks rather than warnings anyway.

AaronBallman

Thank you for working on this! Please be sure to also add a release note to clang/docs/ReleaseNotes.rst so users know about the new diagnostic.

AaronBallman · 2025-11-05T15:47:28Z

clang/include/clang/Basic/DiagnosticSemaKinds.td

  "is always equal to the other value">,
  InGroup<MaxUnsignedZero>;
+def warn_atomic_thread_fence_with_tsan : Warning<
+  "`std::atomic_thread_fence` is not supported with `-fsanitize=thread`">,


Suggested change

"`std::atomic_thread_fence` is not supported with `-fsanitize=thread`">,

"'std::atomic_thread_fence' is not supported with '-fsanitize=thread'">,

Using straight quote instead of backtick

AaronBallman · 2025-11-05T15:49:55Z

clang/include/clang/Basic/DiagnosticGroups.td

 // A warning for options that enable a feature that is not yet complete
 def ExperimentalOption : DiagGroup<"experimental-option">;
+
+def TSan : DiagGroup<"tsan">;


Given that this is only needed for a single diagnostic, I would skip making a group entirely and just use an inline group in DiagnosticSemaKinds.td. As for the string we expose for the group name, I think consistency with GCC is more useful; the sanitize-address group is for controlling remarks rather than warnings anyway.

AaronBallman · 2025-11-05T15:53:08Z

clang/test/SemaCXX/warn-tsan-atomic-fence.cpp

+// WITH-TSAN: `std::atomic_thread_fence` is not supported with `-fsanitize=thread`
+// NO-TSAN-NOT: `std::atomic_thread_fence` is not supported with `-fsanitize=thread`
+
+#include <atomic>


Please do not include system headers; that makes the test non-hermetic. Instead, manually add just the declarations needed for the test case.

Some other test cases to add: use of no_sanitize("thread") and no_sanitize_thread attributes on the function using atomic_thread_fence.

AaronBallman · 2025-11-05T15:54:22Z

clang/test/SemaCXX/warn-tsan-atomic-fence.cpp

+// RUN: %clang -std=c++17 %s 2>&1 | FileCheck %s --check-prefix=NO-TSAN --allow-empty
+// RUN: %clang -std=c++17 -fsanitize=thread %s 2>&1 | FileCheck %s --check-prefix=WITH-TSAN
+
+// WITH-TSAN: `std::atomic_thread_fence` is not supported with `-fsanitize=thread`
+// NO-TSAN-NOT: `std::atomic_thread_fence` is not supported with `-fsanitize=thread`


Suggested change

// RUN: %clang -std=c++17 %s 2>&1 | FileCheck %s --check-prefix=NO-TSAN --allow-empty

// RUN: %clang -std=c++17 -fsanitize=thread %s 2>&1 | FileCheck %s --check-prefix=WITH-TSAN

// WITH-TSAN: `std::atomic_thread_fence` is not supported with `-fsanitize=thread`

// NO-TSAN-NOT: `std::atomic_thread_fence` is not supported with `-fsanitize=thread`

// RUN: %clang -std=c++17 %s 2>&1 | FileCheck %s --check-prefix=NO-TSAN --allow-empty

// RUN: %clang -std=c++17 -fsanitize=thread %s 2>&1 | FileCheck %s --check-prefix=WITH-TSAN

// WITH-TSAN: `std::atomic_thread_fence` is not supported with `-fsanitize=thread`

// NO-TSAN-NOT: `std::atomic_thread_fence` is not supported with `-fsanitize=thread`

These should be running %clang_cc1 so we're not executing the driver and the frontend. Also, please use -verify instead of | FileCheck %s to verify the diagnostics appear on the expected line. You can use -verify= to give different RUN lines a different prefix so you can show that the diagnostic is only emitted for some RUN lines.

…nitize attributes

BStott6 · 2025-11-06T10:59:40Z

Thanks for the feedback, I have implemented your suggestions
In upgrading the test I found I had made a mistake in how I was handling the calling function being in the ignorelist and also had forgot to handle the no-sanitize attributes, I have implemented this and made sure the test checks for it

github-actions · 2025-11-06T16:00:34Z

✅ With the latest revision this PR passed the C/C++ code formatter.

AaronBallman · 2025-11-06T16:12:30Z

clang/lib/Sema/SemaChecking.cpp

+    if (NoSanitizeList.containsFunction(EnabledTSanMask,
+                                        Caller->getQualifiedNameAsString()))


We should have a test that this works. e.g.,

struct S { void func(int) { std::atomic_thread_fence(std::memory_order_relaxed); } void func(float) { std::atomic_thread_fence(std::memory_order_relaxed); } };

(I don't know the answer to this, but should the ignore list have S::func and silence both, or does it support something like S::func(float) to silence one but not the other? Or perhaps use a mangled name instead of the qualified name? Does the qualified name need to support ::foo::bar as well as foo::bar?)

It seems function names in ignore lists should be mangled names, so my code's use of getQualifiedName() is wrong.

AaronBallman · 2025-11-06T16:12:40Z

clang/test/SemaCXX/warn-tsan-atomic-fence.cpp

+
+int main() {
+  std::atomic_thread_fence(std::memory_order_relaxed); // with-tsan-warning {{'std::atomic_thread_fence' is not supported with '-fsanitize=thread'}}
+}


I think other tests that would be interesting to try out would be:

__attribute__((no_sanitize("thread"))) void func() { auto lam = [](){ std::atomic_thread_fence(std::memory_order_relaxed); // This should still diagnose, right? }; } void other() { auto lam = []() __attribute__((no_sanitize("thread"))) { std::atomic_thread_fence(std::memory_order_relaxed); // This should not diagnose, right? }; }

and we probably should document that there can be false positives, e.g.,

inline void inline_func() { std::atomic_thread_fence(std::memory_order_relaxed); // Still diagnosed even though it's an inline function } __attribute__((no_sanitize("thread"))) void caller() { inline_func(); if (0) { std::atomic_thread_fence(std::memory_order_relaxed); // Still diagnosed even though it's unreachable } }

The lambda cases exposed another oversight in my code (it does not take into account the attributes applied to the lambda directly, fixed by also checking the attributes of getCurFunctionDecl(/*AllowLambdas*/ true)
Interestingly, GCC does not emit the warning in the if (0) case; it seems to do some reachability check - should I try to match this behaviour?

The lambda cases exposed another oversight in my code (it does not take into account the attributes applied to the lambda directly, fixed by also checking the attributes of getCurFunctionDecl(/*AllowLambdas*/ true) Interestingly, GCC does not emit the warning in the if (0) case; it seems to do some reachability check - should I try to match this behaviour?

I'd say let's try to land this without any control flow analysis and see whether the false positive rate is acceptable or not. We do have CFG-based diagnostics, but they're off-by-default due to the compile time overhead.

…up in ignore list

BStott6 · 2025-11-07T17:42:38Z

I wasn't sure how I should write the test for the name mangling for the ignore list lookup since it is platform dependent. Should I make two tests that use REQUIRES and include mangled symbols for that platform?

I have verified locally that it works, just not sure how best to formalise the test

gbMattN

The changes I requested are done- I'd wait for an OK from Aaron too before merging though

AaronBallman

Precommit CI found a relevant failure:

# .---command stderr------------
  # | error: 'no-tsan-warning' diagnostics seen but not expected: 
  # |   File C:\_work\llvm-project\llvm-project\clang\test\SemaCXX\warn-tsan-atomic-fence.cpp Line 33: 'std::atomic_thread_fence' is not supported with '-fsanitize=thread'
  # | 1 error generated.
  # `-----------------------------

I wasn't sure how I should write the test for the name mangling for the ignore list lookup since it is platform dependent. Should I make two tests that use REQUIRES and include mangled symbols for that platform?

I would have two RUN lines in the test, one using -triple %itanium_abi_triple and the other using -triple x86_64-windows-msvc, and you can use FileCheck --check-prefix=whatever to give each RUN line a unique prefix to use with the CHECK lines.

ojhunt · 2025-11-13T22:30:07Z

clang/lib/Sema/SemaChecking.cpp

+    return;
+
+  // Check that TSan is enabled in this context
+  const auto EnabledTSanMask =


Sorry this is a pedant/nit - rather than auto you should use the actual type. As I understand it this is the preferred model unless the RHS includes the target type explicitly (e.g X.getAsSomeType(), cast<SomeTYpe>, etc

Thanks, I'm quite new to LLVM so code style tips like this are very appreciated, I will change it

ojhunt · 2025-11-13T22:38:59Z

clang/lib/Sema/SemaChecking.cpp

+  if (IsNotSanitized(getCurFunctionOrMethodDecl()))
+    return;
+  if (IsNotSanitized(getCurFunctionDecl(/*AllowLambdas*/ true)))
+    return;


Rather than this, you should just update getCurFunctionOfMethodDecl() to getCurFunctionOrMethodDecl(bool AllowsLambdas = false)

and

- NamedDecl *Sema::getCurFunctionOrMethodDecl() const { - DeclContext *DC = getFunctionLevelDeclContext(); + NamedDecl *Sema::getCurFunctionOrMethodDecl(bool AllowsLambdas) const { + DeclContext *DC = getFunctionLevelDeclContext(AllowsLambdas);

Then this simply turns into a single

if (!IsNotSanitized(getCurFunctionOrMethodDecl(/*AllowsLambdas=*/true)) return;

Which also saves duplicate walks of the context

Sounds like a good improvement, I'll do it

@ojhunt I think we actually do need to check with and without lambdas separately, due to the case of a non-ignored lambda in an ignored function. I think one would expect the warning to be ignored as the enclosing function is ignored, but if we only check once and include lambdas, it won't pick up the function attribute. We can only do the "AllowLambda = false" check if the first check returns a lambda

[Clang] Warn when std::atomic_thread_fence is used with `fsanitize=…

8b4ff4f

…thread`

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Nov 5, 2025

gbMattN requested review from AaronBallman and ojhunt November 5, 2025 14:13

gbMattN requested changes Nov 5, 2025

View reviewed changes

AaronBallman reviewed Nov 5, 2025

View reviewed changes

Implement suggestions, fix handling of ignored functions, handle nosa…

2558549

…nitize attributes

AaronBallman reviewed Nov 6, 2025

View reviewed changes

BStott6 added 2 commits November 6, 2025 16:44

Fix formatting mistake

30835da

Improve test, account for lambda attributes, use mangled name to look…

8a1b662

…up in ignore list

gbMattN approved these changes Nov 13, 2025

View reviewed changes

AaronBallman reviewed Nov 13, 2025

View reviewed changes

ojhunt requested changes Nov 13, 2025

View reviewed changes

	"`std::atomic_thread_fence` is not supported with `-fsanitize=thread`">,
	"'std::atomic_thread_fence' is not supported with '-fsanitize=thread'">,

		if (NoSanitizeList.containsFunction(EnabledTSanMask,
		Caller->getQualifiedNameAsString()))

[Clang] Warn when std::atomic_thread_fence is used with fsanitize=thread #166542

Are you sure you want to change the base?

[Clang] Warn when std::atomic_thread_fence is used with fsanitize=thread #166542

Conversation

BStott6 commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 5, 2025

Uh oh!

llvmbot commented Nov 5, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BStott6 Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gbMattN Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AaronBallman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BStott6 commented Nov 6, 2025

Uh oh!

github-actions bot commented Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BStott6 Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BStott6 commented Nov 7, 2025

Uh oh!

gbMattN left a comment

Choose a reason for hiding this comment

Uh oh!

AaronBallman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BStott6 Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

[Clang] Warn when `std::atomic_thread_fence` is used with `fsanitize=thread` #166542

[Clang] Warn when `std::atomic_thread_fence` is used with `fsanitize=thread` #166542

BStott6 commented Nov 5, 2025 •

edited

Loading

BStott6 Nov 5, 2025 •

edited

Loading

gbMattN Nov 5, 2025 •

edited

Loading

github-actions bot commented Nov 6, 2025 •

edited

Loading

BStott6 Nov 7, 2025 •

edited

Loading

BStott6 Nov 14, 2025 •

edited

Loading