https://github.com/vtjnash updated https://github.com/llvm/llvm-project/pull/212615
>From df2e601645b9b1342458abe05da42b5a0ca31266 Mon Sep 17 00:00:00 2001 From: Jameson Nash <[email protected]> Date: Tue, 28 Jul 2026 15:34:53 +0000 Subject: [PATCH 1/3] [clang][ThreadSafety] Late parse capability attributes under the experimental extension guarded_by, pt_guarded_by, acquired_after and acquired_before are marked LateAttrParseExperimentalExt, but requires_capability, acquire_capability, release_capability, assert_capability, try_acquire_capability, locks_excluded and lock_returned are only LateAttrParseStandard. So within one feature -fexperimental-late-parse-attributes extends some attributes and not others, and a requirement cannot name a member declared later in the same record -- including the shape ThreadSafetyAnalysis.rst itself shows: struct Cache { Mutex mu; void (*read)(void) REQUIRES(mu); }; Mark the remaining seven LateAttrParseExperimentalExt so the family agrees. Late parsing alone would regress the opposite case. An attribute on a function pointer declarator routinely names a parameter of the pointee type, void (*unlock)(struct BDev *bdev) UNLOCK_FUNCTION(bdev->lock); which the eager path resolved because the prototype's scope was still open. Late parsing runs at the end of the record, after that scope is popped, so 'bdev' no longer resolves; Sema/warn-thread-safety-analysis.c covers this. It is not a hypothetical shape either: the Linux kernel writes it today in blkdev.h, libata.h, arm_vgic.h and landlock, and it is the only form that stays qualified per instance. So keep the prototype's parameters on the LateParsedAttribute and make them visible again while the arguments are parsed. Two details matter: - The parameters go into the current scope, not a nested one. A sibling member of the enclosing record is found only while the record's scope is innermost, and a callback field's requirement may name either a sibling member or a pointee parameter; entering a scope for the parameters loses the former. - Only for a declarator that is not itself a function. A real function keeps its parameters in scope for its body, so taking them out after the attribute would break the body -- and ActOnReenterFunctionContext already covers that case. Both forms now work, including together in one record. --- clang/docs/ReleaseNotes.md | 18 ++++++ clang/include/clang/Basic/Attr.td | 14 ++--- clang/include/clang/Parse/Parser.h | 5 ++ clang/include/clang/Sema/Sema.h | 9 +++ clang/lib/Parse/ParseCXXInlineMethods.cpp | 14 +++++ clang/lib/Parse/ParseDecl.cpp | 16 ++++++ clang/lib/Sema/SemaDecl.cpp | 28 +++++++--- clang/test/Sema/thread-safety-late-parse.c | 65 ++++++++++++++++++++++ 8 files changed, 154 insertions(+), 15 deletions(-) create mode 100644 clang/test/Sema/thread-safety-late-parse.c diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index e5da258b9950a3..787971ad4a9b21 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -282,6 +282,24 @@ features cannot lower the translation-unit ABI level; - Clang now recognizes the `[[gnu::flag_enum]]` attribute and treats it equivalent to `[[clang::flag_enum]]` +- Under `-fexperimental-late-parse-attributes`, the thread safety capability + attributes (`requires_capability`, `acquire_capability`, + `release_capability`, `assert_capability`, `try_acquire_capability`, + `locks_excluded` and `lock_returned`) are now late parsed, as `guarded_by` + and `pt_guarded_by` already were, so a requirement may name a member declared + later in the same record: + + ```c++ + struct Cache { + void (*read)(void) REQUIRES(mu); // 'mu' declared below + Mutex mu; + }; + ``` + + A requirement naming a parameter of the pointee, such as + `void (*unlock)(struct BDev *bdev) UNLOCK_FUNCTION(bdev->lock)`, keeps + working. Without the flag the forward reference remains an error. + ### Improvements to Clang's diagnostics - `-Wfortify-source` now diagnoses when `strlcat`, `__builtin_strlcat`, `strlcpy`, or diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index b9eb41654a81b0..8604256e5038b3 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -4166,7 +4166,7 @@ def AssertCapability : InheritableAttr { GNU<"assert_exclusive_lock">, GNU<"assert_shared_lock">]; let Subjects = SubjectList<[Function, Var, Field]>; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -4185,7 +4185,7 @@ def AcquireCapability : InheritableAttr { GNU<"exclusive_lock_function">, GNU<"shared_lock_function">]; let Subjects = SubjectList<[Function, Var, Field]>; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -4204,7 +4204,7 @@ def TryAcquireCapability : InheritableAttr { GNU<"exclusive_trylock_function">, GNU<"shared_trylock_function">]; let Subjects = SubjectList<[Function, Var, Field]>; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -4223,7 +4223,7 @@ def ReleaseCapability : InheritableAttr { Clang<"release_generic_capability", 0>, Clang<"unlock_function", 0>]; let Subjects = SubjectList<[Function, Var, Field]>; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -4245,7 +4245,7 @@ def RequiresCapability : InheritableAttr { Clang<"shared_locks_required", 0>]; let Args = [VariadicExprArgument<"Args">]; let AcceptsExprPack = 1; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; @@ -4312,7 +4312,7 @@ def AcquiredBefore : InheritableAttr { def LockReturned : InheritableAttr { let Spellings = [GNU<"lock_returned">]; let Args = [ExprArgument<"Arg">]; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let Subjects = SubjectList<[Function]>; @@ -4323,7 +4323,7 @@ def LocksExcluded : InheritableAttr { let Spellings = [GNU<"locks_excluded">]; let Args = [VariadicExprArgument<"Args">]; let AcceptsExprPack = 1; - let LateParsed = LateAttrParseStandard; + let LateParsed = LateAttrParseExperimentalExt; let TemplateDependent = 1; let ParseArgumentsAsUnevaluated = 1; let InheritEvenIfAlreadyPresent = 1; diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index 6de876b1510977..24f1b239929585 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -203,6 +203,11 @@ struct LateParsedAttribute : public LateParsedDeclaration { SourceLocation AttrNameLoc; SmallVector<Decl *, 2> Decls; + /// Parameters of the prototype the attribute was written on, kept because + /// late parsing runs after their scope is popped and the arguments may name + /// one: 'void (*unlock)(struct BDev *bdev) UNLOCK_FUNCTION(bdev->lock)'. + SmallVector<ParmVarDecl *, 4> ProtoParams; + private: Kind K; diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 5f24adbf9e9cf3..7eaf7bb7974cce 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -4508,6 +4508,15 @@ class Sema final : public SemaBase { /// Push the parameters of D, which must be a function, into scope. void ActOnReenterFunctionContext(Scope *S, Decl *D); + + /// Add \p Params to scope, so a late-parsed attribute can name them. + void ActOnReenterFunctionPrototypeParams(Scope *S, + ArrayRef<ParmVarDecl *> Params); + /// Undo the above. Needed because these go into a scope that outlives the + /// attribute, so unlike ActOnReenterFunctionContext no scope pop removes + /// them. + void ActOnExitFunctionPrototypeParams(Scope *S, + ArrayRef<ParmVarDecl *> Params); void ActOnExitFunctionContext(); /// Add this decl to the scope shadowed decl chains. diff --git a/clang/lib/Parse/ParseCXXInlineMethods.cpp b/clang/lib/Parse/ParseCXXInlineMethods.cpp index 35dbec8dfb2f7c..73a13f5a179dbd 100644 --- a/clang/lib/Parse/ParseCXXInlineMethods.cpp +++ b/clang/lib/Parse/ParseCXXInlineMethods.cpp @@ -744,9 +744,23 @@ void Parser::ParseLexedAttribute(LateParsedAttribute &LPA, bool EnterScope, Actions.ActOnReenterFunctionContext(Actions.CurScope, D); } + // For a function pointer field or variable, the arguments may name a + // parameter of the pointee. Add them to the current scope rather than a + // nested one: while late parsing a record's attributes, a member of that + // record resolves only if the record's scope is innermost, and the + // attribute could name either a member or a parameter. + bool HasProtoParams = !HasFuncScope && !LPA.ProtoParams.empty(); + if (HasProtoParams) + Actions.ActOnReenterFunctionPrototypeParams(Actions.getCurScope(), + LPA.ProtoParams); + ParsedAttributes Parsed = ParseLexedAttributeTokens(LPA); Attrs.takeAllAppendingFrom(Parsed); + if (HasProtoParams) + Actions.ActOnExitFunctionPrototypeParams(Actions.getCurScope(), + LPA.ProtoParams); + if (HasFuncScope) Actions.ActOnExitFunctionContext(); } else { diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp index 5976f5a7ccdea0..b4089a197d33e1 100644 --- a/clang/lib/Parse/ParseDecl.cpp +++ b/clang/lib/Parse/ParseDecl.cpp @@ -195,6 +195,22 @@ bool Parser::ParseSingleGNUAttribute(ParsedAttributes &Attrs, // Handle attributes with arguments that require late parsing. LateParsedAttribute *LA = new LateParsedAttribute(this, *AttrName, AttrNameLoc); + + // Keep the innermost prototype's parameters available in case they are needed + // by late-parsing attributes. + if (D && !D->isFunctionDeclarator()) { + for (unsigned I = 0, E = D->getNumTypeObjects(); I != E; ++I) { + const DeclaratorChunk &Chunk = D->getTypeObject(I); + if (Chunk.Kind != DeclaratorChunk::Function) + continue; + const DeclaratorChunk::FunctionTypeInfo &FTI = Chunk.Fun; + for (unsigned P = 0, NumParams = FTI.NumParams; P != NumParams; ++P) + if (auto *Param = dyn_cast_or_null<ParmVarDecl>(FTI.Params[P].Param)) + LA->ProtoParams.push_back(Param); + break; + } + } + LateAttrs->push_back(LA); // Attributes in a class are parsed at the end of the class, along diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index db5e66cb96c3ec..457d34bccee2b1 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -1505,6 +1505,25 @@ void Sema::EnterTemplatedContext(Scope *S, DeclContext *DC) { } } +void Sema::ActOnReenterFunctionPrototypeParams(Scope *S, + ArrayRef<ParmVarDecl *> Params) { + for (ParmVarDecl *Param : Params) + if (Param->getIdentifier()) { + S->AddDecl(Param); + IdResolver.AddDecl(Param); + } +} + +void Sema::ActOnExitFunctionPrototypeParams(Scope *S, + ArrayRef<ParmVarDecl *> Params) { + // Remove in reverse so each name is the most recent one when it is removed. + for (ParmVarDecl *Param : llvm::reverse(Params)) + if (Param->getIdentifier()) { + IdResolver.RemoveDecl(Param); + S->RemoveDecl(Param); + } +} + void Sema::ActOnReenterFunctionContext(Scope* S, Decl *D) { // We assume that the caller has already called // ActOnReenterTemplateScope so getTemplatedDecl() works. @@ -1519,14 +1538,7 @@ void Sema::ActOnReenterFunctionContext(Scope* S, Decl *D) { CurContext = FD; S->setEntity(CurContext); - for (unsigned P = 0, NumParams = FD->getNumParams(); P < NumParams; ++P) { - ParmVarDecl *Param = FD->getParamDecl(P); - // If the parameter has an identifier, then add it to the scope - if (Param->getIdentifier()) { - S->AddDecl(Param); - IdResolver.AddDecl(Param); - } - } + ActOnReenterFunctionPrototypeParams(S, FD->parameters()); } void Sema::ActOnExitFunctionContext() { diff --git a/clang/test/Sema/thread-safety-late-parse.c b/clang/test/Sema/thread-safety-late-parse.c new file mode 100644 index 00000000000000..54df3c6e45caa7 --- /dev/null +++ b/clang/test/Sema/thread-safety-late-parse.c @@ -0,0 +1,65 @@ +// Capability attributes are late parsed under -fexperimental-late-parse-attributes, +// like guarded_by and pt_guarded_by already were, so they can name a member +// declared later in the same struct. +// +// RUN: %clang_cc1 -fexperimental-late-parse-attributes -fsyntax-only -verify=late %s +// RUN: %clang_cc1 -fsyntax-only -verify=early %s + +// late-no-diagnostics + +#define REQUIRES(...) __attribute__((requires_capability(__VA_ARGS__))) +#define ACQUIRE(...) __attribute__((acquire_capability(__VA_ARGS__))) +#define RELEASE(...) __attribute__((release_capability(__VA_ARGS__))) +#define ASSERT_CAP(...) __attribute__((assert_capability(__VA_ARGS__))) +#define TRY_ACQUIRE(...) __attribute__((try_acquire_capability(__VA_ARGS__))) +#define EXCLUDES(...) __attribute__((locks_excluded(__VA_ARGS__))) +#define RETURN_CAP(x) __attribute__((lock_returned(x))) +#define GUARDED_BY(x) __attribute__((guarded_by(x))) + +struct __attribute__((capability("mutex"))) Mutex { + int dummy; +}; + +struct Requires { + void (*cb)(void) REQUIRES(mu); // early-error{{use of undeclared identifier 'mu'}} + struct Mutex mu; +}; + +struct Acquire { + void (*cb)(void) ACQUIRE(mu); // early-error{{use of undeclared identifier 'mu'}} + struct Mutex mu; +}; + +struct Release { + void (*cb)(void) RELEASE(mu); // early-error{{use of undeclared identifier 'mu'}} + struct Mutex mu; +}; + +struct Assert { + void (*cb)(void) ASSERT_CAP(mu); // early-error{{use of undeclared identifier 'mu'}} + struct Mutex mu; +}; + +struct TryAcquire { + int (*cb)(void) TRY_ACQUIRE(1, mu); // early-error{{use of undeclared identifier 'mu'}} + struct Mutex mu; +}; + +struct Excludes { + void (*cb)(void) EXCLUDES(mu); // early-error{{use of undeclared identifier 'mu'}} + struct Mutex mu; +}; + +// guarded_by was already late parsed; it is here to show the family now agrees. +struct Guarded { + int data GUARDED_BY(mu); // early-error{{use of undeclared identifier 'mu'}} + struct Mutex mu; +}; + +// An attribute after a complete parameter list already sees those parameters +// without late parsing; this must keep working in both modes. +struct WithGetter { + struct Mutex mu; +}; +struct Mutex *get_mu(struct WithGetter *w) RETURN_CAP(w->mu); +void use_getter(struct WithGetter *w) REQUIRES(get_mu(w)); >From 66c962875af276302690ab27d91ef572b4c5b1f5 Mon Sep 17 00:00:00 2001 From: Jameson Nash <[email protected]> Date: Tue, 28 Jul 2026 16:10:09 +0000 Subject: [PATCH 2/3] [clang][ThreadSafety] Late parse a parameter's attribute to the end of the prototype A capability attribute on a parameter routinely names another parameter of the same prototype, and that parameter may be declared later: int kref_put_lock(struct kref *kref, void (*release)(struct kref *) RELEASE(lock), spinlock_t *lock); Parameter attributes are parsed eagerly, so the forward reference is a hard error -- 'use of undeclared identifier lock' -- and the contract Linux's include/linux/kref.h states in prose ("The @release function will release the lock") cannot be written down at all. Naming an *earlier* parameter already works, so the restriction is purely one of parse order. Give ParseParameterDeclarationClause a LateParsedAttrList and run it after the parameter loop. Unlike the record case, no scope has to be re-entered: the prototype scope is still open there, so every parameter is already visible. Each deferred attribute is bound to its own parameter by DistributeCLateParsedAttrs as that parameter is created, the same way fields do it. The list is experimental-ext-only, so this is gated on -fexperimental-late-parse-attributes and the error is unchanged without it. The requirement is then honored at the indirect call inside the callee, so a release through such a callback transfers the lock state and a second release is reported. --- clang/docs/ReleaseNotes.md | 11 +++++++++- clang/lib/Parse/ParseDecl.cpp | 22 ++++++++++++++++++- clang/test/Sema/thread-safety-late-parse.c | 18 +++++++++++++++ clang/test/Sema/warn-thread-safety-analysis.c | 19 ++++++++++++++++ 4 files changed, 68 insertions(+), 2 deletions(-) diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index 787971ad4a9b21..d6641469feffee 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -296,9 +296,18 @@ features cannot lower the translation-unit ABI level; }; ``` + and a requirement on a parameter may name another parameter of the same + prototype, declared later: + + ```c++ + int kref_put_lock(struct kref *kref, + void (*release)(struct kref *) RELEASE(lock), + spinlock_t *lock); + ``` + A requirement naming a parameter of the pointee, such as `void (*unlock)(struct BDev *bdev) UNLOCK_FUNCTION(bdev->lock)`, keeps - working. Without the flag the forward reference remains an error. + working. Without the flag both forward references remain errors. ### Improvements to Clang's diagnostics diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp index b4089a197d33e1..bec5e30d620b52 100644 --- a/clang/lib/Parse/ParseDecl.cpp +++ b/clang/lib/Parse/ParseDecl.cpp @@ -7604,6 +7604,18 @@ void Parser::ParseParameterDeclarationClause( AllowImplicitTypename = ImplicitTypenameContext::Yes; } + // A capability attribute on a parameter may name another parameter of the + // same prototype, declared later: + // + // int kref_put_lock(struct kref *kref, + // void (*release)(struct kref *) RELEASE(lock), + // spinlock_t *lock); + // + // Defer those to the end of the clause, where every parameter is declared and + // the prototype scope is still open, so no scope need be re-entered. + LateParsedAttrList LateParamAttrs(/*PSoon=*/true, + /*LateAttrParseExperimentalExtOnly=*/true); + do { // FIXME: Issue a diagnostic if we parsed an attribute-specifier-seq // before deciding this was a parameter-declaration-clause. @@ -7661,7 +7673,7 @@ void Parser::ParseParameterDeclarationClause( ParmDeclarator.SetRangeBegin(ThisLoc); // Parse GNU attributes, if present. - MaybeParseGNUAttributes(ParmDeclarator); + MaybeParseGNUAttributes(ParmDeclarator, &LateParamAttrs); if (getLangOpts().HLSL) MaybeParseHLSLAnnotations(DS.getAttributes()); @@ -7741,6 +7753,8 @@ void Parser::ParseParameterDeclarationClause( // added to the current scope. Decl *Param = Actions.ActOnParamDeclarator(getCurScope(), ParmDeclarator, ThisLoc); + // Claim deferred attributes now, before the next parameter can. + DistributeCLateParsedAttrs(Param, &LateParamAttrs); // Parse the default argument, if any. We parse the default // arguments in all dialects; the semantic analysis in // ActOnParamDefaultArgument will reject the default argument in @@ -7853,6 +7867,12 @@ void Parser::ParseParameterDeclarationClause( // If the next token is a comma, consume it and keep reading arguments. } while (TryConsumeToken(tok::comma)); + + // Every parameter is declared and still in scope, so a deferred attribute can + // name any of them. + if (!LateParamAttrs.empty()) + ParseLexedAttributeList(LateParamAttrs, /*D=*/nullptr, /*EnterScope=*/false, + /*OnDefinition=*/false); } void Parser::ParseBracketDeclarator(Declarator &D) { diff --git a/clang/test/Sema/thread-safety-late-parse.c b/clang/test/Sema/thread-safety-late-parse.c index 54df3c6e45caa7..2af02fa7efdc06 100644 --- a/clang/test/Sema/thread-safety-late-parse.c +++ b/clang/test/Sema/thread-safety-late-parse.c @@ -63,3 +63,21 @@ struct WithGetter { }; struct Mutex *get_mu(struct WithGetter *w) RETURN_CAP(w->mu); void use_getter(struct WithGetter *w) REQUIRES(get_mu(w)); + +// An attribute on a parameter may name another parameter of the same prototype, +// including one declared later -- the kref_put_lock() shape. +void put_later(void (*release)(int) RELEASE(mu), // early-error{{use of undeclared identifier 'mu'}} + struct Mutex *mu); + +// Naming an earlier parameter needs no late parsing; it works in both modes. +void put_earlier(struct Mutex *mu, void (*release)(int) RELEASE(mu)); + +// The requirement may also name a member reached through a later parameter. +struct Holder { + struct Mutex mu; +}; +void put_member(void (*release)(int) RELEASE(&h->mu), // early-error{{use of undeclared identifier 'h'}} + struct Holder *h); + +// A parameter of the pointee type is still resolved, in both modes. +void pointee_param(void (*release)(struct Holder *inner) RELEASE(&inner->mu)); diff --git a/clang/test/Sema/warn-thread-safety-analysis.c b/clang/test/Sema/warn-thread-safety-analysis.c index c61152d59a7ddf..d88c736b5f08fa 100644 --- a/clang/test/Sema/warn-thread-safety-analysis.c +++ b/clang/test/Sema/warn-thread-safety-analysis.c @@ -376,6 +376,25 @@ void test_bdev_ops_fail(struct BDevOps *ops, struct BDev *bdev) { ops->unlock(bdev); // expected-warning {{releasing mutex 'bdev->lock' that was not held}} } +#ifdef LATE_PARSING +// A requirement on a parameter may name another parameter declared later. The +// deferred attribute has to end up on the parameter it was written on and stay +// live, so check that the call through it is really honored rather than merely +// accepted: the release consumes the lock, making the second one unheld. +void late_param_cb(void (*release)(struct Mutex *) UNLOCK_FUNCTION(mu), + struct Mutex *mu) EXCLUSIVE_LOCKS_REQUIRED(mu) { // expected-note {{mutex acquired here}} + release(mu); // expected-note {{mutex released here}} + mutex_exclusive_unlock(mu); // expected-warning {{releasing mutex 'mu' that was not held}} +} // expected-warning {{expecting mutex 'mu' to be held at the end of function}} + +// It names 'mu', not the neighbouring parameter: only 'mu' is consumed. +void late_param_other(void (*release)(struct Mutex *) UNLOCK_FUNCTION(mu), + struct Mutex *other, struct Mutex *mu) + EXCLUSIVE_LOCKS_REQUIRED(mu, other) { // expected-note {{mutex acquired here}} + release(mu); +} // expected-warning {{expecting mutex 'mu' to be held at the end of function}} +#endif + // Test unusual trylock patterns void do_some_work(void); int work_data GUARDED_BY(mu1); >From 3bb9d977fb5ae79d81569637bcb3fe81129dc07d Mon Sep 17 00:00:00 2001 From: Jameson Nash <[email protected]> Date: Wed, 23 Sep 2026 15:54:44 +0000 Subject: [PATCH 3/3] [clang][ThreadSafety] Address review comments - Rename the test to warn-thread-safety-late-parsing.c, reword its header so it describes the feature rather than the change, and add the suggested cases: a parameter of function type, one attribute naming both a pointee parameter and a later sibling member, and nested function declarators. - Add SemaCXX/warn-thread-safety-late-parsing.cpp, covering functions, lambdas, member functions, templates and a class member. - Describe the parameter-clause deferral in ParseDecl.cpp without quoting kernel code. The new tests found two bugs in re-entering a pointee's parameters: - In C++, the parameters were added to the class's own scope, where lookup took them for members of the class and the access check asserted. A class field's attributes are late parsed in C++ even without the flag, so 'void (*cb)(Holder *h) REQUIRES(h->mu);' in a class crashed. In C++, enter a nested prototype scope for them instead, as the eager parse had. - A parameter declared with a function type ('void release(struct H *h) RELEASE(&h->mu)') is a function declarator, so its parameters were not kept. It is adjusted to a pointer, so keep them in a prototype context. Co-Authored-By: Claude Opus 5.5 <[email protected]> --- clang/lib/Parse/ParseCXXInlineMethods.cpp | 11 ++++-- clang/lib/Parse/ParseDecl.cpp | 19 ++++------ ...se.c => warn-thread-safety-late-parsing.c} | 27 +++++++++++--- .../warn-thread-safety-late-parsing.cpp | 37 +++++++++++++++++++ 4 files changed, 75 insertions(+), 19 deletions(-) rename clang/test/Sema/{thread-safety-late-parse.c => warn-thread-safety-late-parsing.c} (71%) create mode 100644 clang/test/SemaCXX/warn-thread-safety-late-parsing.cpp diff --git a/clang/lib/Parse/ParseCXXInlineMethods.cpp b/clang/lib/Parse/ParseCXXInlineMethods.cpp index 73a13f5a179dbd..14dd1a9d3ac5c8 100644 --- a/clang/lib/Parse/ParseCXXInlineMethods.cpp +++ b/clang/lib/Parse/ParseCXXInlineMethods.cpp @@ -745,11 +745,16 @@ void Parser::ParseLexedAttribute(LateParsedAttribute &LPA, bool EnterScope, } // For a function pointer field or variable, the arguments may name a - // parameter of the pointee. Add them to the current scope rather than a - // nested one: while late parsing a record's attributes, a member of that + // parameter of the pointee. In C, add them to the current scope rather than + // a nested one: while late parsing a record's attributes, a member of that // record resolves only if the record's scope is innermost, and the - // attribute could name either a member or a parameter. + // attribute could name either a member or a parameter. In C++, members are + // found through the class instead, and a parameter added to the class's own + // scope would be looked up as a member of it, so nest a prototype scope as + // there was when the attribute was written. bool HasProtoParams = !HasFuncScope && !LPA.ProtoParams.empty(); + ParseScope ProtoScope(this, Scope::FunctionPrototypeScope | Scope::DeclScope, + HasProtoParams && IsCPlusPlus); if (HasProtoParams) Actions.ActOnReenterFunctionPrototypeParams(Actions.getCurScope(), LPA.ProtoParams); diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp index bec5e30d620b52..573274c295b2d6 100644 --- a/clang/lib/Parse/ParseDecl.cpp +++ b/clang/lib/Parse/ParseDecl.cpp @@ -197,8 +197,9 @@ bool Parser::ParseSingleGNUAttribute(ParsedAttributes &Attrs, new LateParsedAttribute(this, *AttrName, AttrNameLoc); // Keep the innermost prototype's parameters available in case they are needed - // by late-parsing attributes. - if (D && !D->isFunctionDeclarator()) { + // by late-parsing attributes. A function keeps its own parameters in scope, so + // skip it; a parameter of function type is adjusted to a pointer, so keep it. + if (D && (!D->isFunctionDeclarator() || D->isPrototypeContext())) { for (unsigned I = 0, E = D->getNumTypeObjects(); I != E; ++I) { const DeclaratorChunk &Chunk = D->getTypeObject(I); if (Chunk.Kind != DeclaratorChunk::Function) @@ -7604,15 +7605,11 @@ void Parser::ParseParameterDeclarationClause( AllowImplicitTypename = ImplicitTypenameContext::Yes; } - // A capability attribute on a parameter may name another parameter of the - // same prototype, declared later: - // - // int kref_put_lock(struct kref *kref, - // void (*release)(struct kref *) RELEASE(lock), - // spinlock_t *lock); - // - // Defer those to the end of the clause, where every parameter is declared and - // the prototype scope is still open, so no scope need be re-entered. + // An attribute on a parameter may name another parameter of the same + // prototype that is declared later, such as a callback parameter whose + // attribute names a lock passed after it. Defer those to the end of the + // clause, where every parameter is declared and the prototype scope is still + // open, so no scope need be re-entered. LateParsedAttrList LateParamAttrs(/*PSoon=*/true, /*LateAttrParseExperimentalExtOnly=*/true); diff --git a/clang/test/Sema/thread-safety-late-parse.c b/clang/test/Sema/warn-thread-safety-late-parsing.c similarity index 71% rename from clang/test/Sema/thread-safety-late-parse.c rename to clang/test/Sema/warn-thread-safety-late-parsing.c index 2af02fa7efdc06..1b85885897b64d 100644 --- a/clang/test/Sema/thread-safety-late-parse.c +++ b/clang/test/Sema/warn-thread-safety-late-parsing.c @@ -1,6 +1,7 @@ -// Capability attributes are late parsed under -fexperimental-late-parse-attributes, -// like guarded_by and pt_guarded_by already were, so they can name a member -// declared later in the same struct. +// Under -fexperimental-late-parse-attributes, capability attributes are late +// parsed, so they can name a member declared later in the same struct or a +// parameter declared later in the same prototype. Without it, those forward +// references are errors. // // RUN: %clang_cc1 -fexperimental-late-parse-attributes -fsyntax-only -verify=late %s // RUN: %clang_cc1 -fsyntax-only -verify=early %s @@ -50,7 +51,7 @@ struct Excludes { struct Mutex mu; }; -// guarded_by was already late parsed; it is here to show the family now agrees. +// guarded_by behaves the same as the rest of the family. struct Guarded { int data GUARDED_BY(mu); // early-error{{use of undeclared identifier 'mu'}} struct Mutex mu; @@ -65,7 +66,7 @@ struct Mutex *get_mu(struct WithGetter *w) RETURN_CAP(w->mu); void use_getter(struct WithGetter *w) REQUIRES(get_mu(w)); // An attribute on a parameter may name another parameter of the same prototype, -// including one declared later -- the kref_put_lock() shape. +// including one declared later. void put_later(void (*release)(int) RELEASE(mu), // early-error{{use of undeclared identifier 'mu'}} struct Mutex *mu); @@ -81,3 +82,19 @@ void put_member(void (*release)(int) RELEASE(&h->mu), // early-error{{use of und // A parameter of the pointee type is still resolved, in both modes. void pointee_param(void (*release)(struct Holder *inner) RELEASE(&inner->mu)); + +// A parameter declared with a function type is adjusted to a function pointer; +// both a later parameter and a pointee parameter resolve the same way. +void put_later_decayed(void release(int) RELEASE(mu), // early-error{{use of undeclared identifier 'mu'}} + struct Mutex *mu); +void pointee_decayed(void release(struct Holder *inner) RELEASE(&inner->mu)); + +// One attribute may name both a pointee parameter and a later sibling member. +struct Both { + void (*cb)(struct Mutex *pm) REQUIRES(pm, sm); // early-error{{use of undeclared identifier 'sm'}} + struct Mutex sm; +}; + +// For nested function declarators, the parameters of the innermost prototype +// are the ones in scope, in both modes. +void nested(void (*(*f)(struct Mutex *m))(int) RELEASE(m)); diff --git a/clang/test/SemaCXX/warn-thread-safety-late-parsing.cpp b/clang/test/SemaCXX/warn-thread-safety-late-parsing.cpp new file mode 100644 index 00000000000000..0a5bdac70b8bcf --- /dev/null +++ b/clang/test/SemaCXX/warn-thread-safety-late-parsing.cpp @@ -0,0 +1,37 @@ +// Under -fexperimental-late-parse-attributes, an attribute on a parameter may +// name a parameter declared later in the same prototype. Without it, that +// forward reference is an error. +// +// RUN: %clang_cc1 -fexperimental-late-parse-attributes -fsyntax-only -verify=late -Wthread-safety -std=c++17 %s +// RUN: %clang_cc1 -fsyntax-only -verify=early -Wthread-safety -std=c++17 %s + +// late-no-diagnostics + +#define RELEASE(...) __attribute__((release_capability(__VA_ARGS__))) +#define REQUIRES(...) __attribute__((requires_capability(__VA_ARGS__))) + +class __attribute__((capability("mutex"))) Mutex {}; + +struct Holder { + Mutex lock; +}; + +void put_later(void (*release)(int) RELEASE(mu), // early-error{{use of undeclared identifier 'mu'}} + Mutex *mu); + +// A lambda's parameter clause is parsed the same way. +auto lambda = [](void (*release)(int) RELEASE(mu), // early-error{{use of undeclared identifier 'mu'}} + Mutex *mu) {}; + +struct Methods { + void put_later(void (*release)(int) RELEASE(mu), // early-error{{use of undeclared identifier 'mu'}} + Mutex *mu); + // A class's attributes are late parsed in C++ regardless of the flag; one may + // name a pointee parameter and a member of the class together. + void (*cb)(Holder *h) REQUIRES(h->lock, own); + Mutex own; +}; + +template <typename T> +void put_later_template(void (*release)(T) RELEASE(mu), // early-error{{use of undeclared identifier 'mu'}} + Mutex *mu); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
