Support guarded_by attribute and related attributes inside C struct…

…s and support late parsing them

This fixes llvm#20777. This replaces llvm#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

@@ -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
 -----------------------------------

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

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;

clang/include/clang/Parse/Parser.h

@@ -3128,6 +3128,20 @@ 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 ParseAcquiredAttribute(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,

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 SemaBase {
     return const_cast<Sema *>(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

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 ||
+             AttrKind == ParsedAttr::AT_PtGuardedBy) {
+    ParseGuardedByAttribute(*AttrName, AttrNameLoc, Attrs, ScopeName, ScopeLoc,
+                            EndLoc, Form);
+    return;
+  } else if (AttrKind == ParsedAttr::AT_AcquiredAfter ||
+             AttrKind == ParsedAttr::AT_AcquiredBefore) {
+    ParseAcquiredAttribute(*AttrName, AttrNameLoc, Attrs, ScopeName, ScopeLoc,
+                           EndLoc, Form);
+    return;
   } else if (AttrKind == ParsedAttr::AT_CXXAssume) {
     ParseCXXAssumeAttributeArg(Attrs, AttrName, AttrNameLoc, EndLoc, Form);
     return;
@@ -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::Unevaluated, 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::Unevaluated, nullptr,
+        ExpressionKind::EK_AttrArgument);
+
+    ExprResult ArgExpr(
+        Actions.CorrectDelayedTyposInExpr(ParseAssignmentExpression()));
+
+    if (ArgExpr.isInvalid()) {
+      Parens.skipToEnd();
+      return;
+    }
+
+    ArgExprs.push_back(ArgExpr.get());
+
+  } while (TryConsumeToken(tok::comma));
+
+  auto ArgEnd = Tok.getLocation();
+
+  Attrs.addNew(&AttrName, SourceRange(ArgStart, ArgEnd), ScopeName, ScopeLoc,
+               ArgExprs.data(), ArgExprs.size(), Form);
+
+  if (EndLoc)
+    *EndLoc = ArgEnd;
+
+  Parens.consumeClose();
+}
+
 /// Bounds attributes (e.g., counted_by):
 ///   AttrName '(' expression ')'
 void Parser::ParseBoundsAttribute(IdentifierInfo &AttrName,
@@ -3355,7 +3471,7 @@ void Parser::ParseBoundsAttribute(IdentifierInfo &AttrName,
       Sema::ExpressionEvaluationContextRecord::ExpressionKind;
   EnterExpressionEvaluationContext EC(
       Actions, Sema::ExpressionEvaluationContext::PotentiallyEvaluated, nullptr,
-      ExpressionKind::EK_BoundsAttrArgument);
+      ExpressionKind::EK_AttrArgument);
 
   ExprResult ArgExpr(
       Actions.CorrectDelayedTyposInExpr(ParseAssignmentExpression()));

clang/lib/Sema/SemaExpr.cpp

@@ -2719,11 +2719,10 @@ Sema::ActOnIdExpression(Scope *S, CXXScopeSpec &SS,
     return ExprError();
   }
 
-  // BoundsSafety: This specially handles arguments of bounds attributes
-  // appertains to a type of C struct field such that the name lookup
-  // within a struct finds the member name, which is not the case for other
-  // contexts in C.
-  if (isBoundsAttrContext() && !getLangOpts().CPlusPlus && S->isClassScope()) {
+  // This specially handles arguments of attributes appertains to a type of C
+  // struct field such that the name lookup within a struct finds the member
+  // name, which is not the case for other contexts in C.
+  if (isAttrContext() && !getLangOpts().CPlusPlus && S->isClassScope()) {
     // See if this is reference to a field of struct.
     LookupResult R(*this, NameInfo, LookupMemberName);
     // LookupName handles a name lookup from within anonymous struct.
@@ -3357,7 +3356,7 @@ ExprResult Sema::BuildDeclarationNameExpr(
   case Decl::Field:
   case Decl::IndirectField:
   case Decl::ObjCIvar:
-    assert((getLangOpts().CPlusPlus || isBoundsAttrContext()) &&
+    assert((getLangOpts().CPlusPlus || isAttrContext()) &&
            "building reference to field in C?");
 
     // These can't have reference type in well-formed programs, but

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

@@ -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__)))
 #define GUARDED_VAR         __attribute__ ((guarded_var))
-#define PT_GUARDED_BY(x)    __attribute__ ((pt_guarded_by(x)))
+#define PT_GUARDED_BY(...)  __attribute__ ((pt_guarded_by(__VA_ARGS__)))
 #define PT_GUARDED_VAR      __attribute__ ((pt_guarded_var))
 #define ACQUIRED_AFTER(...) __attribute__ ((acquired_after(__VA_ARGS__)))
 #define ACQUIRED_BEFORE(...) __attribute__ ((acquired_before(__VA_ARGS__)))
@@ -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_);
+    struct Mutex *third_mu ACQUIRED_BEFORE(other_mu);
+  } bar;
+
+  int* a_ptr PT_GUARDED_BY(bar.other_mu);
+};
+
+struct LOCKABLE Lock {};
+struct A {
+        struct Lock lock;
+        union {
+                int b __attribute__((guarded_by(lock)));
+        };
 };
 
 // Declare mutex lock/unlock functions.
