Skip to content

Commit

Permalink
[Clang] Implement the 'counted_by' attribute (#76348)
Browse files Browse the repository at this point in the history
The 'counted_by' attribute is used on flexible array members. The
argument for the attribute is the name of the field member holding the
count of elements in the flexible array. This information is used to
improve the results of the array bound sanitizer and the
'__builtin_dynamic_object_size' builtin. The 'count' field member must
be within the same non-anonymous, enclosing struct as the flexible array
member. For example:

```
  struct bar;
  struct foo {
    int count;
    struct inner {
      struct {
        int count; /* The 'count' referenced by 'counted_by' */
      };
      struct {
        /* ... */
        struct bar *array[] __attribute__((counted_by(count)));
      };
    } baz;
  };
```

This example specifies 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 throughout 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:

```
  void use_foo(int index, int val) {
    p->count += 42;
    p->array[index] = val; /* The sanitizer can't properly check this access */
  }
```

In this example, an update to 'p->count' maintains the relationship
requirement:

```
  void use_foo(int index, int val) {
    if (p->count == 0)
      return;
    --p->count;
    p->array[index] = val;
  }
```
  • Loading branch information
bwendling committed Jan 16, 2024
1 parent 1b5f72c commit 00b6d03
Show file tree
Hide file tree
Showing 20 changed files with 2,877 additions and 92 deletions.
5 changes: 5 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,11 @@ C Language Changes
- Enums will now be represented in TBAA metadata using their actual underlying
integer type. Previously they were treated as chars, which meant they could
alias with all other types.
- Clang now supports the C-only attribute ``counted_by``. When applied to a
struct's flexible array member, it points to the struct field that holds the
number of elements in the flexible array member. This information can improve
the results of the array bound sanitizer and the
``__builtin_dynamic_object_size`` builtin.

C23 Feature Support
^^^^^^^^^^^^^^^^^^^
Expand Down
10 changes: 10 additions & 0 deletions clang/include/clang/AST/DeclBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "clang/AST/SelectorLocationsKind.h"
#include "clang/Basic/IdentifierTable.h"
#include "clang/Basic/LLVM.h"
#include "clang/Basic/LangOptions.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/Specifiers.h"
#include "llvm/ADT/ArrayRef.h"
Expand Down Expand Up @@ -488,6 +489,15 @@ class alignas(8) Decl {
// Return true if this is a FileContext Decl.
bool isFileContextDecl() const;

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

ASTContext &getASTContext() const LLVM_READONLY;

/// Helper to get the language options from the ASTContext.
Expand Down
18 changes: 18 additions & 0 deletions clang/include/clang/Basic/Attr.td
Original file line number Diff line number Diff line change
Expand Up @@ -4393,3 +4393,21 @@ def CodeAlign: StmtAttr {
static constexpr int MaximumAlignment = 4096;
}];
}

def CountedBy : InheritableAttr {
let Spellings = [Clang<"counted_by">];
let Subjects = SubjectList<[Field]>;
let Args = [IdentifierArgument<"CountedByField">];
let Documentation = [CountedByDocs];
let LangOpts = [COnly];
// FIXME: This is ugly. Let using a DeclArgument would be nice, but a Decl
// isn't yet available due to the fact that we're still parsing the
// structure. Maybe that code could be changed sometime in the future.
code AdditionalMembers = [{
private:
SourceRange CountedByFieldLoc;
public:
SourceRange getCountedByFieldLoc() const { return CountedByFieldLoc; }
void setCountedByFieldLoc(SourceRange Loc) { CountedByFieldLoc = Loc; }
}];
}
78 changes: 78 additions & 0 deletions clang/include/clang/Basic/AttrDocs.td
Original file line number Diff line number Diff line change
Expand Up @@ -7797,3 +7797,81 @@ but do not pass them to the underlying coroutine or pass them by value.
.. _`CRT`: https://clang.llvm.org/docs/AttributeReference.html#coro-return-type
}];
}

