[clang] Support `guarded_by` attribute and related attributes inside C structs and support late parsing them (PR #95455)
@@ -29,6 +30,22 @@ struct LOCKABLE Mutex {}; struct Foo { struct Mutex *mu_; + int a_value GUARDED_BY(mu_); + + struct Bar { +struct Mutex *other_mu ACQUIRED_AFTER(mu_); pdherbemont wrote: > I'm not entirely sure how nested structs work in C, but should we be able to > reference the parent from the child? I think so, it does read nicely it feels – I see why this could cause a problem though: if the child structure is used outside of the parent, there is an ambiguity. Maybe this should be restricted to anonymous struct? Or maybe we need to taint the child struct so that it's not reused outside of the parent? Anyhow, I'll add a comment as suggested! https://github.com/llvm/llvm-project/pull/95455 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support `guarded_by` attribute and related attributes inside C structs and support late parsing them (PR #95455)
https://github.com/pdherbemont updated https://github.com/llvm/llvm-project/pull/95455 >From 4edb5194a6c03cf7bcf18eb998728da7cbc51dc0 Mon Sep 17 00:00:00 2001 From: Pierre d'Herbemont Date: Wed, 12 Jun 2024 20:20:05 +0200 Subject: [PATCH] Support `guarded_by` attribute and related attributes inside C structs and support late parsing them This fixes #20777. This replaces #94216 which was reverted after being merged. Previously the `guarded_by`, `pt_guarded_by`, `acquired_after`, and `acquired_before` attributes were only supported inside C++ classes or top level C/C++ declaration. This patch allows these attributes to be added to struct members in C. These attributes have also now support experimental late parsing. This is off by default but can be enabled by passing `-fexperimental-late-parse-attributes`. This is useful for referring to a struct member after the annotated member. E.g. ``` struct Example { int a_value_defined_before __attribute__ ((guarded_by(a_mutex))); struct Mutex *a_mutex; }; ``` --- clang/docs/ReleaseNotes.rst | 4 + clang/docs/ThreadSafetyAnalysis.rst | 5 +- clang/include/clang/Basic/Attr.td | 8 +- clang/include/clang/Sema/Sema.h | 7 +- clang/lib/Parse/ParseDecl.cpp | 15 ++-- clang/lib/Sema/SemaExpr.cpp | 11 ++- clang/test/Sema/warn-thread-safety-analysis.c | 76 ++- .../SemaCXX/warn-thread-safety-analysis.cpp | 30 8 files changed, 133 insertions(+), 23 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 8c2f737836a9d..3e6cc43652ae0 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -471,6 +471,10 @@ Attribute Changes in Clang size_t count; }; +- The ``guarded_by``, ``pt_guarded_by``, ``acquired_after``, ``acquired_before`` + attributes now support referencing struct members in C. The arguments are also + now late parsed when ``-fexperimental-late-parse-attributes`` is passed like + for ``counted_by``. Improvements to Clang's diagnostics --- diff --git a/clang/docs/ThreadSafetyAnalysis.rst b/clang/docs/ThreadSafetyAnalysis.rst index dcde0c706c704..98a5f0cbdcb7a 100644 --- a/clang/docs/ThreadSafetyAnalysis.rst +++ b/clang/docs/ThreadSafetyAnalysis.rst @@ -764,10 +764,11 @@ doesn't know that munl.mu == mutex. The SCOPED_CAPABILITY attribute handles aliasing for MutexLocker, but does so only for that particular pattern. -ACQUIRED_BEFORE(...) and ACQUIRED_AFTER(...) are currently unimplemented. +ACQUIRED_BEFORE(...) and ACQUIRED_AFTER(...) support is still experimental. - -To be fixed in a future update. +ACQUIRED_BEFORE(...) and ACQUIRED_AFTER(...) are currently being developed under +the ``-Wthread-safety-beta`` flag. .. _mutexheader: diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index b70b0c8b836a5..6032b934279dd 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -3633,7 +3633,7 @@ def NoThreadSafetyAnalysis : InheritableAttr { def GuardedBy : InheritableAttr { let Spellings = [GNU<"guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3644,7 +3644,7 @@ def GuardedBy : InheritableAttr { def PtGuardedBy : InheritableAttr { let Spellings = [GNU<"pt_guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3655,7 +3655,7 @@ def PtGuardedBy : InheritableAttr { def AcquiredAfter : InheritableAttr { let Spellings = [GNU<"acquired_after">]; let Args = [VariadicExprArgument<"Args">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3666,7 +3666,7 @@ def AcquiredAfter : InheritableAttr { def AcquiredBefore : InheritableAttr { let Spellings = [GNU<"acquired_before">]; let Args = [VariadicExprArgument<"Args">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 4d4579fcfd456..53b5e18905ca7 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -5142,7 +5142,7 @@ class Sema final : public SemaBase { enum ExpressionKind { EK
[clang] Support `guarded_by` attribute and related attributes inside C structs and support late parsing them (PR #95455)
@@ -764,12 +764,6 @@ doesn't know that munl.mu == mutex. The SCOPED_CAPABILITY attribute handles aliasing for MutexLocker, but does so only for that particular pattern. -ACQUIRED_BEFORE(...) and ACQUIRED_AFTER(...) are currently unimplemented. -- - -To be fixed in a future update. - - pdherbemont wrote: Thanks! I documented those are "experimental". https://github.com/llvm/llvm-project/pull/95455 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support `guarded_by` attribute and related attributes inside C structs and support late parsing them (PR #95455)
https://github.com/pdherbemont updated https://github.com/llvm/llvm-project/pull/95455 >From 2a8eb78119421fa63f635f9f4e5edad18c635d9c Mon Sep 17 00:00:00 2001 From: Pierre d'Herbemont Date: Wed, 12 Jun 2024 20:20:05 +0200 Subject: [PATCH] Support `guarded_by` attribute and related attributes inside C structs and support late parsing them This fixes #20777. This replaces #94216 which was reverted after being merged. Previously the `guarded_by`, `pt_guarded_by`, `acquired_after`, and `acquired_before` attributes were only supported inside C++ classes or top level C/C++ declaration. This patch allows these attributes to be added to struct members in C. These attributes have also now support experimental late parsing. This is off by default but can be enabled by passing `-fexperimental-late-parse-attributes`. This is useful for referring to a struct member after the annotated member. E.g. ``` struct Example { int a_value_defined_before __attribute__ ((guarded_by(a_mutex))); struct Mutex *a_mutex; }; ``` --- clang/docs/ReleaseNotes.rst | 4 + clang/docs/ThreadSafetyAnalysis.rst | 7 +- clang/include/clang/Basic/Attr.td | 8 +- clang/include/clang/Sema/Sema.h | 7 +- clang/lib/Parse/ParseDecl.cpp | 15 ++-- clang/lib/Sema/SemaExpr.cpp | 11 ++- clang/test/Sema/warn-thread-safety-analysis.c | 76 ++- .../SemaCXX/warn-thread-safety-analysis.cpp | 30 8 files changed, 134 insertions(+), 24 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 8c2f737836a9d..3e6cc43652ae0 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -471,6 +471,10 @@ Attribute Changes in Clang size_t count; }; +- The ``guarded_by``, ``pt_guarded_by``, ``acquired_after``, ``acquired_before`` + attributes now support referencing struct members in C. The arguments are also + now late parsed when ``-fexperimental-late-parse-attributes`` is passed like + for ``counted_by``. Improvements to Clang's diagnostics --- diff --git a/clang/docs/ThreadSafetyAnalysis.rst b/clang/docs/ThreadSafetyAnalysis.rst index dcde0c706c704..cc4089b97b492 100644 --- a/clang/docs/ThreadSafetyAnalysis.rst +++ b/clang/docs/ThreadSafetyAnalysis.rst @@ -764,10 +764,11 @@ doesn't know that munl.mu == mutex. The SCOPED_CAPABILITY attribute handles aliasing for MutexLocker, but does so only for that particular pattern. -ACQUIRED_BEFORE(...) and ACQUIRED_AFTER(...) are currently unimplemented. -- +ACQUIRED_BEFORE(...) and ACQUIRED_AFTER(...) support is still experimental. +--- -To be fixed in a future update. +ACQUIRED_BEFORE(...) and ACQUIRED_AFTER(...) are currently being developed under +the ``-Wthread-safety-beta`` flag. .. _mutexheader: diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index b70b0c8b836a5..6032b934279dd 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -3633,7 +3633,7 @@ def NoThreadSafetyAnalysis : InheritableAttr { def GuardedBy : InheritableAttr { let Spellings = [GNU<"guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3644,7 +3644,7 @@ def GuardedBy : InheritableAttr { def PtGuardedBy : InheritableAttr { let Spellings = [GNU<"pt_guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3655,7 +3655,7 @@ def PtGuardedBy : InheritableAttr { def AcquiredAfter : InheritableAttr { let Spellings = [GNU<"acquired_after">]; let Args = [VariadicExprArgument<"Args">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3666,7 +3666,7 @@ def AcquiredAfter : InheritableAttr { def AcquiredBefore : InheritableAttr { let Spellings = [GNU<"acquired_before">]; let Args = [VariadicExprArgument<"Args">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 4d4579fcfd456..53b5e18905ca7 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -5142,7 +5142,7
[clang] Support `guarded_by` attribute and related attributes inside C structs and support late parsing them (PR #95455)
pdherbemont wrote: > LGTM. Let me know if you need me to merge this change. Yes, please! https://github.com/llvm/llvm-project/pull/95455 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
pdherbemont wrote: > One question I have is whether there will be follow-up work for other thread > safety attributes (`acquire_capability`, `try_acquire_capability`, etc)? They already work fine in C so far. But if you are aware of some issues don't hesitate to let me know! https://github.com/llvm/llvm-project/pull/94216 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
https://github.com/pdherbemont updated https://github.com/llvm/llvm-project/pull/94216 >From 0fd6864d7f0a85ca82f3c926a51403383c7ea0c7 Mon Sep 17 00:00:00 2001 From: Pierre d'Herbemont 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/Basic/Attr.td | 4 +- clang/include/clang/Parse/Parser.h| 8 +++ clang/include/clang/Sema/Sema.h | 6 +- clang/lib/Parse/ParseDecl.cpp | 64 ++- clang/lib/Sema/SemaExpr.cpp | 4 +- clang/test/AST/ast-dump-color.cpp | 4 +- clang/test/Sema/warn-thread-safety-analysis.c | 10 ++- .../SemaCXX/warn-thread-safety-parsing.cpp| 6 +- 8 files changed, 90 insertions(+), 16 deletions(-) diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index c6f08860f203a..15663c9eac686 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -3633,7 +3633,7 @@ def NoThreadSafetyAnalysis : InheritableAttr { def GuardedBy : InheritableAttr { let Spellings = [GNU<"guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3644,7 +3644,7 @@ def GuardedBy : InheritableAttr { def PtGuardedBy : InheritableAttr { let Spellings = [GNU<"pt_guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; 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/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 7dea2b6826cfd..d41557270fe31 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -5078,7 +5078,7 @@ class Sema final : public SemaBase { enum ExpressionKind { EK_Decltype, EK_TemplateArgument, - EK_BoundsAttrArgument, + EK_AttrArgument, EK_Other } ExprContext; @@ -5185,10 +5185,10 @@ class Sema final : public SemaBase { return const_cast(this)->parentEvaluationContext(); }; - bool isBoundsAttrContext() const { + bool isAttrContext() const { return ExprEvalContexts.back().ExprContext == ExpressionEvaluationContextRecord::ExpressionKind:: - EK_BoundsAttrArgument; + EK_AttrArgument; } /// Increment when we find a reference; decrement when we find an ignored diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp index c528917437332..cfa6fa8b4926f 100644 --- a/clang/lib/Parse/ParseDecl.cpp +++ b/clang/lib/Parse/ParseDecl.cpp @@ -671,6 +671,12 @@ void Parser::ParseGNUAttributeArgs( ParseBoundsAttribute(*AttrName, AttrNameLoc, Attrs, ScopeName, ScopeLoc, Form); return; + } else if (AttrKind == ParsedAttr::AT_GuardedBy || + 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 +3336,62 @@ void Parser::DistributeCLateParsedAttrs(Decl *
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
https://github.com/pdherbemont updated https://github.com/llvm/llvm-project/pull/94216 >From 7d01e8aae371e02f23b118d5b92f39fcee72723f Mon Sep 17 00:00:00 2001 From: Pierre d'Herbemont 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/Basic/Attr.td | 4 +- clang/include/clang/Parse/Parser.h| 7 +++ clang/include/clang/Sema/Sema.h | 7 +-- clang/lib/Parse/ParseDecl.cpp | 60 ++- clang/lib/Sema/SemaExpr.cpp | 4 +- clang/test/AST/ast-dump-color.cpp | 4 +- clang/test/Sema/warn-thread-safety-analysis.c | 8 +++ .../SemaCXX/warn-thread-safety-parsing.cpp| 6 +- 8 files changed, 84 insertions(+), 16 deletions(-) diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index c6f08860f203a..15663c9eac686 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -3633,7 +3633,7 @@ def NoThreadSafetyAnalysis : InheritableAttr { def GuardedBy : InheritableAttr { let Spellings = [GNU<"guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3644,7 +3644,7 @@ def GuardedBy : InheritableAttr { def PtGuardedBy : InheritableAttr { let Spellings = [GNU<"pt_guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index d054b8cf0d240..fa60ef0515df1 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -3128,6 +3128,13 @@ 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/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 7dea2b6826cfd..5207a03bb7853 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -5078,7 +5078,7 @@ class Sema final : public SemaBase { enum ExpressionKind { EK_Decltype, EK_TemplateArgument, - EK_BoundsAttrArgument, + EK_AttrArgument, EK_Other } ExprContext; @@ -5185,10 +5185,9 @@ class Sema final : public SemaBase { return const_cast(this)->parentEvaluationContext(); }; - bool isBoundsAttrContext() const { + bool isAttrContext() const { return ExprEvalContexts.back().ExprContext == - ExpressionEvaluationContextRecord::ExpressionKind:: - EK_BoundsAttrArgument; + ExpressionEvaluationContextRecord::ExpressionKind::EK_AttrArgument; } /// Increment when we find a reference; decrement when we find an ignored diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp index c528917437332..a2d2522a2a6ba 100644 --- a/clang/lib/Parse/ParseDecl.cpp +++ b/clang/lib/Parse/ParseDecl.cpp @@ -671,6 +671,11 @@ void Parser::ParseGNUAttributeArgs( ParseBoundsAttribute(*AttrName, AttrNameLoc, Attrs, ScopeName, ScopeLoc, Form); return; + } else if (AttrKind == ParsedAttr::AT_GuardedBy || + 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 +3335,59 @@ void Parser::DistributeCLateParsedAttrs(Decl *Dcl, } }
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
@@ -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(); + } pdherbemont wrote: Switched to `EK_AttrArgument` and fixed the style (using the clang-format) thank you for the feedback! https://github.com/llvm/llvm-project/pull/94216 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
@@ -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; pdherbemont wrote: Done! https://github.com/llvm/llvm-project/pull/94216 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
@@ -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_); pdherbemont wrote: Fixed https://github.com/llvm/llvm-project/pull/94216 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
pdherbemont wrote: > You may also want to consider making the attribute late parsed in C when > `-fexperimental-late-parse-attributes` is enabled. See > https://github.com/llvm/llvm-project/pull/93121/files#diff-ae2ec9524bdbeea1f06917607482634dd89af5bcbb929805032463e5dafe79e7R2260 > > That will allow the code like below: > > ``` > struct Foo { >int a_value GUARDED_BY(mu_); // attribute comes before `mu_` which needs > to be late parsed >struct Mutext *mu_; > } > ``` Adopted `LateAttrParseExperimentalExt`. Let me know if that looks okay. https://github.com/llvm/llvm-project/pull/94216 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
pdherbemont wrote: > > > One question I have is whether there will be follow-up work for other > > > thread safety attributes (`acquire_capability`, `try_acquire_capability`, > > > etc)? > > > > > > They already work fine in C so far. But if you are aware of some issues > > don't hesitate to let me know! > > Ah, sorry, of course I picked examples that appertain to functions rather > than fields. But some of the other thread safety attributes have the same > problem as `guarded_by`: https://godbolt.org/z/hWTMxzrns Oh yes – maybe it makes sense to fix them in this commit. Let me try to look into that quickly. https://github.com/llvm/llvm-project/pull/94216 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
https://github.com/pdherbemont updated https://github.com/llvm/llvm-project/pull/94216 >From 94212789ca0b22380de88521279056ffdce5b3e6 Mon Sep 17 00:00:00 2001 From: Pierre d'Herbemont Date: Wed, 29 May 2024 11:11:03 +0200 Subject: [PATCH] Support [[guarded_by(mutex)]] and friends attributes 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". Then I enabled the use of the experimental late parsing used by [[counted_by]] because that will enable the adoption of attributes without restructuring the data layout as requested by @rapidsna. --- clang/docs/ReleaseNotes.rst | 4 ++ clang/include/clang/Basic/Attr.td | 8 +-- clang/include/clang/Parse/Parser.h| 7 +++ clang/include/clang/Sema/Sema.h | 7 +-- clang/lib/Parse/ParseDecl.cpp | 62 ++- clang/lib/Sema/SemaExpr.cpp | 4 +- clang/test/AST/ast-dump-color.cpp | 4 +- clang/test/Sema/warn-thread-safety-analysis.c | 17 + .../SemaCXX/warn-thread-safety-parsing.cpp| 6 +- 9 files changed, 101 insertions(+), 18 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 99580c0d28a4f..c6bcf25d95dab 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -451,6 +451,10 @@ Attribute Changes in Clang size_t count; }; +- The ``guarded_by``, ``pt_guarded_by``, ``acquired_after``, ``acquired_before`` + attributes now support referencing struct members in C. The arguments are also + now late parsed when ``-fexperimental-late-parse-attributes`` is passed like + for ``counted_by``. Improvements to Clang's diagnostics --- diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index c6f08860f203a..cb424965732b3 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -3633,7 +3633,7 @@ def NoThreadSafetyAnalysis : InheritableAttr { def GuardedBy : InheritableAttr { let Spellings = [GNU<"guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3644,7 +3644,7 @@ def GuardedBy : InheritableAttr { def PtGuardedBy : InheritableAttr { let Spellings = [GNU<"pt_guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3655,7 +3655,7 @@ def PtGuardedBy : InheritableAttr { def AcquiredAfter : InheritableAttr { let Spellings = [GNU<"acquired_after">]; let Args = [VariadicExprArgument<"Args">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3666,7 +3666,7 @@ def AcquiredAfter : InheritableAttr { def AcquiredBefore : InheritableAttr { let Spellings = [GNU<"acquired_before">]; let Args = [VariadicExprArgument<"Args">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index d054b8cf0d240..fa60ef0515df1 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -3128,6 +3128,13 @@ 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/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 7dea2b6826cfd..5207a03bb7853 100644
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
https://github.com/pdherbemont updated https://github.com/llvm/llvm-project/pull/94216 >From 94212789ca0b22380de88521279056ffdce5b3e6 Mon Sep 17 00:00:00 2001 From: Pierre d'Herbemont Date: Wed, 29 May 2024 11:11:03 +0200 Subject: [PATCH 1/2] Support [[guarded_by(mutex)]] and friends attributes 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". Then I enabled the use of the experimental late parsing used by [[counted_by]] because that will enable the adoption of attributes without restructuring the data layout as requested by @rapidsna. --- clang/docs/ReleaseNotes.rst | 4 ++ clang/include/clang/Basic/Attr.td | 8 +-- clang/include/clang/Parse/Parser.h| 7 +++ clang/include/clang/Sema/Sema.h | 7 +-- clang/lib/Parse/ParseDecl.cpp | 62 ++- clang/lib/Sema/SemaExpr.cpp | 4 +- clang/test/AST/ast-dump-color.cpp | 4 +- clang/test/Sema/warn-thread-safety-analysis.c | 17 + .../SemaCXX/warn-thread-safety-parsing.cpp| 6 +- 9 files changed, 101 insertions(+), 18 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 99580c0d28a4f..c6bcf25d95dab 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -451,6 +451,10 @@ Attribute Changes in Clang size_t count; }; +- The ``guarded_by``, ``pt_guarded_by``, ``acquired_after``, ``acquired_before`` + attributes now support referencing struct members in C. The arguments are also + now late parsed when ``-fexperimental-late-parse-attributes`` is passed like + for ``counted_by``. Improvements to Clang's diagnostics --- diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index c6f08860f203a..cb424965732b3 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -3633,7 +3633,7 @@ def NoThreadSafetyAnalysis : InheritableAttr { def GuardedBy : InheritableAttr { let Spellings = [GNU<"guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3644,7 +3644,7 @@ def GuardedBy : InheritableAttr { def PtGuardedBy : InheritableAttr { let Spellings = [GNU<"pt_guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3655,7 +3655,7 @@ def PtGuardedBy : InheritableAttr { def AcquiredAfter : InheritableAttr { let Spellings = [GNU<"acquired_after">]; let Args = [VariadicExprArgument<"Args">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3666,7 +3666,7 @@ def AcquiredAfter : InheritableAttr { def AcquiredBefore : InheritableAttr { let Spellings = [GNU<"acquired_before">]; let Args = [VariadicExprArgument<"Args">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index d054b8cf0d240..fa60ef0515df1 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -3128,6 +3128,13 @@ 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/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 7dea2b6826cfd..5207a03bb7853 10
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
pdherbemont wrote: > > > > One question I have is whether there will be follow-up work for other > > > > thread safety attributes (`acquire_capability`, > > > > `try_acquire_capability`, etc)? > > > > > > > > > They already work fine in C so far. But if you are aware of some issues > > > don't hesitate to let me know! > > > > > > Ah, sorry, of course I picked examples that appertain to functions rather > > than fields. But some of the other thread safety attributes have the same > > problem as `guarded_by`: https://godbolt.org/z/hWTMxzrns > > Oh yes – maybe it makes sense to fix them in this commit. Let me try to look > into that quickly. Seems like it's an easy change for `acquired_after` and `acquired_before`! Added support & a test. https://github.com/llvm/llvm-project/pull/94216 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
@@ -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); pdherbemont wrote: Renamed! https://github.com/llvm/llvm-project/pull/94216 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
pdherbemont wrote: Pushed an additional change on the documentation: the doc was incorrectly stating that ACQUIRED_{AFTER,BEFORE} were not implemented but they are. https://github.com/llvm/llvm-project/pull/94216 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
https://github.com/pdherbemont updated https://github.com/llvm/llvm-project/pull/94216 >From bb2effbaf8e06ae933791d8533c006a6db3aadc9 Mon Sep 17 00:00:00 2001 From: Pierre d'Herbemont Date: Wed, 29 May 2024 11:11:03 +0200 Subject: [PATCH] Support [[guarded_by(mutex)]] and friends attributes 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". Then I enabled the use of the experimental late parsing used by [[counted_by]] because that will enable the adoption of attributes without restructuring the data layout as requested by @rapidsna. --- clang/docs/ReleaseNotes.rst | 4 + clang/docs/ThreadSafetyAnalysis.rst | 6 - clang/include/clang/Basic/Attr.td | 8 +- clang/include/clang/Parse/Parser.h| 14 +++ clang/include/clang/Sema/Sema.h | 7 +- clang/lib/Parse/ParseDecl.cpp | 116 +- clang/lib/Sema/SemaExpr.cpp | 4 +- clang/test/AST/ast-dump-color.cpp | 4 +- clang/test/Sema/warn-thread-safety-analysis.c | 17 +++ .../SemaCXX/warn-thread-safety-parsing.cpp| 6 +- 10 files changed, 162 insertions(+), 24 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 99580c0d28a4f..c6bcf25d95dab 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -451,6 +451,10 @@ Attribute Changes in Clang size_t count; }; +- The ``guarded_by``, ``pt_guarded_by``, ``acquired_after``, ``acquired_before`` + attributes now support referencing struct members in C. The arguments are also + now late parsed when ``-fexperimental-late-parse-attributes`` is passed like + for ``counted_by``. Improvements to Clang's diagnostics --- diff --git a/clang/docs/ThreadSafetyAnalysis.rst b/clang/docs/ThreadSafetyAnalysis.rst index dcde0c706c704..6eefa306e3c89 100644 --- a/clang/docs/ThreadSafetyAnalysis.rst +++ b/clang/docs/ThreadSafetyAnalysis.rst @@ -764,12 +764,6 @@ doesn't know that munl.mu == mutex. The SCOPED_CAPABILITY attribute handles aliasing for MutexLocker, but does so only for that particular pattern. -ACQUIRED_BEFORE(...) and ACQUIRED_AFTER(...) are currently unimplemented. -- - -To be fixed in a future update. - - .. _mutexheader: mutex.h diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index c6f08860f203a..cb424965732b3 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -3633,7 +3633,7 @@ def NoThreadSafetyAnalysis : InheritableAttr { def GuardedBy : InheritableAttr { let Spellings = [GNU<"guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3644,7 +3644,7 @@ def GuardedBy : InheritableAttr { def PtGuardedBy : InheritableAttr { let Spellings = [GNU<"pt_guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3655,7 +3655,7 @@ def PtGuardedBy : InheritableAttr { def AcquiredAfter : InheritableAttr { let Spellings = [GNU<"acquired_after">]; let Args = [VariadicExprArgument<"Args">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3666,7 +3666,7 @@ def AcquiredAfter : InheritableAttr { def AcquiredBefore : InheritableAttr { let Spellings = [GNU<"acquired_before">]; let Args = [VariadicExprArgument<"Args">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index d054b8cf0d240..9e11078382756 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -3128,6 +3128,20 @@ class Parser : public CodeCompletionHandler { IdentifierInfo *ScopeName, SourceLocation ScopeLoc, ParsedAttr::Fo
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
https://github.com/pdherbemont updated https://github.com/llvm/llvm-project/pull/94216 >From 0f7d369ac81fd9523e90404c6b863baaf1f6afd9 Mon Sep 17 00:00:00 2001 From: Pierre d'Herbemont Date: Wed, 29 May 2024 11:11:03 +0200 Subject: [PATCH] Support [[guarded_by(mutex)]] and friends attributes 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". Then I enabled the use of the experimental late parsing used by [[counted_by]] because that will enable the adoption of attributes without restructuring the data layout as requested by @rapidsna. --- clang/docs/ReleaseNotes.rst | 4 + clang/docs/ThreadSafetyAnalysis.rst | 6 - clang/include/clang/Basic/Attr.td | 8 +- clang/include/clang/Parse/Parser.h| 14 +++ clang/include/clang/Sema/Sema.h | 7 +- clang/lib/Parse/ParseDecl.cpp | 116 +- clang/lib/Sema/SemaExpr.cpp | 4 +- clang/test/AST/ast-dump-color.cpp | 4 +- clang/test/Sema/warn-thread-safety-analysis.c | 20 ++- .../SemaCXX/warn-thread-safety-parsing.cpp| 6 +- 10 files changed, 164 insertions(+), 25 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 99580c0d28a4f..c6bcf25d95dab 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -451,6 +451,10 @@ Attribute Changes in Clang size_t count; }; +- The ``guarded_by``, ``pt_guarded_by``, ``acquired_after``, ``acquired_before`` + attributes now support referencing struct members in C. The arguments are also + now late parsed when ``-fexperimental-late-parse-attributes`` is passed like + for ``counted_by``. Improvements to Clang's diagnostics --- diff --git a/clang/docs/ThreadSafetyAnalysis.rst b/clang/docs/ThreadSafetyAnalysis.rst index dcde0c706c704..6eefa306e3c89 100644 --- a/clang/docs/ThreadSafetyAnalysis.rst +++ b/clang/docs/ThreadSafetyAnalysis.rst @@ -764,12 +764,6 @@ doesn't know that munl.mu == mutex. The SCOPED_CAPABILITY attribute handles aliasing for MutexLocker, but does so only for that particular pattern. -ACQUIRED_BEFORE(...) and ACQUIRED_AFTER(...) are currently unimplemented. -- - -To be fixed in a future update. - - .. _mutexheader: mutex.h diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index c6f08860f203a..cb424965732b3 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -3633,7 +3633,7 @@ def NoThreadSafetyAnalysis : InheritableAttr { def GuardedBy : InheritableAttr { let Spellings = [GNU<"guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3644,7 +3644,7 @@ def GuardedBy : InheritableAttr { def PtGuardedBy : InheritableAttr { let Spellings = [GNU<"pt_guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3655,7 +3655,7 @@ def PtGuardedBy : InheritableAttr { def AcquiredAfter : InheritableAttr { let Spellings = [GNU<"acquired_after">]; let Args = [VariadicExprArgument<"Args">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3666,7 +3666,7 @@ def AcquiredAfter : InheritableAttr { def AcquiredBefore : InheritableAttr { let Spellings = [GNU<"acquired_before">]; let Args = [VariadicExprArgument<"Args">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index d054b8cf0d240..9e11078382756 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -3128,6 +3128,20 @@ class Parser : public CodeCompletionHandler { IdentifierInfo *ScopeName, SourceLocation ScopeLoc, ParsedAttr::Fo
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
pdherbemont wrote: > `-fexperimental-late-parse-attributes` Done > > > You may also want to consider making the attribute late parsed in C when > > > `-fexperimental-late-parse-attributes` is enabled. See > > > https://github.com/llvm/llvm-project/pull/93121/files#diff-ae2ec9524bdbeea1f06917607482634dd89af5bcbb929805032463e5dafe79e7R2260 > > > That will allow the code like below: > > > ``` > > > struct Foo { > > >int a_value GUARDED_BY(mu_); // attribute comes before `mu_` which > > > needs to be late parsed > > >struct Mutext *mu_; > > > } > > > ``` > > > > > > Adopted `LateAttrParseExperimentalExt`. Let me know if that looks okay. > > Thanks! Could you please also add a test taking advantage of late parsing? Done! https://github.com/llvm/llvm-project/pull/94216 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
https://github.com/pdherbemont updated https://github.com/llvm/llvm-project/pull/94216 >From 313335523ecdfec8cbf7e703cb720bb4adc81a52 Mon Sep 17 00:00:00 2001 From: Pierre d'Herbemont Date: Wed, 29 May 2024 11:11:03 +0200 Subject: [PATCH] Support [[guarded_by(mutex)]] and friends attributes 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". Then I enabled the use of the experimental late parsing used by [[counted_by]] because that will enable the adoption of attributes without restructuring the data layout as requested by @rapidsna. --- clang/docs/ReleaseNotes.rst | 4 + clang/docs/ThreadSafetyAnalysis.rst | 6 - clang/include/clang/Basic/Attr.td | 8 +- clang/include/clang/Parse/Parser.h| 14 +++ clang/include/clang/Sema/Sema.h | 7 +- clang/lib/Parse/ParseDecl.cpp | 117 +- clang/lib/Sema/SemaExpr.cpp | 4 +- clang/test/AST/ast-dump-color.cpp | 4 +- clang/test/Sema/warn-thread-safety-analysis.c | 20 ++- .../SemaCXX/warn-thread-safety-parsing.cpp| 6 +- 10 files changed, 165 insertions(+), 25 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 99580c0d28a4f..c6bcf25d95dab 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -451,6 +451,10 @@ Attribute Changes in Clang size_t count; }; +- The ``guarded_by``, ``pt_guarded_by``, ``acquired_after``, ``acquired_before`` + attributes now support referencing struct members in C. The arguments are also + now late parsed when ``-fexperimental-late-parse-attributes`` is passed like + for ``counted_by``. Improvements to Clang's diagnostics --- diff --git a/clang/docs/ThreadSafetyAnalysis.rst b/clang/docs/ThreadSafetyAnalysis.rst index dcde0c706c704..6eefa306e3c89 100644 --- a/clang/docs/ThreadSafetyAnalysis.rst +++ b/clang/docs/ThreadSafetyAnalysis.rst @@ -764,12 +764,6 @@ doesn't know that munl.mu == mutex. The SCOPED_CAPABILITY attribute handles aliasing for MutexLocker, but does so only for that particular pattern. -ACQUIRED_BEFORE(...) and ACQUIRED_AFTER(...) are currently unimplemented. -- - -To be fixed in a future update. - - .. _mutexheader: mutex.h diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index c6f08860f203a..cb424965732b3 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -3633,7 +3633,7 @@ def NoThreadSafetyAnalysis : InheritableAttr { def GuardedBy : InheritableAttr { let Spellings = [GNU<"guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3644,7 +3644,7 @@ def GuardedBy : InheritableAttr { def PtGuardedBy : InheritableAttr { let Spellings = [GNU<"pt_guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3655,7 +3655,7 @@ def PtGuardedBy : InheritableAttr { def AcquiredAfter : InheritableAttr { let Spellings = [GNU<"acquired_after">]; let Args = [VariadicExprArgument<"Args">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3666,7 +3666,7 @@ def AcquiredAfter : InheritableAttr { def AcquiredBefore : InheritableAttr { let Spellings = [GNU<"acquired_before">]; let Args = [VariadicExprArgument<"Args">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index d054b8cf0d240..9e11078382756 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -3128,6 +3128,20 @@ class Parser : public CodeCompletionHandler { IdentifierInfo *ScopeName, SourceLocation ScopeLoc, ParsedAttr::Fo
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
https://github.com/pdherbemont updated https://github.com/llvm/llvm-project/pull/94216 >From 958ff53f3c3f31550e093494d5240ec1f350124e Mon Sep 17 00:00:00 2001 From: Pierre d'Herbemont Date: Wed, 29 May 2024 11:11:03 +0200 Subject: [PATCH] Support [[guarded_by(mutex)]] and friends attributes 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". Then I enabled the use of the experimental late parsing used by [[counted_by]] because that will enable the adoption of attributes without restructuring the data layout as requested by @rapidsna. --- clang/docs/ReleaseNotes.rst | 4 + clang/docs/ThreadSafetyAnalysis.rst | 6 - clang/include/clang/Basic/Attr.td | 8 +- clang/include/clang/Parse/Parser.h| 14 +++ clang/include/clang/Sema/Sema.h | 7 +- clang/lib/Parse/ParseDecl.cpp | 118 +- clang/lib/Sema/SemaExpr.cpp | 4 +- clang/test/AST/ast-dump-color.cpp | 4 +- clang/test/Sema/warn-thread-safety-analysis.c | 20 ++- .../SemaCXX/warn-thread-safety-parsing.cpp| 6 +- 10 files changed, 166 insertions(+), 25 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 99580c0d28a4f..c6bcf25d95dab 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -451,6 +451,10 @@ Attribute Changes in Clang size_t count; }; +- The ``guarded_by``, ``pt_guarded_by``, ``acquired_after``, ``acquired_before`` + attributes now support referencing struct members in C. The arguments are also + now late parsed when ``-fexperimental-late-parse-attributes`` is passed like + for ``counted_by``. Improvements to Clang's diagnostics --- diff --git a/clang/docs/ThreadSafetyAnalysis.rst b/clang/docs/ThreadSafetyAnalysis.rst index dcde0c706c704..6eefa306e3c89 100644 --- a/clang/docs/ThreadSafetyAnalysis.rst +++ b/clang/docs/ThreadSafetyAnalysis.rst @@ -764,12 +764,6 @@ doesn't know that munl.mu == mutex. The SCOPED_CAPABILITY attribute handles aliasing for MutexLocker, but does so only for that particular pattern. -ACQUIRED_BEFORE(...) and ACQUIRED_AFTER(...) are currently unimplemented. -- - -To be fixed in a future update. - - .. _mutexheader: mutex.h diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index c6f08860f203a..cb424965732b3 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -3633,7 +3633,7 @@ def NoThreadSafetyAnalysis : InheritableAttr { def GuardedBy : InheritableAttr { let Spellings = [GNU<"guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3644,7 +3644,7 @@ def GuardedBy : InheritableAttr { def PtGuardedBy : InheritableAttr { let Spellings = [GNU<"pt_guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3655,7 +3655,7 @@ def PtGuardedBy : InheritableAttr { def AcquiredAfter : InheritableAttr { let Spellings = [GNU<"acquired_after">]; let Args = [VariadicExprArgument<"Args">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3666,7 +3666,7 @@ def AcquiredAfter : InheritableAttr { def AcquiredBefore : InheritableAttr { let Spellings = [GNU<"acquired_before">]; let Args = [VariadicExprArgument<"Args">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index d054b8cf0d240..9e11078382756 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -3128,6 +3128,20 @@ class Parser : public CodeCompletionHandler { IdentifierInfo *ScopeName, SourceLocation ScopeLoc, ParsedAttr::Fo
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
https://github.com/pdherbemont updated https://github.com/llvm/llvm-project/pull/94216 >From 48f2f8c92ca268f1c92c36b77a5a84a6cd921de9 Mon Sep 17 00:00:00 2001 From: Pierre d'Herbemont Date: Wed, 29 May 2024 11:11:03 +0200 Subject: [PATCH] Support [[guarded_by(mutex)]] and friends attributes 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". Then I enabled the use of the experimental late parsing used by [[counted_by]] because that will enable the adoption of attributes without restructuring the data layout as requested by @rapidsna. --- clang/docs/ReleaseNotes.rst | 4 + clang/docs/ThreadSafetyAnalysis.rst | 6 - clang/include/clang/Basic/Attr.td | 8 +- clang/include/clang/Parse/Parser.h| 14 +++ clang/include/clang/Sema/Sema.h | 7 +- clang/lib/Parse/ParseDecl.cpp | 118 +- clang/lib/Sema/SemaExpr.cpp | 4 +- clang/test/AST/ast-dump-color.cpp | 4 +- clang/test/Sema/warn-thread-safety-analysis.c | 20 ++- .../SemaCXX/warn-thread-safety-parsing.cpp| 6 +- 10 files changed, 166 insertions(+), 25 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 99580c0d28a4f..c6bcf25d95dab 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -451,6 +451,10 @@ Attribute Changes in Clang size_t count; }; +- The ``guarded_by``, ``pt_guarded_by``, ``acquired_after``, ``acquired_before`` + attributes now support referencing struct members in C. The arguments are also + now late parsed when ``-fexperimental-late-parse-attributes`` is passed like + for ``counted_by``. Improvements to Clang's diagnostics --- diff --git a/clang/docs/ThreadSafetyAnalysis.rst b/clang/docs/ThreadSafetyAnalysis.rst index dcde0c706c704..6eefa306e3c89 100644 --- a/clang/docs/ThreadSafetyAnalysis.rst +++ b/clang/docs/ThreadSafetyAnalysis.rst @@ -764,12 +764,6 @@ doesn't know that munl.mu == mutex. The SCOPED_CAPABILITY attribute handles aliasing for MutexLocker, but does so only for that particular pattern. -ACQUIRED_BEFORE(...) and ACQUIRED_AFTER(...) are currently unimplemented. -- - -To be fixed in a future update. - - .. _mutexheader: mutex.h diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index c6f08860f203a..cb424965732b3 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -3633,7 +3633,7 @@ def NoThreadSafetyAnalysis : InheritableAttr { def GuardedBy : InheritableAttr { let Spellings = [GNU<"guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3644,7 +3644,7 @@ def GuardedBy : InheritableAttr { def PtGuardedBy : InheritableAttr { let Spellings = [GNU<"pt_guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3655,7 +3655,7 @@ def PtGuardedBy : InheritableAttr { def AcquiredAfter : InheritableAttr { let Spellings = [GNU<"acquired_after">]; let Args = [VariadicExprArgument<"Args">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3666,7 +3666,7 @@ def AcquiredAfter : InheritableAttr { def AcquiredBefore : InheritableAttr { let Spellings = [GNU<"acquired_before">]; let Args = [VariadicExprArgument<"Args">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index d054b8cf0d240..9e11078382756 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -3128,6 +3128,20 @@ class Parser : public CodeCompletionHandler { IdentifierInfo *ScopeName, SourceLocation ScopeLoc, ParsedAttr::Fo
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
pdherbemont wrote: > LGTM assuming you will be fixing the buildkite failures. > > You may still need to wait on @AaronBallman's approval. Fixed the buildkite issue – they were regression from the latest changes about acquired_{before,after} handling. Also pinged @AaronBallman. Thank you all for the feedback! https://github.com/llvm/llvm-project/pull/94216 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
pdherbemont wrote: > LGTM! Do you need someone to land these changes on your behalf? Yes – that would be ideal! Thanks :) https://github.com/llvm/llvm-project/pull/94216 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
https://github.com/pdherbemont updated https://github.com/llvm/llvm-project/pull/94216 >From 55eb00bafdd19817a2c31af6178a301582f5523b Mon Sep 17 00:00:00 2001 From: Pierre d'Herbemont Date: Wed, 29 May 2024 11:11:03 +0200 Subject: [PATCH] Support [[guarded_by(mutex)]] and friends attributes 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". Then I enabled the use of the experimental late parsing used by [[counted_by]] because that will enable the adoption of attributes without restructuring the data layout as requested by @rapidsna. --- clang/docs/ReleaseNotes.rst | 4 + clang/docs/ThreadSafetyAnalysis.rst | 6 - clang/include/clang/Basic/Attr.td | 8 +- clang/include/clang/Parse/Parser.h| 14 +++ clang/include/clang/Sema/Sema.h | 7 +- clang/lib/Parse/ParseDecl.cpp | 118 +- clang/lib/Sema/SemaExpr.cpp | 4 +- clang/test/AST/ast-dump-color.cpp | 4 +- clang/test/Sema/warn-thread-safety-analysis.c | 43 ++- .../SemaCXX/warn-thread-safety-parsing.cpp| 6 +- 10 files changed, 189 insertions(+), 25 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 99580c0d28a4f..c6bcf25d95dab 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -451,6 +451,10 @@ Attribute Changes in Clang size_t count; }; +- The ``guarded_by``, ``pt_guarded_by``, ``acquired_after``, ``acquired_before`` + attributes now support referencing struct members in C. The arguments are also + now late parsed when ``-fexperimental-late-parse-attributes`` is passed like + for ``counted_by``. Improvements to Clang's diagnostics --- diff --git a/clang/docs/ThreadSafetyAnalysis.rst b/clang/docs/ThreadSafetyAnalysis.rst index dcde0c706c704..6eefa306e3c89 100644 --- a/clang/docs/ThreadSafetyAnalysis.rst +++ b/clang/docs/ThreadSafetyAnalysis.rst @@ -764,12 +764,6 @@ doesn't know that munl.mu == mutex. The SCOPED_CAPABILITY attribute handles aliasing for MutexLocker, but does so only for that particular pattern. -ACQUIRED_BEFORE(...) and ACQUIRED_AFTER(...) are currently unimplemented. -- - -To be fixed in a future update. - - .. _mutexheader: mutex.h diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index c6f08860f203a..cb424965732b3 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -3633,7 +3633,7 @@ def NoThreadSafetyAnalysis : InheritableAttr { def GuardedBy : InheritableAttr { let Spellings = [GNU<"guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3644,7 +3644,7 @@ def GuardedBy : InheritableAttr { def PtGuardedBy : InheritableAttr { let Spellings = [GNU<"pt_guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3655,7 +3655,7 @@ def PtGuardedBy : InheritableAttr { def AcquiredAfter : InheritableAttr { let Spellings = [GNU<"acquired_after">]; let Args = [VariadicExprArgument<"Args">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3666,7 +3666,7 @@ def AcquiredAfter : InheritableAttr { def AcquiredBefore : InheritableAttr { let Spellings = [GNU<"acquired_before">]; let Args = [VariadicExprArgument<"Args">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index d054b8cf0d240..9e11078382756 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -3128,6 +3128,20 @@ class Parser : public CodeCompletionHandler { IdentifierInfo *ScopeName, SourceLocation ScopeLoc, ParsedAttr
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
@@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -Wthread-safety-beta %s +// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -Wthread-safety-beta -fexperimental-late-parse-attributes %s pdherbemont wrote: done! https://github.com/llvm/llvm-project/pull/94216 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
@@ -3330,6 +3340,112 @@ 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); pdherbemont wrote: done! https://github.com/llvm/llvm-project/pull/94216 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
@@ -29,6 +29,13 @@ struct LOCKABLE Mutex {}; struct Foo { struct Mutex *mu_; + int a_value GUARDED_BY(mu_); pdherbemont wrote: done! https://github.com/llvm/llvm-project/pull/94216 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
@@ -3330,6 +3340,112 @@ 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_AttrArgument); + + 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 = RParens; + + if (!Tok.is(tok::r_paren)) { +Diag(Tok.getLocation(), diag::err_attribute_wrong_number_arguments) +<< AL << 1; +Parens.skipToEnd(); +return; + } + + Parens.consumeClose(); +} + +/// Acquired attributes (e.g., acquired_before, acquired_after): +/// AttrName '(' expression-list ')' +void Parser::ParseAcquiredAttribute( +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(); pdherbemont wrote: done! https://github.com/llvm/llvm-project/pull/94216 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
@@ -3330,6 +3340,112 @@ 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_AttrArgument); + + ExprResult ArgExpr( + Actions.CorrectDelayedTyposInExpr(ParseAssignmentExpression())); + + if (ArgExpr.isInvalid()) { +Parens.skipToEnd(); pdherbemont wrote: done! https://github.com/llvm/llvm-project/pull/94216 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
@@ -3330,6 +3340,112 @@ 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_AttrArgument); + + 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 = RParens; + + if (!Tok.is(tok::r_paren)) { +Diag(Tok.getLocation(), diag::err_attribute_wrong_number_arguments) +<< AL << 1; +Parens.skipToEnd(); +return; + } + + Parens.consumeClose(); +} + +/// Acquired attributes (e.g., acquired_before, acquired_after): +/// AttrName '(' expression-list ')' +void Parser::ParseAcquiredAttribute( +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; + } + + auto ArgStart = Tok.getLocation(); + + ArgsVector ArgExprs; + + do { + +// Don't evaluate argument when the attribute is ignored. +using ExpressionKind = +Sema::ExpressionEvaluationContextRecord::ExpressionKind; +EnterExpressionEvaluationContext EC( +Actions, Sema::ExpressionEvaluationContext::PotentiallyEvaluated, +nullptr, ExpressionKind::EK_AttrArgument); + +ExprResult ArgExpr( +Actions.CorrectDelayedTyposInExpr(ParseAssignmentExpression())); + +if (ArgExpr.isInvalid()) { + Parens.skipToEnd(); pdherbemont wrote: done! https://github.com/llvm/llvm-project/pull/94216 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
pdherbemont wrote: > @pdherbemont thanks for working on this. It looks pretty good and its great > to see the late parsing support I added gaining new users. I just have some > nits about missing test cases. I should have added all the requested test cases. Not sure it's the way you wanted it, so let me know if you want any change! https://github.com/llvm/llvm-project/pull/94216 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
https://github.com/pdherbemont updated https://github.com/llvm/llvm-project/pull/94216 >From a9da7c1287142025d8a54e569f6c5f8ecb411961 Mon Sep 17 00:00:00 2001 From: Pierre d'Herbemont Date: Wed, 29 May 2024 11:11:03 +0200 Subject: [PATCH] Support [[guarded_by(mutex)]] and friends attributes 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". Then I enabled the use of the experimental late parsing used by [[counted_by]] because that will enable the adoption of attributes without restructuring the data layout as requested by @rapidsna. --- clang/docs/ReleaseNotes.rst | 4 + clang/docs/ThreadSafetyAnalysis.rst | 6 - clang/include/clang/Basic/Attr.td | 8 +- clang/include/clang/Parse/Parser.h| 14 +++ clang/include/clang/Sema/Sema.h | 7 +- clang/lib/Parse/ParseDecl.cpp | 118 +- clang/lib/Sema/SemaExpr.cpp | 4 +- clang/test/AST/ast-dump-color.cpp | 4 +- clang/test/Sema/warn-thread-safety-analysis.c | 68 +- .../SemaCXX/warn-thread-safety-parsing.cpp| 6 +- 10 files changed, 213 insertions(+), 26 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 99580c0d28a4f..c6bcf25d95dab 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -451,6 +451,10 @@ Attribute Changes in Clang size_t count; }; +- The ``guarded_by``, ``pt_guarded_by``, ``acquired_after``, ``acquired_before`` + attributes now support referencing struct members in C. The arguments are also + now late parsed when ``-fexperimental-late-parse-attributes`` is passed like + for ``counted_by``. Improvements to Clang's diagnostics --- diff --git a/clang/docs/ThreadSafetyAnalysis.rst b/clang/docs/ThreadSafetyAnalysis.rst index dcde0c706c704..6eefa306e3c89 100644 --- a/clang/docs/ThreadSafetyAnalysis.rst +++ b/clang/docs/ThreadSafetyAnalysis.rst @@ -764,12 +764,6 @@ doesn't know that munl.mu == mutex. The SCOPED_CAPABILITY attribute handles aliasing for MutexLocker, but does so only for that particular pattern. -ACQUIRED_BEFORE(...) and ACQUIRED_AFTER(...) are currently unimplemented. -- - -To be fixed in a future update. - - .. _mutexheader: mutex.h diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index c6f08860f203a..cb424965732b3 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -3633,7 +3633,7 @@ def NoThreadSafetyAnalysis : InheritableAttr { def GuardedBy : InheritableAttr { let Spellings = [GNU<"guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3644,7 +3644,7 @@ def GuardedBy : InheritableAttr { def PtGuardedBy : InheritableAttr { let Spellings = [GNU<"pt_guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3655,7 +3655,7 @@ def PtGuardedBy : InheritableAttr { def AcquiredAfter : InheritableAttr { let Spellings = [GNU<"acquired_after">]; let Args = [VariadicExprArgument<"Args">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3666,7 +3666,7 @@ def AcquiredAfter : InheritableAttr { def AcquiredBefore : InheritableAttr { let Spellings = [GNU<"acquired_before">]; let Args = [VariadicExprArgument<"Args">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index d054b8cf0d240..9e11078382756 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -3128,6 +3128,20 @@ class Parser : public CodeCompletionHandler { IdentifierInfo *ScopeName, SourceLocation ScopeLoc, ParsedA
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
@@ -74,6 +83,15 @@ int get_value(int *p) SHARED_LOCKS_REQUIRED(foo_.mu_){ void unlock_scope(struct Mutex *const *mu) __attribute__((release_capability(**mu))); +// Verify late parsing: +#ifdef LATE_PARSING +struct LateParsing { + int a_value_defined_before GUARDED_BY(a_mutex_defined_late); + struct Mutex *a_mutex_defined_late ACQUIRED_AFTER(a_mutex_defined_very_late); + struct Mutex *a_mutex_defined_very_late; pdherbemont wrote: Done! https://github.com/llvm/llvm-project/pull/94216 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
pdherbemont wrote: Ah nice catch! Thank you @aeubanks and @delcypher – I am going to try to understand why (and add a new test) https://github.com/llvm/llvm-project/pull/94216 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
pdherbemont wrote: So I found the cause of the regression: We used to parse the argument in an `Unevaluated` context in C++ mode. We now parse them in a `PotentiallyEvaluated` context. Switching to `Unevaluated` makes the C parsing code fail. A simple fix is to switch the context based on the language detected. https://github.com/llvm/llvm-project/pull/94216 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support `guarded_by` attribute and related attributes inside C structs and support late parsing them (PR #95455)
https://github.com/pdherbemont created https://github.com/llvm/llvm-project/pull/95455 This fixes #20777. This replaces #94216 which was reverted after being merged. Previously the `guarded_by`, `pt_guarded_by`, `acquired_after`, and `acquired_before` attributes were only supported inside C++ classes or top level C/C++ declaration. This patch allows these attributes to be added to struct members in C. These attributes have also now support experimental late parsing. This is off by default but can be enabled by passing `-fexperimental-late-parse-attributes`. This is useful for referring to a struct member after the annotated member. E.g. ``` struct Example { int a_value_defined_before __attribute__ ((guarded_by(a_mutex))); struct Mutex *a_mutex; }; ``` >From 97c7912a3d67ed3709530587b8bdd190203df89a Mon Sep 17 00:00:00 2001 From: Pierre d'Herbemont Date: Wed, 12 Jun 2024 20:20:05 +0200 Subject: [PATCH] Support `guarded_by` attribute and related attributes inside C structs and support late parsing them This fixes #20777. This replaces #94216 which was reverted after being merged. Previously the `guarded_by`, `pt_guarded_by`, `acquired_after`, and `acquired_before` attributes were only supported inside C++ classes or top level C/C++ declaration. This patch allows these attributes to be added to struct members in C. These attributes have also now support experimental late parsing. This is off by default but can be enabled by passing `-fexperimental-late-parse-attributes`. This is useful for referring to a struct member after the annotated member. E.g. ``` struct Example { int a_value_defined_before __attribute__ ((guarded_by(a_mutex))); struct Mutex *a_mutex; }; ``` --- clang/docs/ReleaseNotes.rst | 4 + clang/docs/ThreadSafetyAnalysis.rst | 6 - clang/include/clang/Basic/Attr.td | 8 +- clang/include/clang/Parse/Parser.h| 14 ++ clang/include/clang/Sema/Sema.h | 7 +- clang/lib/Parse/ParseDecl.cpp | 124 +- clang/lib/Sema/SemaExpr.cpp | 11 +- clang/test/Sema/warn-thread-safety-analysis.c | 76 ++- .../SemaCXX/warn-thread-safety-analysis.cpp | 30 + .../SemaCXX/warn-thread-safety-parsing.cpp| 6 +- 10 files changed, 260 insertions(+), 26 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 8c2f737836a9d..3e6cc43652ae0 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -471,6 +471,10 @@ Attribute Changes in Clang size_t count; }; +- The ``guarded_by``, ``pt_guarded_by``, ``acquired_after``, ``acquired_before`` + attributes now support referencing struct members in C. The arguments are also + now late parsed when ``-fexperimental-late-parse-attributes`` is passed like + for ``counted_by``. Improvements to Clang's diagnostics --- diff --git a/clang/docs/ThreadSafetyAnalysis.rst b/clang/docs/ThreadSafetyAnalysis.rst index dcde0c706c704..6eefa306e3c89 100644 --- a/clang/docs/ThreadSafetyAnalysis.rst +++ b/clang/docs/ThreadSafetyAnalysis.rst @@ -764,12 +764,6 @@ doesn't know that munl.mu == mutex. The SCOPED_CAPABILITY attribute handles aliasing for MutexLocker, but does so only for that particular pattern. -ACQUIRED_BEFORE(...) and ACQUIRED_AFTER(...) are currently unimplemented. -- - -To be fixed in a future update. - - .. _mutexheader: mutex.h diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index b70b0c8b836a5..6032b934279dd 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -3633,7 +3633,7 @@ def NoThreadSafetyAnalysis : InheritableAttr { def GuardedBy : InheritableAttr { let Spellings = [GNU<"guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3644,7 +3644,7 @@ def GuardedBy : InheritableAttr { def PtGuardedBy : InheritableAttr { let Spellings = [GNU<"pt_guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3655,7 +3655,7 @@ def PtGuardedBy : InheritableAttr { def AcquiredAfter : InheritableAttr { let Spellings = [GNU<"acquired_after">]; let Args = [VariadicExprArgument<"Args">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3666,7 +3666,7 @@ def AcquiredAfter : InheritableAttr {
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
pdherbemont wrote: Follow-up PR #95455 https://github.com/llvm/llvm-project/pull/94216 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support `guarded_by` attribute and related attributes inside C structs and support late parsing them (PR #95455)
https://github.com/pdherbemont updated https://github.com/llvm/llvm-project/pull/95455 >From 783a6b58367d99eb4b2be3ee1d7177419138885b Mon Sep 17 00:00:00 2001 From: Pierre d'Herbemont Date: Wed, 12 Jun 2024 20:20:05 +0200 Subject: [PATCH] Support `guarded_by` attribute and related attributes inside C structs and support late parsing them This fixes #20777. This replaces #94216 which was reverted after being merged. Previously the `guarded_by`, `pt_guarded_by`, `acquired_after`, and `acquired_before` attributes were only supported inside C++ classes or top level C/C++ declaration. This patch allows these attributes to be added to struct members in C. These attributes have also now support experimental late parsing. This is off by default but can be enabled by passing `-fexperimental-late-parse-attributes`. This is useful for referring to a struct member after the annotated member. E.g. ``` struct Example { int a_value_defined_before __attribute__ ((guarded_by(a_mutex))); struct Mutex *a_mutex; }; ``` --- clang/docs/ReleaseNotes.rst | 4 + clang/docs/ThreadSafetyAnalysis.rst | 6 - clang/include/clang/Basic/Attr.td | 8 +- clang/include/clang/Parse/Parser.h| 14 +++ clang/include/clang/Sema/Sema.h | 7 +- clang/lib/Parse/ParseDecl.cpp | 118 +- clang/lib/Sema/SemaExpr.cpp | 11 +- clang/test/Sema/warn-thread-safety-analysis.c | 76 ++- .../SemaCXX/warn-thread-safety-analysis.cpp | 30 + .../SemaCXX/warn-thread-safety-parsing.cpp| 6 +- 10 files changed, 254 insertions(+), 26 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 8c2f737836a9d..3e6cc43652ae0 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -471,6 +471,10 @@ Attribute Changes in Clang size_t count; }; +- The ``guarded_by``, ``pt_guarded_by``, ``acquired_after``, ``acquired_before`` + attributes now support referencing struct members in C. The arguments are also + now late parsed when ``-fexperimental-late-parse-attributes`` is passed like + for ``counted_by``. Improvements to Clang's diagnostics --- diff --git a/clang/docs/ThreadSafetyAnalysis.rst b/clang/docs/ThreadSafetyAnalysis.rst index dcde0c706c704..6eefa306e3c89 100644 --- a/clang/docs/ThreadSafetyAnalysis.rst +++ b/clang/docs/ThreadSafetyAnalysis.rst @@ -764,12 +764,6 @@ doesn't know that munl.mu == mutex. The SCOPED_CAPABILITY attribute handles aliasing for MutexLocker, but does so only for that particular pattern. -ACQUIRED_BEFORE(...) and ACQUIRED_AFTER(...) are currently unimplemented. -- - -To be fixed in a future update. - - .. _mutexheader: mutex.h diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index b70b0c8b836a5..6032b934279dd 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -3633,7 +3633,7 @@ def NoThreadSafetyAnalysis : InheritableAttr { def GuardedBy : InheritableAttr { let Spellings = [GNU<"guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3644,7 +3644,7 @@ def GuardedBy : InheritableAttr { def PtGuardedBy : InheritableAttr { let Spellings = [GNU<"pt_guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3655,7 +3655,7 @@ def PtGuardedBy : InheritableAttr { def AcquiredAfter : InheritableAttr { let Spellings = [GNU<"acquired_after">]; let Args = [VariadicExprArgument<"Args">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3666,7 +3666,7 @@ def AcquiredAfter : InheritableAttr { def AcquiredBefore : InheritableAttr { let Spellings = [GNU<"acquired_before">]; let Args = [VariadicExprArgument<"Args">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index d054b8cf0d240..9e11078382756 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -3128,6 +3128,20 @@ class Parser : public CodeCompletionHandler { IdentifierInfo *ScopeName, SourceLocation Scope
[clang] Support `guarded_by` attribute and related attributes inside C structs and support late parsing them (PR #95455)
https://github.com/pdherbemont updated https://github.com/llvm/llvm-project/pull/95455 >From 8bfc79e42062e1343dbf96faadcd48cb55134079 Mon Sep 17 00:00:00 2001 From: Pierre d'Herbemont Date: Wed, 12 Jun 2024 20:20:05 +0200 Subject: [PATCH] Support `guarded_by` attribute and related attributes inside C structs and support late parsing them This fixes #20777. This replaces #94216 which was reverted after being merged. Previously the `guarded_by`, `pt_guarded_by`, `acquired_after`, and `acquired_before` attributes were only supported inside C++ classes or top level C/C++ declaration. This patch allows these attributes to be added to struct members in C. These attributes have also now support experimental late parsing. This is off by default but can be enabled by passing `-fexperimental-late-parse-attributes`. This is useful for referring to a struct member after the annotated member. E.g. ``` struct Example { int a_value_defined_before __attribute__ ((guarded_by(a_mutex))); struct Mutex *a_mutex; }; ``` --- clang/docs/ReleaseNotes.rst | 4 + clang/docs/ThreadSafetyAnalysis.rst | 6 -- clang/include/clang/Basic/Attr.td | 8 +- clang/include/clang/Sema/Sema.h | 7 +- clang/lib/Parse/ParseDecl.cpp | 15 ++-- clang/lib/Sema/SemaExpr.cpp | 11 ++- clang/test/Sema/warn-thread-safety-analysis.c | 76 ++- .../SemaCXX/warn-thread-safety-analysis.cpp | 30 8 files changed, 130 insertions(+), 27 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 8c2f737836a9d..3e6cc43652ae0 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -471,6 +471,10 @@ Attribute Changes in Clang size_t count; }; +- The ``guarded_by``, ``pt_guarded_by``, ``acquired_after``, ``acquired_before`` + attributes now support referencing struct members in C. The arguments are also + now late parsed when ``-fexperimental-late-parse-attributes`` is passed like + for ``counted_by``. Improvements to Clang's diagnostics --- diff --git a/clang/docs/ThreadSafetyAnalysis.rst b/clang/docs/ThreadSafetyAnalysis.rst index dcde0c706c704..6eefa306e3c89 100644 --- a/clang/docs/ThreadSafetyAnalysis.rst +++ b/clang/docs/ThreadSafetyAnalysis.rst @@ -764,12 +764,6 @@ doesn't know that munl.mu == mutex. The SCOPED_CAPABILITY attribute handles aliasing for MutexLocker, but does so only for that particular pattern. -ACQUIRED_BEFORE(...) and ACQUIRED_AFTER(...) are currently unimplemented. -- - -To be fixed in a future update. - - .. _mutexheader: mutex.h diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index b70b0c8b836a5..6032b934279dd 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -3633,7 +3633,7 @@ def NoThreadSafetyAnalysis : InheritableAttr { def GuardedBy : InheritableAttr { let Spellings = [GNU<"guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3644,7 +3644,7 @@ def GuardedBy : InheritableAttr { def PtGuardedBy : InheritableAttr { let Spellings = [GNU<"pt_guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3655,7 +3655,7 @@ def PtGuardedBy : InheritableAttr { def AcquiredAfter : InheritableAttr { let Spellings = [GNU<"acquired_after">]; let Args = [VariadicExprArgument<"Args">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3666,7 +3666,7 @@ def AcquiredAfter : InheritableAttr { def AcquiredBefore : InheritableAttr { let Spellings = [GNU<"acquired_before">]; let Args = [VariadicExprArgument<"Args">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 4d4579fcfd456..53b5e18905ca7 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -5142,7 +5142,7 @@ class Sema final : public SemaBase { enum ExpressionKind { EK_Decltype, EK_TemplateArgument, - EK_BoundsAttrArgument, + EK_AttrArgument, EK_Other } ExprContext; @@ -5249,10 +5249,9 @@ class Sema final : public SemaBa
[clang] Support `guarded_by` attribute and related attributes inside C structs and support late parsing them (PR #95455)
https://github.com/pdherbemont updated https://github.com/llvm/llvm-project/pull/95455 >From db251415e4e8c110d8842b1242d51d6ec5e839e5 Mon Sep 17 00:00:00 2001 From: Pierre d'Herbemont Date: Wed, 12 Jun 2024 20:20:05 +0200 Subject: [PATCH] Support `guarded_by` attribute and related attributes inside C structs and support late parsing them This fixes #20777. This replaces #94216 which was reverted after being merged. Previously the `guarded_by`, `pt_guarded_by`, `acquired_after`, and `acquired_before` attributes were only supported inside C++ classes or top level C/C++ declaration. This patch allows these attributes to be added to struct members in C. These attributes have also now support experimental late parsing. This is off by default but can be enabled by passing `-fexperimental-late-parse-attributes`. This is useful for referring to a struct member after the annotated member. E.g. ``` struct Example { int a_value_defined_before __attribute__ ((guarded_by(a_mutex))); struct Mutex *a_mutex; }; ``` --- clang/docs/ReleaseNotes.rst | 4 + clang/docs/ThreadSafetyAnalysis.rst | 6 -- clang/include/clang/Basic/Attr.td | 8 +- clang/include/clang/Sema/Sema.h | 7 +- clang/lib/Parse/ParseDecl.cpp | 15 ++-- clang/lib/Sema/SemaExpr.cpp | 11 ++- clang/test/Sema/warn-thread-safety-analysis.c | 76 ++- .../SemaCXX/warn-thread-safety-analysis.cpp | 30 .../SemaCXX/warn-thread-safety-parsing.cpp| 6 +- 9 files changed, 133 insertions(+), 30 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 8c2f737836a9d..3e6cc43652ae0 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -471,6 +471,10 @@ Attribute Changes in Clang size_t count; }; +- The ``guarded_by``, ``pt_guarded_by``, ``acquired_after``, ``acquired_before`` + attributes now support referencing struct members in C. The arguments are also + now late parsed when ``-fexperimental-late-parse-attributes`` is passed like + for ``counted_by``. Improvements to Clang's diagnostics --- diff --git a/clang/docs/ThreadSafetyAnalysis.rst b/clang/docs/ThreadSafetyAnalysis.rst index dcde0c706c704..6eefa306e3c89 100644 --- a/clang/docs/ThreadSafetyAnalysis.rst +++ b/clang/docs/ThreadSafetyAnalysis.rst @@ -764,12 +764,6 @@ doesn't know that munl.mu == mutex. The SCOPED_CAPABILITY attribute handles aliasing for MutexLocker, but does so only for that particular pattern. -ACQUIRED_BEFORE(...) and ACQUIRED_AFTER(...) are currently unimplemented. -- - -To be fixed in a future update. - - .. _mutexheader: mutex.h diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index b70b0c8b836a5..6032b934279dd 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -3633,7 +3633,7 @@ def NoThreadSafetyAnalysis : InheritableAttr { def GuardedBy : InheritableAttr { let Spellings = [GNU<"guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3644,7 +3644,7 @@ def GuardedBy : InheritableAttr { def PtGuardedBy : InheritableAttr { let Spellings = [GNU<"pt_guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3655,7 +3655,7 @@ def PtGuardedBy : InheritableAttr { def AcquiredAfter : InheritableAttr { let Spellings = [GNU<"acquired_after">]; let Args = [VariadicExprArgument<"Args">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3666,7 +3666,7 @@ def AcquiredAfter : InheritableAttr { def AcquiredBefore : InheritableAttr { let Spellings = [GNU<"acquired_before">]; let Args = [VariadicExprArgument<"Args">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 4d4579fcfd456..53b5e18905ca7 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -5142,7 +5142,7 @@ class Sema final : public SemaBase { enum ExpressionKind { EK_Decltype, EK_TemplateArgument, - EK_BoundsAttrArgument, + EK_AttrArgument, EK_Other } ExprContext;
[clang] Support `guarded_by` attribute and related attributes inside C structs and support late parsing them (PR #95455)
@@ -3330,6 +3340,118 @@ 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, + getLangOpts().CPlusPlus + ? Sema::ExpressionEvaluationContext::Unevaluated + : Sema::ExpressionEvaluationContext::PotentiallyEvaluated, pdherbemont wrote: I thought it wasn't working properly with `Unevaluated` but it seems like it actually does! I have iterated on the patch further and simplified a bit by directly passing `EK_AttrArgument` to `EnterExpressionEvaluationContext` from within `ParseAttributeArgsCommon`. This allows me to remove `ParseGuardedAttribute()` and `ParseAcquiredAttribute()` and only rely on `ParseAttributeArgsCommon`. This may have unforeseen side effects on some GNU/Clang/Microsoft attributes, but those are not caught by the test suite... So I am tempted to propose the simpler patch. https://github.com/llvm/llvm-project/pull/95455 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support `guarded_by` attribute and related attributes inside C structs and support late parsing them (PR #95455)
@@ -1,10 +1,11 @@ // RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -Wthread-safety-beta %s +// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -Wthread-safety-beta -fexperimental-late-parse-attributes -DLATE_PARSING %s #define LOCKABLE__attribute__ ((lockable)) #define SCOPED_LOCKABLE __attribute__ ((scoped_lockable)) -#define GUARDED_BY(x) __attribute__ ((guarded_by(x))) +#define GUARDED_BY(...) __attribute__ ((guarded_by(__VA_ARGS__))) pdherbemont wrote: To be able to test passing multiple arguments. On the previous iteration of the patch the attribute was parsed manually and we needed to make sure all cases where covered. https://github.com/llvm/llvm-project/pull/95455 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support `guarded_by` attribute and related attributes inside C structs and support late parsing them (PR #95455)
https://github.com/pdherbemont edited https://github.com/llvm/llvm-project/pull/95455 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support `guarded_by` attribute and related attributes inside C structs and support late parsing them (PR #95455)
https://github.com/pdherbemont edited https://github.com/llvm/llvm-project/pull/95455 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
https://github.com/pdherbemont created https://github.com/llvm/llvm-project/pull/94216 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. >From 7b5c9e566c326a16d192ae478c806cbc Mon Sep 17 00:00:00 2001 From: Pierre d'Herbemont 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. --- clang/include/clang/Parse/Parser.h| 6 ++ clang/lib/Parse/ParseDecl.cpp | 59 +++ clang/test/Sema/warn-thread-safety-analysis.c | 10 +++- .../SemaCXX/warn-thread-safety-parsing.cpp| 6 +- 4 files changed, 75 insertions(+), 6 deletions(-) diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index d054b8cf0d240..308f6ea121eab 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -3128,6 +3128,12 @@ class Parser : public CodeCompletionHandler { IdentifierInfo *ScopeName, SourceLocation ScopeLoc, ParsedAttr::Form Form); + void ParseGuardedByAttribute(IdentifierInfo &AttrName, + SourceLocation AttrNameLoc, + ParsedAttributes &Attrs, + IdentifierInfo *ScopeName, + SourceLocation ScopeLoc, 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..58a3de7210455 100644 --- a/clang/lib/Parse/ParseDecl.cpp +++ b/clang/lib/Parse/ParseDecl.cpp @@ -671,6 +671,14 @@ void Parser::ParseGNUAttributeArgs( ParseBoundsAttribute(*AttrName, AttrNameLoc, Attrs, ScopeName, ScopeLoc, Form); return; + } else if (AttrKind == ParsedAttr::AT_GuardedBy) { +ParseGuardedByAttribute(*AttrName, AttrNameLoc, Attrs, ScopeName, ScopeLoc, +Form); +return; + } else if (AttrKind == ParsedAttr::AT_PtGuardedBy) { +ParseGuardedByAttribute(*AttrName, AttrNameLoc, Attrs, ScopeName, ScopeLoc, +Form); +return; } else if (AttrKind == ParsedAttr::AT_CXXAssume) { ParseCXXAssumeAttributeArg(Attrs, AttrName, AttrNameLoc, EndLoc, Form); return; @@ -3330,6 +3338,57 @@ 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, + 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 &AL = *Attrs.addNew( + &AttrName, SourceRange(AttrNameLoc, Parens.getCloseLocation()), ScopeName, + ScopeLoc, ArgExprs.data(), ArgExprs.size(), Form); + + 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/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_; +
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
https://github.com/pdherbemont updated https://github.com/llvm/llvm-project/pull/94216 >From ccf6699197b608f95deea2b03b1ee87e29fbc8e1 Mon Sep 17 00:00:00 2001 From: Pierre d'Herbemont 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. --- clang/include/clang/Parse/Parser.h| 6 ++ clang/lib/Parse/ParseDecl.cpp | 59 +++ clang/test/Sema/warn-thread-safety-analysis.c | 10 +++- .../SemaCXX/warn-thread-safety-parsing.cpp| 6 +- 4 files changed, 75 insertions(+), 6 deletions(-) diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index d054b8cf0d240..308f6ea121eab 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -3128,6 +3128,12 @@ class Parser : public CodeCompletionHandler { IdentifierInfo *ScopeName, SourceLocation ScopeLoc, ParsedAttr::Form Form); + void ParseGuardedByAttribute(IdentifierInfo &AttrName, + SourceLocation AttrNameLoc, + ParsedAttributes &Attrs, + IdentifierInfo *ScopeName, + SourceLocation ScopeLoc, 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..58a3de7210455 100644 --- a/clang/lib/Parse/ParseDecl.cpp +++ b/clang/lib/Parse/ParseDecl.cpp @@ -671,6 +671,14 @@ void Parser::ParseGNUAttributeArgs( ParseBoundsAttribute(*AttrName, AttrNameLoc, Attrs, ScopeName, ScopeLoc, Form); return; + } else if (AttrKind == ParsedAttr::AT_GuardedBy) { +ParseGuardedByAttribute(*AttrName, AttrNameLoc, Attrs, ScopeName, ScopeLoc, +Form); +return; + } else if (AttrKind == ParsedAttr::AT_PtGuardedBy) { +ParseGuardedByAttribute(*AttrName, AttrNameLoc, Attrs, ScopeName, ScopeLoc, +Form); +return; } else if (AttrKind == ParsedAttr::AT_CXXAssume) { ParseCXXAssumeAttributeArg(Attrs, AttrName, AttrNameLoc, EndLoc, Form); return; @@ -3330,6 +3338,57 @@ 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, + 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 &AL = *Attrs.addNew( + &AttrName, SourceRange(AttrNameLoc, Parens.getCloseLocation()), ScopeName, + ScopeLoc, ArgExprs.data(), ArgExprs.size(), Form); + + 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/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 l
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
https://github.com/pdherbemont updated https://github.com/llvm/llvm-project/pull/94216 >From bf9a2ba20be8271c30fef456536dd058d599b4cc Mon Sep 17 00:00:00 2001 From: Pierre d'Herbemont 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. --- clang/include/clang/Parse/Parser.h| 8 +++ clang/lib/Parse/ParseDecl.cpp | 67 +++ clang/test/Sema/warn-thread-safety-analysis.c | 10 ++- .../SemaCXX/warn-thread-safety-parsing.cpp| 6 +- 4 files changed, 85 insertions(+), 6 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..34dde77bf3035 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::Unevaluated, 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/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
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
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 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.consu
[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)
@@ -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); pdherbemont wrote: I am looking for guidance here: should I introduce a new `ExpressionKind` for `GuardedByAttr`, or simply rename it? https://github.com/llvm/llvm-project/pull/94216 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] thread-safety: Support the new capability-based names for all related attributes. (PR #99919)
pdherbemont wrote: > Thank you for these changes! Can you also add a release note to > `clang/docs/ReleaseNotes.rst` so users know about the new attribute spellings? They are already sort of documented as such – but will update the ReleaseNotes.rst. Does anyone know why the new names were documented but not actually implemented? https://github.com/llvm/llvm-project/pull/99919 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] thread-safety: Support the new capability-based names for all related attributes. (PR #99919)
https://github.com/pdherbemont created https://github.com/llvm/llvm-project/pull/99919 For some reason some attributes weren't renamed in code but were documented with the new names. I am not sure why this was the case and maybe I am missing something. Those are: - scoped_lockable -> scoped_capability - lock_returned -> capability_returned - locks_excluded -> capabilities_excluded This patch makes it so that we support the new name as documented. And change the test to support the old and new name like for other attributes. >From 7454358c9a7a2e3e182175d5cf9d4e3792c81764 Mon Sep 17 00:00:00 2001 From: Pierre d'Herbemont Date: Mon, 22 Jul 2024 16:22:28 +0200 Subject: [PATCH] thread-safety: Support the new capability-based names for all related attributes. For some reason some attributes weren't renamed in code but were documented with the new names. I am not sure why this was the case and maybe I am missing something. Those are: - scoped_lockable -> scoped_capability - lock_returned -> capability_returned - locks_excluded -> capabilities_excluded This patch makes it so that we support the new name as documented. And change the test to support the old and new name like for other attributes. --- clang/docs/ThreadSafetyAnalysis.rst | 6 ++--- .../Analysis/Analyses/ThreadSafetyCommon.h| 2 +- clang/include/clang/Basic/Attr.td | 15 +++- clang/lib/AST/ASTImporter.cpp | 8 +++ clang/lib/Analysis/ThreadSafety.cpp | 12 +- clang/lib/Analysis/ThreadSafetyCommon.cpp | 4 ++-- clang/lib/Sema/SemaDeclAttr.cpp | 20 +--- clang/lib/Sema/SemaDeclCXX.cpp| 4 ++-- ...a-attribute-supported-attributes-list.test | 2 +- clang/test/Parser/cxx11-stmt-attributes.cpp | 4 .../test/SemaCXX/thread-safety-annotations.h | 9 --- clang/unittests/AST/ASTImporterTest.cpp | 24 +-- 12 files changed, 61 insertions(+), 49 deletions(-) diff --git a/clang/docs/ThreadSafetyAnalysis.rst b/clang/docs/ThreadSafetyAnalysis.rst index cc4089b97b492..995d29ffb6ef0 100644 --- a/clang/docs/ThreadSafetyAnalysis.rst +++ b/clang/docs/ThreadSafetyAnalysis.rst @@ -801,7 +801,7 @@ implementation. THREAD_ANNOTATION_ATTRIBUTE__(capability(x)) #define SCOPED_CAPABILITY \ -THREAD_ANNOTATION_ATTRIBUTE__(scoped_lockable) +THREAD_ANNOTATION_ATTRIBUTE__(scoped_capability) #define GUARDED_BY(x) \ THREAD_ANNOTATION_ATTRIBUTE__(guarded_by(x)) @@ -843,7 +843,7 @@ implementation. THREAD_ANNOTATION_ATTRIBUTE__(try_acquire_shared_capability(__VA_ARGS__)) #define EXCLUDES(...) \ -THREAD_ANNOTATION_ATTRIBUTE__(locks_excluded(__VA_ARGS__)) +THREAD_ANNOTATION_ATTRIBUTE__(capabilities_excluded(__VA_ARGS__)) #define ASSERT_CAPABILITY(x) \ THREAD_ANNOTATION_ATTRIBUTE__(assert_capability(x)) @@ -852,7 +852,7 @@ implementation. THREAD_ANNOTATION_ATTRIBUTE__(assert_shared_capability(x)) #define RETURN_CAPABILITY(x) \ -THREAD_ANNOTATION_ATTRIBUTE__(lock_returned(x)) +THREAD_ANNOTATION_ATTRIBUTE__(capability_returned(x)) #define NO_THREAD_SAFETY_ANALYSIS \ THREAD_ANNOTATION_ATTRIBUTE__(no_thread_safety_analysis) diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h index e99c5b2466334..b76188cbe7b74 100644 --- a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h +++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h @@ -346,7 +346,7 @@ class SExprBuilder { /// the appropriate mutex expression in the lexical context where the function /// is called. PrevCtx holds the context in which the arguments themselves /// should be evaluated; multiple calling contexts can be chained together - /// by the lock_returned attribute. + /// by the capability_returned attribute. struct CallingContext { // The previous context; or 0 if none. CallingContext *Prev; diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 4825979a974d2..cfa51d3ebca3d 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -3583,8 +3583,9 @@ def Lockable : InheritableAttr { let ASTNode = 0; // Replaced by Capability } -def ScopedLockable : InheritableAttr { - let Spellings = [Clang<"scoped_lockable", 0>]; +def ScopedCapability : InheritableAttr { + let Spellings = [Clang<"scoped_capability", 0>, + Clang<"scoped_lockable", 0>]; let Subjects = SubjectList<[Record]>; let Documentation = [Undocumented]; let SimpleHandler = 1; @@ -3779,8 +3780,9 @@ def SharedTrylockFunction : InheritableAttr { let Documentation = [Undocumented]; } -def LockReturned : InheritableAttr { - let Spellings = [GNU<"lock_returned">]; +def CapabilityReturned : InheritableAttr { + let Spellings = [GNU<"capability_returned">, + GNU<"lock_returned">];
[clang] Support `guarded_by` attribute and related attributes inside C structs and support late parsing them (PR #95455)
https://github.com/pdherbemont updated https://github.com/llvm/llvm-project/pull/95455 >From 4fc74f23410c954d83d5e99fc20b78a4e1044834 Mon Sep 17 00:00:00 2001 From: Pierre d'Herbemont Date: Wed, 12 Jun 2024 20:20:05 +0200 Subject: [PATCH] Support `guarded_by` attribute and related attributes inside C structs and support late parsing them This fixes #20777. This replaces #94216 which was reverted after being merged. Previously the `guarded_by`, `pt_guarded_by`, `acquired_after`, and `acquired_before` attributes were only supported inside C++ classes or top level C/C++ declaration. This patch allows these attributes to be added to struct members in C. These attributes have also now support experimental late parsing. This is off by default but can be enabled by passing `-fexperimental-late-parse-attributes`. This is useful for referring to a struct member after the annotated member. E.g. ``` struct Example { int a_value_defined_before __attribute__ ((guarded_by(a_mutex))); struct Mutex *a_mutex; }; ``` --- clang/docs/ReleaseNotes.rst | 4 + clang/docs/ThreadSafetyAnalysis.rst | 7 +- clang/include/clang/Basic/Attr.td | 8 +- clang/include/clang/Sema/Sema.h | 7 +- clang/lib/Parse/ParseDecl.cpp | 15 ++-- clang/lib/Sema/SemaExpr.cpp | 11 ++- clang/test/Sema/warn-thread-safety-analysis.c | 76 ++- .../SemaCXX/warn-thread-safety-analysis.cpp | 30 8 files changed, 134 insertions(+), 24 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 8c2f737836a9d..3e6cc43652ae0 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -471,6 +471,10 @@ Attribute Changes in Clang size_t count; }; +- The ``guarded_by``, ``pt_guarded_by``, ``acquired_after``, ``acquired_before`` + attributes now support referencing struct members in C. The arguments are also + now late parsed when ``-fexperimental-late-parse-attributes`` is passed like + for ``counted_by``. Improvements to Clang's diagnostics --- diff --git a/clang/docs/ThreadSafetyAnalysis.rst b/clang/docs/ThreadSafetyAnalysis.rst index dcde0c706c704..cc4089b97b492 100644 --- a/clang/docs/ThreadSafetyAnalysis.rst +++ b/clang/docs/ThreadSafetyAnalysis.rst @@ -764,10 +764,11 @@ doesn't know that munl.mu == mutex. The SCOPED_CAPABILITY attribute handles aliasing for MutexLocker, but does so only for that particular pattern. -ACQUIRED_BEFORE(...) and ACQUIRED_AFTER(...) are currently unimplemented. -- +ACQUIRED_BEFORE(...) and ACQUIRED_AFTER(...) support is still experimental. +--- -To be fixed in a future update. +ACQUIRED_BEFORE(...) and ACQUIRED_AFTER(...) are currently being developed under +the ``-Wthread-safety-beta`` flag. .. _mutexheader: diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index b70b0c8b836a5..6032b934279dd 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -3633,7 +3633,7 @@ def NoThreadSafetyAnalysis : InheritableAttr { def GuardedBy : InheritableAttr { let Spellings = [GNU<"guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3644,7 +3644,7 @@ def GuardedBy : InheritableAttr { def PtGuardedBy : InheritableAttr { let Spellings = [GNU<"pt_guarded_by">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3655,7 +3655,7 @@ def PtGuardedBy : InheritableAttr { def AcquiredAfter : InheritableAttr { let Spellings = [GNU<"acquired_after">]; let Args = [VariadicExprArgument<"Args">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -3666,7 +3666,7 @@ def AcquiredAfter : InheritableAttr { def AcquiredBefore : InheritableAttr { let Spellings = [GNU<"acquired_before">]; let Args = [VariadicExprArgument<"Args">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 4d4579fcfd456..53b5e18905ca7 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -5142,7 +5142,7
[clang] Support `guarded_by` attribute and related attributes inside C structs and support late parsing them (PR #95455)
pdherbemont wrote: > @pdherbemont Thanks for working on this LGTM. Please confirm that the > comments that @aaronpuchert wanted have been added because I couldn't see > them. > > Once you've done that we can get this merged :) Done! https://github.com/llvm/llvm-project/pull/95455 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Support `guarded_by` attribute and related attributes inside C structs and support late parsing them (PR #95455)
pdherbemont wrote: I think this still needs review from @delcypher and @rapidsna https://github.com/llvm/llvm-project/pull/95455 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits