[clang] [clang][TSA] Make RequiresCapability a DeclOrType attribute (PR #67095)
Timm Baeder via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 13 01:24:30 PDT 2023
Timm =?utf-8?q?Bäder?= <tbaeder at redhat.com>,
Timm =?utf-8?q?Bäder?= <tbaeder at redhat.com>
Message-ID:
In-Reply-To: <llvm/llvm-project/pull/67095/clang at 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?= <tbaeder at 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?= <tbaeder at 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?= <tbaeder at 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
More information about the cfe-commits
mailing list