Author: Kees Cook Date: 2025-12-04T23:18:39-08:00 New Revision: 722026886fb65f0d61e0384886ffc30e83623edf
URL: https://github.com/llvm/llvm-project/commit/722026886fb65f0d61e0384886ffc30e83623edf DIFF: https://github.com/llvm/llvm-project/commit/722026886fb65f0d61e0384886ffc30e83623edf.diff LOG: [Clang] Extend __builtin_counted_by_ref to support pointers with 'counted_by' (#170750) The __builtin_counted_by_ref builtin was previously limited to flexible array members (FAMs). This change extends it to also support pointer members that have the 'counted_by' attribute. The 'counted_by' attribute can be applied to both FAMs and pointer members: struct fam_struct { int count; int array[] __attribute__((counted_by(count))); }; struct ptr_struct { int count; int *buf __attribute__((counted_by(count))); }; With this change, __builtin_counted_by_ref works with both: *__builtin_counted_by_ref(p->array) = size; // FAM - already worked *__builtin_counted_by_ref(p->buf) = size; // pointer - now works This enables the same allocation pattern for pointer members that was previously only available for FAMs: #define alloc_buf(P, MEMBER, COUNT) ({ \ typeof(P) __p = malloc(sizeof(*__p)); \ __p->MEMBER = malloc(sizeof(*__p->MEMBER) * COUNT); \ *_Generic(__builtin_counted_by_ref(__p->MEMBER), \ void *: &(size_t){0}, \ default: __builtin_counted_by_ref(__p->MEMBER)) = COUNT; \ __p; \ }) The builtin returns: - A pointer to the count field if the member has 'counted_by' - void* (null) if the member is an array or pointer without 'counted_by' - An error for other member types (e.g., int, struct) This was requested by upstream Linux kernel devs: https://lore.kernel.org/linux-hardening/202512041215.44484FCACD@keescook/ Added: Modified: clang/docs/LanguageExtensions.rst clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/CodeGen/CGBuiltin.cpp clang/lib/Sema/SemaChecking.cpp clang/test/CodeGen/builtin-counted-by-ref.c clang/test/Sema/builtin-counted-by-ref.c Removed: ################################################################################ diff --git a/clang/docs/LanguageExtensions.rst b/clang/docs/LanguageExtensions.rst index 80cea2166bc83..87d38e7d99e50 100644 --- a/clang/docs/LanguageExtensions.rst +++ b/clang/docs/LanguageExtensions.rst @@ -4313,9 +4313,9 @@ as ``unsigned __int128`` and C23 ``unsigned _BitInt(N)``. ``__builtin_counted_by_ref`` returns a pointer to the count field from the ``counted_by`` attribute. -The argument must be a flexible array member. If the argument isn't a flexible -array member or doesn't have the ``counted_by`` attribute, the builtin returns -``(void *)0``. +The argument must be a flexible array member or a pointer with the ``counted_by`` +attribute. If the argument doesn't have the ``counted_by`` attribute, the builtin +returns ``(void *)0``. **Syntax**: @@ -4346,9 +4346,9 @@ array member or doesn't have the ``counted_by`` attribute, the builtin returns The ``__builtin_counted_by_ref`` builtin allows the programmer to prevent a common error associated with the ``counted_by`` attribute. When using the ``counted_by`` attribute, the ``count`` field **must** be set before the -flexible array member can be accessed. Otherwise, the sanitizers may view such -accesses as false positives. For instance, it's not uncommon for programmers to -initialize the flexible array before setting the ``count`` field: +flexible array member or pointer can be accessed. Otherwise, the sanitizers may +view such accesses as false positives. For instance, it's not uncommon for +programmers to initialize the flexible array before setting the ``count`` field: .. code-block:: c @@ -4366,10 +4366,9 @@ initialize the flexible array before setting the ``count`` field: ptr->count = COUNT; Enforcing the rule that ``ptr->count = COUNT;`` must occur after every -allocation of a struct with a flexible array member with the ``counted_by`` -attribute is prone to failure in large code bases. This builtin mitigates this -for allocators (like in Linux) that are implemented in a way where the counter -assignment can happen automatically. +allocation of a struct with a ``counted_by`` member is prone to failure in large +code bases. This builtin mitigates this for allocators (like in Linux) that are +implemented in a way where the counter assignment can happen automatically. **Note:** The value returned by ``__builtin_counted_by_ref`` cannot be assigned to a variable, have its address taken, or passed into or returned from a diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index a2180fdb36473..509ede2027719 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -7021,8 +7021,9 @@ def warn_counted_by_attr_elt_type_unknown_size : InGroup<BoundsSafetyCountedByEltTyUnknownSize>; // __builtin_counted_by_ref diagnostics: -def err_builtin_counted_by_ref_must_be_flex_array_member : Error< - "'__builtin_counted_by_ref' argument must reference a flexible array member">; +def err_builtin_counted_by_ref_invalid_arg + : Error<"'__builtin_counted_by_ref' argument must reference a member with " + "the 'counted_by' attribute">; def err_builtin_counted_by_ref_has_side_effects : Error< "'__builtin_counted_by_ref' argument cannot have side-effects">; diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp index da72a43643a54..86b0bc68f1525 100644 --- a/clang/lib/CodeGen/CGBuiltin.cpp +++ b/clang/lib/CodeGen/CGBuiltin.cpp @@ -3693,9 +3693,9 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID, if (auto *CATy = ME->getMemberDecl()->getType()->getAs<CountAttributedType>(); CATy && CATy->getKind() == CountAttributedType::CountedBy) { - const auto *FAMDecl = cast<FieldDecl>(ME->getMemberDecl()); - if (const FieldDecl *CountFD = FAMDecl->findCountedByField()) - Result = GetCountedByFieldExprGEP(Arg, FAMDecl, CountFD); + const auto *MemberDecl = cast<FieldDecl>(ME->getMemberDecl()); + if (const FieldDecl *CountFD = MemberDecl->findCountedByField()) + Result = GetCountedByFieldExprGEP(Arg, MemberDecl, CountFD); else llvm::report_fatal_error("Cannot find the counted_by 'count' field"); } diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 58de9fe48162b..833b33de5a75a 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -6362,13 +6362,13 @@ bool Sema::BuiltinCountedByRef(CallExpr *TheCall) { return true; // For simplicity, we support only limited expressions for the argument. - // Specifically a pointer to a flexible array member:'ptr->array'. This - // allows us to reject arguments with complex casting, which really shouldn't - // be a huge problem. + // Specifically a flexible array member or a pointer with counted_by: + // 'ptr->array' or 'ptr->pointer'. This allows us to reject arguments with + // complex casting, which really shouldn't be a huge problem. const Expr *Arg = ArgRes.get()->IgnoreParenImpCasts(); - if (!isa<PointerType>(Arg->getType()) && !Arg->getType()->isArrayType()) + if (!Arg->getType()->isPointerType() && !Arg->getType()->isArrayType()) return Diag(Arg->getBeginLoc(), - diag::err_builtin_counted_by_ref_must_be_flex_array_member) + diag::err_builtin_counted_by_ref_invalid_arg) << Arg->getSourceRange(); if (Arg->HasSideEffects(Context)) @@ -6377,24 +6377,27 @@ bool Sema::BuiltinCountedByRef(CallExpr *TheCall) { << Arg->getSourceRange(); if (const auto *ME = dyn_cast<MemberExpr>(Arg)) { - if (!ME->isFlexibleArrayMemberLike( - Context, getLangOpts().getStrictFlexArraysLevel())) - return Diag(Arg->getBeginLoc(), - diag::err_builtin_counted_by_ref_must_be_flex_array_member) - << Arg->getSourceRange(); + const auto *CATy = + ME->getMemberDecl()->getType()->getAs<CountAttributedType>(); - if (auto *CATy = - ME->getMemberDecl()->getType()->getAs<CountAttributedType>(); - CATy && CATy->getKind() == CountAttributedType::CountedBy) { - const auto *FAMDecl = cast<FieldDecl>(ME->getMemberDecl()); - if (const FieldDecl *CountFD = FAMDecl->findCountedByField()) { + if (CATy && CATy->getKind() == CountAttributedType::CountedBy) { + // Member has counted_by attribute - return pointer to count field + const auto *MemberDecl = cast<FieldDecl>(ME->getMemberDecl()); + if (const FieldDecl *CountFD = MemberDecl->findCountedByField()) { TheCall->setType(Context.getPointerType(CountFD->getType())); return false; } } + + // FAMs and pointers without counted_by return void* + QualType MemberTy = ME->getMemberDecl()->getType(); + if (!MemberTy->isArrayType() && !MemberTy->isPointerType()) + return Diag(Arg->getBeginLoc(), + diag::err_builtin_counted_by_ref_invalid_arg) + << Arg->getSourceRange(); } else { return Diag(Arg->getBeginLoc(), - diag::err_builtin_counted_by_ref_must_be_flex_array_member) + diag::err_builtin_counted_by_ref_invalid_arg) << Arg->getSourceRange(); } diff --git a/clang/test/CodeGen/builtin-counted-by-ref.c b/clang/test/CodeGen/builtin-counted-by-ref.c index 8ad715879aa76..d44dbf5d0c1a2 100644 --- a/clang/test/CodeGen/builtin-counted-by-ref.c +++ b/clang/test/CodeGen/builtin-counted-by-ref.c @@ -175,3 +175,61 @@ struct c *test3(int size) { return p; } + +// Test for pointer member with counted_by attribute +struct d { + int x; + short count; + int *ptr __attribute__((counted_by(count))); +}; + +// X86_64-LABEL: define dso_local ptr @test4( +// X86_64-SAME: i32 noundef [[SIZE:%.*]], ptr noundef [[DATA:%.*]]) #[[ATTR0]] { +// X86_64-NEXT: [[ENTRY:.*:]] +// X86_64-NEXT: [[SIZE_ADDR:%.*]] = alloca i32, align 4 +// X86_64-NEXT: [[DATA_ADDR:%.*]] = alloca ptr, align 8 +// X86_64-NEXT: [[P:%.*]] = alloca ptr, align 8 +// X86_64-NEXT: store i32 [[SIZE]], ptr [[SIZE_ADDR]], align 4 +// X86_64-NEXT: store ptr [[DATA]], ptr [[DATA_ADDR]], align 8 +// X86_64-NEXT: [[CALL:%.*]] = call ptr @malloc(i64 noundef 16) #[[ATTR2]] +// X86_64-NEXT: store ptr [[CALL]], ptr [[P]], align 8 +// X86_64-NEXT: [[TMP0:%.*]] = load ptr, ptr [[DATA_ADDR]], align 8 +// X86_64-NEXT: [[TMP1:%.*]] = load ptr, ptr [[P]], align 8 +// X86_64-NEXT: [[PTR:%.*]] = getelementptr inbounds nuw [[STRUCT_D:%.*]], ptr [[TMP1]], i32 0, i32 2 +// X86_64-NEXT: store ptr [[TMP0]], ptr [[PTR]], align 8 +// X86_64-NEXT: [[TMP2:%.*]] = load i32, ptr [[SIZE_ADDR]], align 4 +// X86_64-NEXT: [[CONV:%.*]] = trunc i32 [[TMP2]] to i16 +// X86_64-NEXT: [[TMP3:%.*]] = load ptr, ptr [[P]], align 8 +// X86_64-NEXT: [[DOT_COUNTED_BY_GEP:%.*]] = getelementptr inbounds [[STRUCT_D]], ptr [[TMP3]], i32 0, i32 1 +// X86_64-NEXT: store i16 [[CONV]], ptr [[DOT_COUNTED_BY_GEP]], align 2 +// X86_64-NEXT: [[TMP4:%.*]] = load ptr, ptr [[P]], align 8 +// X86_64-NEXT: ret ptr [[TMP4]] +// +// I386-LABEL: define dso_local ptr @test4( +// I386-SAME: i32 noundef [[SIZE:%.*]], ptr noundef [[DATA:%.*]]) #[[ATTR0]] { +// I386-NEXT: [[ENTRY:.*:]] +// I386-NEXT: [[SIZE_ADDR:%.*]] = alloca i32, align 4 +// I386-NEXT: [[DATA_ADDR:%.*]] = alloca ptr, align 4 +// I386-NEXT: [[P:%.*]] = alloca ptr, align 4 +// I386-NEXT: store i32 [[SIZE]], ptr [[SIZE_ADDR]], align 4 +// I386-NEXT: store ptr [[DATA]], ptr [[DATA_ADDR]], align 4 +// I386-NEXT: [[CALL:%.*]] = call ptr @malloc(i32 noundef 12) #[[ATTR2]] +// I386-NEXT: store ptr [[CALL]], ptr [[P]], align 4 +// I386-NEXT: [[TMP0:%.*]] = load ptr, ptr [[DATA_ADDR]], align 4 +// I386-NEXT: [[TMP1:%.*]] = load ptr, ptr [[P]], align 4 +// I386-NEXT: [[PTR:%.*]] = getelementptr inbounds nuw [[STRUCT_D:%.*]], ptr [[TMP1]], i32 0, i32 2 +// I386-NEXT: store ptr [[TMP0]], ptr [[PTR]], align 4 +// I386-NEXT: [[TMP2:%.*]] = load i32, ptr [[SIZE_ADDR]], align 4 +// I386-NEXT: [[CONV:%.*]] = trunc i32 [[TMP2]] to i16 +// I386-NEXT: [[TMP3:%.*]] = load ptr, ptr [[P]], align 4 +// I386-NEXT: [[DOT_COUNTED_BY_GEP:%.*]] = getelementptr inbounds [[STRUCT_D]], ptr [[TMP3]], i32 0, i32 1 +// I386-NEXT: store i16 [[CONV]], ptr [[DOT_COUNTED_BY_GEP]], align 2 +// I386-NEXT: [[TMP4:%.*]] = load ptr, ptr [[P]], align 4 +// I386-NEXT: ret ptr [[TMP4]] +// +struct d *test4(int size, int *data) { + struct d *p = __builtin_malloc(sizeof(struct d)); + p->ptr = data; + *__builtin_counted_by_ref(p->ptr) = size; + return p; +} diff --git a/clang/test/Sema/builtin-counted-by-ref.c b/clang/test/Sema/builtin-counted-by-ref.c index a9f46a3223006..ed766d3d593d2 100644 --- a/clang/test/Sema/builtin-counted-by-ref.c +++ b/clang/test/Sema/builtin-counted-by-ref.c @@ -32,14 +32,14 @@ void test2(struct fam_struct *ptr, int idx) { } void test3(struct fam_struct *ptr, int idx) { - __builtin_counted_by_ref(&ptr->array[0]); // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}} - __builtin_counted_by_ref(&ptr->array[idx]); // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}} - __builtin_counted_by_ref(&ptr->array); // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}} - __builtin_counted_by_ref(ptr->x); // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}} - __builtin_counted_by_ref(&ptr->x); // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}} - __builtin_counted_by_ref(global_array); // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}} - __builtin_counted_by_ref(global_int); // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}} - __builtin_counted_by_ref(&global_int); // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}} + __builtin_counted_by_ref(&ptr->array[0]); // expected-error {{'__builtin_counted_by_ref' argument must reference a member with the 'counted_by' attribute}} + __builtin_counted_by_ref(&ptr->array[idx]); // expected-error {{'__builtin_counted_by_ref' argument must reference a member with the 'counted_by' attribute}} + __builtin_counted_by_ref(&ptr->array); // expected-error {{'__builtin_counted_by_ref' argument must reference a member with the 'counted_by' attribute}} + __builtin_counted_by_ref(ptr->x); // expected-error {{'__builtin_counted_by_ref' argument must reference a member with the 'counted_by' attribute}} + __builtin_counted_by_ref(&ptr->x); // expected-error {{'__builtin_counted_by_ref' argument must reference a member with the 'counted_by' attribute}} + __builtin_counted_by_ref(global_array); // expected-error {{'__builtin_counted_by_ref' argument must reference a member with the 'counted_by' attribute}} + __builtin_counted_by_ref(global_int); // expected-error {{'__builtin_counted_by_ref' argument must reference a member with the 'counted_by' attribute}} + __builtin_counted_by_ref(&global_int); // expected-error {{'__builtin_counted_by_ref' argument must reference a member with the 'counted_by' attribute}} } void test4(struct fam_struct *ptr, int idx) { @@ -78,10 +78,12 @@ struct non_fam_struct { }; void *test7(struct non_fam_struct *ptr, int size) { - *__builtin_counted_by_ref(ptr->array) = size // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}} - *__builtin_counted_by_ref(&ptr->array[0]) = size; // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}} - *__builtin_counted_by_ref(ptr->pointer) = size; // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}} - *__builtin_counted_by_ref(&ptr->pointer[0]) = size; // expected-error {{'__builtin_counted_by_ref' argument must reference a flexible array member}} + // Arrays and pointers without counted_by return void* + _Static_assert(_Generic(__builtin_counted_by_ref(ptr->array), void * : 1, default : 0) == 1, "should be void*"); + _Static_assert(_Generic(__builtin_counted_by_ref(ptr->pointer), void * : 1, default : 0) == 1, "should be void*"); + // These are not direct member accesses, so they error + __builtin_counted_by_ref(&ptr->array[0]); // expected-error {{'__builtin_counted_by_ref' argument must reference a member with the 'counted_by' attribute}} + __builtin_counted_by_ref(&ptr->pointer[0]); // expected-error {{'__builtin_counted_by_ref' argument must reference a member with the 'counted_by' attribute}} } struct char_count { @@ -122,3 +124,64 @@ void test8(void) { _Static_assert(_Generic(__builtin_counted_by_ref(lp->array), long * : 1, default : 0) == 1, "wrong return type"); _Static_assert(_Generic(__builtin_counted_by_ref(ulp->array), unsigned long * : 1, default : 0) == 1, "wrong return type"); } + +// Tests for pointer members with counted_by attribute +struct ptr_char_count { + char count; + int *ptr __attribute__((counted_by(count))); +} *pcp; + +struct ptr_short_count { + short count; + int *ptr __attribute__((counted_by(count))); +} *psp; + +struct ptr_int_count { + int count; + int *ptr __attribute__((counted_by(count))); +} *pip; + +struct ptr_unsigned_count { + unsigned count; + int *ptr __attribute__((counted_by(count))); +} *pup; + +struct ptr_long_count { + long count; + int *ptr __attribute__((counted_by(count))); +} *plp; + +struct ptr_unsigned_long_count { + unsigned long count; + int *ptr __attribute__((counted_by(count))); +} *pulp; + +void test9(struct ptr_int_count *ptr, int size) { + size_t size_of = sizeof(__builtin_counted_by_ref(ptr->ptr)); // ok + int align_of = __alignof(__builtin_counted_by_ref(ptr->ptr)); // ok + size_t __ignored_assignment; + + *__builtin_counted_by_ref(ptr->ptr) = size; // ok + *_Generic(__builtin_counted_by_ref(ptr->ptr), + void *: &__ignored_assignment, + default: __builtin_counted_by_ref(ptr->ptr)) = 42; // ok +} + +void test10(void) { + _Static_assert(_Generic(__builtin_counted_by_ref(pcp->ptr), char * : 1, default : 0) == 1, "wrong return type"); + _Static_assert(_Generic(__builtin_counted_by_ref(psp->ptr), short * : 1, default : 0) == 1, "wrong return type"); + _Static_assert(_Generic(__builtin_counted_by_ref(pip->ptr), int * : 1, default : 0) == 1, "wrong return type"); + _Static_assert(_Generic(__builtin_counted_by_ref(pup->ptr), unsigned int * : 1, default : 0) == 1, "wrong return type"); + _Static_assert(_Generic(__builtin_counted_by_ref(plp->ptr), long * : 1, default : 0) == 1, "wrong return type"); + _Static_assert(_Generic(__builtin_counted_by_ref(pulp->ptr), unsigned long * : 1, default : 0) == 1, "wrong return type"); +} + +// Pointer without counted_by returns void* +struct ptr_no_attr { + int count; + int *ptr; // No counted_by attribute +}; + +void test11(struct ptr_no_attr *p) { + _Static_assert(_Generic(__builtin_counted_by_ref(p->ptr), void * : 1, default : 0) == 1, "should be void*"); +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
