diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 7ac0fa0141b473..28ec3d54a5a773 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -71,6 +71,11 @@ C++ Specific Potentially Breaking Changes To fix this, update libstdc++ to version 14.1.1 or greater. +- Clang now emits errors when Thread Safety Analysis trylock attributes are + applied to functions or methods with incompatible return values, such as + constructors, destructors, and void-returning functions. This only affects the + ``TRY_ACQUIRE`` and ``TRY_ACQUIRE_SHARED`` attributes (and any synonyms). + ABI Changes in This Version --------------------------- - Fixed Microsoft name mangling of implicitly defined variables used for thread @@ -720,6 +725,11 @@ Bug Fixes in This Version - Fixed `static_cast` to array of unknown bound. Fixes (#GH62863). +- Clang's Thread Safety Analysis now evaluates trylock success arguments of enum + types rather than silently defaulting to false. This fixes a class of false + negatives where the analysis failed to detect unchecked access to guarded + data. + Bug Fixes to Compiler Builtins ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/docs/ThreadSafetyAnalysis.rst b/clang/docs/ThreadSafetyAnalysis.rst index dcde0c706c7045..0ecbebe7a692f4 100644 --- a/clang/docs/ThreadSafetyAnalysis.rst +++ b/clang/docs/ThreadSafetyAnalysis.rst @@ -420,10 +420,17 @@ TRY_ACQUIRE(, ...), TRY_ACQUIRE_SHARED(, ...) *Previously:* ``EXCLUSIVE_TRYLOCK_FUNCTION``, ``SHARED_TRYLOCK_FUNCTION`` These are attributes on a function or method that tries to acquire the given -capability, and returns a boolean value indicating success or failure. -The first argument must be ``true`` or ``false``, to specify which return value -indicates success, and the remaining arguments are interpreted in the same way -as ``ACQUIRE``. See :ref:`mutexheader`, below, for example uses. +capability, and returns a boolean, integer, or pointer value indicating success +or failure. + +The attribute's first argument defines whether a zero or non-zero return value +indicates success. Syntactically, it accepts ``NULL`` or ``nullptr``, ``bool`` +and ``int`` literals, as well as enumerator values. *The analysis only cares +whether this success value is zero or non-zero.* This leads to some subtle +consequences, discussed in the next section. + +The remaining arguments are interpreted in the same way as ``ACQUIRE``. See +:ref:`mutexheader`, below, for example uses. Because the analysis doesn't support conditional locking, a capability is treated as acquired after the first branch on the return value of a try-acquire @@ -445,6 +452,43 @@ function. } } +Subtle Consequences of Non-Boolean Success Values +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +The trylock attributes accept non-boolean expressions for the success value, but +the analysis only cares whether the value is zero or non-zero. + +Suppose you define an enum with two non-zero enumerators: ``LockAcquired = 1`` +and ``LockNotAcquired = 2``. If your trylock function returns ``LockAcquired`` +on success and ``LockNotAcquired`` on failure, the analysis may fail to detect +access to guarded data without holding the mutex because they are both non-zero. + +.. code-block:: c++ + // *** Beware: this code demonstrates incorrect usage. *** + + enum TrylockResult { LockAcquired = 1, LockNotAcquired = 2 }; + + class CAPABILITY("mutex") Mutex { + public: + TrylockResult TryLock() TRY_ACQUIRE(LockAcquired); + }; + + Mutex mu; + int a GUARDED_BY(mu); + + void foo() { + if (mu.TryLock()) { // This branch satisfies the analysis, but permits + // unguarded access to `a`! + a = 0; + mu.Unlock(); + } + } + +It's also possible to return a pointer from the trylock function. Similarly, all +that matters is whether the success value is zero or non-zero. For instance, a +success value of `true` means the function returns a non-null pointer on +success. + ASSERT_CAPABILITY(...) and ASSERT_SHARED_CAPABILITY(...) -------------------------------------------------------- diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 25a87078a57093..a564a12fd572fc 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -3272,11 +3272,23 @@ def warn_aligned_attr_underaligned : Warning, def err_attribute_sizeless_type : Error< "%0 attribute cannot be applied to sizeless type %1">; def err_attribute_argument_n_type : Error< - "%0 attribute requires parameter %1 to be %select{int or bool|an integer " - "constant|a string|an identifier|a constant expression|a builtin function}2">; + "%0 attribute requires parameter %1 to be %select{" + "int or bool" + "|an integer constant" + "|a string" + "|an identifier" + "|a constant expression" + "|a builtin function" + "|nullptr or a bool, int, or enum literal}2">; def err_attribute_argument_type : Error< - "%0 attribute requires %select{int or bool|an integer " - "constant|a string|an identifier}1">; + "%0 attribute requires %select{" + "int or bool" + "|an integer constant" + "|a string" + "|an identifier" + "|a constant expression" + "|a builtin function" + "|a pointer or a bool, int, or enum literal}1">; def err_attribute_argument_out_of_range : Error< "%0 attribute requires integer constant between %1 and %2 inclusive">; def err_init_priority_object_attr : Error< @@ -3728,7 +3740,8 @@ def warn_attribute_wrong_decl_type : Warning< "|types and namespaces" "|variables, functions and classes" "|kernel functions" - "|non-K&R-style functions}2">, + "|non-K&R-style functions" + "|functions that return bool, integer, or a pointer type}2">, InGroup; def err_attribute_wrong_decl_type : Error; def warn_type_attribute_wrong_type : Warning< diff --git a/clang/include/clang/Sema/ParsedAttr.h b/clang/include/clang/Sema/ParsedAttr.h index 22cbd0d90ee432..65d73f6cd44f6e 100644 --- a/clang/include/clang/Sema/ParsedAttr.h +++ b/clang/include/clang/Sema/ParsedAttr.h @@ -1083,6 +1083,7 @@ enum AttributeArgumentNType { AANT_ArgumentIdentifier, AANT_ArgumentConstantExpr, AANT_ArgumentBuiltinFunction, + AANT_ArgumentNullptrOrBoolIntOrEnumLiteral, }; /// These constants match the enumerated choices of @@ -1101,6 +1102,7 @@ enum AttributeDeclKind { ExpectedFunctionVariableOrClass, ExpectedKernelFunction, ExpectedFunctionWithProtoType, + ExpectedFunctionReturningPointerBoolIntOrEnum, }; inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB, diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp index e25b843c9bf83e..550c2a3ffe522f 100644 --- a/clang/lib/Analysis/ThreadSafety.cpp +++ b/clang/lib/Analysis/ThreadSafety.cpp @@ -37,6 +37,7 @@ #include "clang/Basic/OperatorKinds.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/Specifiers.h" +#include "llvm/ADT/APSInt.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/ImmutableMap.h" @@ -1034,9 +1035,10 @@ class ThreadSafetyAnalyzer { template void getMutexIDs(CapExprSet &Mtxs, AttrType *Attr, const Expr *Exp, - const NamedDecl *D, - const CFGBlock *PredBlock, const CFGBlock *CurrBlock, - Expr *BrE, bool Neg); + const NamedDecl *D, const CFGBlock *PredBlock, + const CFGBlock *CurrBlock, + const ASTContext &TrylockAttrContext, + Expr *TrylockAttrSuccessValue, bool Neg); const CallExpr* getTrylockCallExpr(const Stmt *Cond, LocalVarContext C, bool &Negate); @@ -1359,17 +1361,18 @@ void ThreadSafetyAnalyzer::getMutexIDs(CapExprSet &Mtxs, AttrType *Attr, const Expr *Exp, const NamedDecl *D, const CFGBlock *PredBlock, const CFGBlock *CurrBlock, - Expr *BrE, bool Neg) { - // Find out which branch has the lock - bool branch = false; - if (const auto *BLE = dyn_cast_or_null(BrE)) - branch = BLE->getValue(); - else if (const auto *ILE = dyn_cast_or_null(BrE)) - branch = ILE->getValue().getBoolValue(); - - int branchnum = branch ? 0 : 1; - if (Neg) - branchnum = !branchnum; + const ASTContext &TrylockAttrContext, + Expr *TrylockAttrSuccess, + bool Neg) { + // Evaluate the trylock's success value as a boolean. + bool trylockSuccessValue = false; + if (!TrylockAttrSuccess->EvaluateAsBooleanCondition( + trylockSuccessValue, TrylockAttrContext, + /*InConstantContext=*/true)) { + llvm_unreachable("Trylock success value could not be evaluated."); + } + + const int branchnum = Neg ? !!trylockSuccessValue : !trylockSuccessValue; // If we've taken the trylock branch, then add the lock int i = 0; @@ -1390,8 +1393,15 @@ static bool getStaticBooleanValue(Expr *E, bool &TCond) { } else if (const auto *ILE = dyn_cast(E)) { TCond = ILE->getValue().getBoolValue(); return true; - } else if (auto *CE = dyn_cast(E)) + } else if (auto *CE = dyn_cast(E)) { return getStaticBooleanValue(CE->getSubExpr(), TCond); + } else if (auto *DRE = dyn_cast(E)) { + if (auto *ECD = dyn_cast(DRE->getDecl())) { + llvm::APSInt IV = ECD->getInitVal(); + TCond = IV.getBoolValue(); + return true; + } + } return false; } @@ -1497,27 +1507,27 @@ void ThreadSafetyAnalyzer::getEdgeLockset(FactSet& Result, // If the condition is a call to a Trylock function, then grab the attributes for (const auto *Attr : FunDecl->attrs()) { switch (Attr->getKind()) { - case attr::TryAcquireCapability: { - auto *A = cast(Attr); - getMutexIDs(A->isShared() ? SharedLocksToAdd : ExclusiveLocksToAdd, A, - Exp, FunDecl, PredBlock, CurrBlock, A->getSuccessValue(), - Negate); - break; - }; - case attr::ExclusiveTrylockFunction: { - const auto *A = cast(Attr); - getMutexIDs(ExclusiveLocksToAdd, A, Exp, FunDecl, PredBlock, CurrBlock, - A->getSuccessValue(), Negate); - break; - } - case attr::SharedTrylockFunction: { - const auto *A = cast(Attr); - getMutexIDs(SharedLocksToAdd, A, Exp, FunDecl, PredBlock, CurrBlock, - A->getSuccessValue(), Negate); - break; - } - default: - break; + case attr::TryAcquireCapability: { + auto *A = cast(Attr); + getMutexIDs(A->isShared() ? SharedLocksToAdd : ExclusiveLocksToAdd, A, + Exp, FunDecl, PredBlock, CurrBlock, FunDecl->getASTContext(), + A->getSuccessValue(), Negate); + break; + }; + case attr::ExclusiveTrylockFunction: { + const auto *A = cast(Attr); + getMutexIDs(ExclusiveLocksToAdd, A, Exp, FunDecl, PredBlock, CurrBlock, + FunDecl->getASTContext(), A->getSuccessValue(), Negate); + break; + } + case attr::SharedTrylockFunction: { + const auto *A = cast(Attr); + getMutexIDs(SharedLocksToAdd, A, Exp, FunDecl, PredBlock, CurrBlock, + FunDecl->getASTContext(), A->getSuccessValue(), Negate); + break; + } + default: + break; } } diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index ce6b5b1ff6f931..99b400307d6447 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -14,6 +14,7 @@ #include "clang/AST/ASTContext.h" #include "clang/AST/ASTMutationListener.h" #include "clang/AST/CXXInheritance.h" +#include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/DeclTemplate.h" @@ -25,6 +26,7 @@ #include "clang/Basic/CharInfo.h" #include "clang/Basic/Cuda.h" #include "clang/Basic/DarwinSDKInfo.h" +#include "clang/Basic/DiagnosticSema.h" #include "clang/Basic/HLSLRuntime.h" #include "clang/Basic/IdentifierTable.h" #include "clang/Basic/LangOptions.h" @@ -65,10 +67,13 @@ #include "llvm/Demangle/Demangle.h" #include "llvm/IR/Assumptions.h" #include "llvm/MC/MCSectionMachO.h" +#include "llvm/Support/Casting.h" #include "llvm/Support/Error.h" +#include "llvm/Support/ErrorHandling.h" #include "llvm/Support/MathExtras.h" #include "llvm/Support/raw_ostream.h" #include "llvm/TargetParser/Triple.h" +#include #include using namespace clang; @@ -166,13 +171,6 @@ bool Sema::checkStringLiteralArgumentAttr(const ParsedAttr &AL, unsigned ArgNum, return checkStringLiteralArgumentAttr(AL, ArgExpr, Str, ArgLocation); } -/// Check if the passed-in expression is of type int or bool. -static bool isIntOrBool(Expr *Exp) { - QualType QT = Exp->getType(); - return QT->isBooleanType() || QT->isIntegerType(); -} - - // Check to see if the type is a smart pointer of some kind. We assume // it's a smart pointer if it defines both operator-> and operator*. static bool threadSafetyCheckIsSmartPointer(Sema &S, const RecordType* RT) { @@ -608,15 +606,31 @@ static bool checkTryLockFunAttrCommon(Sema &S, Decl *D, const ParsedAttr &AL, if (!AL.checkAtLeastNumArgs(S, 1)) return false; - if (!isIntOrBool(AL.getArgAsExpr(0))) { + // The attribute's first argument defines the success value. + const Expr *SuccessArg = AL.getArgAsExpr(0); + if (!isa(SuccessArg) && + !isa(SuccessArg) && !isa(SuccessArg) && + !isa(SuccessArg) && !SuccessArg->getEnumConstantDecl()) { S.Diag(AL.getLoc(), diag::err_attribute_argument_n_type) - << AL << 1 << AANT_ArgumentIntOrBool; + << AL << 1 << AANT_ArgumentNullptrOrBoolIntOrEnumLiteral; return false; } - // check that all arguments are lockable objects + // All remaining arguments must be lockable objects. checkAttrArgsAreCapabilityObjs(S, D, AL, Args, 1); + // The function must return a pointer, boolean, integer, or enum. We already + // know that `D` is a function because `ExclusiveTrylockFunction` and friends + // are defined in Attr.td with subject lists that only include functions. + QualType ReturnType = D->getAsFunction()->getReturnType(); + if (!ReturnType->isPointerType() && !ReturnType->isBooleanType() && + !ReturnType->isIntegerType() && !ReturnType->isEnumeralType()) { + S.Diag(AL.getLoc(), diag::err_attribute_wrong_decl_type) + << AL << AL.isRegularKeywordAttribute() + << ExpectedFunctionReturningPointerBoolIntOrEnum; + return false; + } + return true; } diff --git a/clang/test/Sema/attr-capabilities.c b/clang/test/Sema/attr-capabilities.c index 5138803bd5eb7a..91a43c79d5b917 100644 --- a/clang/test/Sema/attr-capabilities.c +++ b/clang/test/Sema/attr-capabilities.c @@ -54,14 +54,14 @@ void Func18(void) __attribute__((release_capability())) {} // expected-warning { void Func19(void) __attribute__((release_shared_capability())) {} // expected-warning {{'release_shared_capability' attribute without capability arguments can only be applied to non-static methods of a class}} void Func20(void) __attribute__((release_generic_capability())) {} // expected-warning {{'release_generic_capability' attribute without capability arguments can only be applied to non-static methods of a class}} -void Func21(void) __attribute__((try_acquire_capability(1))) {} // expected-warning {{'try_acquire_capability' attribute without capability arguments can only be applied to non-static methods of a class}} -void Func22(void) __attribute__((try_acquire_shared_capability(1))) {} // expected-warning {{'try_acquire_shared_capability' attribute without capability arguments can only be applied to non-static methods of a class}} +int Func21(void) __attribute__((try_acquire_capability(1))) {} // expected-warning {{'try_acquire_capability' attribute without capability arguments can only be applied to non-static methods of a class}} +int Func22(void) __attribute__((try_acquire_shared_capability(1))) {} // expected-warning {{'try_acquire_shared_capability' attribute without capability arguments can only be applied to non-static methods of a class}} -void Func23(void) __attribute__((try_acquire_capability(1, GUI))) {} -void Func24(void) __attribute__((try_acquire_shared_capability(1, GUI))) {} +int Func23(void) __attribute__((try_acquire_capability(1, GUI))) {} +int Func24(void) __attribute__((try_acquire_shared_capability(1, GUI))) {} -void Func25(void) __attribute__((try_acquire_capability())) {} // expected-error {{'try_acquire_capability' attribute takes at least 1 argument}} -void Func26(void) __attribute__((try_acquire_shared_capability())) {} // expected-error {{'try_acquire_shared_capability' attribute takes at least 1 argument}} +int Func25(void) __attribute__((try_acquire_capability())) {} // expected-error {{'try_acquire_capability' attribute takes at least 1 argument}} +int Func26(void) __attribute__((try_acquire_shared_capability())) {} // expected-error {{'try_acquire_shared_capability' attribute takes at least 1 argument}} // Test that boolean logic works with capability attributes void Func27(void) __attribute__((requires_capability(!GUI))); diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp index 73cc946ca0ce1c..a6ddf028e1fadb 100644 --- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp +++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp @@ -1954,6 +1954,125 @@ struct TestTryLock { } // end namespace TrylockTest +// Regression test for trylock attributes with an enumerator success argument. +// Prior versions of the analysis silently failed to evaluate success arguments +// that were neither `CXXBoolLiteralExpr` nor `IntegerLiteral` and assumed the +// value was false. +namespace TrylockSuccessEnumFalseNegative { + +enum TrylockResult { Failure = 0, Success = 1 }; + +class LOCKABLE Mutex { +public: + TrylockResult TryLock() EXCLUSIVE_TRYLOCK_FUNCTION(Success); + void Unlock() EXCLUSIVE_UNLOCK_FUNCTION(); +}; + +class TrylockTest { + Mutex mu_; + int a_ GUARDED_BY(mu_) = 0; + + void test_bool_expression() { + if (!mu_.TryLock()) { // expected-note {{mutex acquired here}} + a_++; // expected-warning {{writing variable 'a_' requires holding mutex 'mu_' exclusively}} + mu_.Unlock(); // expected-warning {{releasing mutex 'mu_' that was not held}} + } + } // expected-warning {{mutex 'mu_' is not held on every path through here}} +}; +} // namespace TrylockSuccessEnumFalseNegative + +// This test demonstrates that the analysis does not distinguish between +// different non-zero enumerators. +namespace TrylockFalseNegativeWhenEnumHasMultipleFailures { + +enum TrylockResult { Failure1 = 0, Failure2, Success }; + +class LOCKABLE Mutex { +public: + TrylockResult TryLock() EXCLUSIVE_TRYLOCK_FUNCTION(Success); + void Unlock() EXCLUSIVE_UNLOCK_FUNCTION(); +}; + +class TrylockTest { + Mutex mu_; + int a_ GUARDED_BY(mu_) = 0; + + void test_eq_success() { + if (mu_.TryLock() == Success) { + a_++; + mu_.Unlock(); + } + } + + void test_bool_expression() { + // This branch checks whether the trylock function returned a non-zero + // value. This satisfies the analysis, but fails to account for `Failure2`. + // From the analysis's perspective, `Failure2` and `Success` are equivalent! + if (mu_.TryLock()) { + a_++; + mu_.Unlock(); + } + } +}; +} // namespace TrylockSuccessEnumMultipleFailureModesFalseNegative + + +// This test demonstrates that the analysis does not detect when all enumerators +// are positive, and thus incapable of representing a failure. +namespace TrylockSuccessEnumNoZeroFalseNegative { + +enum TrylockResult { Failure = 1, Success = 2 }; + +class LOCKABLE Mutex { +public: + TrylockResult TryLock() EXCLUSIVE_TRYLOCK_FUNCTION(Success); + void Unlock() EXCLUSIVE_UNLOCK_FUNCTION(); +}; + +class TrylockTest { + Mutex mu_; + int a_ GUARDED_BY(mu_) = 0; + + void test_bool_expression() { + // This branch checks whether the trylock function returned a non-zero value + // This satisfies the analysis, but is actually incorrect because `Failure` + // and `Success` are both non-zero. + if (mu_.TryLock()) { + a_++; + mu_.Unlock(); + } + } +}; +} // namespace TrylockSuccessEnumNoZeroFalseNegative + + +// Demonstrate a mutex with a trylock function that conditionally vends a +// pointer to guarded data. +namespace TrylockFunctionReturnPointer { + +class LOCKABLE OwningMutex { +public: + int *TryLock() EXCLUSIVE_TRYLOCK_FUNCTION(true); + void Unlock() EXCLUSIVE_UNLOCK_FUNCTION(); + +private: + int guarded_ GUARDED_BY(this) = 0; +}; + +class TrylockTest { + OwningMutex mu_; + + void test_ptr_return() { + if (int *p = mu_.TryLock()) { + *p = 0; + mu_.Unlock(); + } + } +}; + +} // namespace TrylockFunctionReturnPointer + + namespace TestTemplateAttributeInstantiation { class Foo1 { @@ -3657,8 +3776,8 @@ class Foo { int a GUARDED_BY(mu_); bool c; - int tryLockMutexI() EXCLUSIVE_TRYLOCK_FUNCTION(1, mu_); - Mutex* tryLockMutexP() EXCLUSIVE_TRYLOCK_FUNCTION(1, mu_); + int tryLockMutexI() EXCLUSIVE_TRYLOCK_FUNCTION(1, mu_); + Mutex *tryLockMutexP() EXCLUSIVE_TRYLOCK_FUNCTION(1, mu_); void unlock() UNLOCK_FUNCTION(mu_); void test1(); diff --git a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp index 0c5b0cc85897be..4921a75b84e398 100644 --- a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp +++ b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp @@ -716,15 +716,17 @@ int slf_function_bad_7() SHARED_LOCK_FUNCTION(0); // \ #error "Should support exclusive_trylock_function attribute" #endif -// takes a mandatory boolean or integer argument specifying the retval -// plus an optional list of locks (vars/fields) +// The attribute takes a mandatory boolean or integer argument specifying the +// retval plus an optional list of locks (vars/fields). The annotated function's +// return type should be boolean or integer. void etf_function() __attribute__((exclusive_trylock_function)); // \ // expected-error {{'exclusive_trylock_function' attribute takes at least 1 argument}} -void etf_function_args() EXCLUSIVE_TRYLOCK_FUNCTION(1, mu2); +void etf_function_args() EXCLUSIVE_TRYLOCK_FUNCTION(1, mu2); // \ + // expected-error {{'exclusive_trylock_function' attribute only applies to functions that return bool, integer, or a pointer type}} -void etf_function_arg() EXCLUSIVE_TRYLOCK_FUNCTION(1); // \ +int etf_function_arg() EXCLUSIVE_TRYLOCK_FUNCTION(1); // \ // expected-warning {{'exclusive_trylock_function' attribute without capability arguments can only be applied to non-static methods of a class}} int etf_testfn(int y) EXCLUSIVE_TRYLOCK_FUNCTION(1); // \ @@ -743,10 +745,23 @@ class EtfFoo { private: int test_field EXCLUSIVE_TRYLOCK_FUNCTION(1); // \ // expected-warning {{'exclusive_trylock_function' attribute only applies to functions}} - void test_method() EXCLUSIVE_TRYLOCK_FUNCTION(1); // \ + bool test_method() EXCLUSIVE_TRYLOCK_FUNCTION(1); // \ // expected-warning {{'exclusive_trylock_function' attribute without capability arguments refers to 'this', but 'EtfFoo' isn't annotated with 'capability' or 'scoped_lockable' attribute}} }; +// It does not make sense to annotate constructors/destructors as exclusive +// trylock functions because they cannot return a value. See +// . +class EtfConstructorDestructor { + EtfConstructorDestructor() EXCLUSIVE_TRYLOCK_FUNCTION(1, mu); // \ + // expected-error {{'exclusive_trylock_function' attribute only applies to functions that return bool, integer, or a pointer type}} + + ~EtfConstructorDestructor() EXCLUSIVE_TRYLOCK_FUNCTION(1, mu); // \ + // expected-error {{'exclusive_trylock_function' attribute only applies to functions that return bool, integer, or a pointer type}} + + Mutex mu; +}; + class EXCLUSIVE_TRYLOCK_FUNCTION(1) EtfTestClass { // \ // expected-warning {{'exclusive_trylock_function' attribute only applies to functions}} }; @@ -756,7 +771,68 @@ void etf_fun_params(int lvar EXCLUSIVE_TRYLOCK_FUNCTION(1)); // \ // Check argument parsing. -// legal attribute arguments +int global_int = 0; +int* etf_fun_with_global_ptr_success() EXCLUSIVE_TRYLOCK_FUNCTION(&global_int, &mu1); //\ + // expected-error {{'exclusive_trylock_function' attribute requires parameter 1 to be nullptr or a bool, int, or enum literal}} + +#ifdef CPP11 +constexpr int kTrylockSuccess = 1; +int etf_succ_constexpr() EXCLUSIVE_TRYLOCK_FUNCTION(kTrylockSuccess, mu2); // \ + // expected-error {{'exclusive_trylock_function' attribute requires parameter 1 to be nullptr or a bool, int, or enum literal}} + +int success_value_non_constant = 1; + +bool etf_succ_variable() EXCLUSIVE_TRYLOCK_FUNCTION(success_value_non_constant, mu2); //\ + // expected-error {{attribute requires parameter 1 to be nullptr or a bool, int, or enum literal}} +#endif + + +// Legal permutations of the first argument and function return type. +struct TrylockResult; + +#ifdef CPP11 +int* etf_fun_with_nullptr_success() EXCLUSIVE_TRYLOCK_FUNCTION(nullptr, &mu1); +#endif + +#ifndef __cplusplus +int* etf_fun_with_nullptr_success() EXCLUSIVE_TRYLOCK_FUNCTION(NULL, &mu1); +#endif + +int etf_succ_1_ret_i() EXCLUSIVE_TRYLOCK_FUNCTION(1, mu2); +bool etf_succ_1_ret_b() EXCLUSIVE_TRYLOCK_FUNCTION(1, mu2); +TrylockResult *etf_succ_1_ret_p() EXCLUSIVE_TRYLOCK_FUNCTION(1, mu2); + +int etf_succ_true_ret_i() EXCLUSIVE_TRYLOCK_FUNCTION(true, mu2); +bool etf_succ_true_ret_b() EXCLUSIVE_TRYLOCK_FUNCTION(true, mu2); +TrylockResult *etf_succ_true_ret_p() EXCLUSIVE_TRYLOCK_FUNCTION(true, mu2); + +int etf_succ_false_ret_i() EXCLUSIVE_TRYLOCK_FUNCTION(false, mu2); +bool etf_succ_false_ret_b() EXCLUSIVE_TRYLOCK_FUNCTION(false, mu2); +TrylockResult *etf_succ_false_ret_p() EXCLUSIVE_TRYLOCK_FUNCTION(false, mu2); + +enum TrylockSuccessEnum { TrylockNotAcquired = 0, TrylockAcquired }; +int etf_succ_enum_ret_i() EXCLUSIVE_TRYLOCK_FUNCTION(TrylockAcquired, mu2); +bool etf_succ_enum_ret_b() EXCLUSIVE_TRYLOCK_FUNCTION(TrylockAcquired, mu2); +TrylockResult *etf_succ_enum_ret_p() + EXCLUSIVE_TRYLOCK_FUNCTION(TrylockAcquired, mu2); + +#ifdef CPP11 +enum class TrylockSuccessEnumClass { NotAcquired = 0, Acquired}; +int etf_succ_enum_class_ret_i() + EXCLUSIVE_TRYLOCK_FUNCTION(TrylockSuccessEnumClass::Acquired, mu2); +bool etf_succ_enum_class_ret_b() + EXCLUSIVE_TRYLOCK_FUNCTION(TrylockSuccessEnumClass::Acquired, mu2); +TrylockResult *etf_succ_enum_class_ret_p() + EXCLUSIVE_TRYLOCK_FUNCTION(TrylockSuccessEnumClass::Acquired, mu2); +#endif + +// Demonstrate that we do not detect enum type mismatches between the success +// argument and the function's return type. +enum SomeOtherEnum { Foo = 1 }; +TrylockSuccessEnum etf_succ_enum_mismatch() + EXCLUSIVE_TRYLOCK_FUNCTION(Foo, mu2); + +// legal capability attribute arguments int etf_function_1() EXCLUSIVE_TRYLOCK_FUNCTION(1, muWrapper.mu); int etf_function_2() EXCLUSIVE_TRYLOCK_FUNCTION(1, muDoubleWrapper.muWrapper->mu); int etf_function_3() EXCLUSIVE_TRYLOCK_FUNCTION(1, muWrapper.getMu()); @@ -768,14 +844,13 @@ int etf_functetfn_8() EXCLUSIVE_TRYLOCK_FUNCTION(1, muPointer); int etf_function_9() EXCLUSIVE_TRYLOCK_FUNCTION(true); // \ // expected-warning {{'exclusive_trylock_function' attribute without capability arguments can only be applied to non-static methods of a class}} - // illegal attribute arguments int etf_function_bad_1() EXCLUSIVE_TRYLOCK_FUNCTION(mu1); // \ - // expected-error {{'exclusive_trylock_function' attribute requires parameter 1 to be int or bool}} + // expected-error {{'exclusive_trylock_function' attribute requires parameter 1 to be nullptr or a bool, int, or enum literal}} int etf_function_bad_2() EXCLUSIVE_TRYLOCK_FUNCTION("mu"); // \ - // expected-error {{'exclusive_trylock_function' attribute requires parameter 1 to be int or bool}} + // expected-error {{'exclusive_trylock_function' attribute requires parameter 1 to be nullptr or a bool, int, or enum literal}} int etf_function_bad_3() EXCLUSIVE_TRYLOCK_FUNCTION(muDoublePointer); // \ - // expected-error {{'exclusive_trylock_function' attribute requires parameter 1 to be int or bool}} + // expected-error {{'exclusive_trylock_function' attribute requires parameter 1 to be nullptr or a bool, int, or enum literal}} int etf_function_bad_4() EXCLUSIVE_TRYLOCK_FUNCTION(1, "mu"); // \ // expected-warning {{ignoring 'exclusive_trylock_function' attribute because its argument is invalid}} @@ -799,9 +874,9 @@ int etf_function_bad_6() EXCLUSIVE_TRYLOCK_FUNCTION(1, umu); // \ void stf_function() __attribute__((shared_trylock_function)); // \ // expected-error {{'shared_trylock_function' attribute takes at least 1 argument}} -void stf_function_args() SHARED_TRYLOCK_FUNCTION(1, mu2); +bool stf_function_args() SHARED_TRYLOCK_FUNCTION(1, mu2); -void stf_function_arg() SHARED_TRYLOCK_FUNCTION(1); // \ +bool stf_function_arg() SHARED_TRYLOCK_FUNCTION(1); // \ // expected-warning {{'shared_trylock_function' attribute without capability arguments can only be applied to non-static methods of a class}} int stf_testfn(int y) SHARED_TRYLOCK_FUNCTION(1); // \ @@ -824,7 +899,7 @@ class StfFoo { private: int test_field SHARED_TRYLOCK_FUNCTION(1); // \ // expected-warning {{'shared_trylock_function' attribute only applies to functions}} - void test_method() SHARED_TRYLOCK_FUNCTION(1); // \ + bool test_method() SHARED_TRYLOCK_FUNCTION(1); // \ // expected-warning {{'shared_trylock_function' attribute without capability arguments refers to 'this', but 'StfFoo' isn't annotated with 'capability' or 'scoped_lockable' attribute}} }; @@ -849,11 +924,11 @@ int stf_function_9() SHARED_TRYLOCK_FUNCTION(true); // \ // illegal attribute arguments int stf_function_bad_1() SHARED_TRYLOCK_FUNCTION(mu1); // \ - // expected-error {{'shared_trylock_function' attribute requires parameter 1 to be int or bool}} + // expected-error {{'shared_trylock_function' attribute requires parameter 1 to be nullptr or a bool, int, or enum literal}} int stf_function_bad_2() SHARED_TRYLOCK_FUNCTION("mu"); // \ - // expected-error {{'shared_trylock_function' attribute requires parameter 1 to be int or bool}} + // expected-error {{'shared_trylock_function' attribute requires parameter 1 to be nullptr or a bool, int, or enum literal}} int stf_function_bad_3() SHARED_TRYLOCK_FUNCTION(muDoublePointer); // \ - // expected-error {{'shared_trylock_function' attribute requires parameter 1 to be int or bool}} + // expected-error {{'shared_trylock_function' attribute requires parameter 1 to be nullptr or a bool, int, or enum literal}} int stf_function_bad_4() SHARED_TRYLOCK_FUNCTION(1, "mu"); // \ // expected-warning {{ignoring 'shared_trylock_function' attribute because its argument is invalid}} diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index 92f9bae6cb064b..4c33546de6f923 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -7851,7 +7851,7 @@ TEST_P(ImportAttributes, ImportAcquireCapability) { TEST_P(ImportAttributes, ImportTryAcquireCapability) { TryAcquireCapabilityAttr *FromAttr, *ToAttr; importAttr( - "void test(int A1, int A2) __attribute__((try_acquire_capability(1, A1, " + "bool test(int A1, int A2) __attribute__((try_acquire_capability(1, A1, " "A2)));", FromAttr, ToAttr); checkImported(FromAttr->getSuccessValue(), ToAttr->getSuccessValue()); @@ -7948,7 +7948,7 @@ TEST_P(ImportAttributes, ImportAssertSharedLock) { TEST_P(ImportAttributes, ImportExclusiveTrylockFunction) { ExclusiveTrylockFunctionAttr *FromAttr, *ToAttr; importAttr( - "void test(int A1, int A2) __attribute__((exclusive_trylock_function(1, " + "bool test(int A1, int A2) __attribute__((exclusive_trylock_function(1, " "A1, A2)));", FromAttr, ToAttr); checkImported(FromAttr->getSuccessValue(), ToAttr->getSuccessValue()); @@ -7958,7 +7958,7 @@ TEST_P(ImportAttributes, ImportExclusiveTrylockFunction) { TEST_P(ImportAttributes, ImportSharedTrylockFunction) { SharedTrylockFunctionAttr *FromAttr, *ToAttr; importAttr( - "void test(int A1, int A2) __attribute__((shared_trylock_function(1, A1, " + "bool test(int A1, int A2) __attribute__((shared_trylock_function(1, A1, " "A2)));", FromAttr, ToAttr); checkImported(FromAttr->getSuccessValue(), ToAttr->getSuccessValue());