https://github.com/aaronpuchert created https://github.com/llvm/llvm-project/pull/137477
Support for attribute parameter packs was added some time ago in commit ead1690d31f815c00fdd2bc23db4766191bbeabc. But template substitution didn't expand the packs yet. For now expansion can only happen within a `VariadicExprArgument`: i.e. in `try_acquire_capability`, which takes a regular and a variadic argument, the template can't have a single pack that then expands to cover both arguments. This is a prerequisite for #42000. >From a8a89dacbdc6f3db80a062e02d86beeeb8516b4c Mon Sep 17 00:00:00 2001 From: Aaron Puchert <aaronpuch...@alice-dsl.net> Date: Sat, 26 Apr 2025 23:00:08 +0200 Subject: [PATCH] Support pack expansion for Clang Thread Safety attributes Support for attribute parameter packs was added some time ago in commit ead1690d31f815c00fdd2bc23db4766191bbeabc. But template substitution didn't expand the packs yet. For now expansion can only happen within a `VariadicExprArgument`: i.e. in `try_acquire_capability`, which takes a regular and a variadic argument, the template can't have a single pack that then expands to cover both arguments. This is a prerequisite for #42000. --- clang/include/clang/Basic/Attr.td | 8 + .../SemaCXX/warn-thread-safety-analysis.cpp | 47 ++++++ .../SemaCXX/warn-thread-safety-parsing.cpp | 147 +++++++++++++++++- clang/utils/TableGen/ClangAttrEmitter.cpp | 30 ++-- 4 files changed, 214 insertions(+), 18 deletions(-) diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index dcdcff8c46fe2..2548e60312a0d 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -3856,6 +3856,7 @@ def AssertCapability : InheritableAttr { let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; let Args = [VariadicExprArgument<"Args">]; + let AcceptsExprPack = 1; let Accessors = [Accessor<"isShared", [Clang<"assert_shared_capability", 0>, GNU<"assert_shared_lock">]>]; @@ -3873,6 +3874,7 @@ def AcquireCapability : InheritableAttr { let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; let Args = [VariadicExprArgument<"Args">]; + let AcceptsExprPack = 1; let Accessors = [Accessor<"isShared", [Clang<"acquire_shared_capability", 0>, GNU<"shared_lock_function">]>]; @@ -3890,6 +3892,7 @@ def TryAcquireCapability : InheritableAttr { let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; let Args = [ExprArgument<"SuccessValue">, VariadicExprArgument<"Args">]; + let AcceptsExprPack = 1; let Accessors = [Accessor<"isShared", [Clang<"try_acquire_shared_capability", 0>, GNU<"shared_trylock_function">]>]; @@ -3907,6 +3910,7 @@ def ReleaseCapability : InheritableAttr { let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; let Args = [VariadicExprArgument<"Args">]; + let AcceptsExprPack = 1; let Accessors = [Accessor<"isShared", [Clang<"release_shared_capability", 0>]>, Accessor<"isGeneric", @@ -3921,6 +3925,7 @@ def RequiresCapability : InheritableAttr { Clang<"requires_shared_capability", 0>, Clang<"shared_locks_required", 0>]; let Args = [VariadicExprArgument<"Args">]; + let AcceptsExprPack = 1; let LateParsed = LateAttrParseStandard; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; @@ -3963,6 +3968,7 @@ def PtGuardedBy : InheritableAttr { def AcquiredAfter : InheritableAttr { let Spellings = [GNU<"acquired_after">]; let Args = [VariadicExprArgument<"Args">]; + let AcceptsExprPack = 1; let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; @@ -3974,6 +3980,7 @@ def AcquiredAfter : InheritableAttr { def AcquiredBefore : InheritableAttr { let Spellings = [GNU<"acquired_before">]; let Args = [VariadicExprArgument<"Args">]; + let AcceptsExprPack = 1; let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; @@ -3995,6 +4002,7 @@ def LockReturned : InheritableAttr { def LocksExcluded : InheritableAttr { let Spellings = [GNU<"locks_excluded">]; let Args = [VariadicExprArgument<"Args">]; + let AcceptsExprPack = 1; let LateParsed = LateAttrParseStandard; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp index 2a04e820eb095..4c17294d11e79 100644 --- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp +++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp @@ -57,6 +57,20 @@ class SCOPED_LOCKABLE DoubleMutexLock { ~DoubleMutexLock() UNLOCK_FUNCTION(); }; +template<typename Mu> +class SCOPED_LOCKABLE TemplateMutexLock { +public: + TemplateMutexLock(Mu *mu) EXCLUSIVE_LOCK_FUNCTION(mu); + ~TemplateMutexLock() UNLOCK_FUNCTION(); +}; + +template<typename... Mus> +class SCOPED_LOCKABLE VariadicMutexLock { +public: + VariadicMutexLock(Mus *...mus) EXCLUSIVE_LOCK_FUNCTION(mus...); + ~VariadicMutexLock() UNLOCK_FUNCTION(); +}; + // The universal lock, written "*", allows checking to be selectively turned // off for a particular piece of code. void beginNoWarnOnReads() SHARED_LOCK_FUNCTION("*"); @@ -1821,6 +1835,18 @@ struct TestScopedLockable { a = b + 1; b = a + 1; } + + void foo6() { + TemplateMutexLock<Mutex> mulock1(&mu1), mulock2(&mu2); + a = b + 1; + b = a + 1; + } + + void foo7() { + VariadicMutexLock<Mutex, Mutex> mulock(&mu1, &mu2); + a = b + 1; + b = a + 1; + } }; } // end namespace test_scoped_lockable @@ -3114,6 +3140,16 @@ class SCOPED_LOCKABLE ReaderMutexUnlock { void Unlock() EXCLUSIVE_LOCK_FUNCTION(); }; +template<typename... Mus> +class SCOPED_LOCKABLE VariadicMutexUnlock { +public: + VariadicMutexUnlock(Mus *...mus) EXCLUSIVE_UNLOCK_FUNCTION(mus...); + ~VariadicMutexUnlock() EXCLUSIVE_UNLOCK_FUNCTION(); + + void Lock() EXCLUSIVE_UNLOCK_FUNCTION(); + void Unlock() EXCLUSIVE_LOCK_FUNCTION(); +}; + Mutex mu; int x GUARDED_BY(mu); bool c; @@ -3176,6 +3212,17 @@ void doubleUnlock() EXCLUSIVE_LOCKS_REQUIRED(mu) { scope.Unlock(); // expected-warning {{releasing mutex 'mu' that was not held}} } +Mutex mu2; +int y GUARDED_BY(mu2); + +void variadic() EXCLUSIVE_LOCKS_REQUIRED(mu, mu2) { + VariadicMutexUnlock<Mutex, Mutex> scope(&mu, &mu2); + x = 2; // expected-warning {{writing variable 'x' requires holding mutex 'mu' exclusively}} + y = 3; // expected-warning {{writing variable 'y' requires holding mutex 'mu2' exclusively}} + scope.Lock(); + x = y = 4; +} + class SCOPED_LOCKABLE MutexLockUnlock { public: MutexLockUnlock(Mutex *mu1, Mutex *mu2) EXCLUSIVE_UNLOCK_FUNCTION(mu1) EXCLUSIVE_LOCK_FUNCTION(mu2); diff --git a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp index 752803e4a0543..bad71f7cdd07c 100644 --- a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp +++ b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp @@ -1,6 +1,6 @@ // RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety %s // RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -std=c++98 %s -// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -std=c++11 %s -D CPP11 +// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -std=c++11 %s #define LOCKABLE __attribute__ ((lockable)) #define SCOPED_LOCKABLE __attribute__ ((scoped_lockable)) @@ -643,6 +643,24 @@ int elf_function_bad_6(Mutex x, Mutex y) EXCLUSIVE_LOCK_FUNCTION(0); // \ int elf_function_bad_7() EXCLUSIVE_LOCK_FUNCTION(0); // \ // expected-error {{'exclusive_lock_function' attribute parameter 1 is out of bounds: no parameters to index into}} +template<typename Mu> +int elf_template(Mu& mu) EXCLUSIVE_LOCK_FUNCTION(mu) {} + +template int elf_template<Mutex>(Mutex&); +// FIXME: warn on template instantiation. +template int elf_template<UnlockableMu>(UnlockableMu&); + +#if __cplusplus >= 201103 + +template<typename... Mus> +int elf_variadic_template(Mus&... mus) EXCLUSIVE_LOCK_FUNCTION(mus...) {} + +template int elf_variadic_template<Mutex, Mutex>(Mutex&, Mutex&); +// FIXME: warn on template instantiation. +template int elf_variadic_template<Mutex, UnlockableMu>(Mutex&, UnlockableMu&); + +#endif + //-----------------------------------------// // Shared Lock Function (slf) @@ -719,6 +737,24 @@ int slf_function_bad_6(Mutex x, Mutex y) SHARED_LOCK_FUNCTION(0); // \ int slf_function_bad_7() SHARED_LOCK_FUNCTION(0); // \ // expected-error {{'shared_lock_function' attribute parameter 1 is out of bounds: no parameters to index into}} +template<typename Mu> +int slf_template(Mu& mu) SHARED_LOCK_FUNCTION(mu) {} + +template int slf_template<Mutex>(Mutex&); +// FIXME: warn on template instantiation. +template int slf_template<UnlockableMu>(UnlockableMu&); + +#if __cplusplus >= 201103 + +template<typename... Mus> +int slf_variadic_template(Mus&... mus) SHARED_LOCK_FUNCTION(mus...) {} + +template int slf_variadic_template<Mutex, Mutex>(Mutex&, Mutex&); +// FIXME: warn on template instantiation. +template int slf_variadic_template<Mutex, UnlockableMu>(Mutex&, UnlockableMu&); + +#endif + //-----------------------------------------// // Exclusive TryLock Function (etf) @@ -796,6 +832,24 @@ int etf_function_bad_5() EXCLUSIVE_TRYLOCK_FUNCTION(1, muDoublePointer); // \ int etf_function_bad_6() EXCLUSIVE_TRYLOCK_FUNCTION(1, umu); // \ // expected-warning {{'exclusive_trylock_function' attribute requires arguments whose type is annotated with 'capability' attribute}} +template<typename Mu> +int etf_template(Mu& mu) EXCLUSIVE_TRYLOCK_FUNCTION(1, mu) {} + +template int etf_template<Mutex>(Mutex&); +// FIXME: warn on template instantiation. +template int etf_template<UnlockableMu>(UnlockableMu&); + +#if __cplusplus >= 201103 + +template<typename... Mus> +int etf_variadic_template(Mus&... mus) EXCLUSIVE_TRYLOCK_FUNCTION(1, mus...) {} + +template int etf_variadic_template<Mutex, Mutex>(Mutex&, Mutex&); +// FIXME: warn on template instantiation. +template int etf_variadic_template<Mutex, UnlockableMu>(Mutex&, UnlockableMu&); + +#endif + //-----------------------------------------// // Shared TryLock Function (stf) @@ -874,6 +928,24 @@ int stf_function_bad_5() SHARED_TRYLOCK_FUNCTION(1, muDoublePointer); // \ int stf_function_bad_6() SHARED_TRYLOCK_FUNCTION(1, umu); // \ // expected-warning {{'shared_trylock_function' attribute requires arguments whose type is annotated with 'capability' attribute}} +template<typename Mu> +int stf_template(Mu& mu) SHARED_TRYLOCK_FUNCTION(1, mu) {} + +template int stf_template<Mutex>(Mutex&); +// FIXME: warn on template instantiation. +template int stf_template<UnlockableMu>(UnlockableMu&); + +#if __cplusplus >= 201103 + +template<typename... Mus> +int stf_variadic_template(Mus&... mus) SHARED_TRYLOCK_FUNCTION(1, mus...) {} + +template int stf_variadic_template<Mutex, Mutex>(Mutex&, Mutex&); +// FIXME: warn on template instantiation. +template int stf_variadic_template<Mutex, UnlockableMu>(Mutex&, UnlockableMu&); + +#endif + //-----------------------------------------// // Unlock Function (uf) @@ -953,6 +1025,24 @@ int uf_function_bad_6(Mutex x, Mutex y) UNLOCK_FUNCTION(0); // \ int uf_function_bad_7() UNLOCK_FUNCTION(0); // \ // expected-error {{'unlock_function' attribute parameter 1 is out of bounds: no parameters to index into}} +template<typename Mu> +int uf_template(Mu& mu) UNLOCK_FUNCTION(mu) {} + +template int uf_template<Mutex>(Mutex&); +// FIXME: warn on template instantiation. +template int uf_template<UnlockableMu>(UnlockableMu&); + +#if __cplusplus >= 201103 + +template<typename... Mus> +int uf_variadic_template(Mus&... mus) UNLOCK_FUNCTION(mus...) {} + +template int uf_variadic_template<Mutex, Mutex>(Mutex&, Mutex&); +// FIXME: warn on template instantiation. +template int uf_variadic_template<Mutex, UnlockableMu>(Mutex&, UnlockableMu&); + +#endif + //-----------------------------------------// // Lock Returned (lr) @@ -1088,6 +1178,23 @@ int le_function_bad_3() LOCKS_EXCLUDED(muDoublePointer); // \ int le_function_bad_4() LOCKS_EXCLUDED(umu); // \ // expected-warning {{'locks_excluded' attribute requires arguments whose type is annotated with 'capability' attribute}} +template<typename Mu> +int le_template(Mu& mu) LOCKS_EXCLUDED(mu) {} + +template int le_template<Mutex>(Mutex&); +// FIXME: warn on template instantiation. +template int le_template<UnlockableMu>(UnlockableMu&); + +#if __cplusplus >= 201103 + +template<typename... Mus> +int le_variadic_template(Mus&... mus) LOCKS_EXCLUDED(mus...) {} + +template int le_variadic_template<Mutex, Mutex>(Mutex&, Mutex&); +// FIXME: warn on template instantiation. +template int le_variadic_template<Mutex, UnlockableMu>(Mutex&, UnlockableMu&); + +#endif //-----------------------------------------// @@ -1156,6 +1263,24 @@ int elr_function_bad_3() EXCLUSIVE_LOCKS_REQUIRED(muDoublePointer); // \ int elr_function_bad_4() EXCLUSIVE_LOCKS_REQUIRED(umu); // \ // expected-warning {{'exclusive_locks_required' attribute requires arguments whose type is annotated with 'capability' attribute}} +template<typename Mu> +int elr_template(Mu& mu) EXCLUSIVE_LOCKS_REQUIRED(mu) {} + +template int elr_template<Mutex>(Mutex&); +// FIXME: warn on template instantiation. +template int elr_template<UnlockableMu>(UnlockableMu&); + +#if __cplusplus >= 201103 + +template<typename... Mus> +int elr_variadic_template(Mus&... mus) EXCLUSIVE_LOCKS_REQUIRED(mus...) {} + +template int elr_variadic_template<Mutex, Mutex>(Mutex&, Mutex&); +// FIXME: warn on template instantiation. +template int elr_variadic_template<Mutex, UnlockableMu>(Mutex&, UnlockableMu&); + +#endif + @@ -1225,6 +1350,24 @@ int slr_function_bad_3() SHARED_LOCKS_REQUIRED(muDoublePointer); // \ int slr_function_bad_4() SHARED_LOCKS_REQUIRED(umu); // \ // expected-warning {{'shared_locks_required' attribute requires arguments whose type is annotated with 'capability' attribute}} +template<typename Mu> +int slr_template(Mu& mu) SHARED_LOCKS_REQUIRED(mu) {} + +template int slr_template<Mutex>(Mutex&); +// FIXME: warn on template instantiation. +template int slr_template<UnlockableMu>(UnlockableMu&); + +#if __cplusplus >= 201103 + +template<typename... Mus> +int slr_variadic_template(Mus&... mus) SHARED_LOCKS_REQUIRED(mus...) {} + +template int slr_variadic_template<Mutex, Mutex>(Mutex&, Mutex&); +// FIXME: warn on template instantiation. +template int slr_variadic_template<Mutex, UnlockableMu>(Mutex&, UnlockableMu&); + +#endif + //-----------------------------------------// // Regression tests for unusual cases. @@ -1582,7 +1725,7 @@ class Foo { } // end namespace FunctionAttributesInsideClass_ICE_Test -#ifdef CPP11 +#if __cplusplus >= 201103 namespace CRASH_POST_R301735 { class SomeClass { public: diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp b/clang/utils/TableGen/ClangAttrEmitter.cpp index 00df99d04d873..b7f7f8f0b2fe2 100644 --- a/clang/utils/TableGen/ClangAttrEmitter.cpp +++ b/clang/utils/TableGen/ClangAttrEmitter.cpp @@ -1329,28 +1329,26 @@ namespace { void writeTemplateInstantiationArgs(raw_ostream &OS) const override { OS << "tempInst" << getUpperName() << ", " - << "A->" << getLowerName() << "_size()"; + << "numTempInst" << getUpperName(); } void writeTemplateInstantiation(raw_ostream &OS) const override { - OS << " auto *tempInst" << getUpperName() - << " = new (C, 16) " << getType() - << "[A->" << getLowerName() << "_size()];\n"; + OS << " size_t numTempInst" << getUpperName() << ";\n"; + OS << " " << getType() << "*tempInst" << getUpperName() << ";\n"; OS << " {\n"; OS << " EnterExpressionEvaluationContext " << "Unevaluated(S, Sema::ExpressionEvaluationContext::Unevaluated);\n"; - OS << " " << getType() << " *TI = tempInst" << getUpperName() - << ";\n"; - OS << " " << getType() << " *I = A->" << getLowerName() - << "_begin();\n"; - OS << " " << getType() << " *E = A->" << getLowerName() - << "_end();\n"; - OS << " for (; I != E; ++I, ++TI) {\n"; - OS << " ExprResult Result = S.SubstExpr(*I, TemplateArgs);\n"; - OS << " if (Result.isInvalid())\n"; - OS << " return nullptr;\n"; - OS << " *TI = Result.get();\n"; - OS << " }\n"; + OS << " ArrayRef<" << getType() << "> ArgsToInstantiate(A->" + << getLowerName() << "_begin(), A->" << getLowerName() << "_end());\n"; + OS << " SmallVector<" << getType() << ", 4> InstArgs;\n"; + OS << " if (S.SubstExprs(ArgsToInstantiate, /*IsCall=*/false, " + "TemplateArgs, InstArgs))\n"; + OS << " return nullptr;\n"; + OS << " numTempInst" << getUpperName() << " = InstArgs.size();\n"; + OS << " tempInst" << getUpperName() << " = new (C, 16) " + << getType() << "[numTempInst" << getUpperName() << "];\n"; + OS << " std::copy(InstArgs.begin(), InstArgs.end(), tempInst" + << getUpperName() << ");\n"; OS << " }\n"; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits