[clang] [Clang][counted_by] Don't treat a __bdos argument as an array if it isn't (PR #125298)
Bill Wendling via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 31 16:19:55 PST 2025
https://github.com/bwendling updated https://github.com/llvm/llvm-project/pull/125298
>From df6b80c82f1a9ce4f1eef580f008c86fd691ba71 Mon Sep 17 00:00:00 2001
From: Bill Wendling <morbo at google.com>
Date: Fri, 31 Jan 2025 12:48:36 -0800
Subject: [PATCH 1/3] [Clang][counted_by] Don't treat a pointer as an array
If the __bdos field isn't an array, don't process it. We'll default to
using the llvm.objectsize intrinsic.
---
clang/lib/CodeGen/CGBuiltin.cpp | 30 ++++++++++++++++++++++++------
1 file changed, 24 insertions(+), 6 deletions(-)
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 11fa295dad9524..21faf85a16e2d2 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -1319,12 +1319,36 @@ CodeGenFunction::emitCountedByMemberSize(const Expr *E, llvm::Value *EmittedE,
// size_t field_offset = offsetof (struct s, field);
Value *FieldOffset = nullptr;
+ llvm::ConstantInt *FieldBaseSize = nullptr;
if (FlexibleArrayMemberFD != FD) {
std::optional<int64_t> Offset = GetFieldOffset(Ctx, RD, FD);
if (!Offset)
return nullptr;
FieldOffset =
llvm::ConstantInt::get(ResType, *Offset / CharWidth, IsSigned);
+
+ if (Idx) {
+ // From option (4):
+ // size_t field_base_size = sizeof (*ptr->field_array);
+ if (!FieldTy->isArrayType())
+ // The field isn't an array. For example:
+ //
+ // struct {
+ // int count;
+ // char *string;
+ // int array[] __counted_by(count);
+ // } x;
+ //
+ // __builtin_dynamic_object_size(x.string[42], 0);
+ //
+ // If built with '-Wno-int-conversion', FieldTy won't be an array here.
+ return nullptr;
+
+ const ArrayType *ArrayTy = Ctx.getAsArrayType(FieldTy);
+ CharUnits BaseSize = Ctx.getTypeSizeInChars(ArrayTy->getElementType());
+ FieldBaseSize =
+ llvm::ConstantInt::get(ResType, BaseSize.getQuantity(), IsSigned);
+ }
}
// size_t count = (size_t) ptr->count;
@@ -1376,12 +1400,6 @@ CodeGenFunction::emitCountedByMemberSize(const Expr *E, llvm::Value *EmittedE,
llvm::ConstantInt::get(ResType, Size.getKnownMinValue() / CharWidth);
if (Idx) { // Option (4) '&ptr->field_array[idx]'
- // size_t field_base_size = sizeof (*ptr->field_array);
- const ArrayType *ArrayTy = Ctx.getAsArrayType(FieldTy);
- CharUnits BaseSize = Ctx.getTypeSizeInChars(ArrayTy->getElementType());
- auto *FieldBaseSize =
- llvm::ConstantInt::get(ResType, BaseSize.getQuantity(), IsSigned);
-
// field_offset += index * field_base_size;
Value *Mul = Builder.CreateMul(Index, FieldBaseSize, "field_offset",
!IsSigned, IsSigned);
>From c79f5a6f828be21910fb8a86b835474206786185 Mon Sep 17 00:00:00 2001
From: Bill Wendling <morbo at google.com>
Date: Fri, 31 Jan 2025 16:05:18 -0800
Subject: [PATCH 2/3] Add testcases.
---
clang/test/CodeGen/attr-counted-by-bug.c | 85 ++++++++++++++++++++++++
1 file changed, 85 insertions(+)
create mode 100644 clang/test/CodeGen/attr-counted-by-bug.c
diff --git a/clang/test/CodeGen/attr-counted-by-bug.c b/clang/test/CodeGen/attr-counted-by-bug.c
new file mode 100644
index 00000000000000..6cf1e3cbbdc001
--- /dev/null
+++ b/clang/test/CodeGen/attr-counted-by-bug.c
@@ -0,0 +1,85 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 5
+// RUN: %clang_cc1 -triple aarch64-unknown-linux-gnu -DCOUNTED_BY -O2 -Wall -Wno-int-conversion -fsanitize=array-bounds,object-size,local-bounds -fstrict-flex-arrays=3 -emit-llvm -o - %s | FileCheck --check-prefix=SANITIZE-WITH-ATTR %s
+// RUN: %clang_cc1 -triple aarch64-unknown-linux-gnu -DCOUNTED_BY -O2 -Wall -Wno-int-conversion -fsanitize=array-bounds,object-size,local-bounds -fstrict-flex-arrays=3 -emit-llvm -o - %s | FileCheck --check-prefix=NO-SANITIZE-WITH-ATTR %s
+// RUN: %clang_cc1 -triple aarch64-unknown-linux-gnu -O2 -Wall -Wno-int-conversion -fsanitize=array-bounds,object-size,local-bounds -fstrict-flex-arrays=3 -emit-llvm -o - %s | FileCheck --check-prefix=SANITIZE-WITHOUT-ATTR %s
+// RUN: %clang_cc1 -triple aarch64-unknown-linux-gnu -O2 -Wall -Wno-int-conversion -fsanitize=array-bounds,object-size,local-bounds -fstrict-flex-arrays=3 -emit-llvm -o - %s | FileCheck --check-prefix=NO-SANITIZE-WITHOUT-ATTR %s
+
+// See https://github.com/llvm/llvm-project/pull/122198#issuecomment-2627868702
+
+#if !__has_attribute(counted_by)
+#error "has attribute broken"
+#endif
+
+#ifdef COUNTED_BY
+#define __counted_by(member) __attribute__((__counted_by__(member)))
+#else
+#define __counted_by(member)
+#endif
+
+#define __bdos(P) __builtin_dynamic_object_size(P, 0)
+
+typedef long unsigned int size_t;
+
+struct test1_struct {
+ int a;
+ char *b;
+ char c[] __counted_by(a);
+} d;
+
+// SANITIZE-WITH-ATTR-LABEL: define dso_local i64 @test1(
+// SANITIZE-WITH-ATTR-SAME: ) local_unnamed_addr #[[ATTR0:[0-9]+]] {
+// SANITIZE-WITH-ATTR-NEXT: [[ENTRY:.*:]]
+// SANITIZE-WITH-ATTR-NEXT: ret i64 -1
+//
+// NO-SANITIZE-WITH-ATTR-LABEL: define dso_local i64 @test1(
+// NO-SANITIZE-WITH-ATTR-SAME: ) local_unnamed_addr #[[ATTR0:[0-9]+]] {
+// NO-SANITIZE-WITH-ATTR-NEXT: [[ENTRY:.*:]]
+// NO-SANITIZE-WITH-ATTR-NEXT: ret i64 -1
+//
+// SANITIZE-WITHOUT-ATTR-LABEL: define dso_local i64 @test1(
+// SANITIZE-WITHOUT-ATTR-SAME: ) local_unnamed_addr #[[ATTR0:[0-9]+]] {
+// SANITIZE-WITHOUT-ATTR-NEXT: [[ENTRY:.*:]]
+// SANITIZE-WITHOUT-ATTR-NEXT: ret i64 -1
+//
+// NO-SANITIZE-WITHOUT-ATTR-LABEL: define dso_local i64 @test1(
+// NO-SANITIZE-WITHOUT-ATTR-SAME: ) local_unnamed_addr #[[ATTR0:[0-9]+]] {
+// NO-SANITIZE-WITHOUT-ATTR-NEXT: [[ENTRY:.*:]]
+// NO-SANITIZE-WITHOUT-ATTR-NEXT: ret i64 -1
+//
+size_t test1(void) {
+ return __builtin_dynamic_object_size(d.b[4], 0);
+}
+
+typedef struct {
+ char __padding[0];
+} spinlock_t;
+struct {
+ int priv_len;
+ spinlock_t addr_list_lock;
+ char *dev_addr;
+ char priv[] __attribute__((__counted_by__(priv_len)));
+} x;
+
+// SANITIZE-WITH-ATTR-LABEL: define dso_local i64 @falcon_reconfigure_xmac_core(
+// SANITIZE-WITH-ATTR-SAME: ) local_unnamed_addr #[[ATTR1:[0-9]+]] {
+// SANITIZE-WITH-ATTR-NEXT: [[ENTRY:.*:]]
+// SANITIZE-WITH-ATTR-NEXT: ret i64 -1
+//
+// NO-SANITIZE-WITH-ATTR-LABEL: define dso_local i64 @falcon_reconfigure_xmac_core(
+// NO-SANITIZE-WITH-ATTR-SAME: ) local_unnamed_addr #[[ATTR1:[0-9]+]] {
+// NO-SANITIZE-WITH-ATTR-NEXT: [[ENTRY:.*:]]
+// NO-SANITIZE-WITH-ATTR-NEXT: ret i64 -1
+//
+// SANITIZE-WITHOUT-ATTR-LABEL: define dso_local i64 @falcon_reconfigure_xmac_core(
+// SANITIZE-WITHOUT-ATTR-SAME: ) local_unnamed_addr #[[ATTR1:[0-9]+]] {
+// SANITIZE-WITHOUT-ATTR-NEXT: [[ENTRY:.*:]]
+// SANITIZE-WITHOUT-ATTR-NEXT: ret i64 -1
+//
+// NO-SANITIZE-WITHOUT-ATTR-LABEL: define dso_local i64 @falcon_reconfigure_xmac_core(
+// NO-SANITIZE-WITHOUT-ATTR-SAME: ) local_unnamed_addr #[[ATTR1:[0-9]+]] {
+// NO-SANITIZE-WITHOUT-ATTR-NEXT: [[ENTRY:.*:]]
+// NO-SANITIZE-WITHOUT-ATTR-NEXT: ret i64 -1
+//
+size_t test2() {
+ return __builtin_dynamic_object_size(&x.dev_addr[4], 1);
+}
>From eb882a4e540d0896e805ec900c564c7a3a5a7b67 Mon Sep 17 00:00:00 2001
From: Bill Wendling <morbo at google.com>
Date: Fri, 31 Jan 2025 16:18:48 -0800
Subject: [PATCH 3/3] Clarify comment
---
clang/lib/CodeGen/CGBuiltin.cpp | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 21faf85a16e2d2..a57fc8109fa1a7 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -1331,7 +1331,7 @@ CodeGenFunction::emitCountedByMemberSize(const Expr *E, llvm::Value *EmittedE,
// From option (4):
// size_t field_base_size = sizeof (*ptr->field_array);
if (!FieldTy->isArrayType())
- // The field isn't an array. For example:
+ // The field isn't an array. It could be a pointer, for example:
//
// struct {
// int count;
@@ -1339,9 +1339,10 @@ CodeGenFunction::emitCountedByMemberSize(const Expr *E, llvm::Value *EmittedE,
// int array[] __counted_by(count);
// } x;
//
- // __builtin_dynamic_object_size(x.string[42], 0);
+ // __builtin_dynamic_object_size(&x.string[42], 0);
//
- // If built with '-Wno-int-conversion', FieldTy won't be an array here.
+ // This __bdos isn't wanting the size of the struct, but the size of
+ // 'string's allocation, which __bdos isn't currently able to provide.
return nullptr;
const ArrayType *ArrayTy = Ctx.getAsArrayType(FieldTy);
More information about the cfe-commits
mailing list