[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
Mon Jul 1 07:24:28 PDT 2024
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 <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 | 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 @@ 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() {
More information about the cfe-commits
mailing list