Skip to content

Commit 9a954c6

Browse files
bwendlingisanbard
authored andcommitted
[Clang] Implement the 'counted_by' attribute
The 'counted_by' attribute is used on flexible array members. The argument for the attribute is the name of the field member in the same structure holding the count of elements in the flexible array. This information can be used to improve the results of the array bound sanitizer and the '__builtin_dynamic_object_size' builtin. This example specifies the that the flexible array member 'array' has the number of elements allocated for it in 'count': struct bar; struct foo { size_t count; /* ... */ struct bar *array[] __attribute__((counted_by(count))); }; This establishes a relationship between 'array' and 'count', specifically that 'p->array' must have *at least* 'p->count' number of elements available. It's the user's responsibility to ensure that this relationship is maintained through changes to the structure. In the following, the allocated array erroneously has fewer elements than what's specified by 'p->count'. This would result in an out-of-bounds access not not being detected: struct foo *p; void foo_alloc(size_t count) { p = malloc(MAX(sizeof(struct foo), offsetof(struct foo, array[0]) + count * sizeof(struct bar *))); p->count = count + 42; } The next example updates 'p->count', breaking the relationship requirement that 'p->array' must have at least 'p->count' number of elements available: struct foo *p; void foo_alloc(size_t count) { p = malloc(MAX(sizeof(struct foo), offsetof(struct foo, array[0]) + count * sizeof(struct bar *))); p->count = count + 42; } void use_foo(int index) { p->count += 42; p->array[index] = 0; /* The sanitizer cannot properly check this access */ } Reviewed By: nickdesaulniers, aaron.ballman Differential Revision: https://reviews.llvm.org/D148381
1 parent 8fd02d5 commit 9a954c6

18 files changed

+770
-77
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,11 @@ C Language Changes
132132
- ``structs``, ``unions``, and ``arrays`` that are const may now be used as
133133
constant expressions. This change is more consistent with the behavior of
134134
GCC.
135+
- Clang now supports the C-only attribute ``counted_by``. When applied to a
136+
struct's flexible array member, it points to the struct field that holds the
137+
number of elements in the flexible array member. This information can improve
138+
the results of the array bound sanitizer and the
139+
``__builtin_dynamic_object_size`` builtin.
135140

136141
C23 Feature Support
137142
^^^^^^^^^^^^^^^^^^^

clang/include/clang/AST/Decl.h

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4304,6 +4304,30 @@ class RecordDecl : public TagDecl {
43044304
return field_begin() == field_end();
43054305
}
43064306

4307+
FieldDecl *getLastField() {
4308+
FieldDecl *FD = nullptr;
4309+
for (FieldDecl *Field : fields())
4310+
FD = Field;
4311+
return FD;
4312+
}
4313+
const FieldDecl *getLastField() const {
4314+
return const_cast<RecordDecl *>(this)->getLastField();
4315+
}
4316+
4317+
template <typename Functor>
4318+
const FieldDecl *findFieldIf(Functor &Pred) const {
4319+
for (const Decl *D : decls()) {
4320+
if (const auto *FD = dyn_cast<FieldDecl>(D); FD && Pred(FD))
4321+
return FD;
4322+
4323+
if (const auto *RD = dyn_cast<RecordDecl>(D))
4324+
if (const FieldDecl *FD = RD->findFieldIf(Pred))
4325+
return FD;
4326+
}
4327+
4328+
return nullptr;
4329+
}
4330+
43074331
/// Note that the definition of this type is now complete.
43084332
virtual void completeDefinition();
43094333

