https://github.com/tbaederr updated
https://github.com/llvm/llvm-project/pull/67095
>From 62f499be459107de97a697f42a04f1888af1218b 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] [clang][TSA] Make RequiresCapability a DeclOrType attribute
---
clang/include/clang/Basic/Attr.td | 12 +++---
clang/include/clang/Basic/AttrDocs.td | 7 +++
clang/include/clang/Sema/Sema.h | 6 +++
clang/lib/AST/TypePrinter.cpp | 3 ++
clang/lib/Sema/SemaDeclAttr.cpp | 43 +++++++++----------
clang/lib/Sema/SemaType.cpp | 25 +++++++++++
clang/test/Sema/warn-thread-safety-analysis.c | 13 ++++++
.../SemaCXX/warn-thread-safety-parsing.cpp | 2 +
8 files changed, 82 insertions(+), 29 deletions(-)
diff --git a/clang/include/clang/Basic/Attr.td
b/clang/include/clang/Basic/Attr.td
index d48aed5b73cf5..040826127ea3d 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -3902,7 +3902,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>,
@@ -3912,16 +3912,16 @@ def RequiresCapability : InheritableAttr {
let TemplateDependent = 1;
let ParseArgumentsAsUnevaluated = 1;
let InheritEvenIfAlreadyPresent = 1;
- let Subjects = SubjectList<[Function, ParmVar]>;
+ let Subjects = SubjectList<[Function, FunctionPointer, ParmVar]>;
let Accessors = [Accessor<"isShared", [Clang<"requires_shared_capability",
0>,
Clang<"shared_locks_required", 0>]>];
- let Documentation = [Undocumented];
+ let Documentation = [ThreadSafetyDocs];
}
def NoThreadSafetyAnalysis : InheritableAttr {
let Spellings = [Clang<"no_thread_safety_analysis">];
let Subjects = SubjectList<[Function]>;
- let Documentation = [Undocumented];
+ let Documentation = [ThreadSafetyDocs];
let SimpleHandler = 1;
}
@@ -3933,7 +3933,7 @@ def GuardedBy : InheritableAttr {
let ParseArgumentsAsUnevaluated = 1;
let InheritEvenIfAlreadyPresent = 1;
let Subjects = SubjectList<[Field, SharedVar]>;
- let Documentation = [Undocumented];
+ let Documentation = [ThreadSafetyDocs];
}
def PtGuardedBy : InheritableAttr {
@@ -3944,7 +3944,7 @@ def PtGuardedBy : InheritableAttr {
let ParseArgumentsAsUnevaluated = 1;
let InheritEvenIfAlreadyPresent = 1;
let Subjects = SubjectList<[Field, SharedVar]>;
- let Documentation = [Undocumented];
+ let Documentation = [ThreadSafetyDocs];
}
def AcquiredAfter : InheritableAttr {
diff --git a/clang/include/clang/Basic/AttrDocs.td
b/clang/include/clang/Basic/AttrDocs.td
index 98468034e71a8..0b49268d79571 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -9283,3 +9283,10 @@ Declares that a function potentially allocates heap
memory, and prevents any pot
of ``nonallocating`` by the compiler.
}];
}
+
+def ThreadSafetyDocs : Documentation {
+ let Category = DocCatFunction;
+ let Content = [{
+Part of Thread Safety Analysis (TSA) in Clang, as documented at
https://clang.llvm.org/docs/ThreadSafetyAnalysis.html.
+ }];
+}
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 96d81e618494a..7c79b1b34cc11 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -14106,6 +14106,12 @@ class Sema final : public SemaBase {
LocalInstantiationScope &Scope,
const MultiLevelTemplateArgumentList &TemplateArgs);
+public:
+ void checkAttrArgsAreCapabilityObjs(Decl *D, const ParsedAttr &AL,
+ SmallVectorImpl<Expr *> &Args,
+ unsigned Sidx = 0,
+ bool ParamIdxOk = false);
+
int ParsingClassDepth = 0;
class SavePendingParsedClassStateRAII {
diff --git a/clang/lib/AST/TypePrinter.cpp b/clang/lib/AST/TypePrinter.cpp
index cba1a2d98d660..479a35b28d3f5 100644
--- a/clang/lib/AST/TypePrinter.cpp
+++ b/clang/lib/AST/TypePrinter.cpp
@@ -2098,6 +2098,9 @@ void TypePrinter::printAttributedAfter(const
AttributedType *T,
case attr::ExtVectorType:
OS << "ext_vector_type";
break;
+ case attr::RequiresCapability:
+ OS << "requires_capability(...)";
+ break;
}
OS << "))";
}
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 3b5cf3661a52f..32e003c1eb938 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -339,12 +339,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<Expr *> &Args,
- unsigned Sidx = 0,
- bool ParamIdxOk = false) {
- if (Sidx == AL.getNumArgs()) {
+void Sema::checkAttrArgsAreCapabilityObjs(Decl *D, const ParsedAttr &AL,
+ SmallVectorImpl<Expr *> &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.
@@ -354,11 +352,10 @@ static void checkAttrArgsAreCapabilityObjs(Sema &S, Decl
*D,
// FIXME -- need to check this again on template instantiation
if (!checkRecordDeclForAttr<CapabilityAttr>(RD) &&
!checkRecordDeclForAttr<ScopedLockableAttr>(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;
}
}
@@ -383,7 +380,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;
}
@@ -402,7 +399,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<FunctionDecl>(D);
const auto *IL = dyn_cast<IntegerLiteral>(ArgExp);
if(FD && IL) {
@@ -411,8 +408,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;
}
@@ -424,8 +421,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);
@@ -460,7 +457,7 @@ static bool checkGuardedByAttrCommon(Sema &S, Decl *D,
const ParsedAttr &AL,
Expr *&Arg) {
SmallVector<Expr *, 1> 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;
@@ -502,7 +499,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;
@@ -533,7 +530,7 @@ static bool checkLockFunAttrCommon(Sema &S, Decl *D, const
ParsedAttr &AL,
SmallVectorImpl<Expr *> &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;
}
@@ -612,7 +609,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;
}
@@ -620,7 +617,7 @@ static bool checkTryLockFunAttrCommon(Sema &S, Decl *D,
const ParsedAttr &AL,
static void handleLockReturnedAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
// check that the argument is lockable object
SmallVector<Expr*, 1> Args;
- checkAttrArgsAreCapabilityObjs(S, D, AL, Args);
+ S.checkAttrArgsAreCapabilityObjs(D, AL, Args);
unsigned Size = Args.size();
if (Size == 0)
return;
@@ -638,7 +635,7 @@ static void handleLocksExcludedAttr(Sema &S, Decl *D, const
ParsedAttr &AL) {
// check that all arguments are lockable objects
SmallVector<Expr*, 1> Args;
- checkAttrArgsAreCapabilityObjs(S, D, AL, Args);
+ S.checkAttrArgsAreCapabilityObjs(D, AL, Args);
unsigned Size = Args.size();
if (Size == 0)
return;
@@ -6269,7 +6266,7 @@ static void handleReleaseCapabilityAttr(Sema &S, Decl *D,
return;
// Check that all arguments are lockable objects.
SmallVector<Expr *, 1> 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()));
@@ -6286,7 +6283,7 @@ static void handleRequiresCapabilityAttr(Sema &S, Decl *D,
// check that all arguments are lockable objects
SmallVector<Expr*, 1> 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 6e7ee8b5506ff..86c5590053864 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -8682,6 +8682,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<Expr *, 4> 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) {
@@ -9034,6 +9053,12 @@ 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 c58b7bed61183..5d5dffe08b9b9 100644
--- a/clang/test/Sema/warn-thread-safety-analysis.c
+++ b/clang/test/Sema/warn-thread-safety-analysis.c
@@ -212,6 +212,19 @@ int main(void) {
mutex_exclusive_unlock(late_parsing.a_mutex_defined_very_late);
mutex_exclusive_unlock(late_parsing.a_mutex_defined_late);
#endif
+ /// 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);
+
+ int (*function_ptr2)(int) = function_ptr;
+ function_ptr2(5);
+ }
return 0;
}
diff --git a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp
b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp
index 752803e4a0543..b6fd0016e7fa1 100644
--- a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp
@@ -1109,6 +1109,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}}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