def CountedByDocs : Documentation {
let Category = DocCatField;
let Content = [{
Clang supports the ``counted_by`` attribute on the flexible array member of a
structure in C. The argument for the attribute is the name of a field member
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. The ``count`` field member must be
within the same non-anonymous, enclosing struct as the flexible array member.

This example specifies that the flexible array member ``array`` has the number
of elements allocated for it in ``count``:

.. code-block:: c

struct bar;

struct foo {
size_t count;
char other;
struct bar *array[] __attribute__((counted_by(count)));
};

This establishes a relationship between ``array`` and ``count``. Specifically,
``array`` must have at least ``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 example, the allocated array erroneously has fewer elements
than what's specified by ``p->count``. This would result in an out-of-bounds
access not being detected.

.. code-block:: c

#define SIZE_INCR 42

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 + SIZE_INCR;
}

The next example updates ``p->count``, but breaks the relationship requirement
that ``p->array`` must have at least ``p->count`` number of elements available:

.. code-block:: c

#define SIZE_INCR 42

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;
}

void use_foo(int index, int val) {
p->count += SIZE_INCR + 1; /* 'count' is now larger than the number of elements of 'array'. */
p->array[index] = val; /* The sanitizer can't properly check this access. */
}

In this example, an update to ``p->count`` maintains the relationship
requirement:

.. code-block:: c

void use_foo(int index, int val) {
if (p->count == 0)
return;
--p->count;
p->array[index] = val;
}
}];
}
13 changes: 13 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -6447,6 +6447,19 @@ def warn_superclass_variable_sized_type_not_at_end : Warning<
"field %0 can overwrite instance variable %1 with variable sized type %2"
" in superclass %3">, InGroup<ObjCFlexibleArray>;

def err_flexible_array_count_not_in_same_struct : Error<
"'counted_by' field %0 isn't within the same struct as the flexible array">;
def err_counted_by_attr_not_on_flexible_array_member : Error<
"'counted_by' only applies to C99 flexible array members">;
def err_counted_by_attr_refers_to_flexible_array : Error<
"'counted_by' cannot refer to the flexible array %0">;
def err_counted_by_must_be_in_structure : Error<
"field %0 in 'counted_by' not inside structure">;
def err_flexible_array_counted_by_attr_field_not_integer : Error<
"field %0 in 'counted_by' must be a non-boolean integer type">;
def note_flexible_array_counted_by_attr_field : Note<
"field %0 declared here">;

