https://github.com/abhijeetsharma200 updated https://github.com/llvm/llvm-project/pull/211697
>From b18b1bc6f7f51978a191f7482b8a601f69e6dad4 Mon Sep 17 00:00:00 2001 From: Abhijeet Sharma <[email protected]> Date: Fri, 24 Jul 2026 01:44:31 +0200 Subject: [PATCH 1/4] [Clang] Add -Wcounted-by-static-allocation for __counted_by FAMs in fixed-size storage (#206541) --- clang/docs/ReleaseNotes.md | 7 ++ clang/include/clang/Basic/DiagnosticGroups.td | 1 + .../clang/Basic/DiagnosticSemaKinds.td | 9 +++ clang/include/clang/Sema/Sema.h | 12 ++++ clang/lib/Sema/SemaBoundsSafety.cpp | 69 +++++++++++++++++++ clang/lib/Sema/SemaDecl.cpp | 2 + .../Sema/attr-counted-by-static-allocation.c | 63 +++++++++++++++++ 7 files changed, 163 insertions(+) create mode 100644 clang/test/Sema/attr-counted-by-static-allocation.c diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index 639f7b09ae002..7b60c35ae71ea 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -153,6 +153,13 @@ features cannot lower the translation-unit ABI level; - More consistent rendering of Unicode characters in diagnostic messages. +- Added `-Wcounted-by-static-allocation` (off by default) which warns when a + variable with static, thread, or automatic storage duration has a struct type + whose flexible array member is annotated with `__counted_by`. The annotation + promises that the count field describes the size of the trailing array, which + can only be kept when the object is dynamically allocated with a size derived + from the count. (#GH206541) + - Fixed bug in `-Wdocumentation` so that it correctly handles explicit function template instantiations (#64087). diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index b7072634cccf3..5170e86f16dae 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -1870,6 +1870,7 @@ def NoDeref : DiagGroup<"noderef">; // -fbounds-safety and bounds annotation related warnings def BoundsSafetyCountedByEltTyUnknownSize : DiagGroup<"bounds-safety-counted-by-elt-type-unknown-size">; +def CountedByStaticAllocation : DiagGroup<"counted-by-static-allocation">; // A group for cross translation unit static analysis related warnings. def CrossTU : DiagGroup<"ctu">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 89e2f956971b3..6bd79fa2fa6c5 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -7231,6 +7231,15 @@ def warn_counted_by_attr_elt_type_unknown_size : Warning<err_counted_by_attr_pointee_unknown_size.Summary>, InGroup<BoundsSafetyCountedByEltTyUnknownSize>; +def warn_counted_by_attr_fam_static_storage : Warning< + "flexible array member %0 with '%1' attribute in an object with " + "%select{automatic|thread|static}2 storage duration; the count in %3 " + "cannot grow or shrink the fixed-size allocation">, + InGroup<CountedByStaticAllocation>, DefaultIgnore; +def note_counted_by_intended_for_dynamic_allocation : Note< + "'%0' is intended for dynamically allocated objects, where the size of the " + "allocation can be computed from the count">; + // __builtin_counted_by_ref diagnostics: def err_builtin_counted_by_ref_invalid_arg : Error<"'__builtin_counted_by_ref' argument must reference a member with " diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 8a30f6319bcef..63fde69acfd86 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -2558,6 +2558,18 @@ class Sema final : public SemaBase { /// /// \returns True iff no diagnostic where emitted, false otherwise. bool BoundsSafetyCheckUseOfCountAttrPtr(const Expr *E); + + /// Warn when a variable definition gives static, thread, or automatic + /// storage duration to a struct type whose flexible array member has a + /// `counted_by` attribute (-Wcounted-by-static-allocation). The attribute + /// promises that the count field describes the size of the trailing array, + /// which can only be kept when the object is dynamically allocated with a + /// size derived from the count. + /// + /// \param VD The variable declaration to check + /// + /// \returns True iff no diagnostic were emitted, false otherwise. + bool BoundsSafetyCheckCountedByFAMInStaticStorage(const VarDecl *VD); ///@} // diff --git a/clang/lib/Sema/SemaBoundsSafety.cpp b/clang/lib/Sema/SemaBoundsSafety.cpp index 066dab2f0bef2..c1dd859373172 100644 --- a/clang/lib/Sema/SemaBoundsSafety.cpp +++ b/clang/lib/Sema/SemaBoundsSafety.cpp @@ -412,4 +412,73 @@ bool Sema::BoundsSafetyCheckUseOfCountAttrPtr(const Expr *E) { return false; } +bool Sema::BoundsSafetyCheckCountedByFAMInStaticStorage(const VarDecl *VD) { + if (getDiagnostics().isIgnored(diag::warn_counted_by_attr_fam_static_storage, + VD->getLocation())) + return true; + + if (VD->isInvalidDecl()) + return true; + + // Any object with a VarDecl is a fixed-size allocation; only heap objects + // (which have no VarDecl) can honor the count. Diagnose once per object: + // skip non-defining declarations, and all but the first tentative definition. + if (!VD->isThisDeclarationADefinition()) + return true; + for (const VarDecl *Prev = VD->getPreviousDecl(); Prev; + Prev = Prev->getPreviousDecl()) + if (Prev->isThisDeclarationADefinition()) + return true; + + // Arrays of such structs are still fixed-size allocations. + const RecordDecl *RD = + Context.getBaseElementType(VD->getType())->getAsRecordDecl(); + if (!RD || RD->isInvalidDecl() || !RD->hasFlexibleArrayMember()) + return true; + + // Find the last field, descending through anonymous struct/union members. + const FieldDecl *FAM = nullptr; + for (const FieldDecl *FD : RD->fields()) + FAM = FD; + while (FAM) { + const RecordDecl *FieldRD = FAM->getType()->getAsRecordDecl(); + if (!FieldRD || !FieldRD->isAnonymousStructOrUnion()) + break; + FAM = nullptr; + for (const FieldDecl *FD : FieldRD->fields()) + FAM = FD; + } + if (!FAM) + return true; + const auto *CATy = FAM->getType()->getAs<CountAttributedType>(); + if (!CATy) + return true; + const FieldDecl *CountFD = FAM->findCountedByField(); + if (!CountFD) + return true; + + unsigned StorageKind; + switch (VD->getStorageDuration()) { + case SD_Automatic: + StorageKind = 0; + break; + case SD_Thread: + StorageKind = 1; + break; + case SD_Static: + StorageKind = 2; + break; + case SD_FullExpression: + case SD_Dynamic: + llvm_unreachable("variable with full-expression or dynamic storage"); + } + + Diag(VD->getLocation(), diag::warn_counted_by_attr_fam_static_storage) + << FAM << CATy->getAttributeName(/*WithMacroPrefix=*/true) << StorageKind + << CountFD; + Diag(FAM->getLocation(), diag::note_counted_by_intended_for_dynamic_allocation) + << CATy->getAttributeName(/*WithMacroPrefix=*/true); + return false; +} + } // namespace clang diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 7de5542e72559..be693c84f0a70 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -15380,6 +15380,8 @@ void Sema::FinalizeDeclaration(Decl *ThisDecl) { CheckInvalidBuiltinCountedByRef(VD->getInit(), BuiltinCountedByRefKind::Initializer); + BoundsSafetyCheckCountedByFAMInStaticStorage(VD); + checkAttributesAfterMerging(*this, *VD); if (VD->isStaticLocal()) diff --git a/clang/test/Sema/attr-counted-by-static-allocation.c b/clang/test/Sema/attr-counted-by-static-allocation.c new file mode 100644 index 0000000000000..caa89a0abd570 --- /dev/null +++ b/clang/test/Sema/attr-counted-by-static-allocation.c @@ -0,0 +1,63 @@ +// RUN: %clang_cc1 -fsyntax-only -Wcounted-by-static-allocation -verify %s +// RUN: %clang_cc1 -fsyntax-only -verify=quiet %s +// quiet-no-diagnostics + +#define __counted_by(f) __attribute__((counted_by(f))) + +struct flex { + int count; + char fam[] __counted_by(count); // expected-note 6 {{'__counted_by' is intended for dynamically allocated objects, where the size of the allocation can be computed from the count}} +}; + +// Global definition with an initializer. +struct flex global_init = {.count = 10}; // expected-warning {{flexible array member 'fam' with '__counted_by' attribute in an object with static storage duration; the count in 'count' cannot grow or shrink the fixed-size allocation}} + +// Tentative definitions are diagnosed once per object. +struct flex tentative; // expected-warning {{flexible array member 'fam' with '__counted_by' attribute in an object with static storage duration; the count in 'count' cannot grow or shrink the fixed-size allocation}} +struct flex tentative; + +// Thread-local storage. +_Thread_local struct flex tls_var; // expected-warning {{flexible array member 'fam' with '__counted_by' attribute in an object with thread storage duration; the count in 'count' cannot grow or shrink the fixed-size allocation}} + +// Arrays of such structs are still fixed-size allocations. +struct flex array_of_flex[4]; // expected-warning {{flexible array member 'fam' with '__counted_by' attribute in an object with static storage duration; the count in 'count' cannot grow or shrink the fixed-size allocation}} + +// Non-defining declarations are not diagnosed; the definition is. +extern struct flex external_obj; + +// Pointers to such structs are fine; the pointee can be heap-allocated. +struct flex *ptr; +struct flex **ptr_ptr; + +// A flexible array member without a count annotation is not diagnosed. +struct plain_flex { + int count; + char fam[]; +}; +struct plain_flex plain_global = {.count = 10}; + +// Named nested FAM struct (GNU extension): not diagnosed. +struct outer { + int x; + struct flex inner; +}; +struct outer outer_global; + +// FAM reached through a C11 anonymous struct: diagnosed. +struct anon_holder { + int count; + struct { + int pad; + char fam[] __counted_by(count); // expected-note {{'__counted_by' is intended for dynamically allocated objects, where the size of the allocation can be computed from the count}} + }; +}; +struct anon_holder anon_global; // expected-warning {{flexible array member 'fam' with '__counted_by' attribute in an object with static storage duration; the count in 'count' cannot grow or shrink the fixed-size allocation}} + +void func(void) { + struct flex local; // expected-warning {{flexible array member 'fam' with '__counted_by' attribute in an object with automatic storage duration; the count in 'count' cannot grow or shrink the fixed-size allocation}} + static struct flex static_local; // expected-warning {{flexible array member 'fam' with '__counted_by' attribute in an object with static storage duration; the count in 'count' cannot grow or shrink the fixed-size allocation}} + struct flex *local_ptr; + (void)local; + (void)static_local; + (void)local_ptr; +} >From b23580d211984fa7af4d32a56e746106e8312150 Mon Sep 17 00:00:00 2001 From: Abhijeet Sharma <[email protected]> Date: Fri, 24 Jul 2026 02:14:47 +0200 Subject: [PATCH 2/4] Fixed failing linter --- clang/lib/Sema/SemaBoundsSafety.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/lib/Sema/SemaBoundsSafety.cpp b/clang/lib/Sema/SemaBoundsSafety.cpp index c1dd859373172..6eb479ddf7952 100644 --- a/clang/lib/Sema/SemaBoundsSafety.cpp +++ b/clang/lib/Sema/SemaBoundsSafety.cpp @@ -476,7 +476,8 @@ bool Sema::BoundsSafetyCheckCountedByFAMInStaticStorage(const VarDecl *VD) { Diag(VD->getLocation(), diag::warn_counted_by_attr_fam_static_storage) << FAM << CATy->getAttributeName(/*WithMacroPrefix=*/true) << StorageKind << CountFD; - Diag(FAM->getLocation(), diag::note_counted_by_intended_for_dynamic_allocation) + Diag(FAM->getLocation(), + diag::note_counted_by_intended_for_dynamic_allocation) << CATy->getAttributeName(/*WithMacroPrefix=*/true); return false; } >From 9768bbb3c286ce5e7ecf9f5da671ecf27fd04ece Mon Sep 17 00:00:00 2001 From: Abhijeet Sharma <[email protected]> Date: Fri, 24 Jul 2026 13:13:45 +0200 Subject: [PATCH 3/4] Fix review comments --- clang/include/clang/AST/Decl.h | 8 ++++++ clang/lib/AST/Decl.cpp | 21 ++++++++++++++ clang/lib/Sema/SemaBoundsSafety.cpp | 15 ++-------- .../Sema/attr-counted-by-static-allocation.c | 28 +++++++++++++++---- 4 files changed, 54 insertions(+), 18 deletions(-) diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h index 0a6f256afa2cc..92fd2daf28ea4 100644 --- a/clang/include/clang/AST/Decl.h +++ b/clang/include/clang/AST/Decl.h @@ -4407,6 +4407,14 @@ class RecordDecl : public TagDecl { RecordDeclBits.HasFlexibleArrayMember = V; } + /// Find this record's flexible array member, or null if it has none. + /// + /// A struct-typed last field (named or anonymous) is descended into, since + /// its flexible array member then lies at this record's tail. Unions are not + /// descended into, so this may return null even when hasFlexibleArrayMember() + /// is true. + const FieldDecl *findFlexibleArrayMember() const; + /// Whether this is an anonymous struct or union. To be an anonymous /// struct or union, it must have been declared without a name and /// there must be no objects of this type declared, e.g., diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index 4eaef0d87f3e5..b6c83862558f1 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -5413,6 +5413,27 @@ const FieldDecl *RecordDecl::findFirstNamedDataMember() const { return nullptr; } +const FieldDecl *RecordDecl::findFlexibleArrayMember() const { + if (!hasFlexibleArrayMember()) + return nullptr; + + const FieldDecl *Last = nullptr; + for (const FieldDecl *FD : fields()) + Last = FD; + while (Last) { + const RecordDecl *FieldRD = Last->getType()->getAsRecordDecl(); + if (!FieldRD || FieldRD->isUnion()) + break; + Last = nullptr; + for (const FieldDecl *FD : FieldRD->fields()) + Last = FD; + } + + if (Last && Last->getType()->isIncompleteArrayType()) + return Last; + return nullptr; +} + unsigned RecordDecl::getODRHash() { if (hasODRHash()) return RecordDeclBits.ODRHash; diff --git a/clang/lib/Sema/SemaBoundsSafety.cpp b/clang/lib/Sema/SemaBoundsSafety.cpp index 6eb479ddf7952..3bb438e53be67 100644 --- a/clang/lib/Sema/SemaBoundsSafety.cpp +++ b/clang/lib/Sema/SemaBoundsSafety.cpp @@ -433,21 +433,10 @@ bool Sema::BoundsSafetyCheckCountedByFAMInStaticStorage(const VarDecl *VD) { // Arrays of such structs are still fixed-size allocations. const RecordDecl *RD = Context.getBaseElementType(VD->getType())->getAsRecordDecl(); - if (!RD || RD->isInvalidDecl() || !RD->hasFlexibleArrayMember()) + if (!RD || RD->isInvalidDecl()) return true; - // Find the last field, descending through anonymous struct/union members. - const FieldDecl *FAM = nullptr; - for (const FieldDecl *FD : RD->fields()) - FAM = FD; - while (FAM) { - const RecordDecl *FieldRD = FAM->getType()->getAsRecordDecl(); - if (!FieldRD || !FieldRD->isAnonymousStructOrUnion()) - break; - FAM = nullptr; - for (const FieldDecl *FD : FieldRD->fields()) - FAM = FD; - } + const FieldDecl *FAM = RD->findFlexibleArrayMember(); if (!FAM) return true; const auto *CATy = FAM->getType()->getAs<CountAttributedType>(); diff --git a/clang/test/Sema/attr-counted-by-static-allocation.c b/clang/test/Sema/attr-counted-by-static-allocation.c index caa89a0abd570..f76f0fae5bd69 100644 --- a/clang/test/Sema/attr-counted-by-static-allocation.c +++ b/clang/test/Sema/attr-counted-by-static-allocation.c @@ -1,12 +1,12 @@ -// RUN: %clang_cc1 -fsyntax-only -Wcounted-by-static-allocation -verify %s -// RUN: %clang_cc1 -fsyntax-only -verify=quiet %s +// RUN: %clang_cc1 -fsyntax-only -Wcounted-by-static-allocation -Wno-gnu-variable-sized-type-not-at-end -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wno-gnu-variable-sized-type-not-at-end -verify=quiet %s // quiet-no-diagnostics #define __counted_by(f) __attribute__((counted_by(f))) struct flex { int count; - char fam[] __counted_by(count); // expected-note 6 {{'__counted_by' is intended for dynamically allocated objects, where the size of the allocation can be computed from the count}} + char fam[] __counted_by(count); // expected-note 7 {{'__counted_by' is intended for dynamically allocated objects, where the size of the allocation can be computed from the count}} }; // Global definition with an initializer. @@ -36,12 +36,30 @@ struct plain_flex { }; struct plain_flex plain_global = {.count = 10}; -// Named nested FAM struct (GNU extension): not diagnosed. +// Nested struct whose FAM is the last field: diagnosed (the FAM is at the tail). struct outer { int x; struct flex inner; }; -struct outer outer_global; +struct outer outer_global; // expected-warning {{flexible array member 'fam' with '__counted_by' attribute in an object with static storage duration; the count in 'count' cannot grow or shrink the fixed-size allocation}} + +// FAM struct not the last field, so the FAM is not at the tail: not diagnosed. +struct middle_fam { + int x; + struct flex inner; + int foo; +}; +struct middle_fam middle_global; + +// Unions are not descended into (no counted_by in unions): not diagnosed. +struct union_holder { + int count; + union { + struct flex inner; + long long y; + }; +}; +struct union_holder union_global; // FAM reached through a C11 anonymous struct: diagnosed. struct anon_holder { >From 8ab0c366c34d6704df3887723c04b3575521669e Mon Sep 17 00:00:00 2001 From: Abhijeet Sharma <[email protected]> Date: Sun, 26 Jul 2026 22:58:42 +0200 Subject: [PATCH 4/4] Don't search unions for FAM --- clang/include/clang/AST/Decl.h | 6 +++--- clang/lib/AST/Decl.cpp | 2 +- clang/test/Sema/attr-counted-by-static-allocation.c | 13 +++++++++++++ 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h index 92fd2daf28ea4..62adc33495253 100644 --- a/clang/include/clang/AST/Decl.h +++ b/clang/include/clang/AST/Decl.h @@ -4410,9 +4410,9 @@ class RecordDecl : public TagDecl { /// Find this record's flexible array member, or null if it has none. /// /// A struct-typed last field (named or anonymous) is descended into, since - /// its flexible array member then lies at this record's tail. Unions are not - /// descended into, so this may return null even when hasFlexibleArrayMember() - /// is true. + /// its flexible array member then lies at this record's tail. Unions are + /// neither descended into nor searched, so this may return null even when + /// hasFlexibleArrayMember() is true. const FieldDecl *findFlexibleArrayMember() const; /// Whether this is an anonymous struct or union. To be an anonymous diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index b6c83862558f1..eeae7206ba2cc 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -5414,7 +5414,7 @@ const FieldDecl *RecordDecl::findFirstNamedDataMember() const { } const FieldDecl *RecordDecl::findFlexibleArrayMember() const { - if (!hasFlexibleArrayMember()) + if (!hasFlexibleArrayMember() || isUnion()) return nullptr; const FieldDecl *Last = nullptr; diff --git a/clang/test/Sema/attr-counted-by-static-allocation.c b/clang/test/Sema/attr-counted-by-static-allocation.c index f76f0fae5bd69..4a97055e88607 100644 --- a/clang/test/Sema/attr-counted-by-static-allocation.c +++ b/clang/test/Sema/attr-counted-by-static-allocation.c @@ -61,6 +61,19 @@ struct union_holder { }; struct union_holder union_global; +// A union has no tail field, so neither member order is diagnosed. +union union_flex_last { + long long y; + struct flex inner; +}; +union union_flex_last union_last_global; + +union union_flex_first { + struct flex inner; + long long y; +}; +union union_flex_first union_first_global; + // FAM reached through a C11 anonymous struct: diagnosed. struct anon_holder { int count; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
