https://github.com/pdherbemont updated https://github.com/llvm/llvm-project/pull/94216
>From 3cfee204f62e029fb1aaadae6a598e1d46b5c465 Mon Sep 17 00:00:00 2001 From: Pierre d'Herbemont <pdherbem...@apple.com> Date: Wed, 29 May 2024 11:11:03 +0200 Subject: [PATCH] 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". --- clang/include/clang/Parse/Parser.h | 8 +++ clang/lib/Parse/ParseDecl.cpp | 67 +++++++++++++++++++ clang/test/AST/ast-dump-color.cpp | 4 +- clang/test/Sema/warn-thread-safety-analysis.c | 10 ++- .../SemaCXX/warn-thread-safety-parsing.cpp | 6 +- 5 files changed, 87 insertions(+), 8 deletions(-) diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index d054b8cf0d240..39cc6b8e1fbeb 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -3128,6 +3128,14 @@ class Parser : public CodeCompletionHandler { IdentifierInfo *ScopeName, SourceLocation ScopeLoc, ParsedAttr::Form Form); + void ParseGuardedByAttribute(IdentifierInfo &AttrName, + SourceLocation AttrNameLoc, + ParsedAttributes &Attrs, + IdentifierInfo *ScopeName, + SourceLocation ScopeLoc, + SourceLocation *EndLoc, + ParsedAttr::Form Form); + void ParseTypeofSpecifier(DeclSpec &DS); SourceLocation ParseDecltypeSpecifier(DeclSpec &DS); void AnnotateExistingDecltypeSpecifier(const DeclSpec &DS, diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp index c528917437332..613973b2525ab 100644 --- a/clang/lib/Parse/ParseDecl.cpp +++ b/clang/lib/Parse/ParseDecl.cpp @@ -671,6 +671,16 @@ void Parser::ParseGNUAttributeArgs( ParseBoundsAttribute(*AttrName, AttrNameLoc, Attrs, ScopeName, ScopeLoc, Form); return; + } else if (AttrKind == ParsedAttr::AT_GuardedBy) { + ParseGuardedByAttribute(*AttrName, AttrNameLoc, Attrs, ScopeName, ScopeLoc, + EndLoc, + Form); + return; + } else if (AttrKind == ParsedAttr::AT_PtGuardedBy) { + ParseGuardedByAttribute(*AttrName, AttrNameLoc, Attrs, ScopeName, ScopeLoc, + EndLoc, + Form); + return; } else if (AttrKind == ParsedAttr::AT_CXXAssume) { ParseCXXAssumeAttributeArg(Attrs, AttrName, AttrNameLoc, EndLoc, Form); return; @@ -3330,6 +3340,63 @@ void Parser::DistributeCLateParsedAttrs(Decl *Dcl, } } +/// GuardedBy attributes (e.g., guarded_by): +/// AttrName '(' expression ')' +void Parser::ParseGuardedByAttribute(IdentifierInfo &AttrName, + SourceLocation AttrNameLoc, + ParsedAttributes &Attrs, + IdentifierInfo *ScopeName, + SourceLocation ScopeLoc, + SourceLocation *EndLoc, + ParsedAttr::Form Form) { + assert(Tok.is(tok::l_paren) && "Attribute arg list not starting with '('"); + + BalancedDelimiterTracker Parens(*this, tok::l_paren); + Parens.consumeOpen(); + + if (Tok.is(tok::r_paren)) { + Diag(Tok.getLocation(), diag::err_argument_required_after_attribute); + Parens.consumeClose(); + return; + } + + ArgsVector ArgExprs; + // Don't evaluate argument when the attribute is ignored. + using ExpressionKind = + Sema::ExpressionEvaluationContextRecord::ExpressionKind; + EnterExpressionEvaluationContext EC( + Actions, Sema::ExpressionEvaluationContext::PotentiallyEvaluated, nullptr, + ExpressionKind::EK_BoundsAttrArgument); + + ExprResult ArgExpr( + Actions.CorrectDelayedTyposInExpr(ParseAssignmentExpression())); + + if (ArgExpr.isInvalid()) { + Parens.skipToEnd(); + return; + } + + ArgExprs.push_back(ArgExpr.get()); + + auto RParens = Tok.getLocation(); + auto &AL = *Attrs.addNew( + &AttrName, SourceRange(AttrNameLoc, RParens), ScopeName, + ScopeLoc, ArgExprs.data(), ArgExprs.size(), Form); + + if (EndLoc) { + *EndLoc = Tok.getLocation(); + } + + if (!Tok.is(tok::r_paren)) { + Diag(Tok.getLocation(), diag::err_attribute_wrong_number_arguments) + << AL << 1; + Parens.skipToEnd(); + return; + } + + Parens.consumeClose(); +} + /// Bounds attributes (e.g., counted_by): /// AttrName '(' expression ')' void Parser::ParseBoundsAttribute(IdentifierInfo &AttrName, diff --git a/clang/test/AST/ast-dump-color.cpp b/clang/test/AST/ast-dump-color.cpp index 87797f6bffc5b..944b3ecb9a4f7 100644 --- a/clang/test/AST/ast-dump-color.cpp +++ b/clang/test/AST/ast-dump-color.cpp @@ -82,13 +82,13 @@ struct Invalid { //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]]{{$}} //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{{ .*$}} //CHECK: {{^}}[[Blue]]| `-[[RESET]][[GREEN]]ParmVarDecl[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:33[[RESET]]> [[Yellow]]col:33[[RESET]] [[Green]]'Mutex &&'[[RESET]]{{$}} -//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]] +//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]] //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]]{{$}} //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]] //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]]{{$}} //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]] //CHECK: {{^}}[[Blue]]| `-[[RESET]][[BLUE]]GuardedByAttr[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:29[[RESET]], [[Yellow]]col:43[[RESET]]>{{$}} -//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{{$}} +//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]]{{$}} //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 //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]] //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]] diff --git a/clang/test/Sema/warn-thread-safety-analysis.c b/clang/test/Sema/warn-thread-safety-analysis.c index 642ea88ec3c96..abad65b0b9154 100644 --- a/clang/test/Sema/warn-thread-safety-analysis.c +++ b/clang/test/Sema/warn-thread-safety-analysis.c @@ -28,7 +28,12 @@ struct LOCKABLE Mutex {}; struct Foo { - struct Mutex *mu_; + struct Mutex *mu_; + struct Bar { + struct Mutex *other_mu; + } bar; + int a_value GUARDED_BY(mu_); + int* a_ptr PT_GUARDED_BY(bar.other_mu); }; // Declare mutex lock/unlock functions. @@ -136,6 +141,9 @@ int main(void) { // Cleanup happens automatically -> no warning. } + foo_.a_value = 0; // expected-warning {{writing variable 'a_value' requires holding mutex 'mu_' exclusively}} + *foo_.a_ptr = 1; // expected-warning {{writing the value pointed to by 'a_ptr' requires holding mutex 'bar.other_mu' exclusively}} + return 0; } diff --git a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp index 0c5b0cc85897b..1626e8375892a 100644 --- a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp +++ b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp @@ -1516,11 +1516,7 @@ class Foo { mutable Mutex mu; int a GUARDED_BY(mu); - static int si GUARDED_BY(mu); -//FIXME: Bug 32066 - Error should be emitted irrespective of C++ dialect -#if __cplusplus <= 199711L - // expected-error@-3 {{invalid use of non-static data member 'mu'}} -#endif + static int si GUARDED_BY(mu); // expected-error {{invalid use of non-static data member 'mu'}} static void foo() EXCLUSIVE_LOCKS_REQUIRED(mu); //FIXME: Bug 32066 - Error should be emitted irrespective of C++ dialect _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits