[Clang][Sema] Allow counted_by on void* in GNU mode #164737
Conversation
The counted_by attribute currently rejects void* members because void has no defined size. However, the sized_by attribute accepts void* since it explicitly measures bytes. In GNU mode, void pointer arithmetic treats void as having size 1 byte, so counted_by on void* should behave identically to sized_by (treating the count as bytes). Allow counted_by on void* when GNU extensions are enabled. The implementation validates this only at declaration time in SemaBoundsSafety.cpp, emitting a -Wpointer-arith warning that the attribute is treated as a GNU extension equivalent to sized_by. Both use-site validation and code generation trust this earlier validation, avoiding redundant checks. In CodeGen, __builtin_dynamic_object_size now correctly handles counted_by on void* by treating any CountAttributedType with zero element size as having 1-byte elements, matching the GNU void pointer arithmetic semantics. Add tests validating both Sema diagnostics (warnings in GNU mode, errors in strict C) and CodeGen behavior (correct byte counts from __builtin_dynamic_object_size). Update existing counted_by tests to explicitly use -std=c11 to preserve their original intent of rejecting void* in strict C mode.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Kees Cook (kees) ChangesThe counted_by attribute currently rejects void* members because void has no defined size. However, the sized_by attribute accepts void* since it explicitly measures bytes. In GNU mode, void pointer arithmetic treats void as having size 1 byte, so counted_by on void* should behave identically to sized_by (treating the count as bytes). Allow counted_by on void* when GNU extensions are enabled. The implementation validates this only at declaration time in SemaBoundsSafety.cpp, emitting a -Wpointer-arith warning that the attribute is treated as a GNU extension equivalent to sized_by. Both use-site validation and code generation trust this earlier validation, avoiding redundant checks. In CodeGen, __builtin_dynamic_object_size now correctly handles counted_by on void* by treating any CountAttributedType with zero element size as having 1-byte elements, matching the GNU void pointer arithmetic semantics. Add tests validating both Sema diagnostics (warnings in GNU mode, errors in strict C) and CodeGen behavior (correct byte counts from __builtin_dynamic_object_size). Update existing counted_by tests to explicitly use -std=c11 to preserve their original intent of rejecting void* in strict C mode. Full diff: https://github.com/llvm/llvm-project/pull/164737.diff 10 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 20b499462ae94..33f28f251f5d0 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8114,6 +8114,12 @@ def ext_gnu_ptr_func_arith : Extension<
"arithmetic on%select{ a|}0 pointer%select{|s}0 to%select{ the|}2 function "
"type%select{|s}2 %1%select{| and %3}2 is a GNU extension">,
InGroup<GNUPointerArith>;
+def ext_gnu_counted_by_void_ptr
+ : Extension<
+ "'%select{counted_by|sized_by|counted_by_or_null|sized_by_or_null}0' "
+ "on a pointer to void is a GNU extension, treated as "
+ "'%select{sized_by|sized_by|sized_by_or_null|sized_by_or_null}0'">,
+ InGroup<GNUPointerArith>;
def err_readonly_message_assignment : Error<
"assigning to 'readonly' return result of an Objective-C message not allowed">;
def ext_c2y_increment_complex : Extension<
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index fd14cd6926fe2..aaf8d59414498 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -1211,11 +1211,13 @@ llvm::Value *CodeGenFunction::emitCountedByPointerSize(
getContext().getTypeSizeInChars(ElementTy->getPointeeType());
if (ElementSize.isZero()) {
- // This might be a __sized_by on a 'void *', which counts bytes, not
- // elements.
+ // This might be a __sized_by (or __counted_by in GNU mode) on a
+ // 'void *', which counts bytes, not elements.
auto *CAT = ElementTy->getAs<CountAttributedType>();
if (!CAT || (CAT->getKind() != CountAttributedType::SizedBy &&
- CAT->getKind() != CountAttributedType::SizedByOrNull))
+ CAT->getKind() != CountAttributedType::SizedByOrNull &&
+ CAT->getKind() != CountAttributedType::CountedBy &&
+ CAT->getKind() != CountAttributedType::CountedByOrNull))
// Okay, not sure what it is now.
// FIXME: Should this be an assert?
return std::optional<CharUnits>();
diff --git a/clang/lib/Sema/SemaBoundsSafety.cpp b/clang/lib/Sema/SemaBoundsSafety.cpp
index 39ab13653f5fe..abc7397aaa888 100644
--- a/clang/lib/Sema/SemaBoundsSafety.cpp
+++ b/clang/lib/Sema/SemaBoundsSafety.cpp
@@ -132,9 +132,20 @@ bool Sema::CheckCountedByAttrOnField(FieldDecl *FD, Expr *E, bool CountInBytes,
// `BoundsSafetyCheckUseOfCountAttrPtr`
//
// * When the pointee type is always an incomplete type (e.g.
- // `void`) the attribute is disallowed by this method because we know the
- // type can never be completed so there's no reason to allow it.
- InvalidTypeKind = CountedByInvalidPointeeTypeKind::INCOMPLETE;
+ // `void` in strict C mode) the attribute is disallowed by this method
+ // because we know the type can never be completed so there's no reason
+ // to allow it.
+ //
+ // Exception: In GNU mode, void has an implicit size of 1 byte for pointer
+ // arithmetic. Therefore, counted_by on void* is allowed as a GNU extension
+ // and behaves equivalently to sized_by (treating the count as bytes).
+ bool IsVoidPtrInGNUMode = PointeeTy->isVoidType() && getLangOpts().GNUMode;
+ if (IsVoidPtrInGNUMode) {
+ // Emit a warning that this is a GNU extension
+ Diag(FD->getBeginLoc(), diag::ext_gnu_counted_by_void_ptr) << Kind;
+ } else {
+ InvalidTypeKind = CountedByInvalidPointeeTypeKind::INCOMPLETE;
+ }
} else if (PointeeTy->isSizelessType()) {
InvalidTypeKind = CountedByInvalidPointeeTypeKind::SIZELESS;
} else if (PointeeTy->isFunctionType()) {
@@ -272,6 +283,11 @@ GetCountedByAttrOnIncompletePointee(QualType Ty, NamedDecl **ND) {
if (!PointeeTy->isIncompleteType(ND))
return {};
+ // If counted_by is on void*, it was already validated at declaration time
+ // as a GNU extension. No need to re-check GNU mode here.
+ if (PointeeTy->isVoidType())
+ return {};
+
return {CATy, PointeeTy};
}
diff --git a/clang/test/CodeGen/attr-counted-by-void-ptr-gnu.c b/clang/test/CodeGen/attr-counted-by-void-ptr-gnu.c
new file mode 100644
index 0000000000000..e22aad306f60c
--- /dev/null
+++ b/clang/test/CodeGen/attr-counted-by-void-ptr-gnu.c
@@ -0,0 +1,65 @@
+// RUN: %clang_cc1 -std=gnu11 -triple x86_64-unknown-linux-gnu -O2 -emit-llvm -o - %s | FileCheck %s
+
+// Test that counted_by on void* in GNU mode treats void as having size 1 (byte count)
+
+#define __counted_by(f) __attribute__((counted_by(f)))
+#define __sized_by(f) __attribute__((sized_by(f)))
+
+struct with_counted_by_void {
+ int count;
+ void* buf __counted_by(count);
+};
+
+struct with_sized_by_void {
+ int size;
+ void* buf __sized_by(size);
+};
+
+struct with_counted_by_int {
+ int count;
+ int* buf __counted_by(count);
+};
+
+// CHECK-LABEL: define dso_local {{.*}}@test_counted_by_void(
+// CHECK: %[[COUNT:.*]] = load i32, ptr %s
+// CHECK: %[[NARROW:.*]] = tail call i32 @llvm.smax.i32(i32 %[[COUNT]], i32 0)
+// CHECK: %[[ZEXT:.*]] = zext nneg i32 %[[NARROW]] to i64
+// CHECK: ret i64 %[[ZEXT]]
+//
+// Verify: counted_by on void* returns the count directly (count * 1 byte)
+long long test_counted_by_void(struct with_counted_by_void *s) {
+ return __builtin_dynamic_object_size(s->buf, 0);
+}
+
+// CHECK-LABEL: define dso_local {{.*}}@test_sized_by_void(
+// CHECK: %[[SIZE:.*]] = load i32, ptr %s
+// CHECK: %[[NARROW:.*]] = tail call i32 @llvm.smax.i32(i32 %[[SIZE]], i32 0)
+// CHECK: %[[ZEXT:.*]] = zext nneg i32 %[[NARROW]] to i64
+// CHECK: ret i64 %[[ZEXT]]
+//
+// Verify: sized_by on void* returns the size directly
+long long test_sized_by_void(struct with_sized_by_void *s) {
+ return __builtin_dynamic_object_size(s->buf, 0);
+}
+
+// CHECK-LABEL: define dso_local {{.*}}@test_counted_by_int(
+// CHECK: %[[COUNT:.*]] = load i32, ptr %s
+// CHECK: %[[SEXT:.*]] = sext i32 %[[COUNT]] to i64
+// CHECK: %[[SIZE:.*]] = shl nsw i64 %[[SEXT]], 2
+// CHECK: ret i64
+//
+// Verify: counted_by on int* returns count * sizeof(int) = count * 4
+long long test_counted_by_int(struct with_counted_by_int *s) {
+ return __builtin_dynamic_object_size(s->buf, 0);
+}
+
+// CHECK-LABEL: define dso_local ptr @test_void_ptr_arithmetic(
+// CHECK: %[[BUF:.*]] = load ptr, ptr
+// CHECK: %[[EXT:.*]] = sext i32 %offset to i64
+// CHECK: %[[PTR:.*]] = getelementptr inbounds i8, ptr %[[BUF]], i64 %[[EXT]]
+// CHECK: ret ptr %[[PTR]]
+//
+// Verify: pointer arithmetic on void* uses i8 (byte offsets), not i32 or other sizes
+void* test_void_ptr_arithmetic(struct with_counted_by_void *s, int offset) {
+ return s->buf + offset; // GNU extension: void* arithmetic
+}
diff --git a/clang/test/Sema/attr-counted-by-late-parsed-struct-ptrs.c b/clang/test/Sema/attr-counted-by-late-parsed-struct-ptrs.c
index 8d4e0c510603a..b6c2ba68732ee 100644
--- a/clang/test/Sema/attr-counted-by-late-parsed-struct-ptrs.c
+++ b/clang/test/Sema/attr-counted-by-late-parsed-struct-ptrs.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fexperimental-late-parse-attributes -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c11 -fexperimental-late-parse-attributes -fsyntax-only -verify %s
#define __counted_by(f) __attribute__((counted_by(f)))
diff --git a/clang/test/Sema/attr-counted-by-or-null-last-field.c b/clang/test/Sema/attr-counted-by-or-null-last-field.c
index 60a1f571b19e9..4cf8cb64557d2 100644
--- a/clang/test/Sema/attr-counted-by-or-null-last-field.c
+++ b/clang/test/Sema/attr-counted-by-or-null-last-field.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -verify=expected,immediate %s
-// RUN: %clang_cc1 -fsyntax-only -fexperimental-late-parse-attributes -verify=expected,late %s
+// RUN: %clang_cc1 -std=c11 -fsyntax-only -verify=expected,immediate %s
+// RUN: %clang_cc1 -std=c11 -fsyntax-only -fexperimental-late-parse-attributes -verify=expected,late %s
#define __counted_by_or_null(f) __attribute__((counted_by_or_null(f)))
diff --git a/clang/test/Sema/attr-counted-by-or-null-late-parsed-struct-ptrs.c b/clang/test/Sema/attr-counted-by-or-null-late-parsed-struct-ptrs.c
index 2150c81f9e9be..29e235463f822 100644
--- a/clang/test/Sema/attr-counted-by-or-null-late-parsed-struct-ptrs.c
+++ b/clang/test/Sema/attr-counted-by-or-null-late-parsed-struct-ptrs.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fexperimental-late-parse-attributes -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c11 -fexperimental-late-parse-attributes -fsyntax-only -verify %s
#define __counted_by_or_null(f) __attribute__((counted_by_or_null(f)))
#define __counted_by(f) __attribute__((counted_by(f)))
diff --git a/clang/test/Sema/attr-counted-by-or-null-struct-ptrs.c b/clang/test/Sema/attr-counted-by-or-null-struct-ptrs.c
index 0bb09059c97f9..7fe43a20c0c6e 100644
--- a/clang/test/Sema/attr-counted-by-or-null-struct-ptrs.c
+++ b/clang/test/Sema/attr-counted-by-or-null-struct-ptrs.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fexperimental-late-parse-attributes -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c11 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c11 -fexperimental-late-parse-attributes -fsyntax-only -verify %s
#define __counted_by_or_null(f) __attribute__((counted_by_or_null(f)))
#define __counted_by(f) __attribute__((counted_by(f)))
diff --git a/clang/test/Sema/attr-counted-by-struct-ptrs.c b/clang/test/Sema/attr-counted-by-struct-ptrs.c
index c05d18262e2b7..d75533d7d135b 100644
--- a/clang/test/Sema/attr-counted-by-struct-ptrs.c
+++ b/clang/test/Sema/attr-counted-by-struct-ptrs.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fsyntax-only -fexperimental-late-parse-attributes %s -verify
+// RUN: %clang_cc1 -std=c11 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c11 -fsyntax-only -fexperimental-late-parse-attributes %s -verify
#define __counted_by(f) __attribute__((counted_by(f)))
diff --git a/clang/test/Sema/attr-counted-by-void-ptr-gnu.c b/clang/test/Sema/attr-counted-by-void-ptr-gnu.c
new file mode 100644
index 0000000000000..c6aa371434bc4
--- /dev/null
+++ b/clang/test/Sema/attr-counted-by-void-ptr-gnu.c
@@ -0,0 +1,65 @@
+// RUN: %clang_cc1 -std=gnu11 -fsyntax-only -verify=expected-nowarn %s
+// RUN: %clang_cc1 -std=gnu11 -Wpointer-arith -fsyntax-only -verify=expected-warn %s
+// RUN: %clang_cc1 -std=c11 -fsyntax-only -verify=expected-strict %s
+
+// expected-nowarn-no-diagnostics
+
+#define __counted_by(f) __attribute__((counted_by(f)))
+#define __counted_by_or_null(f) __attribute__((counted_by_or_null(f)))
+#define __sized_by(f) __attribute__((sized_by(f)))
+
+//==============================================================================
+// Test: counted_by on void* is allowed in GNU mode, rejected in strict mode
+//==============================================================================
+
+struct test_void_ptr_gnu {
+ int count;
+ // expected-warn-warning@+2{{'counted_by' on a pointer to void is a GNU extension, treated as 'sized_by'}}
+ // expected-strict-error@+1{{'counted_by' cannot be applied to a pointer with pointee of unknown size because 'void' is an incomplete type}}
+ void* buf __counted_by(count);
+};
+
+struct test_const_void_ptr_gnu {
+ int count;
+ // expected-warn-warning@+2{{'counted_by' on a pointer to void is a GNU extension, treated as 'sized_by'}}
+ // expected-strict-error@+1{{'counted_by' cannot be applied to a pointer with pointee of unknown size because 'const void' is an incomplete type}}
+ const void* buf __counted_by(count);
+};
+
+struct test_volatile_void_ptr_gnu {
+ int count;
+ // expected-warn-warning@+2{{'counted_by' on a pointer to void is a GNU extension, treated as 'sized_by'}}
+ // expected-strict-error@+1{{'counted_by' cannot be applied to a pointer with pointee of unknown size because 'volatile void' is an incomplete type}}
+ volatile void* buf __counted_by(count);
+};
+
+struct test_const_volatile_void_ptr_gnu {
+ int count;
+ // expected-warn-warning@+2{{'counted_by' on a pointer to void is a GNU extension, treated as 'sized_by'}}
+ // expected-strict-error@+1{{'counted_by' cannot be applied to a pointer with pointee of unknown size because 'const volatile void' is an incomplete type}}
+ const volatile void* buf __counted_by(count);
+};
+
+// Verify sized_by still works the same way (always allowed, no warning)
+struct test_sized_by_void_ptr {
+ int size;
+ void* buf __sized_by(size); // OK in both modes, no warning
+};
+
+//==============================================================================
+// Test: counted_by_or_null on void* behaves the same
+//==============================================================================
+
+struct test_void_ptr_or_null_gnu {
+ int count;
+ // expected-warn-warning@+2{{'counted_by_or_null' on a pointer to void is a GNU extension, treated as 'sized_by_or_null'}}
+ // expected-strict-error@+1{{'counted_by_or_null' cannot be applied to a pointer with pointee of unknown size because 'void' is an incomplete type}}
+ void* buf __counted_by_or_null(count);
+};
+
+struct test_const_void_ptr_or_null_gnu {
+ int count;
+ // expected-warn-warning@+2{{'counted_by_or_null' on a pointer to void is a GNU extension, treated as 'sized_by_or_null'}}
+ // expected-strict-error@+1{{'counted_by_or_null' cannot be applied to a pointer with pointee of unknown size because 'const void' is an incomplete type}}
+ const void* buf __counted_by_or_null(count);
+};
|
|
@llvm/pr-subscribers-clang-codegen Author: Kees Cook (kees) ChangesThe counted_by attribute currently rejects void* members because void has no defined size. However, the sized_by attribute accepts void* since it explicitly measures bytes. In GNU mode, void pointer arithmetic treats void as having size 1 byte, so counted_by on void* should behave identically to sized_by (treating the count as bytes). Allow counted_by on void* when GNU extensions are enabled. The implementation validates this only at declaration time in SemaBoundsSafety.cpp, emitting a -Wpointer-arith warning that the attribute is treated as a GNU extension equivalent to sized_by. Both use-site validation and code generation trust this earlier validation, avoiding redundant checks. In CodeGen, __builtin_dynamic_object_size now correctly handles counted_by on void* by treating any CountAttributedType with zero element size as having 1-byte elements, matching the GNU void pointer arithmetic semantics. Add tests validating both Sema diagnostics (warnings in GNU mode, errors in strict C) and CodeGen behavior (correct byte counts from __builtin_dynamic_object_size). Update existing counted_by tests to explicitly use -std=c11 to preserve their original intent of rejecting void* in strict C mode. Full diff: https://github.com/llvm/llvm-project/pull/164737.diff 10 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 20b499462ae94..33f28f251f5d0 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8114,6 +8114,12 @@ def ext_gnu_ptr_func_arith : Extension<
"arithmetic on%select{ a|}0 pointer%select{|s}0 to%select{ the|}2 function "
"type%select{|s}2 %1%select{| and %3}2 is a GNU extension">,
InGroup<GNUPointerArith>;
+def ext_gnu_counted_by_void_ptr
+ : Extension<
+ "'%select{counted_by|sized_by|counted_by_or_null|sized_by_or_null}0' "
+ "on a pointer to void is a GNU extension, treated as "
+ "'%select{sized_by|sized_by|sized_by_or_null|sized_by_or_null}0'">,
+ InGroup<GNUPointerArith>;
def err_readonly_message_assignment : Error<
"assigning to 'readonly' return result of an Objective-C message not allowed">;
def ext_c2y_increment_complex : Extension<
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index fd14cd6926fe2..aaf8d59414498 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -1211,11 +1211,13 @@ llvm::Value *CodeGenFunction::emitCountedByPointerSize(
getContext().getTypeSizeInChars(ElementTy->getPointeeType());
if (ElementSize.isZero()) {
- // This might be a __sized_by on a 'void *', which counts bytes, not
- // elements.
+ // This might be a __sized_by (or __counted_by in GNU mode) on a
+ // 'void *', which counts bytes, not elements.
auto *CAT = ElementTy->getAs<CountAttributedType>();
if (!CAT || (CAT->getKind() != CountAttributedType::SizedBy &&
- CAT->getKind() != CountAttributedType::SizedByOrNull))
+ CAT->getKind() != CountAttributedType::SizedByOrNull &&
+ CAT->getKind() != CountAttributedType::CountedBy &&
+ CAT->getKind() != CountAttributedType::CountedByOrNull))
// Okay, not sure what it is now.
// FIXME: Should this be an assert?
return std::optional<CharUnits>();
diff --git a/clang/lib/Sema/SemaBoundsSafety.cpp b/clang/lib/Sema/SemaBoundsSafety.cpp
index 39ab13653f5fe..abc7397aaa888 100644
--- a/clang/lib/Sema/SemaBoundsSafety.cpp
+++ b/clang/lib/Sema/SemaBoundsSafety.cpp
@@ -132,9 +132,20 @@ bool Sema::CheckCountedByAttrOnField(FieldDecl *FD, Expr *E, bool CountInBytes,
// `BoundsSafetyCheckUseOfCountAttrPtr`
//
// * When the pointee type is always an incomplete type (e.g.
- // `void`) the attribute is disallowed by this method because we know the
- // type can never be completed so there's no reason to allow it.
- InvalidTypeKind = CountedByInvalidPointeeTypeKind::INCOMPLETE;
+ // `void` in strict C mode) the attribute is disallowed by this method
+ // because we know the type can never be completed so there's no reason
+ // to allow it.
+ //
+ // Exception: In GNU mode, void has an implicit size of 1 byte for pointer
+ // arithmetic. Therefore, counted_by on void* is allowed as a GNU extension
+ // and behaves equivalently to sized_by (treating the count as bytes).
+ bool IsVoidPtrInGNUMode = PointeeTy->isVoidType() && getLangOpts().GNUMode;
+ if (IsVoidPtrInGNUMode) {
+ // Emit a warning that this is a GNU extension
+ Diag(FD->getBeginLoc(), diag::ext_gnu_counted_by_void_ptr) << Kind;
+ } else {
+ InvalidTypeKind = CountedByInvalidPointeeTypeKind::INCOMPLETE;
+ }
} else if (PointeeTy->isSizelessType()) {
InvalidTypeKind = CountedByInvalidPointeeTypeKind::SIZELESS;
} else if (PointeeTy->isFunctionType()) {
@@ -272,6 +283,11 @@ GetCountedByAttrOnIncompletePointee(QualType Ty, NamedDecl **ND) {
if (!PointeeTy->isIncompleteType(ND))
return {};
+ // If counted_by is on void*, it was already validated at declaration time
+ // as a GNU extension. No need to re-check GNU mode here.
+ if (PointeeTy->isVoidType())
+ return {};
+
return {CATy, PointeeTy};
}
diff --git a/clang/test/CodeGen/attr-counted-by-void-ptr-gnu.c b/clang/test/CodeGen/attr-counted-by-void-ptr-gnu.c
new file mode 100644
index 0000000000000..e22aad306f60c
--- /dev/null
+++ b/clang/test/CodeGen/attr-counted-by-void-ptr-gnu.c
@@ -0,0 +1,65 @@
+// RUN: %clang_cc1 -std=gnu11 -triple x86_64-unknown-linux-gnu -O2 -emit-llvm -o - %s | FileCheck %s
+
+// Test that counted_by on void* in GNU mode treats void as having size 1 (byte count)
+
+#define __counted_by(f) __attribute__((counted_by(f)))
+#define __sized_by(f) __attribute__((sized_by(f)))
+
+struct with_counted_by_void {
+ int count;
+ void* buf __counted_by(count);
+};
+
+struct with_sized_by_void {
+ int size;
+ void* buf __sized_by(size);
+};
+
+struct with_counted_by_int {
+ int count;
+ int* buf __counted_by(count);
+};
+
+// CHECK-LABEL: define dso_local {{.*}}@test_counted_by_void(
+// CHECK: %[[COUNT:.*]] = load i32, ptr %s
+// CHECK: %[[NARROW:.*]] = tail call i32 @llvm.smax.i32(i32 %[[COUNT]], i32 0)
+// CHECK: %[[ZEXT:.*]] = zext nneg i32 %[[NARROW]] to i64
+// CHECK: ret i64 %[[ZEXT]]
+//
+// Verify: counted_by on void* returns the count directly (count * 1 byte)
+long long test_counted_by_void(struct with_counted_by_void *s) {
+ return __builtin_dynamic_object_size(s->buf, 0);
+}
+
+// CHECK-LABEL: define dso_local {{.*}}@test_sized_by_void(
+// CHECK: %[[SIZE:.*]] = load i32, ptr %s
+// CHECK: %[[NARROW:.*]] = tail call i32 @llvm.smax.i32(i32 %[[SIZE]], i32 0)
+// CHECK: %[[ZEXT:.*]] = zext nneg i32 %[[NARROW]] to i64
+// CHECK: ret i64 %[[ZEXT]]
+//
+// Verify: sized_by on void* returns the size directly
+long long test_sized_by_void(struct with_sized_by_void *s) {
+ return __builtin_dynamic_object_size(s->buf, 0);
+}
+
+// CHECK-LABEL: define dso_local {{.*}}@test_counted_by_int(
+// CHECK: %[[COUNT:.*]] = load i32, ptr %s
+// CHECK: %[[SEXT:.*]] = sext i32 %[[COUNT]] to i64
+// CHECK: %[[SIZE:.*]] = shl nsw i64 %[[SEXT]], 2
+// CHECK: ret i64
+//
+// Verify: counted_by on int* returns count * sizeof(int) = count * 4
+long long test_counted_by_int(struct with_counted_by_int *s) {
+ return __builtin_dynamic_object_size(s->buf, 0);
+}
+
+// CHECK-LABEL: define dso_local ptr @test_void_ptr_arithmetic(
+// CHECK: %[[BUF:.*]] = load ptr, ptr
+// CHECK: %[[EXT:.*]] = sext i32 %offset to i64
+// CHECK: %[[PTR:.*]] = getelementptr inbounds i8, ptr %[[BUF]], i64 %[[EXT]]
+// CHECK: ret ptr %[[PTR]]
+//
+// Verify: pointer arithmetic on void* uses i8 (byte offsets), not i32 or other sizes
+void* test_void_ptr_arithmetic(struct with_counted_by_void *s, int offset) {
+ return s->buf + offset; // GNU extension: void* arithmetic
+}
diff --git a/clang/test/Sema/attr-counted-by-late-parsed-struct-ptrs.c b/clang/test/Sema/attr-counted-by-late-parsed-struct-ptrs.c
index 8d4e0c510603a..b6c2ba68732ee 100644
--- a/clang/test/Sema/attr-counted-by-late-parsed-struct-ptrs.c
+++ b/clang/test/Sema/attr-counted-by-late-parsed-struct-ptrs.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fexperimental-late-parse-attributes -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c11 -fexperimental-late-parse-attributes -fsyntax-only -verify %s
#define __counted_by(f) __attribute__((counted_by(f)))
diff --git a/clang/test/Sema/attr-counted-by-or-null-last-field.c b/clang/test/Sema/attr-counted-by-or-null-last-field.c
index 60a1f571b19e9..4cf8cb64557d2 100644
--- a/clang/test/Sema/attr-counted-by-or-null-last-field.c
+++ b/clang/test/Sema/attr-counted-by-or-null-last-field.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -verify=expected,immediate %s
-// RUN: %clang_cc1 -fsyntax-only -fexperimental-late-parse-attributes -verify=expected,late %s
+// RUN: %clang_cc1 -std=c11 -fsyntax-only -verify=expected,immediate %s
+// RUN: %clang_cc1 -std=c11 -fsyntax-only -fexperimental-late-parse-attributes -verify=expected,late %s
#define __counted_by_or_null(f) __attribute__((counted_by_or_null(f)))
diff --git a/clang/test/Sema/attr-counted-by-or-null-late-parsed-struct-ptrs.c b/clang/test/Sema/attr-counted-by-or-null-late-parsed-struct-ptrs.c
index 2150c81f9e9be..29e235463f822 100644
--- a/clang/test/Sema/attr-counted-by-or-null-late-parsed-struct-ptrs.c
+++ b/clang/test/Sema/attr-counted-by-or-null-late-parsed-struct-ptrs.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fexperimental-late-parse-attributes -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c11 -fexperimental-late-parse-attributes -fsyntax-only -verify %s
#define __counted_by_or_null(f) __attribute__((counted_by_or_null(f)))
#define __counted_by(f) __attribute__((counted_by(f)))
diff --git a/clang/test/Sema/attr-counted-by-or-null-struct-ptrs.c b/clang/test/Sema/attr-counted-by-or-null-struct-ptrs.c
index 0bb09059c97f9..7fe43a20c0c6e 100644
--- a/clang/test/Sema/attr-counted-by-or-null-struct-ptrs.c
+++ b/clang/test/Sema/attr-counted-by-or-null-struct-ptrs.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fexperimental-late-parse-attributes -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c11 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c11 -fexperimental-late-parse-attributes -fsyntax-only -verify %s
#define __counted_by_or_null(f) __attribute__((counted_by_or_null(f)))
#define __counted_by(f) __attribute__((counted_by(f)))
diff --git a/clang/test/Sema/attr-counted-by-struct-ptrs.c b/clang/test/Sema/attr-counted-by-struct-ptrs.c
index c05d18262e2b7..d75533d7d135b 100644
--- a/clang/test/Sema/attr-counted-by-struct-ptrs.c
+++ b/clang/test/Sema/attr-counted-by-struct-ptrs.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fsyntax-only -fexperimental-late-parse-attributes %s -verify
+// RUN: %clang_cc1 -std=c11 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c11 -fsyntax-only -fexperimental-late-parse-attributes %s -verify
#define __counted_by(f) __attribute__((counted_by(f)))
diff --git a/clang/test/Sema/attr-counted-by-void-ptr-gnu.c b/clang/test/Sema/attr-counted-by-void-ptr-gnu.c
new file mode 100644
index 0000000000000..c6aa371434bc4
--- /dev/null
+++ b/clang/test/Sema/attr-counted-by-void-ptr-gnu.c
@@ -0,0 +1,65 @@
+// RUN: %clang_cc1 -std=gnu11 -fsyntax-only -verify=expected-nowarn %s
+// RUN: %clang_cc1 -std=gnu11 -Wpointer-arith -fsyntax-only -verify=expected-warn %s
+// RUN: %clang_cc1 -std=c11 -fsyntax-only -verify=expected-strict %s
+
+// expected-nowarn-no-diagnostics
+
+#define __counted_by(f) __attribute__((counted_by(f)))
+#define __counted_by_or_null(f) __attribute__((counted_by_or_null(f)))
+#define __sized_by(f) __attribute__((sized_by(f)))
+
+//==============================================================================
+// Test: counted_by on void* is allowed in GNU mode, rejected in strict mode
+//==============================================================================
+
+struct test_void_ptr_gnu {
+ int count;
+ // expected-warn-warning@+2{{'counted_by' on a pointer to void is a GNU extension, treated as 'sized_by'}}
+ // expected-strict-error@+1{{'counted_by' cannot be applied to a pointer with pointee of unknown size because 'void' is an incomplete type}}
+ void* buf __counted_by(count);
+};
+
+struct test_const_void_ptr_gnu {
+ int count;
+ // expected-warn-warning@+2{{'counted_by' on a pointer to void is a GNU extension, treated as 'sized_by'}}
+ // expected-strict-error@+1{{'counted_by' cannot be applied to a pointer with pointee of unknown size because 'const void' is an incomplete type}}
+ const void* buf __counted_by(count);
+};
+
+struct test_volatile_void_ptr_gnu {
+ int count;
+ // expected-warn-warning@+2{{'counted_by' on a pointer to void is a GNU extension, treated as 'sized_by'}}
+ // expected-strict-error@+1{{'counted_by' cannot be applied to a pointer with pointee of unknown size because 'volatile void' is an incomplete type}}
+ volatile void* buf __counted_by(count);
+};
+
+struct test_const_volatile_void_ptr_gnu {
+ int count;
+ // expected-warn-warning@+2{{'counted_by' on a pointer to void is a GNU extension, treated as 'sized_by'}}
+ // expected-strict-error@+1{{'counted_by' cannot be applied to a pointer with pointee of unknown size because 'const volatile void' is an incomplete type}}
+ const volatile void* buf __counted_by(count);
+};
+
+// Verify sized_by still works the same way (always allowed, no warning)
+struct test_sized_by_void_ptr {
+ int size;
+ void* buf __sized_by(size); // OK in both modes, no warning
+};
+
+//==============================================================================
+// Test: counted_by_or_null on void* behaves the same
+//==============================================================================
+
+struct test_void_ptr_or_null_gnu {
+ int count;
+ // expected-warn-warning@+2{{'counted_by_or_null' on a pointer to void is a GNU extension, treated as 'sized_by_or_null'}}
+ // expected-strict-error@+1{{'counted_by_or_null' cannot be applied to a pointer with pointee of unknown size because 'void' is an incomplete type}}
+ void* buf __counted_by_or_null(count);
+};
+
+struct test_const_void_ptr_or_null_gnu {
+ int count;
+ // expected-warn-warning@+2{{'counted_by_or_null' on a pointer to void is a GNU extension, treated as 'sized_by_or_null'}}
+ // expected-strict-error@+1{{'counted_by_or_null' cannot be applied to a pointer with pointee of unknown size because 'const void' is an incomplete type}}
+ const void* buf __counted_by_or_null(count);
+};
|
delcypher
added
the
clang:bounds-safety
|
I think this particular gcc feature falls into "this is a hazard rather than a feature" - it feels like if people want to use a pointer as a byte array they should simply convert the type to an appropriate type. |
|
I'm not huge fan of diverging the behavior of the That being said, as this is a GNU extension and presumably GCC already implemented this we probably need to allow this in order to be source compatible. |
|
What happens with this? Previously this was completely disallowed and when using the GNU language mode it will be. I don't think you're testing this. @yeoul We probably need to check what happens when we lift this restriction in |
| bool IsVoidPtrInGNUMode = PointeeTy->isVoidType() && getLangOpts().GNUMode; | ||
| if (IsVoidPtrInGNUMode) { | ||
| // Emit a warning that this is a GNU extension | ||
| Diag(FD->getBeginLoc(), diag::ext_gnu_counted_by_void_ptr) << Kind; |
There was a problem hiding this comment.
We should emit a note diagnostic here that suggests using __sized_by (or __sized_by_or_null) to remove this warning. Ideally we should add a fix-it to that too but that can be quite painful to implement because we don't have standardized way of writing the attributes (e.g. raw GNU attribute vs macro).
There was a problem hiding this comment.
I have it say what it is being interpreted as. What language would you prefer in here? I'm happy to swap it to whatever:
def ext_gnu_counted_by_void_ptr
: Extension<
"'%select{counted_by|sized_by|counted_by_or_null|sized_by_or_null}0' "
"on a pointer to void is a GNU extension, treated as "
"'%select{sized_by|sized_by|sized_by_or_null|sized_by_or_null}0'">,
InGroup<GNUPointerArith>;
| // Exception: In GNU mode, void has an implicit size of 1 byte for pointer | ||
| // arithmetic. Therefore, counted_by on void* is allowed as a GNU extension | ||
| // and behaves equivalently to sized_by (treating the count as bytes). | ||
| bool IsVoidPtrInGNUMode = PointeeTy->isVoidType() && getLangOpts().GNUMode; |
There was a problem hiding this comment.
@rapidsna What do you think about changing this to
bool IsVoidPtrInGNUMode = PointeeTy->isVoidType() && getLangOpts().GNUMode && !getLangOpts().BoundsSafety;
?
This would mean in the -fbounds-safety language model we would still disallow this. Or for consistency with -fno-bounds-safety do you think we should allow this language extension?
There was a problem hiding this comment.
Why does -fbounds-safety need this? I'd like to be bringing -fbounds-safety incrementally to Linux...
This is exactly what does happen, but it's usually as a generic allocation space, and the type usage comes later via implicit casts. For |
I don't understand what you mean? This builds with gnu and not with C11. I have tests in this PR for it? I must be misunderstanding something. $ clang -std=gnu11 -o wat.o -c wat.c
$ clang -std=c11 -o wat.o -c wat.c
wat.c:4:5: error: 'counted_by' cannot be applied to a pointer with pointee of
unknown size because 'void' is an incomplete type
4 | void* __counted_by(count) ptr;
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated. |
I didn't quite get the context for why this is the case - maybe I missed something. If a user can write: why would they not be able to instead write this? |
| @@ -1,4 +1,4 @@ | |||
| // RUN: %clang_cc1 -fexperimental-late-parse-attributes -fsyntax-only -verify %s | |||
| // RUN: %clang_cc1 -std=c11 -fexperimental-late-parse-attributes -fsyntax-only -verify %s | |||
There was a problem hiding this comment.
What happens if -std=c11 is left unspecified?
There was a problem hiding this comment.
Clang defaults to GNU mode AFAICT, so the negative tests fail.
Please know that I'm not trying to be difficult with this reply; I'm just trying to state the reality of the situation: because they(we) just will not. The Linux developer community is extraordinarily opinionated about these kinds of things and from their(our) perspective, Since I have no preprocessor way to interrogate the type of the member when applying an attribute, I have no way to make Linux's Alternatively, if there were some other attribute name (strawman: I just need to find a way to keep both camps happy: both have legitimate reasons for why they want things the way they do, so I just need a knob to select between them (without compromising either side's implementation details). |
I.e. the only logic change (i.e. keep all the tests) needed would be this: diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 964a2a791e18..c97ec9cea0c6 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -6577,6 +6577,20 @@ static void handleCountedByAttrField(Sema &S, Decl *D, const ParsedAttr &AL) {
llvm_unreachable("unexpected counted_by family attribute");
}
+ // In GNU mode, silently convert counted_by/counted_by_or_null on void*
+ // to sized_by/sized_by_or_null, since void has size 1 in GNU pointer
+ // arithmetic.
+ if (!CountInBytes && FD->getType()->isPointerType()) {
+ QualType PointeeTy = FD->getType()->getPointeeType();
+ if (PointeeTy->isVoidType() && S.getLangOpts().GNUMode) {
+ // Emit diagnostic before conversion
+ S.Diag(FD->getBeginLoc(), diag::ext_gnu_counted_by_void_ptr)
+ << (OrNull ? CountAttributedType::CountedByOrNull
+ : CountAttributedType::CountedBy);
+ CountInBytes = true; // Convert to sized_by variant
+ }
+ }
+
if (S.CheckCountedByAttrOnField(FD, CountExpr, CountInBytes, OrNull))
return;
This is, perhaps, much cleaner? |
Interesting... I wonder what their reaction to
I'm not personally opposed to allowing As an potential alternative I think you might be able to do something this for pointers (but not arrays): |
That seems a lot easier to maintain! You may have to live with diagnostics saying |
|
Weirdly, I think a mismatched diagnostic string would be much more well tolerated by the Linux dev community. :) That kind of thing I can document along with the |
5 participants
The counted_by attribute currently rejects void* members because void has no defined size. However, the sized_by attribute accepts void* since it explicitly measures bytes. In GNU mode, void pointer arithmetic treats void as having size 1 byte, so counted_by on void* should behave identically to sized_by (treating the count as bytes).
Allow counted_by on void* when GNU extensions are enabled. The implementation validates this only at declaration time in SemaBoundsSafety.cpp, emitting a -Wpointer-arith warning that the attribute is treated as a GNU extension equivalent to sized_by. Both use-site validation and code generation trust this earlier validation, avoiding redundant checks.
In CodeGen, __builtin_dynamic_object_size now correctly handles counted_by on void* by treating any CountAttributedType with zero element size as having 1-byte elements, matching the GNU void pointer arithmetic semantics.
Add tests validating both Sema diagnostics (warnings in GNU mode, errors in strict C) and CodeGen behavior (correct byte counts from __builtin_dynamic_object_size). Update existing counted_by tests to explicitly use -std=c11 to preserve their original intent of rejecting void* in strict C mode.