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(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..73b4e4798f644 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_); // Note: referencing the parent structure is convenient here, but this should probably be disallowed if the child structure is re-used outside of the parent. + 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))); // Note: referencing the parent structure is convenient here, but this should probably be disallowed if the child is re-used outside of the parent. + }; }; // 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() {