Skip to content

Commit

Permalink
Allow arbitrary capability name in Thread Safety Analysis
Browse files Browse the repository at this point in the history
Restricting the names of capabilities to only "role" or "mutex" makes
for awkward diagnostic text, such as with:
https://chromium-review.googlesource.com/c/chromium/src/+/1948098/19/base/sequence_checker_unittest.nc#33
  • Loading branch information
Etienne Pierre-Doray authored and AaronBallman committed Jan 21, 2020
1 parent b6c62ef commit 5260bc2
Show file tree
Hide file tree
Showing 4 changed files with 3 additions and 14 deletions.
4 changes: 0 additions & 4 deletions clang/include/clang/Basic/Attr.td
Expand Up @@ -2567,10 +2567,6 @@ def Capability : InheritableAttr {
let Accessors = [Accessor<"isShared",
[Clang<"shared_capability", 0>]>];
let Documentation = [Undocumented];
let AdditionalMembers = [{
bool isMutex() const { return getName().equals_lower("mutex"); }
bool isRole() const { return getName().equals_lower("role"); }
}];
}

def AssertCapability : InheritableAttr {
Expand Down
3 changes: 0 additions & 3 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Expand Up @@ -3252,9 +3252,6 @@ def warn_at_available_unchecked_use : Warning<
InGroup<DiagGroup<"unsupported-availability-guard">>;

// Thread Safety Attributes
def warn_invalid_capability_name : Warning<
"invalid capability name '%0'; capability name must be 'mutex' or 'role'">,
InGroup<ThreadSafetyAttributes>, DefaultIgnore;
def warn_thread_attribute_ignored : Warning<
"ignoring %0 attribute because its argument is invalid">,
InGroup<ThreadSafetyAttributes>, DefaultIgnore;
Expand Down
5 changes: 0 additions & 5 deletions clang/lib/Sema/SemaDeclAttr.cpp
Expand Up @@ -6195,11 +6195,6 @@ static void handleCapabilityAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
!S.checkStringLiteralArgumentAttr(AL, 0, N, &LiteralLoc))
return;

// Currently, there are only two names allowed for a capability: role and
// mutex (case insensitive). Diagnose other capability names.
if (!N.equals_lower("mutex") && !N.equals_lower("role"))
S.Diag(LiteralLoc, diag::warn_invalid_capability_name) << N;

D->addAttr(::new (S.Context) CapabilityAttr(S.Context, AL, N));
}

Expand Down
5 changes: 3 additions & 2 deletions clang/test/Sema/attr-capabilities.c
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 -fsyntax-only -Wthread-safety -verify %s

typedef int __attribute__((capability("role"))) ThreadRole;
typedef int __attribute__((capability("role"))) ThreadRole;
struct __attribute__((shared_capability("mutex"))) Mutex {};
struct NotACapability {};
Expand All @@ -8,8 +9,8 @@ struct NotACapability {};
union __attribute__((capability("mutex"))) MutexUnion { int a; char* b; };
typedef union { int a; char* b; } __attribute__((capability("mutex"))) MutexUnion2;

// Test an invalid capability name
struct __attribute__((capability("wrong"))) IncorrectName {}; // expected-warning {{invalid capability name 'wrong'; capability name must be 'mutex' or 'role'}}
// Test a different capability name
struct __attribute__((capability("custom"))) CustomName {};

int Test1 __attribute__((capability("test1"))); // expected-error {{'capability' attribute only applies to structs, unions, classes, and typedefs}}
int Test2 __attribute__((shared_capability("test2"))); // expected-error {{'shared_capability' attribute only applies to structs, unions, classes, and typedefs}}
Expand Down

0 comments on commit 5260bc2

Please sign in to comment.