[clang] Support `guarded_by` attribute and related attributes inside C structs and support late parsing them (PR #95455)
Pierre d'Herbemont via cfe-commits
cfe-commits at lists.llvm.org
Fri Jun 14 08:08:54 PDT 2024
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 <pdherbemont at apple.com>
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;
@@ -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
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index c528917437332..4e484efc2edfe 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -565,7 +565,9 @@ unsigned Parser::ParseAttributeArgsCommon(
EnterExpressionEvaluationContext Unevaluated(
Actions,
Uneval ? Sema::ExpressionEvaluationContext::Unevaluated
- : Sema::ExpressionEvaluationContext::ConstantEvaluated);
+ : Sema::ExpressionEvaluationContext::ConstantEvaluated,
+ nullptr,
+ Sema::ExpressionEvaluationContextRecord::EK_AttrArgument);
ExprResult ArgExpr(
Actions.CorrectDelayedTyposInExpr(ParseAssignmentExpression()));
@@ -582,9 +584,12 @@ unsigned Parser::ParseAttributeArgsCommon(
// General case. Parse all available expressions.
bool Uneval = attributeParsedArgsUnevaluated(*AttrName);
EnterExpressionEvaluationContext Unevaluated(
- Actions, Uneval
- ? Sema::ExpressionEvaluationContext::Unevaluated
- : Sema::ExpressionEvaluationContext::ConstantEvaluated);
+ Actions,
+ Uneval ? Sema::ExpressionEvaluationContext::Unevaluated
+ : Sema::ExpressionEvaluationContext::ConstantEvaluated,
+ nullptr,
+ Sema::ExpressionEvaluationContextRecord::ExpressionKind::
+ EK_AttrArgument);
ExprVector ParsedExprs;
ParsedAttributeArgumentsProperties ArgProperties =
@@ -3355,7 +3360,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()));
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 99a8704298314..2c0d9d75fcc8c 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/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
diff --git a/clang/test/Sema/warn-thread-safety-analysis.c b/clang/test/Sema/warn-thread-safety-analysis.c
index 642ea88ec3c96..50934f5476157 100644
--- a/clang/test/Sema/warn-thread-safety-analysis.c
+++ b/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;
diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index 73cc946ca0ce1..af9254508d805 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -141,6 +141,36 @@ class MutexWrapper {
void MyLock() EXCLUSIVE_LOCK_FUNCTION(mu);
};
+struct TestingMoreComplexAttributes {
+ Mutex lock;
+ struct { Mutex lock; } strct;
+ union {
+ bool a __attribute__((guarded_by(lock)));
+ bool b __attribute__((guarded_by(strct.lock)));
+ bool *ptr_a __attribute__((pt_guarded_by(lock)));
+ bool *ptr_b __attribute__((pt_guarded_by(strct.lock)));
+ Mutex lock1 __attribute__((acquired_before(lock))) __attribute__((acquired_before(strct.lock)));
+ Mutex lock2 __attribute__((acquired_after(lock))) __attribute__((acquired_after(strct.lock)));
+ };
+} more_complex_atttributes;
+
+void more_complex_attributes() {
+ more_complex_atttributes.a = true; // expected-warning{{writing variable 'a' requires holding mutex 'lock' exclusively}}
+ more_complex_atttributes.b = true; // expected-warning{{writing variable 'b' requires holding mutex 'strct.lock' exclusively}}
+ *more_complex_atttributes.ptr_a = true; // expected-warning{{writing the value pointed to by 'ptr_a' requires holding mutex 'lock' exclusively}}
+ *more_complex_atttributes.ptr_b = true; // expected-warning{{writing the value pointed to by 'ptr_b' requires holding mutex 'strct.lock' exclusively}}
+
+ more_complex_atttributes.lock.Lock();
+ more_complex_atttributes.lock1.Lock(); // expected-warning{{mutex 'lock1' must be acquired before 'lock'}}
+ more_complex_atttributes.lock1.Unlock();
+ more_complex_atttributes.lock.Unlock();
+
+ more_complex_atttributes.lock2.Lock();
+ more_complex_atttributes.lock.Lock(); // expected-warning{{mutex 'lock' must be acquired before 'lock2'}}
+ more_complex_atttributes.lock.Unlock();
+ more_complex_atttributes.lock2.Unlock();
+}
+
MutexWrapper sls_mw;
void sls_fun_0() {
diff --git a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp
index 0c5b0cc85897b..462cd2437814b 100644
--- a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp
@@ -1517,10 +1517,10 @@ class Foo {
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
+ //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
+ #endif
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