diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 92fc9381a5868..d5a1a7fa9b5fc 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -349,6 +349,8 @@ Improvements to Clang's diagnostics - Fixed false positives in ``-Waddress-of-packed-member`` diagnostics when potential misaligned members get processed before they can get discarded. (#GH144729) +- Clang now emits a warning when `std::atomic_thread_fence` is used with `-fsanitize=thread` as this can + lead to false positives. (This can be disabled with ``-Wno-tsan``) - Clang now emits dignostic with correct message in case of assigning to const reference captured in lambda. (#GH105647) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 4e369be0bbb92..c90b018f64f81 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; +def warn_atomic_thread_fence_with_tsan : Warning< + "'std::atomic_thread_fence' is not supported with '-fsanitize=thread'">, + InGroup>; 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..d78d912c16104 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -3033,6 +3033,9 @@ 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..52ba36ab93642 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -29,6 +29,7 @@ #include "clang/AST/ExprObjC.h" #include "clang/AST/FormatString.h" #include "clang/AST/IgnoreExpr.h" +#include "clang/AST/Mangle.h" #include "clang/AST/NSAPI.h" #include "clang/AST/NonTrivialTypeVisitor.h" #include "clang/AST/OperationKinds.h" @@ -45,6 +46,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 +4102,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 +9825,65 @@ 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 positives. Example issue: + // https://github.com/llvm/llvm-project/issues/52942 + + if (!Call || !FDecl) + return; + + if (!IsStdFunction(FDecl, "atomic_thread_fence")) + return; + + // Check that TSan is enabled in this context + const auto EnabledTSanMask = + Context.getLangOpts().Sanitize.Mask & (SanitizerKind::Thread); + if (!EnabledTSanMask) + return; + + // Check that the file isn't in the no-sanitize list + const auto &NoSanitizeList = Context.getNoSanitizeList(); + if (NoSanitizeList.containsLocation(EnabledTSanMask, + Call->getSourceRange().getBegin())) + return; + + std::unique_ptr MC(Context.createMangleContext()); + + // Check that the calling function or lambda: + // - Does not have any attributes preventing TSan checking + // - Is not in the ignore list + auto IsNotSanitized = [&](NamedDecl *Decl) { + const auto SpecificAttrs = Decl->specific_attrs(); + const auto IsNoSanitizeThreadAttr = [](NoSanitizeAttr *Attr) { + return static_cast(Attr->getMask() & SanitizerKind::Thread); + }; + + // Get mangled name for ignorelist lookup + std::string MangledName; + if (MC->shouldMangleDeclName(Decl)) { + llvm::raw_string_ostream S = llvm::raw_string_ostream(MangledName); + MC->mangleName(Decl, S); + } else { + MangledName = Decl->getName(); + } + + return Decl && + (Decl->hasAttr() || + std::any_of(SpecificAttrs.begin(), SpecificAttrs.end(), + IsNoSanitizeThreadAttr) || + NoSanitizeList.containsFunction(EnabledTSanMask, MangledName)); + }; + if (IsNotSanitized(getCurFunctionOrMethodDecl())) + return; + if (IsNotSanitized(getCurFunctionDecl(/*AllowLambdas*/ true))) + 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..a7db3958696db --- /dev/null +++ b/clang/test/SemaCXX/warn-tsan-atomic-fence.cpp @@ -0,0 +1,54 @@ +// No warnings in regular compile +// RUN: %clang_cc1 -verify=no-tsan %s + +// Emits warning with `-fsanitize=thread` +// RUN: %clang_cc1 -verify=with-tsan -fsanitize=thread %s + +// No warnings if `-Wno-tsan` is passed +// RUN: %clang_cc1 -verify=no-tsan -fsanitize=thread -Wno-tsan %s + +// Ignoring func1 +// RUN: echo "fun:*func1*" > %t +// RUN: %clang_cc1 -verify=no-tsan -fsanitize=thread -fsanitize-ignorelist=%t %s + +// Ignoring source file +// RUN: echo "src:%s" > %t +// RUN: %clang_cc1 -verify=no-tsan -fsanitize=thread -fsanitize-ignorelist=%t %s + +// no-tsan-no-diagnostics + +namespace std { + enum memory_order { + memory_order_relaxed, + memory_order_consume, + memory_order_acquire, + memory_order_release, + memory_order_acq_rel, + memory_order_seq_cst, + }; + void atomic_thread_fence(memory_order) {} +}; + +void func1() { // extern "C" to stop name mangling + std::atomic_thread_fence(std::memory_order_relaxed); // with-tsan-warning {{'std::atomic_thread_fence' is not supported with '-fsanitize=thread'}} + + auto lam = []() __attribute__((no_sanitize("thread"))) { + std::atomic_thread_fence(std::memory_order_relaxed); + }; +} + +__attribute__((no_sanitize("thread"))) +void func2() { + std::atomic_thread_fence(std::memory_order_relaxed); + + auto lam = []() { + std::atomic_thread_fence(std::memory_order_relaxed); + }; +} + +__attribute__((no_sanitize_thread)) +void func3() { + std::atomic_thread_fence(std::memory_order_relaxed); +} + +int main() {}