Skip to content

Commit 62f499b

Browse files
committed
[clang][TSA] Make RequiresCapability a DeclOrType attribute
1 parent 1a78ef9 commit 62f499b

File tree

8 files changed

+82
-29
lines changed

8 files changed

+82
-29
lines changed

clang/include/clang/Basic/Attr.td

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3902,7 +3902,7 @@ def ReleaseCapability : InheritableAttr {
39023902
let Documentation = [ReleaseCapabilityDocs];
39033903
}
39043904

3905-
def RequiresCapability : InheritableAttr {
3905+
def RequiresCapability : DeclOrTypeAttr {
39063906
let Spellings = [Clang<"requires_capability", 0>,
39073907
Clang<"exclusive_locks_required", 0>,
39083908
Clang<"requires_shared_capability", 0>,
@@ -3912,16 +3912,16 @@ def RequiresCapability : InheritableAttr {
39123912
let TemplateDependent = 1;
39133913
let ParseArgumentsAsUnevaluated = 1;
39143914
let InheritEvenIfAlreadyPresent = 1;
3915-
let Subjects = SubjectList<[Function, ParmVar]>;
3915+
let Subjects = SubjectList<[Function, FunctionPointer, ParmVar]>;
39163916
let Accessors = [Accessor<"isShared", [Clang<"requires_shared_capability", 0>,
39173917
Clang<"shared_locks_required", 0>]>];
3918-
let Documentation = [Undocumented];
3918+
let Documentation = [ThreadSafetyDocs];
39193919
}
39203920

39213921
def NoThreadSafetyAnalysis : InheritableAttr {
39223922
let Spellings = [Clang<"no_thread_safety_analysis">];
39233923
let Subjects = SubjectList<[Function]>;
3924-
let Documentation = [Undocumented];
3924+
let Documentation = [ThreadSafetyDocs];
39253925
let SimpleHandler = 1;
39263926
}
39273927

@@ -3933,7 +3933,7 @@ def GuardedBy : InheritableAttr {
39333933
let ParseArgumentsAsUnevaluated = 1;
39343934
let InheritEvenIfAlreadyPresent = 1;
39353935
let Subjects = SubjectList<[Field, SharedVar]>;
3936-
let Documentation = [Undocumented];
3936+
let Documentation = [ThreadSafetyDocs];
39373937
}
39383938

39393939
def PtGuardedBy : InheritableAttr {
@@ -3944,7 +3944,7 @@ def PtGuardedBy : InheritableAttr {
39443944
let ParseArgumentsAsUnevaluated = 1;
39453945
let InheritEvenIfAlreadyPresent = 1;
39463946
let Subjects = SubjectList<[Field, SharedVar]>;
3947-
let Documentation = [Undocumented];
3947+
let Documentation = [ThreadSafetyDocs];
39483948
}
39493949

39503950
def AcquiredAfter : InheritableAttr {

clang/include/clang/Basic/AttrDocs.td

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9283,3 +9283,10 @@ Declares that a function potentially allocates heap memory, and prevents any pot
92839283
of ``nonallocating`` by the compiler.
92849284
}];
92859285
}
9286+
9287+
def ThreadSafetyDocs : Documentation {
9288+
let Category = DocCatFunction;
9289+
let Content = [{
9290+
Part of Thread Safety Analysis (TSA) in Clang, as documented at https://clang.llvm.org/docs/ThreadSafetyAnalysis.html.
9291+
}];
9292+
}

clang/include/clang/Sema/Sema.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14106,6 +14106,12 @@ class Sema final : public SemaBase {
1410614106
LocalInstantiationScope &Scope,
1410714107
const MultiLevelTemplateArgumentList &TemplateArgs);
1410814108

14109+
public:
14110+
void checkAttrArgsAreCapabilityObjs(Decl *D, const ParsedAttr &AL,
14111+
SmallVectorImpl<Expr *> &Args,
14112+
unsigned Sidx = 0,
14113+
bool ParamIdxOk = false);
14114+
1410914115
int ParsingClassDepth = 0;
1411014116

1411114117
class SavePendingParsedClassStateRAII {

clang/lib/AST/TypePrinter.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2098,6 +2098,9 @@ void TypePrinter::printAttributedAfter(const AttributedType *T,
20982098
case attr::ExtVectorType:
20992099
OS << "ext_vector_type";
21002100
break;
2101+
case attr::RequiresCapability:
2102+
OS << "requires_capability(...)";
2103+
break;
21012104
}
21022105
OS << "))";
21032106
}

clang/lib/Sema/SemaDeclAttr.cpp

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -339,12 +339,10 @@ static bool isCapabilityExpr(Sema &S, const Expr *Ex) {
339339
/// \param Sidx The attribute argument index to start checking with.
340340
/// \param ParamIdxOk Whether an argument can be indexing into a function
341341
/// parameter list.
342-
static void checkAttrArgsAreCapabilityObjs(Sema &S, Decl *D,
343-
const ParsedAttr &AL,
344-
SmallVectorImpl<Expr *> &Args,
345-
unsigned Sidx = 0,
346-
bool ParamIdxOk = false) {
347-
if (Sidx == AL.getNumArgs()) {
342+
void Sema::checkAttrArgsAreCapabilityObjs(Decl *D, const ParsedAttr &AL,
343+
SmallVectorImpl<Expr *> &Args,
344+
unsigned Sidx, bool ParamIdxOk) {
345+
if (D && Sidx == AL.getNumArgs()) {
348346
// If we don't have any capability arguments, the attribute implicitly
349347
// refers to 'this'. So we need to make sure that 'this' exists, i.e. we're
350348
// a non-static method, and that the class is a (scoped) capability.
@@ -354,11 +352,10 @@ static void checkAttrArgsAreCapabilityObjs(Sema &S, Decl *D,
354352
// FIXME -- need to check this again on template instantiation
355353
if (!checkRecordDeclForAttr<CapabilityAttr>(RD) &&
356354
!checkRecordDeclForAttr<ScopedLockableAttr>(RD))
357-
S.Diag(AL.getLoc(),
358-
diag::warn_thread_attribute_not_on_capability_member)
355+
Diag(AL.getLoc(), diag::warn_thread_attribute_not_on_capability_member)
359356
<< AL << MD->getParent();
360357
} else {
361-
S.Diag(AL.getLoc(), diag::warn_thread_attribute_not_on_non_static_member)
358+
Diag(AL.getLoc(), diag::warn_thread_attribute_not_on_non_static_member)
362359
<< AL;
363360
}
364361
}
@@ -383,7 +380,7 @@ static void checkAttrArgsAreCapabilityObjs(Sema &S, Decl *D,
383380

384381
// We allow constant strings to be used as a placeholder for expressions
385382
// that are not valid C++ syntax, but warn that they are ignored.
386-
S.Diag(AL.getLoc(), diag::warn_thread_attribute_ignored) << AL;
383+
Diag(AL.getLoc(), diag::warn_thread_attribute_ignored) << AL;
387384
Args.push_back(ArgExp);
388385
continue;
389386
}
@@ -402,7 +399,7 @@ static void checkAttrArgsAreCapabilityObjs(Sema &S, Decl *D,
402399
const RecordType *RT = getRecordType(ArgTy);
403400

404401
// Now check if we index into a record type function param.
405-
if(!RT && ParamIdxOk) {
402+
if (D && !RT && ParamIdxOk) {
406403
const auto *FD = dyn_cast<FunctionDecl>(D);
407404
const auto *IL = dyn_cast<IntegerLiteral>(ArgExp);
408405
if(FD && IL) {
@@ -411,8 +408,8 @@ static void checkAttrArgsAreCapabilityObjs(Sema &S, Decl *D,
411408
uint64_t ParamIdxFromOne = ArgValue.getZExtValue();
412409
uint64_t ParamIdxFromZero = ParamIdxFromOne - 1;
413410
if (!ArgValue.isStrictlyPositive() || ParamIdxFromOne > NumParams) {
414-
S.Diag(AL.getLoc(),
415-
diag::err_attribute_argument_out_of_bounds_extra_info)
411+
Diag(AL.getLoc(),
412+
diag::err_attribute_argument_out_of_bounds_extra_info)
416413
<< AL << Idx + 1 << NumParams;
417414
continue;
418415
}
@@ -424,8 +421,8 @@ static void checkAttrArgsAreCapabilityObjs(Sema &S, Decl *D,
424421
// expression have capabilities. This allows for writing C code where the
425422
// capability may be on the type, and the expression is a capability
426423
// boolean logic expression. Eg) requires_capability(A || B && !C)
427-
if (!typeHasCapability(S, ArgTy) && !isCapabilityExpr(S, ArgExp))
428-
S.Diag(AL.getLoc(), diag::warn_thread_attribute_argument_not_lockable)
424+
if (!typeHasCapability(*this, ArgTy) && !isCapabilityExpr(*this, ArgExp))
425+
Diag(AL.getLoc(), diag::warn_thread_attribute_argument_not_lockable)
429426
<< AL << ArgTy;
430427

431428
Args.push_back(ArgExp);
@@ -460,7 +457,7 @@ static bool checkGuardedByAttrCommon(Sema &S, Decl *D, const ParsedAttr &AL,
460457
Expr *&Arg) {
461458
SmallVector<Expr *, 1> Args;
462459
// check that all arguments are lockable objects
463-
checkAttrArgsAreCapabilityObjs(S, D, AL, Args);
460+
S.checkAttrArgsAreCapabilityObjs(D, AL, Args);
464461
unsigned Size = Args.size();
465462
if (Size != 1)
466463
return false;
@@ -502,7 +499,7 @@ static bool checkAcquireOrderAttrCommon(Sema &S, Decl *D, const ParsedAttr &AL,
502499
}
503500

504501
// Check that all arguments are lockable objects.
505-
checkAttrArgsAreCapabilityObjs(S, D, AL, Args);
502+
S.checkAttrArgsAreCapabilityObjs(D, AL, Args);
506503
if (Args.empty())
507504
return false;
508505

@@ -533,7 +530,7 @@ static bool checkLockFunAttrCommon(Sema &S, Decl *D, const ParsedAttr &AL,
533530
SmallVectorImpl<Expr *> &Args) {
534531
// zero or more arguments ok
535532
// check that all arguments are lockable objects
536-
checkAttrArgsAreCapabilityObjs(S, D, AL, Args, 0, /*ParamIdxOk=*/true);
533+
S.checkAttrArgsAreCapabilityObjs(D, AL, Args, 0, /*ParamIdxOk=*/true);
537534

538535
return true;
539536
}
@@ -612,15 +609,15 @@ static bool checkTryLockFunAttrCommon(Sema &S, Decl *D, const ParsedAttr &AL,
612609
}
613610

614611
// check that all arguments are lockable objects
615-
checkAttrArgsAreCapabilityObjs(S, D, AL, Args, 1);
612+
S.checkAttrArgsAreCapabilityObjs(D, AL, Args, 1);
616613

617614
return true;
618615
}
619616

620617
static void handleLockReturnedAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
621618
// check that the argument is lockable object
622619
SmallVector<Expr*, 1> Args;
623-
checkAttrArgsAreCapabilityObjs(S, D, AL, Args);
620+
S.checkAttrArgsAreCapabilityObjs(D, AL, Args);
624621
unsigned Size = Args.size();
625622
if (Size == 0)
626623
return;
@@ -638,7 +635,7 @@ static void handleLocksExcludedAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
638635

639636
// check that all arguments are lockable objects
640637
SmallVector<Expr*, 1> Args;
641-
checkAttrArgsAreCapabilityObjs(S, D, AL, Args);
638+
S.checkAttrArgsAreCapabilityObjs(D, AL, Args);
642639
unsigned Size = Args.size();
643640
if (Size == 0)
644641
return;
@@ -6269,7 +6266,7 @@ static void handleReleaseCapabilityAttr(Sema &S, Decl *D,
62696266
return;
62706267
// Check that all arguments are lockable objects.
62716268
SmallVector<Expr *, 1> Args;
6272-
checkAttrArgsAreCapabilityObjs(S, D, AL, Args, 0, true);
6269+
S.checkAttrArgsAreCapabilityObjs(D, AL, Args, 0, true);
62736270

62746271
D->addAttr(::new (S.Context) ReleaseCapabilityAttr(S.Context, AL, Args.data(),
62756272
Args.size()));
@@ -6286,7 +6283,7 @@ static void handleRequiresCapabilityAttr(Sema &S, Decl *D,
62866283

62876284
// check that all arguments are lockable objects
62886285
SmallVector<Expr*, 1> Args;
6289-
checkAttrArgsAreCapabilityObjs(S, D, AL, Args);
6286+
S.checkAttrArgsAreCapabilityObjs(D, AL, Args);
62906287
if (Args.empty())
62916288
return;
62926289

clang/lib/Sema/SemaType.cpp

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8682,6 +8682,25 @@ static void HandleAnnotateTypeAttr(TypeProcessingState &State,
86828682
CurType = State.getAttributedType(AnnotateTypeAttr, CurType, CurType);
86838683
}
86848684

8685+
static void HandleRequiresCapabilityAttr(TypeProcessingState &State,
8686+
QualType &CurType,
8687+
const ParsedAttr &PA) {
8688+
Sema &S = State.getSema();
8689+
8690+
if (PA.getNumArgs() < 1) {
8691+
// Already diganosed elsewhere, just ignore.
8692+
return;
8693+
}
8694+
8695+
llvm::SmallVector<Expr *, 4> Args;
8696+
Args.reserve(PA.getNumArgs() - 1);
8697+
State.getSema().checkAttrArgsAreCapabilityObjs(/*Decl=*/nullptr, PA, Args);
8698+
8699+
auto *RCAttr =
8700+
RequiresCapabilityAttr::Create(S.Context, Args.data(), Args.size(), PA);
8701+
CurType = State.getAttributedType(RCAttr, CurType, CurType);
8702+
}
8703+
86858704
static void HandleLifetimeBoundAttr(TypeProcessingState &State,
86868705
QualType &CurType,
86878706
ParsedAttr &Attr) {
@@ -9034,6 +9053,12 @@ static void processTypeAttrs(TypeProcessingState &state, QualType &type,
90349053
attr.setUsedAsTypeAttr();
90359054
break;
90369055
}
9056+
9057+
case ParsedAttr::AT_RequiresCapability: {
9058+
HandleRequiresCapabilityAttr(state, type, attr);
9059+
attr.setUsedAsTypeAttr();
9060+
break;
9061+
}
90379062
}
90389063

90399064
// Handle attributes that are defined in a macro. We do not want this to be

clang/test/Sema/warn-thread-safety-analysis.c

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,19 @@ int main(void) {
212212
mutex_exclusive_unlock(late_parsing.a_mutex_defined_very_late);
213213
mutex_exclusive_unlock(late_parsing.a_mutex_defined_late);
214214
#endif
215+
/// Function pointers
216+
{
217+
int __attribute__((requires_capability(&mu1))) (*function_ptr)(int) = Foo_fun1;
218+
219+
function_ptr(5); // expected-warning {{calling function 'function_ptr' requires holding mutex 'mu1'}}
220+
221+
mutex_exclusive_lock(&mu1);
222+
int five = function_ptr(5);
223+
mutex_exclusive_unlock(&mu1);
224+
225+
int (*function_ptr2)(int) = function_ptr;
226+
function_ptr2(5);
227+
}
215228

216229
return 0;
217230
}

clang/test/SemaCXX/warn-thread-safety-parsing.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1109,6 +1109,8 @@ void elr_function_args() EXCLUSIVE_LOCKS_REQUIRED(mu1, mu2);
11091109

11101110
int elr_testfn(int y) EXCLUSIVE_LOCKS_REQUIRED(mu1);
11111111

1112+
int EXCLUSIVE_LOCKS_REQUIRED(mu1) (*function_ptr)(int);
1113+
11121114
int elr_testfn(int y) {
11131115
int x EXCLUSIVE_LOCKS_REQUIRED(mu1) = y; // \
11141116
// expected-warning {{'exclusive_locks_required' attribute only applies to functions}}

0 commit comments

Comments
 (0)