Timm =?utf-8?q?Bäder?= <tbae...@redhat.com>, Timm =?utf-8?q?Bäder?= <tbae...@redhat.com> Message-ID: In-Reply-To: <llvm/llvm-project/pull/67095/cl...@github.com>
https://github.com/tbaederr updated https://github.com/llvm/llvm-project/pull/67095 >From a7c2b5a2834ef6dc345257e8d67caae909abbe20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbae...@redhat.com> Date: Fri, 22 Sep 2023 08:42:05 +0200 Subject: [PATCH 1/3] [clang][TSA] Make RequiresCapability a DeclOrType attribute --- clang/include/clang/Basic/Attr.td | 6 +++--- clang/test/Sema/warn-thread-safety-analysis.c | 11 +++++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 7a6ec77ae84b15a..fc094c8caf23901 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -3317,7 +3317,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>, @@ -3326,8 +3326,8 @@ def RequiresCapability : InheritableAttr { let LateParsed = 1; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; - let InheritEvenIfAlreadyPresent = 1; - let Subjects = SubjectList<[Function]>; + /*let InheritEvenIfAlreadyPresent = 1;*/ + /*let Subjects = SubjectList<[Function]>;*/ let Accessors = [Accessor<"isShared", [Clang<"requires_shared_capability", 0>, Clang<"shared_locks_required", 0>]>]; let Documentation = [Undocumented]; diff --git a/clang/test/Sema/warn-thread-safety-analysis.c b/clang/test/Sema/warn-thread-safety-analysis.c index 642ea88ec3c96f7..cd852efac21cb81 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; } >From 13393e67525df1565a661ec96e97cae0538fbaad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbae...@redhat.com> Date: Tue, 26 Sep 2023 09:57:41 +0200 Subject: [PATCH 2/3] Restrict to functions and function pointer decls --- clang/include/clang/Basic/Attr.td | 2 +- clang/lib/Sema/SemaDeclAttr.cpp | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index fc094c8caf23901..af3241b3169bc39 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -3326,7 +3326,7 @@ def RequiresCapability : DeclOrTypeAttr { let LateParsed = 1; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; - /*let InheritEvenIfAlreadyPresent = 1;*/ + let InheritEvenIfAlreadyPresent = 1; /*let Subjects = SubjectList<[Function]>;*/ let Accessors = [Accessor<"isShared", [Clang<"requires_shared_capability", 0>, Clang<"shared_locks_required", 0>]>]; diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index ed0b4d29b056397..c967af7bb6914e0 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -8166,6 +8166,16 @@ static void handleRequiresCapabilityAttr(Sema &S, Decl *D, if (!AL.checkAtLeastNumArgs(S, 1)) return; + // We allow this on function declaration as well as + // variable declarations of function pointer type. + if (!D->isFunctionPointerType() && !isa<FunctionDecl>(D)) { + // FIXME: Diagnostic should say "functions and function pointer decls" now I + // guess. + S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type) + << AL << AL.isRegularKeywordAttribute() << ExpectedFunction; + return; + } + // check that all arguments are lockable objects SmallVector<Expr*, 1> Args; checkAttrArgsAreCapabilityObjs(S, D, AL, Args); >From 2fdd4485b94df6c79e3e190d69a7dff70fe17d78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbae...@redhat.com> Date: Fri, 13 Oct 2023 10:23:43 +0200 Subject: [PATCH 3/3] Add processing to SemaType.cpp --- clang/include/clang/Basic/Attr.td | 2 +- clang/lib/Sema/SemaDeclAttr.cpp | 10 ---------- clang/lib/Sema/SemaType.cpp | 30 ++++++++++++++++++++++++++++++ 3 files changed, 31 insertions(+), 11 deletions(-) diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index af3241b3169bc39..15e785538709716 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -3327,7 +3327,7 @@ def RequiresCapability : DeclOrTypeAttr { 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/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index c967af7bb6914e0..ed0b4d29b056397 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -8166,16 +8166,6 @@ static void handleRequiresCapabilityAttr(Sema &S, Decl *D, if (!AL.checkAtLeastNumArgs(S, 1)) return; - // We allow this on function declaration as well as - // variable declarations of function pointer type. - if (!D->isFunctionPointerType() && !isa<FunctionDecl>(D)) { - // FIXME: Diagnostic should say "functions and function pointer decls" now I - // guess. - S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type) - << AL << AL.isRegularKeywordAttribute() << ExpectedFunction; - return; - } - // check that all arguments are lockable objects SmallVector<Expr*, 1> Args; checkAttrArgsAreCapabilityObjs(S, D, AL, Args); diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp index 068971f8130a4aa..26ae7f17d527540 100644 --- a/clang/lib/Sema/SemaType.cpp +++ b/clang/lib/Sema/SemaType.cpp @@ -8602,6 +8602,31 @@ 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) { + S.Diag(PA.getLoc(), diag::err_attribute_too_few_arguments) << PA << 1; + return; + } + + // FIXME: We need to sanity check the arguments here I think? Like we do in + // SemaDeclAtr.cpp. + + llvm::SmallVector<Expr *, 4> Args; + Args.reserve(PA.getNumArgs() - 1); + for (unsigned Idx = 1; Idx < PA.getNumArgs(); Idx++) { + assert(!PA.isArgIdent(Idx)); + Args.push_back(PA.getArgAsExpr(Idx)); + } + + 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) { @@ -8899,6 +8924,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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