clang/include/clang/AST/DeclBase.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "clang/AST/DeclarationName.h"
1919
#include "clang/Basic/IdentifierTable.h"
2020
#include "clang/Basic/LLVM.h"
21+
#include "clang/Basic/LangOptions.h"
2122
#include "clang/Basic/SourceLocation.h"
2223
#include "clang/Basic/Specifiers.h"
2324
#include "llvm/ADT/ArrayRef.h"
@@ -477,6 +478,15 @@ class alignas(8) Decl {
477478
// Return true if this is a FileContext Decl.
478479
bool isFileContextDecl() const;
479480

481+
/// Whether it resembles a flexible array member. This is a static member
482+
/// because we want to be able to call it with a nullptr. That allows us to
483+
/// perform non-Decl specific checks based on the object's type and strict
484+
/// flex array level.
485+
static bool isFlexibleArrayMemberLike(
486+
ASTContext &Context, const Decl *D, QualType Ty,
487+
LangOptions::StrictFlexArraysLevelKind StrictFlexArraysLevel,
488+
bool IgnoreTemplateOrMacroSubstitution);
489+
480490
ASTContext &getASTContext() const LLVM_READONLY;
481491

482492
/// Helper to get the language options from the ASTContext.

clang/include/clang/Basic/Attr.td

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4246,3 +4246,21 @@ def AvailableOnlyInDefaultEvalMethod : InheritableAttr {
42464246
let Subjects = SubjectList<[TypedefName], ErrorDiag>;
42474247
let Documentation = [Undocumented];
42484248
}
4249+
4250+
def CountedBy : InheritableAttr {
4251+
let Spellings = [Clang<"counted_by">];
4252+
let Subjects = SubjectList<[Field]>;
4253+
let Args = [IdentifierArgument<"CountedByField">];
4254+
let Documentation = [CountedByDocs];
4255+
let LangOpts = [COnly];
4256+
// FIXME: This is ugly. Let using a DeclArgument would be nice, but a Decl
4257+
// isn't yet available due to the fact that we're still parsing the
4258+
// structure. Maybe that code could be changed sometime in the future.
4259+
code AdditionalMembers = [{
4260+
private:
4261+
SourceRange CountedByFieldLoc;
4262+
public:
4263+
SourceRange getCountedByFieldLoc() const { return CountedByFieldLoc; }
4264+
void setCountedByFieldLoc(SourceRange Loc) { CountedByFieldLoc = Loc; }
4265+
}];
4266+
}

clang/include/clang/Basic/AttrDocs.td

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7275,3 +7275,69 @@ relative ordering of values is important. For example:
72757275
attribute, they default to the value ``65535``.
72767276
}];
72777277
}
7278+
7279+
def CountedByDocs : Documentation {
7280+
let Category = DocCatField;
7281+
let Content = [{
7282+
Clang supports the ``counted_by`` attribute on the flexible array member of a
7283+
structure in C. The argument for the attribute is the name of a field member in
7284+
the same structure holding the count of elements in the flexible array. This
7285+
information can be used to improve the results of the array bound sanitizer and
7286+
the ``__builtin_dynamic_object_size`` builtin.
7287+
7288+
For example, the following code:
7289+
7290+
.. code-block:: c
7291+
7292+
struct bar;
7293+
7294+
struct foo {
7295+
size_t count;
7296+
char other;
7297+
struct bar *array[] __attribute__((counted_by(count)));
7298+
};
7299+
7300+
specifies that the flexible array member ``array`` has the number of elements
7301+
allocated for it stored in ``count``. This establishes a relationship between
7302+
``array`` and ``count``. Specifically, ``p->array`` must have at least
7303+
``p->count`` number of elements available. It's the user's responsibility to
7304+
ensure that this relationship is maintained through changes to the structure.
7305+
7306+
In the following example, the allocated array erroneously has fewer elements
7307+
than what's specified by ``p->count``. This would result in an out-of-bounds
7308+
access not being detected.
7309+
7310+
.. code-block:: c
7311+
7312+
#define SIZE_INCR 42
7313+
7314+
struct foo *p;
7315+
7316+
void foo_alloc(size_t count) {
7317+
p = malloc(MAX(sizeof(struct foo),
7318+
offsetof(struct foo, array[0]) + count * sizeof(struct bar *)));
7319+
p->count = count + SIZE_INCR;
7320+
}
7321+
7322+
The next example updates ``p->count``, breaking the relationship requirement
7323+
that ``p->array`` must have at least ``p->count`` number of elements available:
7324+
7325+
.. code-block:: c
7326+
7327+
#define SIZE_INCR 42
7328+
7329+
struct foo *p;
7330+
7331+
void foo_alloc(size_t count) {
7332+
p = malloc(MAX(sizeof(struct foo),
7333+
offsetof(struct foo, array[0]) + count * sizeof(struct bar *)));
7334+
p->count = count;
7335+
}
7336+
7337+
void use_foo(int index) {
7338+
p->count += SIZE_INCR + 1; /* 'count' is now larger than the number of elements of 'array'. */
7339+
p->array[index] = 0; /* the sanitizer can't properly check if this is an out-of-bounds access. */
7340+
}
7341+
7342+
}];
7343+
}

clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6383,6 +6383,21 @@ def warn_superclass_variable_sized_type_not_at_end : Warning<
63836383
"field %0 can overwrite instance variable %1 with variable sized type %2"
63846384
" in superclass %3">, InGroup<ObjCFlexibleArray>;
63856385

6386+
def err_counted_by_attr_not_on_flexible_array_member : Error<
6387+
"'counted_by' only applies to flexible array members">;
6388+
def err_flexible_array_counted_by_attr_field_not_found : Error<
6389+
"field %0 in 'counted_by' not found">;
6390+
def err_flexible_array_counted_by_attr_field_not_found_suggest : Error<
6391+
"field %0 in 'counted_by' not found; did you mean %1?">;
6392+
def err_flexible_array_counted_by_attr_field_not_found_in_struct : Error<
6393+
"field %0 in 'counted_by' is not found in struct">;
6394+
def err_flexible_array_counted_by_attr_refers_to_self : Error<
6395+
"field %0 in 'counted_by' cannot refer to the flexible array">;
6396+
def err_flexible_array_counted_by_attr_field_not_integer : Error<
6397+
"field %0 in 'counted_by' is not a non-boolean integer type">;
6398+
def note_flexible_array_counted_by_attr_field : Note<
6399+
"field %0 declared here">;
6400+
63866401
let CategoryName = "ARC Semantic Issue" in {
63876402

63886403
// ARC-mode diagnostics.

clang/include/clang/Sema/Sema.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4804,6 +4804,8 @@ class Sema final {
48044804
bool CheckAlwaysInlineAttr(const Stmt *OrigSt, const Stmt *CurSt,
48054805
const AttributeCommonInfo &A);
48064806

4807+
bool CheckCountedByAttr(Scope *Scope, const FieldDecl *FD);
4808+
48074809
/// Adjust the calling convention of a method to be the ABI default if it
48084810
/// wasn't specified explicitly. This handles method types formed from
48094811
/// function type typedefs and typename template arguments.

clang/lib/AST/ASTImporter.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8977,6 +8977,10 @@ class AttrImporter {
89778977
public:
89788978
AttrImporter(ASTImporter &I) : Importer(I), NImporter(I) {}
89798979

8980+
// Useful for accessing the imported attribute.
8981+
template <typename T> T *castAttrAs() { return cast<T>(ToAttr); }
8982+
template <typename T> const T *castAttrAs() const { return cast<T>(ToAttr); }
8983+
89808984
// Create an "importer" for an attribute parameter.
89818985
// Result of the 'value()' of that object is to be passed to the function
89828986
// 'importAttr', in the order that is expected by the attribute class.
@@ -9183,6 +9187,15 @@ Expected<Attr *> ASTImporter::Import(const Attr *FromAttr) {
91839187
From->args_size());
91849188
break;
91859189
}
9190+
case attr::CountedBy: {
9191+
AI.cloneAttr(FromAttr);
9192+
const auto *CBA = cast<CountedByAttr>(FromAttr);
9193+
Expected<SourceRange> SR = Import(CBA->getCountedByFieldLoc()).get();
9194+
if (!SR)
9195+
return SR.takeError();
9196+
AI.castAttrAs<CountedByAttr>()->setCountedByFieldLoc(SR.get());
9197+
break;
9198+
}
91869199

91879200
default: {
91889201
// The default branch works for attributes that have no arguments to import.

clang/lib/AST/DeclBase.cpp

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
#include "clang/AST/Type.h"
3030
#include "clang/Basic/IdentifierTable.h"
3131
#include "clang/Basic/LLVM.h"
32-
#include "clang/Basic/LangOptions.h"
3332
#include "clang/Basic/Module.h"
3433
#include "clang/Basic/ObjCRuntime.h"
3534
#include "clang/Basic/PartialDiagnostic.h"
@@ -411,6 +410,81 @@ bool Decl::isFileContextDecl() const {
411410
return DC && DC->isFileContext();
412411
}
413412

413+
bool Decl::isFlexibleArrayMemberLike(
414+
ASTContext &Ctx, const Decl *D, QualType Ty,
415+
LangOptions::StrictFlexArraysLevelKind StrictFlexArraysLevel,
416+
bool IgnoreTemplateOrMacroSubstitution) {
417+
// For compatibility with existing code, we treat arrays of length 0 or
418+
// 1 as flexible array members.
419+
const auto *CAT = Ctx.getAsConstantArrayType(Ty);
420+
if (CAT) {
421+
using FAMKind = LangOptions::StrictFlexArraysLevelKind;
422+
423+
llvm::APInt Size = CAT->getSize();
424+
FAMKind StrictFlexArraysLevel =
425+
Ctx.getLangOpts().getStrictFlexArraysLevel();
426+
427+
if (StrictFlexArraysLevel == FAMKind::IncompleteOnly)
428+
return false;
429+
430+
// GCC extension, only allowed to represent a FAM.
431+
if (Size.isZero())
432+
return true;
433+
434+
if (StrictFlexArraysLevel == FAMKind::ZeroOrIncomplete && Size.uge(1))
435+
return false;
436+
437+
if (StrictFlexArraysLevel == FAMKind::OneZeroOrIncomplete && Size.uge(2))
438+
return false;
439+
} else if (!Ctx.getAsIncompleteArrayType(Ty)) {
440+
return false;
441+
}
442+
443+
if (const auto *OID = dyn_cast_if_present<ObjCIvarDecl>(D))
444+
return OID->getNextIvar() == nullptr;
445+
446+
const auto *FD = dyn_cast_if_present<FieldDecl>(D);
447+
if (!FD)
448+
return false;
449+
450+
if (CAT) {
451+
// GCC treats an array memeber of a union as an FAM if the size is one or
452+
// zero.
453+
llvm::APInt Size = CAT->getSize();
454+
if (FD->getParent()->isUnion() && (Size.isZero() || Size.isOne()))
455+
return true;
456+
}
457+
458+
// Don't consider sizes resulting from macro expansions or template argument
459+
// substitution to form C89 tail-padded arrays.
460+
if (IgnoreTemplateOrMacroSubstitution) {
461+
TypeSourceInfo *TInfo = FD->getTypeSourceInfo();
462+
while (TInfo) {
463+
TypeLoc TL = TInfo->getTypeLoc();
464+
465+
// Look through typedefs.
466+
if (TypedefTypeLoc TTL = TL.getAsAdjusted<TypedefTypeLoc>()) {
467+
const TypedefNameDecl *TDL = TTL.getTypedefNameDecl();
468+
TInfo = TDL->getTypeSourceInfo();
469+
continue;
470+
}
471+
472+
if (auto CTL = TL.getAs<ConstantArrayTypeLoc>()) {
473+
const Expr *SizeExpr = dyn_cast<IntegerLiteral>(CTL.getSizeExpr());
474+
if (!SizeExpr || SizeExpr->getExprLoc().isMacroID())
475+
return false;
476+
}
477+
478+
break;
479+
}
480+
}
481+
482+
// Test that the field is the last in the structure.
483+
RecordDecl::field_iterator FI(
484+
DeclContext::decl_iterator(const_cast<FieldDecl *>(FD)));
485+
return ++FI == FD->getParent()->field_end();
486+
}
487+
414488
TranslationUnitDecl *Decl::getTranslationUnitDecl() {
415489
if (auto *TUD = dyn_cast<TranslationUnitDecl>(this))
416490
return TUD;

0 commit comments

Comments
 (0)