@@ -74,6 +91,19 @@ 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);
+  int *a_ptr_defined_before PT_GUARDED_BY(a_mutex_defined_late);
+  struct Mutex *a_mutex_defined_early
+    ACQUIRED_BEFORE(a_mutex_defined_late);
+  struct Mutex *a_mutex_defined_late
+    ACQUIRED_AFTER(a_mutex_defined_very_late);
+  struct Mutex *a_mutex_defined_very_late;
+} late_parsing;
+#endif
+
 int main(void) {
 
   Foo_fun1(1); // expected-warning{{calling function 'Foo_fun1' requires holding mutex 'mu2'}} \
@@ -136,9 +166,51 @@ 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}}
+
+
+  mutex_exclusive_lock(foo_.bar.other_mu);
+  mutex_exclusive_lock(foo_.bar.third_mu); // expected-warning{{mutex 'third_mu' must be acquired before 'other_mu'}}
+  mutex_exclusive_lock(foo_.mu_); // expected-warning{{mutex 'mu_' must be acquired before 'other_mu'}}
+  mutex_exclusive_unlock(foo_.mu_);
+  mutex_exclusive_unlock(foo_.bar.other_mu);
+  mutex_exclusive_unlock(foo_.bar.third_mu);
+
+#ifdef LATE_PARSING
+  late_parsing.a_value_defined_before = 1; // expected-warning{{writing variable 'a_value_defined_before' requires holding mutex 'a_mutex_defined_late' exclusively}}
+  late_parsing.a_ptr_defined_before = 0;
+  mutex_exclusive_lock(late_parsing.a_mutex_defined_late);
+  mutex_exclusive_lock(late_parsing.a_mutex_defined_early); // expected-warning{{mutex 'a_mutex_defined_early' must be acquired before 'a_mutex_defined_late'}}
+  mutex_exclusive_unlock(late_parsing.a_mutex_defined_early);
+  mutex_exclusive_unlock(late_parsing.a_mutex_defined_late);
+  mutex_exclusive_lock(late_parsing.a_mutex_defined_late);
+  mutex_exclusive_lock(late_parsing.a_mutex_defined_very_late); // expected-warning{{mutex 'a_mutex_defined_very_late' must be acquired before 'a_mutex_defined_late'}}
+  mutex_exclusive_unlock(late_parsing.a_mutex_defined_very_late);
+  mutex_exclusive_unlock(late_parsing.a_mutex_defined_late);
+#endif
+
   return 0;
 }
 
 // We had a problem where we'd skip all attributes that follow a late-parsed
 // attribute in a single __attribute__.
 void run(void) __attribute__((guarded_by(mu1), guarded_by(mu1))); // expected-warning 2{{only applies to non-static data members and global variables}}
+
+int value_with_wrong_number_of_args GUARDED_BY(mu1, mu2); // expected-error{{'guarded_by' attribute takes one argument}}
+
+int *ptr_with_wrong_number_of_args PT_GUARDED_BY(mu1, mu2); // expected-error{{'pt_guarded_by' attribute takes one argument}}
+
+int value_with_no_open_brace __attribute__((guarded_by)); // expected-error{{'guarded_by' attribute takes one argument}}
+int *ptr_with_no_open_brace __attribute__((pt_guarded_by)); // expected-error{{'pt_guarded_by' attribute takes one argument}}
+
+int value_with_no_open_brace_on_acquire_after __attribute__((acquired_after)); // expected-error{{'acquired_after' attribute takes at least 1 argument}}
+int value_with_no_open_brace_on_acquire_before __attribute__((acquired_before)); // expected-error{{'acquired_before' attribute takes at least 1 argument}}
+
+int value_with_bad_expr GUARDED_BY(bad_expr); // expected-error{{use of undeclared identifier 'bad_expr'}}
+int *ptr_with_bad_expr PT_GUARDED_BY(bad_expr); // expected-error{{use of undeclared identifier 'bad_expr'}}
+
+int value_with_bad_expr_on_acquire_after __attribute__((acquired_after(other_bad_expr))); //  expected-error{{use of undeclared identifier 'other_bad_expr'}}
+int value_with_bad_expr_on_acquire_before __attribute__((acquired_before(other_bad_expr))); //  expected-error{{use of undeclared identifier 'other_bad_expr'}}
+
+int a_final_expression = 0;