diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 60b549999c155..bce0e2f2ac9f1 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -3331,7 +3331,7 @@ def ReleaseCapability : InheritableAttr { let Documentation = [ReleaseCapabilityDocs]; } -def RequiresCapability : InheritableAttr { +def RequiresCapability : DeclOrTypeAttr { let Spellings = [Clang<"requires_capability", 0>, Clang<"exclusive_locks_required", 0>, Clang<"requires_shared_capability", 0>, @@ -3341,7 +3341,7 @@ def RequiresCapability : InheritableAttr { let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; - let Subjects = SubjectList<[Function]>; + let Subjects = SubjectList<[Function, FunctionPointer]>; let Accessors = [Accessor<"isShared", [Clang<"requires_shared_capability", 0>, Clang<"shared_locks_required", 0>]>]; let Documentation = [Undocumented]; diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index daed24be0a86d..a5b315145e3fc 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -13868,6 +13868,11 @@ class Sema final { SourceLocation BuiltinLoc, SourceLocation RParenLoc); + void checkAttrArgsAreCapabilityObjs(Decl *D, const ParsedAttr &AL, + SmallVectorImpl &Args, + unsigned Sidx = 0, + bool ParamIdxOk = false); + private: bool SemaBuiltinPrefetch(CallExpr *TheCall); bool SemaBuiltinAllocaWithAlign(CallExpr *TheCall); diff --git a/clang/lib/AST/TypePrinter.cpp b/clang/lib/AST/TypePrinter.cpp index e4f5f40cd6259..2d62b6a5bd851 100644 --- a/clang/lib/AST/TypePrinter.cpp +++ b/clang/lib/AST/TypePrinter.cpp @@ -1894,6 +1894,8 @@ void TypePrinter::printAttributedAfter(const AttributedType *T, case attr::ArmMveStrictPolymorphism: OS << "__clang_arm_mve_strict_polymorphism"; break; + case attr::RequiresCapability: + OS << "requires_capability(blah)"; } OS << "))"; } diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 842a01a88cd3c..e45b0f20d7458 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -600,12 +600,10 @@ static bool isCapabilityExpr(Sema &S, const Expr *Ex) { /// \param Sidx The attribute argument index to start checking with. /// \param ParamIdxOk Whether an argument can be indexing into a function /// parameter list. -static void checkAttrArgsAreCapabilityObjs(Sema &S, Decl *D, - const ParsedAttr &AL, - SmallVectorImpl &Args, - unsigned Sidx = 0, - bool ParamIdxOk = false) { - if (Sidx == AL.getNumArgs()) { +void Sema::checkAttrArgsAreCapabilityObjs(Decl *D, const ParsedAttr &AL, + SmallVectorImpl &Args, + unsigned Sidx, bool ParamIdxOk) { + if (D && Sidx == AL.getNumArgs()) { // If we don't have any capability arguments, the attribute implicitly // refers to 'this'. So we need to make sure that 'this' exists, i.e. we're // a non-static method, and that the class is a (scoped) capability. @@ -615,11 +613,10 @@ static void checkAttrArgsAreCapabilityObjs(Sema &S, Decl *D, // FIXME -- need to check this again on template instantiation if (!checkRecordDeclForAttr(RD) && !checkRecordDeclForAttr(RD)) - S.Diag(AL.getLoc(), - diag::warn_thread_attribute_not_on_capability_member) + Diag(AL.getLoc(), diag::warn_thread_attribute_not_on_capability_member) << AL << MD->getParent(); } else { - S.Diag(AL.getLoc(), diag::warn_thread_attribute_not_on_non_static_member) + Diag(AL.getLoc(), diag::warn_thread_attribute_not_on_non_static_member) << AL; } } @@ -644,7 +641,7 @@ static void checkAttrArgsAreCapabilityObjs(Sema &S, Decl *D, // We allow constant strings to be used as a placeholder for expressions // that are not valid C++ syntax, but warn that they are ignored. - S.Diag(AL.getLoc(), diag::warn_thread_attribute_ignored) << AL; + Diag(AL.getLoc(), diag::warn_thread_attribute_ignored) << AL; Args.push_back(ArgExp); continue; } @@ -663,7 +660,7 @@ static void checkAttrArgsAreCapabilityObjs(Sema &S, Decl *D, const RecordType *RT = getRecordType(ArgTy); // Now check if we index into a record type function param. - if(!RT && ParamIdxOk) { + if (D && !RT && ParamIdxOk) { const auto *FD = dyn_cast(D); const auto *IL = dyn_cast(ArgExp); if(FD && IL) { @@ -672,8 +669,8 @@ static void checkAttrArgsAreCapabilityObjs(Sema &S, Decl *D, uint64_t ParamIdxFromOne = ArgValue.getZExtValue(); uint64_t ParamIdxFromZero = ParamIdxFromOne - 1; if (!ArgValue.isStrictlyPositive() || ParamIdxFromOne > NumParams) { - S.Diag(AL.getLoc(), - diag::err_attribute_argument_out_of_bounds_extra_info) + Diag(AL.getLoc(), + diag::err_attribute_argument_out_of_bounds_extra_info) << AL << Idx + 1 << NumParams; continue; } @@ -685,8 +682,8 @@ static void checkAttrArgsAreCapabilityObjs(Sema &S, Decl *D, // expression have capabilities. This allows for writing C code where the // capability may be on the type, and the expression is a capability // boolean logic expression. Eg) requires_capability(A || B && !C) - if (!typeHasCapability(S, ArgTy) && !isCapabilityExpr(S, ArgExp)) - S.Diag(AL.getLoc(), diag::warn_thread_attribute_argument_not_lockable) + if (!typeHasCapability(*this, ArgTy) && !isCapabilityExpr(*this, ArgExp)) + Diag(AL.getLoc(), diag::warn_thread_attribute_argument_not_lockable) << AL << ArgTy; Args.push_back(ArgExp); @@ -708,7 +705,7 @@ 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); + S.checkAttrArgsAreCapabilityObjs(D, AL, Args); unsigned Size = Args.size(); if (Size != 1) return false; @@ -750,7 +747,7 @@ static bool checkAcquireOrderAttrCommon(Sema &S, Decl *D, const ParsedAttr &AL, } // Check that all arguments are lockable objects. - checkAttrArgsAreCapabilityObjs(S, D, AL, Args); + S.checkAttrArgsAreCapabilityObjs(D, AL, Args); if (Args.empty()) return false; @@ -781,7 +778,7 @@ static bool checkLockFunAttrCommon(Sema &S, Decl *D, const ParsedAttr &AL, SmallVectorImpl &Args) { // zero or more arguments ok // check that all arguments are lockable objects - checkAttrArgsAreCapabilityObjs(S, D, AL, Args, 0, /*ParamIdxOk=*/true); + S.checkAttrArgsAreCapabilityObjs(D, AL, Args, 0, /*ParamIdxOk=*/true); return true; } @@ -883,7 +880,7 @@ static bool checkTryLockFunAttrCommon(Sema &S, Decl *D, const ParsedAttr &AL, } // check that all arguments are lockable objects - checkAttrArgsAreCapabilityObjs(S, D, AL, Args, 1); + S.checkAttrArgsAreCapabilityObjs(D, AL, Args, 1); return true; } @@ -911,7 +908,7 @@ static void handleExclusiveTrylockFunctionAttr(Sema &S, Decl *D, static void handleLockReturnedAttr(Sema &S, Decl *D, const ParsedAttr &AL) { // check that the argument is lockable object SmallVector Args; - checkAttrArgsAreCapabilityObjs(S, D, AL, Args); + S.checkAttrArgsAreCapabilityObjs(D, AL, Args); unsigned Size = Args.size(); if (Size == 0) return; @@ -925,7 +922,7 @@ static void handleLocksExcludedAttr(Sema &S, Decl *D, const ParsedAttr &AL) { // check that all arguments are lockable objects SmallVector Args; - checkAttrArgsAreCapabilityObjs(S, D, AL, Args); + S.checkAttrArgsAreCapabilityObjs(D, AL, Args); unsigned Size = Args.size(); if (Size == 0) return; @@ -8176,7 +8173,7 @@ static void handleReleaseCapabilityAttr(Sema &S, Decl *D, const ParsedAttr &AL) { // Check that all arguments are lockable objects. SmallVector Args; - checkAttrArgsAreCapabilityObjs(S, D, AL, Args, 0, true); + S.checkAttrArgsAreCapabilityObjs(D, AL, Args, 0, true); D->addAttr(::new (S.Context) ReleaseCapabilityAttr(S.Context, AL, Args.data(), Args.size())); @@ -8189,7 +8186,7 @@ static void handleRequiresCapabilityAttr(Sema &S, Decl *D, // check that all arguments are lockable objects SmallVector Args; - checkAttrArgsAreCapabilityObjs(S, D, AL, Args); + S.checkAttrArgsAreCapabilityObjs(D, AL, Args); if (Args.empty()) return; diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp index a46deed8e7c58..577f99da6adc8 100644 --- a/clang/lib/Sema/SemaType.cpp +++ b/clang/lib/Sema/SemaType.cpp @@ -8642,6 +8642,25 @@ static void HandleAnnotateTypeAttr(TypeProcessingState &State, CurType = State.getAttributedType(AnnotateTypeAttr, CurType, CurType); } +static void HandleRequiresCapabilityAttr(TypeProcessingState &State, + QualType &CurType, + const ParsedAttr &PA) { + Sema &S = State.getSema(); + + if (PA.getNumArgs() < 1) { + // Already diganosed elsewhere, just ignore. + return; + } + + llvm::SmallVector Args; + Args.reserve(PA.getNumArgs() - 1); + State.getSema().checkAttrArgsAreCapabilityObjs(/*Decl=*/nullptr, PA, Args); + + auto *RCAttr = + RequiresCapabilityAttr::Create(S.Context, Args.data(), Args.size(), PA); + CurType = State.getAttributedType(RCAttr, CurType, CurType); +} + static void HandleLifetimeBoundAttr(TypeProcessingState &State, QualType &CurType, ParsedAttr &Attr) { @@ -8938,6 +8957,11 @@ static void processTypeAttrs(TypeProcessingState &state, QualType &type, attr.setUsedAsTypeAttr(); break; } + case ParsedAttr::AT_RequiresCapability: { + HandleRequiresCapabilityAttr(state, type, attr); + attr.setUsedAsTypeAttr(); + break; + } } // Handle attributes that are defined in a macro. We do not want this to be diff --git a/clang/test/Sema/warn-thread-safety-analysis.c b/clang/test/Sema/warn-thread-safety-analysis.c index 642ea88ec3c96..cd852efac21cb 100644 --- a/clang/test/Sema/warn-thread-safety-analysis.c +++ b/clang/test/Sema/warn-thread-safety-analysis.c @@ -136,6 +136,17 @@ int main(void) { // Cleanup happens automatically -> no warning. } + /// Function pointers + { + int __attribute__((requires_capability(&mu1))) (*function_ptr)(int) = Foo_fun1; + + function_ptr(5); // expected-warning {{calling function 'function_ptr' requires holding mutex 'mu1'}} + + mutex_exclusive_lock(&mu1); + int five = function_ptr(5); + mutex_exclusive_unlock(&mu1); + } + return 0; } diff --git a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp index 0c5b0cc85897b..bc8586b4a9586 100644 --- a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp +++ b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp @@ -1093,6 +1093,8 @@ void elr_function_args() EXCLUSIVE_LOCKS_REQUIRED(mu1, mu2); int elr_testfn(int y) EXCLUSIVE_LOCKS_REQUIRED(mu1); +int EXCLUSIVE_LOCKS_REQUIRED(mu1) (*function_ptr)(int); + int elr_testfn(int y) { int x EXCLUSIVE_LOCKS_REQUIRED(mu1) = y; // \ // expected-warning {{'exclusive_locks_required' attribute only applies to functions}}