let CategoryName = "ARC Semantic Issue" in {

// ARC-mode diagnostics.
Expand Down
3 changes: 3 additions & 0 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -4799,6 +4799,8 @@ class Sema final {
bool CheckAlwaysInlineAttr(const Stmt *OrigSt, const Stmt *CurSt,
const AttributeCommonInfo &A);

bool CheckCountedByAttr(Scope *Scope, const FieldDecl *FD);

/// Adjust the calling convention of a method to be the ABI default if it
/// wasn't specified explicitly. This handles method types formed from
/// function type typedefs and typename template arguments.
Expand Down Expand Up @@ -5642,6 +5644,7 @@ class Sema final {
CorrectionCandidateCallback &CCC,
TemplateArgumentListInfo *ExplicitTemplateArgs = nullptr,
ArrayRef<Expr *> Args = std::nullopt,
DeclContext *LookupCtx = nullptr,
TypoExpr **Out = nullptr);

DeclResult LookupIvarInObjCMethod(LookupResult &Lookup, Scope *S,
Expand Down
12 changes: 8 additions & 4 deletions clang/include/clang/Sema/TypoCorrection.h
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ class CorrectionCandidateCallback {
public:
static const unsigned InvalidDistance = TypoCorrection::InvalidDistance;

explicit CorrectionCandidateCallback(IdentifierInfo *Typo = nullptr,
explicit CorrectionCandidateCallback(const IdentifierInfo *Typo = nullptr,
NestedNameSpecifier *TypoNNS = nullptr)
: Typo(Typo), TypoNNS(TypoNNS) {}

Expand Down Expand Up @@ -319,7 +319,7 @@ class CorrectionCandidateCallback {
/// this method.
virtual std::unique_ptr<CorrectionCandidateCallback> clone() = 0;

void setTypoName(IdentifierInfo *II) { Typo = II; }
void setTypoName(const IdentifierInfo *II) { Typo = II; }
void setTypoNNS(NestedNameSpecifier *NNS) { TypoNNS = NNS; }

// Flags for context-dependent keywords. WantFunctionLikeCasts is only
Expand All @@ -345,13 +345,13 @@ class CorrectionCandidateCallback {
candidate.getCorrectionSpecifier() == TypoNNS;
}

IdentifierInfo *Typo;
const IdentifierInfo *Typo;
NestedNameSpecifier *TypoNNS;
};

class DefaultFilterCCC final : public CorrectionCandidateCallback {
public:
explicit DefaultFilterCCC(IdentifierInfo *Typo = nullptr,
explicit DefaultFilterCCC(const IdentifierInfo *Typo = nullptr,
NestedNameSpecifier *TypoNNS = nullptr)
: CorrectionCandidateCallback(Typo, TypoNNS) {}

Expand All @@ -365,6 +365,10 @@ class DefaultFilterCCC final : public CorrectionCandidateCallback {
template <class C>
class DeclFilterCCC final : public CorrectionCandidateCallback {
public:
explicit DeclFilterCCC(const IdentifierInfo *Typo = nullptr,
NestedNameSpecifier *TypoNNS = nullptr)
: CorrectionCandidateCallback(Typo, TypoNNS) {}

bool ValidateCandidate(const TypoCorrection &candidate) override {
return candidate.getCorrectionDeclAs<C>();
}
Expand Down
13 changes: 13 additions & 0 deletions clang/lib/AST/ASTImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9044,6 +9044,10 @@ class AttrImporter {
public:
AttrImporter(ASTImporter &I) : Importer(I), NImporter(I) {}

// Useful for accessing the imported attribute.
template <typename T> T *castAttrAs() { return cast<T>(ToAttr); }
template <typename T> const T *castAttrAs() const { return cast<T>(ToAttr); }

// Create an "importer" for an attribute parameter.
// Result of the 'value()' of that object is to be passed to the function
// 'importAttr', in the order that is expected by the attribute class.
Expand Down Expand Up @@ -9257,6 +9261,15 @@ Expected<Attr *> ASTImporter::Import(const Attr *FromAttr) {
From->args_size());
break;
}
case attr::CountedBy: {
AI.cloneAttr(FromAttr);
const auto *CBA = cast<CountedByAttr>(FromAttr);
Expected<SourceRange> SR = Import(CBA->getCountedByFieldLoc()).get();
if (!SR)
return SR.takeError();
AI.castAttrAs<CountedByAttr>()->setCountedByFieldLoc(SR.get());
break;
}

default: {
// The default branch works for attributes that have no arguments to import.
Expand Down
74 changes: 73 additions & 1 deletion clang/lib/AST/DeclBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
#include "clang/AST/Type.h"
#include "clang/Basic/IdentifierTable.h"
#include "clang/Basic/LLVM.h"
#include "clang/Basic/LangOptions.h"
#include "clang/Basic/Module.h"
#include "clang/Basic/ObjCRuntime.h"
#include "clang/Basic/PartialDiagnostic.h"
Expand Down Expand Up @@ -411,6 +410,79 @@ bool Decl::isFileContextDecl() const {
return DC && DC->isFileContext();
}

bool Decl::isFlexibleArrayMemberLike(
ASTContext &Ctx, const Decl *D, QualType Ty,
LangOptions::StrictFlexArraysLevelKind StrictFlexArraysLevel,
bool IgnoreTemplateOrMacroSubstitution) {
// For compatibility with existing code, we treat arrays of length 0 or
// 1 as flexible array members.
const auto *CAT = Ctx.getAsConstantArrayType(Ty);
if (CAT) {
using FAMKind = LangOptions::StrictFlexArraysLevelKind;

llvm::APInt Size = CAT->getSize();
if (StrictFlexArraysLevel == FAMKind::IncompleteOnly)
return false;

// GCC extension, only allowed to represent a FAM.
if (Size.isZero())
return true;

if (StrictFlexArraysLevel == FAMKind::ZeroOrIncomplete && Size.uge(1))
return false;

if (StrictFlexArraysLevel == FAMKind::OneZeroOrIncomplete && Size.uge(2))
return false;
} else if (!Ctx.getAsIncompleteArrayType(Ty)) {
return false;
}

if (const auto *OID = dyn_cast_if_present<ObjCIvarDecl>(D))
return OID->getNextIvar() == nullptr;

const auto *FD = dyn_cast_if_present<FieldDecl>(D);
if (!FD)
return false;

if (CAT) {
// GCC treats an array memeber of a union as an FAM if the size is one or
// zero.
llvm::APInt Size = CAT->getSize();
if (FD->getParent()->isUnion() && (Size.isZero() || Size.isOne()))
return true;
}

// Don't consider sizes resulting from macro expansions or template argument
// substitution to form C89 tail-padded arrays.
if (IgnoreTemplateOrMacroSubstitution) {
TypeSourceInfo *TInfo = FD->getTypeSourceInfo();
while (TInfo) {
TypeLoc TL = TInfo->getTypeLoc();

// Look through typedefs.
if (TypedefTypeLoc TTL = TL.getAsAdjusted<TypedefTypeLoc>()) {
const TypedefNameDecl *TDL = TTL.getTypedefNameDecl();
TInfo = TDL->getTypeSourceInfo();
continue;
}

if (auto CTL = TL.getAs<ConstantArrayTypeLoc>()) {
if (const Expr *SizeExpr =
dyn_cast_if_present<IntegerLiteral>(CTL.getSizeExpr());
!SizeExpr || SizeExpr->getExprLoc().isMacroID())
return false;
}

break;
}
}

// Test that the field is the last in the structure.
RecordDecl::field_iterator FI(
DeclContext::decl_iterator(const_cast<FieldDecl *>(FD)));
return ++FI == FD->getParent()->field_end();
}

TranslationUnitDecl *Decl::getTranslationUnitDecl() {
if (auto *TUD = dyn_cast<TranslationUnitDecl>(this))
return TUD;
Expand Down

0 comments on commit 00b6d03

Please sign in to comment.