[clang] cef6387 - [Bounds-Safety] Temporarily relax a `counted_by` attribute restriction on flexible array members
Dan Liew via cfe-commits
cfe-commits at lists.llvm.org
Fri May 17 16:24:14 PDT 2024
Author: Dan Liew
Date: 2024-05-17T16:23:24-07:00
New Revision: cef6387e52578366c2332275dad88b9953b55336
URL: https://github.com/llvm/llvm-project/commit/cef6387e52578366c2332275dad88b9953b55336
DIFF: https://github.com/llvm/llvm-project/commit/cef6387e52578366c2332275dad88b9953b55336.diff
LOG: [Bounds-Safety] Temporarily relax a `counted_by` attribute restriction on flexible array members
In 0ec3b972e58bcbcdc1bebe1696ea37f2931287c3 an additional restriction
was added when applying the `counted_by` attribute to flexible array
members in structs. The restriction prevented the element type being
a struct that itself had a flexible array member. E.g.:
```
struct has_unannotated_VLA {
int count;
char buffer[];
};
struct buffer_of_structs_with_unnannotated_vla {
int count;
struct has_unannotated_VLA Arr[] __counted_by(count);
};
```
In this example assuming the size of `Arr` is `sizeof(struct
has_unannotated_VLA)*count` (which is what the attribute says) is wrong
because it doesn't account for the size of
`has_unannotated_VLA::buffer`. This is why this kind of code construct
was treated as an error.
However, it turns out existing Linux kernel code used the attribute
on a flexible array member in this way
(https://github.com/llvm/llvm-project/pull/90786#issuecomment-2118416515).
To unbreak the build this restriction is downgraded to a warning with
the plan to make it an error again once the errornous use of the
attribute in the Linux kernel is resolved.
Added:
Modified:
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/Sema/attr-counted-by-vla.c
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 4cb4f3d999f7a..4fad4d1a0eca7 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -1447,6 +1447,10 @@ def FunctionMultiVersioning
def NoDeref : DiagGroup<"noderef">;
+// -fbounds-safety and bounds annotation related warnings
+def BoundsSafetyCountedByEltTyUnknownSize :
+ DiagGroup<"bounds-safety-counted-by-elt-type-unknown-size">;
+
// A group for cross translation unit static analysis related warnings.
def CrossTU : DiagGroup<"ctu">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 8e6596410c5d0..1efa3af121c10 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6552,7 +6552,7 @@ def err_counted_by_attr_refer_to_union : Error<
def note_flexible_array_counted_by_attr_field : Note<
"field %0 declared here">;
def err_counted_by_attr_pointee_unknown_size : Error<
- "'counted_by' cannot be applied to %select{"
+ "'counted_by' %select{cannot|should not}3 be applied to %select{"
"a pointer with pointee|" // pointer
"an array with element}0" // array
" of unknown size because %1 is %select{"
@@ -6561,8 +6561,14 @@ def err_counted_by_attr_pointee_unknown_size : Error<
"a function type|" // CountedByInvalidPointeeTypeKind::FUNCTION
// CountedByInvalidPointeeTypeKind::FLEXIBLE_ARRAY_MEMBER
"a struct type with a flexible array member"
+ "%select{|. This will be an error in a future compiler version}3"
+ ""
"}2">;
+def warn_counted_by_attr_elt_type_unknown_size :
+ Warning<err_counted_by_attr_pointee_unknown_size.Summary>,
+ InGroup<BoundsSafetyCountedByEltTyUnknownSize>;
+
let CategoryName = "ARC Semantic Issue" in {
// ARC-mode diagnostics.
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index c8b71631076ba..e816ea3647a7c 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -8687,6 +8687,7 @@ static bool CheckCountedByAttrOnField(
// Note: The `Decl::isFlexibleArrayMemberLike` check earlier on means
// only `PointeeTy->isStructureTypeWithFlexibleArrayMember()` is reachable
// when `FieldTy->isArrayType()`.
+ bool ShouldWarn = false;
if (PointeeTy->isIncompleteType()) {
InvalidTypeKind = CountedByInvalidPointeeTypeKind::INCOMPLETE;
} else if (PointeeTy->isSizelessType()) {
@@ -8694,13 +8695,25 @@ static bool CheckCountedByAttrOnField(
} else if (PointeeTy->isFunctionType()) {
InvalidTypeKind = CountedByInvalidPointeeTypeKind::FUNCTION;
} else if (PointeeTy->isStructureTypeWithFlexibleArrayMember()) {
+ if (FieldTy->isArrayType()) {
+ // This is a workaround for the Linux kernel that has already adopted
+ // `counted_by` on a FAM where the pointee is a struct with a FAM. This
+ // should be an error because computing the bounds of the array cannot be
+ // done correctly without manually traversing every struct object in the
+ // array at runtime. To allow the code to be built this error is
+ // downgraded to a warning.
+ ShouldWarn = true;
+ }
InvalidTypeKind = CountedByInvalidPointeeTypeKind::FLEXIBLE_ARRAY_MEMBER;
}
if (InvalidTypeKind != CountedByInvalidPointeeTypeKind::VALID) {
- S.Diag(FD->getBeginLoc(), diag::err_counted_by_attr_pointee_unknown_size)
+ unsigned DiagID = ShouldWarn
+ ? diag::warn_counted_by_attr_elt_type_unknown_size
+ : diag::err_counted_by_attr_pointee_unknown_size;
+ S.Diag(FD->getBeginLoc(), DiagID)
<< SelectPtrOrArr << PointeeTy << (int)InvalidTypeKind
- << FD->getSourceRange();
+ << (ShouldWarn ? 1 : 0) << FD->getSourceRange();
return true;
}
diff --git a/clang/test/Sema/attr-counted-by-vla.c b/clang/test/Sema/attr-counted-by-vla.c
index 3de6bd55e2d8e..b25f719f3b95a 100644
--- a/clang/test/Sema/attr-counted-by-vla.c
+++ b/clang/test/Sema/attr-counted-by-vla.c
@@ -173,21 +173,24 @@ struct has_annotated_VLA {
struct buffer_of_structs_with_unnannotated_vla {
int count;
- // expected-error at +1{{'counted_by' cannot be applied to an array with element of unknown size because 'struct has_unannotated_VLA' is a struct type with a flexible array member}}
+ // Treating this as a warning is a temporary fix for existing attribute adopters. It **SHOULD BE AN ERROR**.
+ // expected-warning at +1{{'counted_by' should not be applied to an array with element of unknown size because 'struct has_unannotated_VLA' is a struct type with a flexible array member. This will be an error in a future compiler version}}
struct has_unannotated_VLA Arr[] __counted_by(count);
};
struct buffer_of_structs_with_annotated_vla {
int count;
- // expected-error at +1{{'counted_by' cannot be applied to an array with element of unknown size because 'struct has_annotated_VLA' is a struct type with a flexible array member}}
+ // Treating this as a warning is a temporary fix for existing attribute adopters. It **SHOULD BE AN ERROR**.
+ // expected-warning at +1{{'counted_by' should not be applied to an array with element of unknown size because 'struct has_annotated_VLA' is a struct type with a flexible array member. This will be an error in a future compiler version}}
struct has_annotated_VLA Arr[] __counted_by(count);
};
struct buffer_of_const_structs_with_annotated_vla {
int count;
+ // Treating this as a warning is a temporary fix for existing attribute adopters. It **SHOULD BE AN ERROR**.
// Make sure the `const` qualifier is printed when printing the element type.
- // expected-error at +1{{'counted_by' cannot be applied to an array with element of unknown size because 'const struct has_annotated_VLA' is a struct type with a flexible array member}}
+ // expected-warning at +1{{'counted_by' should not be applied to an array with element of unknown size because 'const struct has_annotated_VLA' is a struct type with a flexible array member. This will be an error in a future compiler version}}
const struct has_annotated_VLA Arr[] __counted_by(count);
};
More information about the cfe-commits
mailing list