Skip to content

Commit 3cfee20

Browse files
pdherbemontpdherbemont
committed
Support [[guarded_by(mutex)]] attribute inside C struct
Today, it's only supported inside C++ classes or top level C/C++ declaration. I mostly copied and adapted over the code from the [[counted_by(value)]] lookup. I had to change ast-dump-color.cpp because of the way the expression marking now marks the mutex as "used" instead of "referenced". If I change the expression parsing to use `ExpressionEvaluationContext::Unevaluated` then I get the mutex as "referenced" but I lose the C++ check for "invalid use of non-static data member".
1 parent 50d837e commit 3cfee20

File tree

5 files changed

+87
-8
lines changed

5 files changed

+87
-8
lines changed

‎clang/include/clang/Parse/Parser.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3128,6 +3128,14 @@ class Parser : public CodeCompletionHandler {
31283128
IdentifierInfo *ScopeName, SourceLocation ScopeLoc,
31293129
ParsedAttr::Form Form);
31303130

3131+
void ParseGuardedByAttribute(IdentifierInfo &AttrName,
3132+
SourceLocation AttrNameLoc,
3133+
ParsedAttributes &Attrs,
3134+
IdentifierInfo *ScopeName,
3135+
SourceLocation ScopeLoc,
3136+
SourceLocation *EndLoc,
3137+
ParsedAttr::Form Form);
3138+
31313139
void ParseTypeofSpecifier(DeclSpec &DS);
31323140
SourceLocation ParseDecltypeSpecifier(DeclSpec &DS);
31333141
void AnnotateExistingDecltypeSpecifier(const DeclSpec &DS,

‎clang/lib/Parse/ParseDecl.cpp

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -671,6 +671,16 @@ void Parser::ParseGNUAttributeArgs(
671671
ParseBoundsAttribute(*AttrName, AttrNameLoc, Attrs, ScopeName, ScopeLoc,
672672
Form);
673673
return;
674+
} else if (AttrKind == ParsedAttr::AT_GuardedBy) {
675+
ParseGuardedByAttribute(*AttrName, AttrNameLoc, Attrs, ScopeName, ScopeLoc,
676+
EndLoc,
677+
Form);
678+
return;
679+
} else if (AttrKind == ParsedAttr::AT_PtGuardedBy) {
680+
ParseGuardedByAttribute(*AttrName, AttrNameLoc, Attrs, ScopeName, ScopeLoc,
681+
EndLoc,
682+
Form);
683+
return;
674684
} else if (AttrKind == ParsedAttr::AT_CXXAssume) {
675685
ParseCXXAssumeAttributeArg(Attrs, AttrName, AttrNameLoc, EndLoc, Form);
676686
return;
@@ -3330,6 +3340,63 @@ void Parser::DistributeCLateParsedAttrs(Decl *Dcl,
33303340
}
33313341
}
33323342

3343+
/// GuardedBy attributes (e.g., guarded_by):
3344+
/// AttrName '(' expression ')'
3345+
void Parser::ParseGuardedByAttribute(IdentifierInfo &AttrName,
3346+
SourceLocation AttrNameLoc,
3347+
ParsedAttributes &Attrs,
3348+
IdentifierInfo *ScopeName,
3349+
SourceLocation ScopeLoc,
3350+
SourceLocation *EndLoc,
3351+
ParsedAttr::Form Form) {
3352+
assert(Tok.is(tok::l_paren) && "Attribute arg list not starting with '('");
3353+
3354+
BalancedDelimiterTracker Parens(*this, tok::l_paren);
3355+
Parens.consumeOpen();
3356+
3357+
if (Tok.is(tok::r_paren)) {
3358+
Diag(Tok.getLocation(), diag::err_argument_required_after_attribute);
3359+
Parens.consumeClose();
3360+
return;
3361+
}
3362+
3363+
ArgsVector ArgExprs;
3364+
// Don't evaluate argument when the attribute is ignored.
3365+
using ExpressionKind =
3366+
Sema::ExpressionEvaluationContextRecord::ExpressionKind;
3367+
EnterExpressionEvaluationContext EC(
3368+
Actions, Sema::ExpressionEvaluationContext::PotentiallyEvaluated, nullptr,
3369+
ExpressionKind::EK_BoundsAttrArgument);
3370+
3371+
ExprResult ArgExpr(
3372+
Actions.CorrectDelayedTyposInExpr(ParseAssignmentExpression()));
3373+
3374+
if (ArgExpr.isInvalid()) {
3375+
Parens.skipToEnd();
3376+
return;
3377+
}
3378+
3379+
ArgExprs.push_back(ArgExpr.get());
3380+
3381+
auto RParens = Tok.getLocation();
3382+
auto &AL = *Attrs.addNew(
3383+
&AttrName, SourceRange(AttrNameLoc, RParens), ScopeName,
3384+
ScopeLoc, ArgExprs.data(), ArgExprs.size(), Form);
3385+
3386+
if (EndLoc) {
3387+
*EndLoc = Tok.getLocation();
3388+
}
3389+
3390+
if (!Tok.is(tok::r_paren)) {
3391+
Diag(Tok.getLocation(), diag::err_attribute_wrong_number_arguments)
3392+
<< AL << 1;
3393+
Parens.skipToEnd();
3394+
return;
3395+
}
3396+
3397+
Parens.consumeClose();
3398+
}
3399+
33333400
/// Bounds attributes (e.g., counted_by):
33343401
/// AttrName '(' expression ')'
33353402
void Parser::ParseBoundsAttribute(IdentifierInfo &AttrName,

‎clang/test/AST/ast-dump-color.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,13 +82,13 @@ struct Invalid {
8282
//CHECK: {{^}}[[Blue]]| | `-[[RESET]][[GREEN]]ParmVarDecl[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:33[[RESET]]> [[Yellow]]col:33[[RESET]] [[Green]]'const Mutex &'[[RESET]]{{$}}
8383
//CHECK: {{^}}[[Blue]]| `-[[RESET]][[GREEN]]CXXConstructorDecl[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:33[[RESET]]> [[Yellow]]col:33[[RESET]] implicit constexpr[[CYAN]] Mutex[[RESET]] [[Green]]'void (Mutex &&)'[[RESET]] inline{{ .*$}}
8484
//CHECK: {{^}}[[Blue]]| `-[[RESET]][[GREEN]]ParmVarDecl[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:33[[RESET]]> [[Yellow]]col:33[[RESET]] [[Green]]'Mutex &&'[[RESET]]{{$}}
85-
//CHECK: {{^}}[[Blue]]|-[[RESET]][[GREEN]]VarDecl[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:1[[RESET]], [[Yellow]]line:25:3[[RESET]]> [[Yellow]]col:3[[RESET]] referenced[[CYAN]] mu1[[RESET]] [[Green]]'class Mutex':'Mutex'[[RESET]]
85+
//CHECK: {{^}}[[Blue]]|-[[RESET]][[GREEN]]VarDecl[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:1[[RESET]], [[Yellow]]line:25:3[[RESET]]> [[Yellow]]col:3[[RESET]] used[[CYAN]] mu1[[RESET]] [[Green]]'class Mutex':'Mutex'[[RESET]]
8686
//CHECK: {{^}}[[Blue]]| `-[[RESET]][[MAGENTA]]CXXConstructExpr[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:3[[RESET]]> [[Green]]'class Mutex':'Mutex'[[RESET]][[Cyan]][[RESET]][[Cyan]][[RESET]] [[Green]]'void () noexcept'[[RESET]]{{$}}
8787
//CHECK: {{^}}[[Blue]]|-[[RESET]][[GREEN]]VarDecl[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]line:18:1[[RESET]], [[Yellow]]line:25:8[[RESET]]> [[Yellow]]col:8[[RESET]][[CYAN]] mu2[[RESET]] [[Green]]'class Mutex':'Mutex'[[RESET]]
8888
//CHECK: {{^}}[[Blue]]| `-[[RESET]][[MAGENTA]]CXXConstructExpr[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:8[[RESET]]> [[Green]]'class Mutex':'Mutex'[[RESET]][[Cyan]][[RESET]][[Cyan]][[RESET]] [[Green]]'void () noexcept'[[RESET]]{{$}}
8989
//CHECK: {{^}}[[Blue]]|-[[RESET]][[GREEN]]VarDecl[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]line:26:1[[RESET]], [[Yellow]]col:5[[RESET]]> [[Yellow]]col:5[[RESET]][[CYAN]] TestExpr[[RESET]] [[Green]]'int'[[RESET]]
9090
//CHECK: {{^}}[[Blue]]| `-[[RESET]][[BLUE]]GuardedByAttr[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:29[[RESET]], [[Yellow]]col:43[[RESET]]>{{$}}
91-
//CHECK: {{^}}[[Blue]]| `-[[RESET]][[MAGENTA]]DeclRefExpr[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:40[[RESET]]> [[Green]]'class Mutex':'Mutex'[[RESET]][[Cyan]] lvalue[[RESET]][[Cyan]][[RESET]] [[GREEN]]Var[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]][[CYAN]] 'mu1'[[RESET]] [[Green]]'class Mutex':'Mutex'[[RESET]] non_odr_use_unevaluated{{$}}
91+
//CHECK: {{^}}[[Blue]]| `-[[RESET]][[MAGENTA]]DeclRefExpr[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:40[[RESET]]> [[Green]]'class Mutex':'Mutex'[[RESET]][[Cyan]] lvalue[[RESET]][[Cyan]][[RESET]] [[GREEN]]Var[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]][[CYAN]] 'mu1'[[RESET]] [[Green]]'class Mutex':'Mutex'[[RESET]]{{$}}
9292
//CHECK: {{^}}[[Blue]]|-[[RESET]][[GREEN]]CXXRecordDecl[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]line:28:1[[RESET]], [[Yellow]]line:30:1[[RESET]]> [[Yellow]]line:28:8[[RESET]] struct[[CYAN]] Invalid[[RESET]] definition
9393
//CHECK: {{^}}[[Blue]]| |-[[RESET]][[GREEN]]CXXRecordDecl[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:1[[RESET]], [[Yellow]]col:8[[RESET]]> [[Yellow]]col:8[[RESET]] implicit referenced struct[[CYAN]] Invalid[[RESET]]
9494
//CHECK: {{^}}[[Blue]]| |-[[RESET]][[GREEN]]CXXConstructorDecl[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]line:29:3[[RESET]], [[Yellow]]col:42[[RESET]]> [[Yellow]]col:29[[RESET]] invalid[[CYAN]] Invalid[[RESET]] [[Green]]'void (int)'[[RESET]]

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,12 @@
2828
struct LOCKABLE Mutex {};
2929

3030
struct Foo {
31-
struct Mutex *mu_;
31+
struct Mutex *mu_;
32+
struct Bar {
33+
struct Mutex *other_mu;
34+
} bar;
35+
int a_value GUARDED_BY(mu_);
36+
int* a_ptr PT_GUARDED_BY(bar.other_mu);
3237
};
3338

3439
// Declare mutex lock/unlock functions.
@@ -136,6 +141,9 @@ int main(void) {
136141
// Cleanup happens automatically -> no warning.
137142
}
138143

144+
foo_.a_value = 0; // expected-warning {{writing variable 'a_value' requires holding mutex 'mu_' exclusively}}
145+
*foo_.a_ptr = 1; // expected-warning {{writing the value pointed to by 'a_ptr' requires holding mutex 'bar.other_mu' exclusively}}
146+
139147
return 0;
140148
}
141149

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

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1516,11 +1516,7 @@ class Foo {
15161516
mutable Mutex mu;
15171517
int a GUARDED_BY(mu);
15181518

1519-
static int si GUARDED_BY(mu);
1520-
//FIXME: Bug 32066 - Error should be emitted irrespective of C++ dialect
1521-
#if __cplusplus <= 199711L
1522-
// expected-error@-3 {{invalid use of non-static data member 'mu'}}
1523-
#endif
1519+
static int si GUARDED_BY(mu); // expected-error {{invalid use of non-static data member 'mu'}}
15241520

15251521
static void foo() EXCLUSIVE_LOCKS_REQUIRED(mu);
15261522
//FIXME: Bug 32066 - Error should be emitted irrespective of C++ dialect

0 commit comments

Comments
 (0)