llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: None (BStott6) <details> <summary>Changes</summary> - ThreadSanitizer currently does not support `std::atomic_thread_fence`, leading to false positives: https://github.com/llvm/llvm-project/issues/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 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); +} `````````` </details> https://github.com/llvm/llvm-project/pull/166542 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
