[clang] Support [[guarded_by(mutex)]] attribute inside C struct (PR #94216)

Pierre d'Herbemont via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 3 06:22:10 PDT 2024


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 7b5c9e566c0000326a16d1bbbb92ae478c806cbc Mon Sep 17 00:00:00 2001
From: Pierre d'Herbemont <pdherbemont at apple.com>
Date: Wed, 29 May 2024 11:11:03 +0200
Subject: [PATCH] Support [[guarded_by(mutex)]] attribute inside C struct

Today, it's only supported inside C++ classes or top level C/C++
declaration.

I mostly copied and adapted over the code from the
[[counted_by(value)]] lookup.
---
 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 lock/unlock functions.
@@ -136,6 +141,9 @@ int main(void) {
     // Cleanup happens automatically -> no warning.
   }
 
+  foo_.a_value = 0; // expected-warning {{writing variable 'a_value' requires holding mutex 'mu_' exclusively}}
+  *foo_.a_ptr = 1; // expected-warning {{writing the value pointed to by 'a_ptr' requires holding mutex 'bar.other_mu' exclusively}}
+
   return 0;
 }
 
diff --git a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp
index 0c5b0cc85897b..1626e8375892a 100644
--- a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp
@@ -1516,11 +1516,7 @@ class Foo {
   mutable Mutex mu;
   int a GUARDED_BY(mu);
 
-  static int si GUARDED_BY(mu);
-//FIXME: Bug 32066 - Error should be emitted irrespective of C++ dialect
-#if __cplusplus <= 199711L
-  // expected-error at -3 {{invalid use of non-static data member 'mu'}}
-#endif
+  static int si GUARDED_BY(mu); // expected-error {{invalid use of non-static data member 'mu'}}
 
   static void foo() EXCLUSIVE_LOCKS_REQUIRED(mu);
 //FIXME: Bug 32066 - Error should be emitted irrespective of C++ dialect



More information about the cfe-commits mailing list