diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 804aebe0cc06..6e6c580fcb0f 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -221,6 +221,13 @@ Attribute Changes in Clang foreign language personality with a given function. Note that this does not perform any ABI validation for the personality routine. +- The :doc:`ThreadSafetyAnalysis` attributes ``guarded_by`` and + ``pt_guarded_by`` now accept multiple capability arguments with refined + access semantics: *writing* requires all listed capabilities to be held + exclusively, while *reading* requires at least one to be held. This is + sound because any writer must hold all capabilities, so holding any one + prevents concurrent writes. + Improvements to Clang's diagnostics ----------------------------------- - Added ``-Wlifetime-safety`` to enable lifetime safety analysis, diff --git a/clang/docs/ThreadSafetyAnalysis.rst b/clang/docs/ThreadSafetyAnalysis.rst index d0f96f58dac1..571c6c0e04ea 100644 --- a/clang/docs/ThreadSafetyAnalysis.rst +++ b/clang/docs/ThreadSafetyAnalysis.rst @@ -153,16 +153,20 @@ general capability model. The prior names are still in use, and will be mentioned under the tag *previously* where appropriate. -GUARDED_BY(c) and PT_GUARDED_BY(c) ----------------------------------- +GUARDED_BY(...) and PT_GUARDED_BY(...) +-------------------------------------- ``GUARDED_BY`` is an attribute on data members, which declares that the data member is protected by the given capability. Read operations on the data require shared access, while write operations require exclusive access. +Multiple capabilities may be specified, subject to the following rules: +a writer must hold *all* listed capabilities exclusively, so holding *any one* +of them is sufficient to guarantee at least shared (read) access. + ``PT_GUARDED_BY`` is similar, but is intended for use on pointers and smart pointers. There is no constraint on the data member itself, but the *data that -it points to* is protected by the given capability. +it points to* is protected by the given capabilities. .. code-block:: c++ @@ -181,6 +185,25 @@ it points to* is protected by the given capability. p3.reset(new int); // OK. } +When multiple capabilities are listed: + +* **Write** access requires all listed capabilities to be held exclusively. +* **Read** access requires at least one of them to be held (shared or exclusive). + +.. code-block:: c++ + + Mutex mu1, mu2; + int a GUARDED_BY(mu1, mu2); + + void reader() REQUIRES_SHARED(mu1) { + int x = a; // OK: at least one capability is held. + a = 0; // Warning! Writing requires both mu1 and mu2. + } + + void writer() REQUIRES(mu1, mu2) { + a = 0; // OK: both capabilities are held exclusively. + } + REQUIRES(...), REQUIRES_SHARED(...) ----------------------------------- @@ -860,11 +883,11 @@ implementation. #define SCOPED_CAPABILITY \ THREAD_ANNOTATION_ATTRIBUTE__(scoped_lockable) - #define GUARDED_BY(x) \ - THREAD_ANNOTATION_ATTRIBUTE__(guarded_by(x)) + #define GUARDED_BY(...) \ + THREAD_ANNOTATION_ATTRIBUTE__(guarded_by(__VA_ARGS__)) - #define PT_GUARDED_BY(x) \ - THREAD_ANNOTATION_ATTRIBUTE__(pt_guarded_by(x)) + #define PT_GUARDED_BY(...) \ + THREAD_ANNOTATION_ATTRIBUTE__(pt_guarded_by(__VA_ARGS__)) #define ACQUIRED_BEFORE(...) \ THREAD_ANNOTATION_ATTRIBUTE__(acquired_before(__VA_ARGS__)) diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafety.h b/clang/include/clang/Analysis/Analyses/ThreadSafety.h index 9fb23212aaf9..4fb04fb16b8c 100644 --- a/clang/include/clang/Analysis/Analyses/ThreadSafety.h +++ b/clang/include/clang/Analysis/Analyses/ThreadSafety.h @@ -19,6 +19,7 @@ #define LLVM_CLANG_ANALYSIS_ANALYSES_THREADSAFETY_H #include "clang/Basic/SourceLocation.h" +#include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/StringRef.h" namespace clang { @@ -193,6 +194,17 @@ public: virtual void handleNoMutexHeld(const NamedDecl *D, ProtectedOperationKind POK, AccessKind AK, SourceLocation Loc) {} + /// Warn when a read of a multi-capability guarded_by variable occurs while + /// none of the listed capabilities are held. + /// \param D -- The decl for the protected variable + /// \param POK -- The kind of protected operation (e.g. variable access) + /// \param LockNames -- Names of the capabilities that were not held + /// \param Loc -- The location of the read + virtual void handleGuardedByAnyReadNotHeld(const NamedDecl *D, + ProtectedOperationKind POK, + ArrayRef LockNames, + SourceLocation Loc) {} + /// Warn when a protected operation occurs while the specific mutex protecting /// the operation is not locked. /// \param Kind -- the capability's name parameter (role, mutex, etc). diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 5023011a6814..f697262d603b 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -4155,7 +4155,7 @@ def NoThreadSafetyAnalysis : InheritableAttr { def GuardedBy : InheritableAttr { let Spellings = [GNU<"guarded_by">]; - let Args = [ExprArgument<"Arg">]; + let Args = [VariadicExprArgument<"Args">]; let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; @@ -4166,7 +4166,7 @@ def GuardedBy : InheritableAttr { def PtGuardedBy : InheritableAttr { let Spellings = [GNU<"pt_guarded_by">]; - let Args = [ExprArgument<"Arg">]; + let Args = [VariadicExprArgument<"Args">]; let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index d4d09a8ecef3..5567963b312e 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -4327,6 +4327,10 @@ def warn_var_deref_requires_any_lock : Warning< "%select{reading|writing}1 the value pointed to by %0 requires holding " "%select{any mutex|any mutex exclusively}1">, InGroup, DefaultIgnore; +def warn_requires_any_of_locks : Warning< + "reading %select{variable|the value pointed to by}1 %0 requires holding " + "at least one of %2">, + InGroup, DefaultIgnore; def warn_fun_excludes_mutex : Warning< "cannot call function '%1' while %0 '%2' is held">, InGroup, DefaultIgnore; diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index df7c3928ae23..41ba98c53247 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -9749,12 +9749,16 @@ Expected ASTImporter::Import(const Attr *FromAttr) { } case attr::GuardedBy: { const auto *From = cast(FromAttr); - AI.importAttr(From, AI.importArg(From->getArg()).value()); + AI.importAttr(From, + AI.importArrayArg(From->args(), From->args_size()).value(), + From->args_size()); break; } case attr::PtGuardedBy: { const auto *From = cast(FromAttr); - AI.importAttr(From, AI.importArg(From->getArg()).value()); + AI.importAttr(From, + AI.importArrayArg(From->args(), From->args_size()).value(), + From->args_size()); break; } case attr::AcquiredAfter: { diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp index 2c5976af9f61..099e8590b50e 100644 --- a/clang/lib/Analysis/ThreadSafety.cpp +++ b/clang/lib/Analysis/ThreadSafety.cpp @@ -1279,6 +1279,11 @@ public: const Expr *Exp, AccessKind AK, Expr *MutexExp, ProtectedOperationKind POK, til::SExpr *Self, SourceLocation Loc); + void warnIfAnyMutexNotHeldForRead(const FactSet &FSet, const NamedDecl *D, + const Expr *Exp, + llvm::ArrayRef Args, + ProtectedOperationKind POK, + SourceLocation Loc); void warnIfMutexHeld(const FactSet &FSet, const NamedDecl *D, const Expr *Exp, Expr *MutexExp, til::SExpr *Self, SourceLocation Loc); @@ -1848,6 +1853,38 @@ void ThreadSafetyAnalyzer::warnIfMutexNotHeld( } } +void ThreadSafetyAnalyzer::warnIfAnyMutexNotHeldForRead( + const FactSet &FSet, const NamedDecl *D, const Expr *Exp, + llvm::ArrayRef Args, ProtectedOperationKind POK, + SourceLocation Loc) { + SmallVector Caps; + for (auto *Arg : Args) { + CapabilityExpr Cp = SxBuilder.translateAttrExpr(Arg, D, Exp, nullptr); + if (Cp.isInvalid()) { + warnInvalidLock(Handler, Arg, D, Exp, Cp.getKind()); + continue; + } + if (Cp.shouldIgnore()) + continue; + const FactEntry *LDat = FSet.findLockUniv(FactMan, Cp); + if (LDat && LDat->isAtLeast(LK_Shared)) + return; // At least one held — read access is safe. + // FIXME: try findPartialMatch as a fallback to support + // -Wno-thread-safety-precise, as warnIfMutexNotHeld does. + Caps.push_back(Cp); + } + if (Caps.empty()) + return; + // Materialize names only now that we know we are going to warn. + SmallVector NameStorage; + SmallVector Names; + for (const auto &Cp : Caps) { + NameStorage.push_back(Cp.toString()); + Names.push_back(NameStorage.back()); + } + Handler.handleGuardedByAnyReadNotHeld(D, POK, Names, Loc); +} + /// Warn if the LSet contains the given lock. void ThreadSafetyAnalyzer::warnIfMutexHeld(const FactSet &FSet, const NamedDecl *D, const Expr *Exp, @@ -1934,8 +1971,18 @@ void ThreadSafetyAnalyzer::checkAccess(const FactSet &FSet, const Expr *Exp, Handler.handleNoMutexHeld(D, POK, AK, Loc); } - for (const auto *I : D->specific_attrs()) - warnIfMutexNotHeld(FSet, D, Exp, AK, I->getArg(), POK, nullptr, Loc); + for (const auto *I : D->specific_attrs()) { + if (AK == AK_Written || I->args_size() == 1) { + // Write requires all capabilities; single-arg read uses the normal + // per-lock warning path. + for (auto *Arg : I->args()) + warnIfMutexNotHeld(FSet, D, Exp, AK, Arg, POK, nullptr, Loc); + } else { + // Multi-arg read: holding any one of the listed capabilities is + // sufficient (a writer must hold all, so any one prevents writes). + warnIfAnyMutexNotHeldForRead(FSet, D, Exp, I->args(), POK, Loc); + } + } } /// Checks pt_guarded_by and pt_guarded_var attributes. @@ -1998,9 +2045,20 @@ void ThreadSafetyAnalyzer::checkPtAccess(const FactSet &FSet, const Expr *Exp, if (D->hasAttr() && FSet.isEmpty(FactMan)) Handler.handleNoMutexHeld(D, PtPOK, AK, Exp->getExprLoc()); - for (auto const *I : D->specific_attrs()) - warnIfMutexNotHeld(FSet, D, Exp, AK, I->getArg(), PtPOK, nullptr, - Exp->getExprLoc()); + for (auto const *I : D->specific_attrs()) { + if (AK == AK_Written || I->args_size() == 1) { + // Write requires all capabilities; single-arg read uses the normal + // per-lock warning path. + for (auto *Arg : I->args()) + warnIfMutexNotHeld(FSet, D, Exp, AK, Arg, PtPOK, nullptr, + Exp->getExprLoc()); + } else { + // Multi-arg read: holding any one of the listed capabilities is + // sufficient (a writer must hold all, so any one prevents writes). + warnIfAnyMutexNotHeldForRead(FSet, D, Exp, I->args(), PtPOK, + Exp->getExprLoc()); + } + } } /// Process a function call, method call, constructor call, diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index e2bd8d795656..37ed7488bb92 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -55,6 +55,7 @@ #include "llvm/ADT/PostOrderIterator.h" #include "llvm/ADT/STLFunctionalExtras.h" #include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Debug.h" #include "llvm/Support/TimeProfiler.h" @@ -2141,6 +2142,39 @@ class ThreadSafetyReporter : public clang::threadSafety::ThreadSafetyHandler { Warnings.emplace_back(std::move(Warning), getNotes()); } + void handleGuardedByAnyReadNotHeld(const NamedDecl *D, + ProtectedOperationKind POK, + ArrayRef LockNames, + SourceLocation Loc) override { + bool IsDeref; + switch (POK) { + case POK_VarAccess: + case POK_PassByRef: + case POK_ReturnByRef: + case POK_PassPointer: + case POK_ReturnPointer: + IsDeref = false; + break; + case POK_VarDereference: + case POK_PtPassByRef: + case POK_PtReturnByRef: + case POK_PtPassPointer: + case POK_PtReturnPointer: + IsDeref = true; + break; + case POK_FunctionCall: + llvm_unreachable("POK_FunctionCall not applicable here"); + } + std::string Quoted; + llvm::raw_string_ostream OS(Quoted); + llvm::ListSeparator LS; + for (StringRef Name : LockNames) + OS << LS << "'" << Name << "'"; + PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_requires_any_of_locks) + << D << IsDeref << Quoted); + Warnings.emplace_back(std::move(Warning), getNotes()); + } + void handleMutexNotHeld(StringRef Kind, const NamedDecl *D, ProtectedOperationKind POK, Name LockName, LockKind LK, SourceLocation Loc, diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 0589fb81b606..3c1e00c206dc 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -460,36 +460,33 @@ static void handlePtGuardedVarAttr(Sema &S, Decl *D, const ParsedAttr &AL) { } static bool checkGuardedByAttrCommon(Sema &S, Decl *D, const ParsedAttr &AL, - Expr *&Arg) { - SmallVector Args; - // check that all arguments are lockable objects - checkAttrArgsAreCapabilityObjs(S, D, AL, Args); - unsigned Size = Args.size(); - if (Size != 1) + SmallVectorImpl &Args) { + if (!AL.checkAtLeastNumArgs(S, 1)) return false; - Arg = Args[0]; - - return true; + checkAttrArgsAreCapabilityObjs(S, D, AL, Args); + return !Args.empty(); } static void handleGuardedByAttr(Sema &S, Decl *D, const ParsedAttr &AL) { - Expr *Arg = nullptr; - if (!checkGuardedByAttrCommon(S, D, AL, Arg)) + SmallVector Args; + if (!checkGuardedByAttrCommon(S, D, AL, Args)) return; - D->addAttr(::new (S.Context) GuardedByAttr(S.Context, AL, Arg)); + D->addAttr(::new (S.Context) + GuardedByAttr(S.Context, AL, Args.data(), Args.size())); } static void handlePtGuardedByAttr(Sema &S, Decl *D, const ParsedAttr &AL) { - Expr *Arg = nullptr; - if (!checkGuardedByAttrCommon(S, D, AL, Arg)) + SmallVector Args; + if (!checkGuardedByAttrCommon(S, D, AL, Args)) return; if (!threadSafetyCheckIsPointer(S, D, AL)) return; - D->addAttr(::new (S.Context) PtGuardedByAttr(S.Context, AL, Arg)); + D->addAttr(::new (S.Context) + PtGuardedByAttr(S.Context, AL, Args.data(), Args.size())); } static bool checkAcquireOrderAttrCommon(Sema &S, Decl *D, const ParsedAttr &AL, diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 87e07c8ac0d0..441df43d3d18 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -19476,9 +19476,9 @@ bool Sema::checkThisInStaticMemberFunctionAttributes(CXXMethodDecl *Method) { Expr *Arg = nullptr; ArrayRef Args; if (const auto *G = dyn_cast(A)) - Arg = G->getArg(); + Args = llvm::ArrayRef(G->args_begin(), G->args_size()); else if (const auto *G = dyn_cast(A)) - Arg = G->getArg(); + Args = llvm::ArrayRef(G->args_begin(), G->args_size()); else if (const auto *AA = dyn_cast(A)) Args = llvm::ArrayRef(AA->args_begin(), AA->args_size()); else if (const auto *AB = dyn_cast(A)) diff --git a/clang/test/Sema/warn-thread-safety-analysis.c b/clang/test/Sema/warn-thread-safety-analysis.c index 69ec109e1523..0fd382eb02b7 100644 --- a/clang/test/Sema/warn-thread-safety-analysis.c +++ b/clang/test/Sema/warn-thread-safety-analysis.c @@ -286,12 +286,12 @@ struct TestInit test_init(void) { // attribute in a single __attribute__. void run(void) __attribute__((guarded_by(mu1), guarded_by(mu1))); // expected-warning 2{{only applies to non-static data members and global variables}} -int value_with_wrong_number_of_args GUARDED_BY(mu1, mu2); // expected-error{{'guarded_by' attribute takes one argument}} +int value_with_multiple_guarded_args GUARDED_BY(mu1, mu2); -int *ptr_with_wrong_number_of_args PT_GUARDED_BY(mu1, mu2); // expected-error{{'pt_guarded_by' attribute takes one argument}} +int *ptr_with_multiple_guarded_args PT_GUARDED_BY(mu1, mu2); -int value_with_no_open_brace __attribute__((guarded_by)); // expected-error{{'guarded_by' attribute takes one argument}} -int *ptr_with_no_open_brace __attribute__((pt_guarded_by)); // expected-error{{'pt_guarded_by' attribute takes one argument}} +int value_with_no_open_brace __attribute__((guarded_by)); // expected-error{{'guarded_by' attribute takes at least 1 argument}} +int *ptr_with_no_open_brace __attribute__((pt_guarded_by)); // expected-error{{'pt_guarded_by' attribute takes at least 1 argument}} int value_with_no_open_brace_on_acquire_after __attribute__((acquired_after)); // expected-error{{'acquired_after' attribute takes at least 1 argument}} int value_with_no_open_brace_on_acquire_before __attribute__((acquired_before)); // expected-error{{'acquired_before' attribute takes at least 1 argument}} diff --git a/clang/test/SemaCXX/thread-safety-annotations.h b/clang/test/SemaCXX/thread-safety-annotations.h index 00d432da4b6f..bcda81cc6904 100644 --- a/clang/test/SemaCXX/thread-safety-annotations.h +++ b/clang/test/SemaCXX/thread-safety-annotations.h @@ -31,8 +31,8 @@ // Capabilities only #define EXCLUSIVE_UNLOCK_FUNCTION(...) __attribute__((release_capability(__VA_ARGS__))) #define SHARED_UNLOCK_FUNCTION(...) __attribute__((release_shared_capability(__VA_ARGS__))) -#define GUARDED_BY(x) __attribute__((guarded_by(x))) -#define PT_GUARDED_BY(x) __attribute__((pt_guarded_by(x))) +#define GUARDED_BY(...) __attribute__((guarded_by(__VA_ARGS__))) +#define PT_GUARDED_BY(...) __attribute__((pt_guarded_by(__VA_ARGS__))) // Common #define REENTRANT_CAPABILITY __attribute__((reentrant_capability)) diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp index a6c9a2236fc4..079e068ca781 100644 --- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp +++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp @@ -1526,6 +1526,16 @@ class Foo { f2(); // expected-warning {{cannot call function 'f2' while mutex 'mu1' is held}} \ // expected-warning {{cannot call function 'f2' while mutex 'mu2' is held}} } + + // Holding only mu1 (but not mu2) is insufficient to access x. + void f3() EXCLUSIVE_LOCKS_REQUIRED(mu1) { + x = 5; // expected-warning {{writing variable 'x' requires holding mutex 'mu2' exclusively}} + } + + // Holding only mu2 (but not mu1) is insufficient to access x. + void f4() EXCLUSIVE_LOCKS_REQUIRED(mu2) { + x = 5; // expected-warning {{writing variable 'x' requires holding mutex 'mu1' exclusively}} + } }; Foo *foo; @@ -1537,6 +1547,50 @@ void func() } } // end namespace thread_annot_lock_42 +namespace guarded_by_multi { +// Test multi-arg GUARDED_BY: any one capability suffices for read; all are +// required for write. +class Foo { + Mutex mu1, mu2; + int x GUARDED_BY(mu1, mu2); + int *p PT_GUARDED_BY(mu1, mu2); + + // All locks held exclusively: read and write both OK. + void f1() EXCLUSIVE_LOCKS_REQUIRED(mu1, mu2) { + x = 1; + *p = 1; + } + + // Only mu1 held exclusively: read OK, write warns about mu2. + void f2() EXCLUSIVE_LOCKS_REQUIRED(mu1) { + int y = x; + int z = *p; + x = 1; // expected-warning {{writing variable 'x' requires holding mutex 'mu2' exclusively}} + *p = 1; // expected-warning {{writing the value pointed to by 'p' requires holding mutex 'mu2' exclusively}} + } + + // One lock held shared: read OK, write warns about both. + void f3() SHARED_LOCKS_REQUIRED(mu1) { + int y = x; + int z = *p; + x = 1; // expected-warning {{writing variable 'x' requires holding mutex 'mu1' exclusively}} \ + // expected-warning {{writing variable 'x' requires holding mutex 'mu2' exclusively}} + *p = 1; // expected-warning {{writing the value pointed to by 'p' requires holding mutex 'mu1' exclusively}} \ + // expected-warning {{writing the value pointed to by 'p' requires holding mutex 'mu2' exclusively}} + } + + // No locks held: read and write both warn. + void f4() { + int y = x; // expected-warning {{reading variable 'x' requires holding at least one of 'mu1', 'mu2'}} + int z = *p; // expected-warning {{reading the value pointed to by 'p' requires holding at least one of 'mu1', 'mu2'}} + x = 1; // expected-warning {{writing variable 'x' requires holding mutex 'mu1' exclusively}} \ + // expected-warning {{writing variable 'x' requires holding mutex 'mu2' exclusively}} + *p = 1; // expected-warning {{writing the value pointed to by 'p' requires holding mutex 'mu1' exclusively}} \ + // expected-warning {{writing the value pointed to by 'p' requires holding mutex 'mu2' exclusively}} + } +}; +} // end namespace guarded_by_multi + namespace thread_annot_lock_46 { // Test the support for annotations on virtual functions. class Base { diff --git a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp index 570586ab2f53..862b2d58238c 100644 --- a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp +++ b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp @@ -5,9 +5,9 @@ #define LOCKABLE __attribute__ ((lockable)) #define REENTRANT_CAPABILITY __attribute__ ((reentrant_capability)) #define SCOPED_LOCKABLE __attribute__ ((scoped_lockable)) -#define GUARDED_BY(x) __attribute__ ((guarded_by(x))) +#define GUARDED_BY(...) __attribute__ ((guarded_by(__VA_ARGS__))) #define GUARDED_VAR __attribute__ ((guarded_var)) -#define PT_GUARDED_BY(x) __attribute__ ((pt_guarded_by(x))) +#define PT_GUARDED_BY(...) __attribute__ ((pt_guarded_by(__VA_ARGS__))) #define PT_GUARDED_VAR __attribute__ ((pt_guarded_var)) #define ACQUIRED_AFTER(...) __attribute__ ((acquired_after(__VA_ARGS__))) #define ACQUIRED_BEFORE(...) __attribute__ ((acquired_before(__VA_ARGS__))) @@ -317,8 +317,6 @@ void sl_function_params(int lvar SCOPED_LOCKABLE); // \ // Guarded By Attribute (gb) //-----------------------------------------// -// FIXME: Eventually, would we like this attribute to take more than 1 arg? - #if !__has_attribute(guarded_by) #error "Should support guarded_by attribute" #endif @@ -329,17 +327,17 @@ int gb_var_arg GUARDED_BY(mu1); int gb_non_ascii GUARDED_BY(L"wide"); // expected-warning {{ignoring 'guarded_by' attribute because its argument is invalid}} -int gb_var_args __attribute__((guarded_by(mu1, mu2))); // \ - // expected-error {{'guarded_by' attribute takes one argument}} +int gb_var_args GUARDED_BY(mu1, mu2); int gb_var_noargs __attribute__((guarded_by)); // \ - // expected-error {{'guarded_by' attribute takes one argument}} + // expected-error {{'guarded_by' attribute takes at least 1 argument}} class GBFoo { private: int gb_field_noargs __attribute__((guarded_by)); // \ - // expected-error {{'guarded_by' attribute takes one argument}} + // expected-error {{'guarded_by' attribute takes at least 1 argument}} int gb_field_args GUARDED_BY(mu1); + int gb_field_multi GUARDED_BY(mu1, mu2); }; class GUARDED_BY(mu1) GB { // \ @@ -397,12 +395,11 @@ int gb_var_arg_bad_4 GUARDED_BY(umu); // \ //1. Check applied to the right types & argument number int *pgb_var_noargs __attribute__((pt_guarded_by)); // \ - // expected-error {{'pt_guarded_by' attribute takes one argument}} + // expected-error {{'pt_guarded_by' attribute takes at least 1 argument}} int *pgb_ptr_var_arg PT_GUARDED_BY(mu1); -int *pgb_ptr_var_args __attribute__((pt_guarded_by(mu1, mu2))); // \ - // expected-error {{'pt_guarded_by' attribute takes one argument}} +int *pgb_ptr_var_args PT_GUARDED_BY(mu1, mu2); int pgb_var_args PT_GUARDED_BY(mu1); // \ // expected-warning {{'pt_guarded_by' only applies to pointer types; type here is 'int'}} @@ -410,8 +407,9 @@ int pgb_var_args PT_GUARDED_BY(mu1); // \ class PGBFoo { private: int *pgb_field_noargs __attribute__((pt_guarded_by)); // \ - // expected-error {{'pt_guarded_by' attribute takes one argument}} + // expected-error {{'pt_guarded_by' attribute takes at least 1 argument}} int *pgb_field_args PT_GUARDED_BY(mu1); + int *pgb_field_multi PT_GUARDED_BY(mu1, mu2); }; class PT_GUARDED_BY(mu1) PGB { // \ diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index 164790606ea0..d1d6ea94d315 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -8142,7 +8142,7 @@ TEST_P(ImportAttributes, ImportGuardedBy) { int test __attribute__((guarded_by(G))); )", FromAttr, ToAttr); - checkImported(FromAttr->getArg(), ToAttr->getArg()); + checkImportVariadicArg(FromAttr->args(), ToAttr->args()); } TEST_P(ImportAttributes, ImportPtGuardedBy) { @@ -8153,7 +8153,7 @@ TEST_P(ImportAttributes, ImportPtGuardedBy) { int *test __attribute__((pt_guarded_by(G))); )", FromAttr, ToAttr); - checkImported(FromAttr->getArg(), ToAttr->getArg()); + checkImportVariadicArg(FromAttr->args(), ToAttr->args()); } TEST_P(ImportAttributes, ImportAcquiredAfter) {