Author: Benjamin Stott Date: 2026-01-09T10:20:35Z New Revision: d6d5c5f6a2f468cb5aac7b466d2802841b8d1441
URL: https://github.com/llvm/llvm-project/commit/d6d5c5f6a2f468cb5aac7b466d2802841b8d1441 DIFF: https://github.com/llvm/llvm-project/commit/d6d5c5f6a2f468cb5aac7b466d2802841b8d1441.diff LOG: [Clang] Warn when `std::atomic_thread_fence` is used with `fsanitize=thread` (#166542) - 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. --------- Co-authored-by: Aaron Ballman <[email protected]> Added: clang/test/SemaCXX/warn-tsan-atomic-fence.cpp Modified: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/DiagnosticSemaKinds.td clang/include/clang/Sema/Sema.h clang/lib/Sema/Sema.cpp clang/lib/Sema/SemaChecking.cpp Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 22aa8083d32ee..cbd17dc08e39f 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -435,6 +435,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``) - Fix a false positive warning in ``-Wignored-qualifiers`` when the return type is undeduced. (#GH43054) - Clang now emits a diagnostic with the 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 f3c13841c4bf0..fdd9b9cd3a996 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<DiagGroup<"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 654bebbaf8ddd..cc542c37d888a 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -1165,8 +1165,10 @@ class Sema final : public SemaBase { /// getCurFunctionOrMethodDecl - Return the Decl for the current ObjC method /// or C function we're in, otherwise return null. If we're currently - /// in a 'block', this returns the containing context. - NamedDecl *getCurFunctionOrMethodDecl() const; + /// in a 'block', this returns the containing context. If \p AllowLambda is + /// true, this can return the call operator of an enclosing lambda, otherwise + /// lambdas are skipped when looking for an enclosing function. + NamedDecl *getCurFunctionOrMethodDecl(bool AllowLambda = false) const; /// Warn if we're implicitly casting from a _Nullable pointer type to a /// _Nonnull one. @@ -3038,6 +3040,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/Sema.cpp b/clang/lib/Sema/Sema.cpp index 6eea8f6e9d97e..32453232a0e03 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -1663,8 +1663,8 @@ ObjCMethodDecl *Sema::getCurMethodDecl() { return dyn_cast<ObjCMethodDecl>(DC); } -NamedDecl *Sema::getCurFunctionOrMethodDecl() const { - DeclContext *DC = getFunctionLevelDeclContext(); +NamedDecl *Sema::getCurFunctionOrMethodDecl(bool AllowLambda) const { + DeclContext *DC = getFunctionLevelDeclContext(AllowLambda); if (isa<ObjCMethodDecl>(DC) || isa<FunctionDecl>(DC)) return cast<NamedDecl>(DC); return nullptr; diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index ba4b25961d70d..e70f9dbd93a61 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" @@ -46,6 +47,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" @@ -1140,6 +1142,53 @@ static bool ProcessFormatStringLiteral(const Expr *FormatExpr, return false; } +/// Returns true if: +/// - The sanitizers are enabled. +/// - `Decl` does not have attributes preventing sanitizer instrumentation. +/// - `Decl` or its location is not included in the no-sanitize list. +static bool isSanitizationEnabledForDecl(ASTContext &Context, + const NamedDecl *Decl, + SanitizerMask TheSanitizerMask) { + // Check that the sanitizer is enabled globally. + const SanitizerMask EnabledSanitizerMask = + Context.getLangOpts().Sanitize.Mask; + if (!(EnabledSanitizerMask & TheSanitizerMask)) + return false; + + // Check that the source file is not included in the no sanitize list. + const auto &NoSanitizeList = Context.getNoSanitizeList(); + if (NoSanitizeList.containsLocation(TheSanitizerMask, + Decl->getSourceRange().getBegin())) + return false; + + // Check that the declaration name is not included in the no sanitize list. + // NB no-sanitize lists use mangled names. + std::unique_ptr<MangleContext> MC(Context.createMangleContext()); + 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(); + } + if (NoSanitizeList.containsFunction(TheSanitizerMask, MangledName)) + return false; + + // Check that the declaration does not have the + // "disable_sanitizer_instrumentation" attribute. + if (Decl->hasAttr<DisableSanitizerInstrumentationAttr>()) + return false; + + // Check that the declaration does not have a "no_sanitize" attribute matching + // this sanitizer mask. + for (const NoSanitizeAttr *Attr : Decl->specific_attrs<NoSanitizeAttr>()) { + if (Attr->getMask() & TheSanitizerMask) + return false; + } + + return true; +} + void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, CallExpr *TheCall) { if (TheCall->isValueDependent() || TheCall->isTypeDependent() || @@ -4221,6 +4270,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); @@ -10097,6 +10147,23 @@ 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 || !IsStdFunction(FDecl, "atomic_thread_fence")) + return; + + const NamedDecl *Caller = getCurFunctionOrMethodDecl(/*AllowLambda=*/true); + if (!isSanitizationEnabledForDecl(Context, Caller, SanitizerKind::Thread)) + 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..2c2626e25c859 --- /dev/null +++ b/clang/test/SemaCXX/warn-tsan-atomic-fence.cpp @@ -0,0 +1,70 @@ +// No warnings in regular compile +// RUN: %clang_cc1 -triple=x86_64-linux-unknown -verify=no-warnings %s + +// Emits warnings with `-fsanitize=thread` +// RUN: %clang_cc1 -triple=x86_64-linux-unknown -verify=with-tsan,warn-global-function,warn-member-function -fsanitize=thread %s + +// No warnings if `-Wno-tsan` is passed +// RUN: %clang_cc1 -triple=x86_64-linux-unknown -verify=no-warnings -fsanitize=thread -Wno-tsan %s + +// Ignoring source file +// RUN: echo "src:*%{s:basename}" > %t +// RUN: %clang_cc1 -triple=x86_64-linux-unknown -verify=no-warnings -fsanitize=thread -fsanitize-ignorelist=%t %s + +// Ignoring global function +// RUN: echo "fun:*global_function_to_maybe_ignore*" > %t +// RUN: %clang_cc1 -triple=x86_64-linux-unknown -verify=with-tsan,warn-member-function -fsanitize=thread -fsanitize-ignorelist=%t %s + +// Ignoring "S::member_function" +// RUN: echo "fun:_ZN1S15member_functionEv" > %t +// RUN: %clang_cc1 -triple=x86_64-linux-unknown -verify=with-tsan,warn-global-function -fsanitize=thread -fsanitize-ignorelist=%t %s + +// no-warnings-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 global_function_to_maybe_ignore() { + std::atomic_thread_fence(std::memory_order_relaxed); // warn-global-function-warning {{'std::atomic_thread_fence' is not supported with '-fsanitize=thread'}} +} + +__attribute__((no_sanitize("thread"))) +void no_sanitize_1() { + std::atomic_thread_fence(std::memory_order_relaxed); + + auto lam = []() { + std::atomic_thread_fence(std::memory_order_relaxed); // with-tsan-warning {{'std::atomic_thread_fence' is not supported with '-fsanitize=thread'}} + }; +} + +__attribute__((no_sanitize_thread)) +void no_sanitize_2() { + std::atomic_thread_fence(std::memory_order_relaxed); +} + +__attribute__((disable_sanitizer_instrumentation)) +void no_sanitize_3() { + std::atomic_thread_fence(std::memory_order_relaxed); +} + +void no_sanitize_lambda() { + auto lam = [] () __attribute__((no_sanitize("thread"))) { + std::atomic_thread_fence(std::memory_order_relaxed); + }; +} + +struct S { +public: + void member_function() { + std::atomic_thread_fence(std::memory_order_relaxed); // warn-member-function-warning {{'std::atomic_thread_fence' is not supported with '-fsanitize=thread'}} + } +}; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
