-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[Clang] Extend __builtin_counted_by_ref to support pointers with 'counted_by' #170750
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@llvm/pr-subscribers-clang-codegen @llvm/pr-subscribers-clang Author: Kees Cook (kees) Changes…nted_by' 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: With this change, __builtin_counted_by_ref works with both: This enables the same allocation pattern for pointer members that was previously only available for FAMs: The builtin returns:
This was requested by upstream Linux kernel devs: Full diff: https://github.com/llvm/llvm-project/pull/170750.diff 6 Files Affected:
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..6a93451160835 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7021,8 +7021,10 @@ 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*");
+}
|
bwendling
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one nit, otherwise good.
e38d469 to
b0904e3
Compare
…nted_by'
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/
b0904e3 to
a16daea
Compare
…nted_by' (llvm#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/
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:
With this change, __builtin_counted_by_ref works with both:
This enables the same allocation pattern for pointer members that was previously only available for FAMs:
The builtin returns:
This was requested by upstream Linux kernel devs:
https://lore.kernel.org/linux-hardening/202512041215.44484FCACD@keescook/