Skip to content

Commit cef6387

Browse files
committed
[Bounds-Safety] Temporarily relax a counted_by attribute restriction on flexible array members
In 0ec3b97 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 (#90786 (comment)). 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.
1 parent b80e0fb commit cef6387

File tree

4 files changed

+32
-6
lines changed

4 files changed

+32
-6
lines changed

clang/include/clang/Basic/DiagnosticGroups.td

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1447,6 +1447,10 @@ def FunctionMultiVersioning
14471447

14481448
def NoDeref : DiagGroup<"noderef">;
14491449

1450+
// -fbounds-safety and bounds annotation related warnings
1451+
def BoundsSafetyCountedByEltTyUnknownSize :
1452+
DiagGroup<"bounds-safety-counted-by-elt-type-unknown-size">;
1453+
14501454
// A group for cross translation unit static analysis related warnings.
14511455
def CrossTU : DiagGroup<"ctu">;
14521456

clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6552,7 +6552,7 @@ def err_counted_by_attr_refer_to_union : Error<
65526552
def note_flexible_array_counted_by_attr_field : Note<
65536553
"field %0 declared here">;
65546554
def err_counted_by_attr_pointee_unknown_size : Error<
6555-
"'counted_by' cannot be applied to %select{"
6555+
"'counted_by' %select{cannot|should not}3 be applied to %select{"
65566556
"a pointer with pointee|" // pointer
65576557
"an array with element}0" // array
65586558
" of unknown size because %1 is %select{"
@@ -6561,8 +6561,14 @@ def err_counted_by_attr_pointee_unknown_size : Error<
65616561
"a function type|" // CountedByInvalidPointeeTypeKind::FUNCTION
65626562
// CountedByInvalidPointeeTypeKind::FLEXIBLE_ARRAY_MEMBER
65636563
"a struct type with a flexible array member"
6564+
"%select{|. This will be an error in a future compiler version}3"
6565+
""
65646566
"}2">;
65656567

6568+
def warn_counted_by_attr_elt_type_unknown_size :
6569+
Warning<err_counted_by_attr_pointee_unknown_size.Summary>,
6570+
InGroup<BoundsSafetyCountedByEltTyUnknownSize>;
6571+
65666572
let CategoryName = "ARC Semantic Issue" in {
65676573

65686574
// ARC-mode diagnostics.

clang/lib/Sema/SemaDeclAttr.cpp

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8687,20 +8687,33 @@ static bool CheckCountedByAttrOnField(
86878687
// Note: The `Decl::isFlexibleArrayMemberLike` check earlier on means
86888688
// only `PointeeTy->isStructureTypeWithFlexibleArrayMember()` is reachable
86898689
// when `FieldTy->isArrayType()`.
8690+
bool ShouldWarn = false;
86908691
if (PointeeTy->isIncompleteType()) {
86918692
InvalidTypeKind = CountedByInvalidPointeeTypeKind::INCOMPLETE;
86928693
} else if (PointeeTy->isSizelessType()) {
86938694
InvalidTypeKind = CountedByInvalidPointeeTypeKind::SIZELESS;
86948695
} else if (PointeeTy->isFunctionType()) {
86958696
InvalidTypeKind = CountedByInvalidPointeeTypeKind::FUNCTION;
86968697
} else if (PointeeTy->isStructureTypeWithFlexibleArrayMember()) {
8698+
if (FieldTy->isArrayType()) {
8699+
// This is a workaround for the Linux kernel that has already adopted
8700+
// `counted_by` on a FAM where the pointee is a struct with a FAM. This
8701+
// should be an error because computing the bounds of the array cannot be
8702+
// done correctly without manually traversing every struct object in the
8703+
// array at runtime. To allow the code to be built this error is
8704+
// downgraded to a warning.
8705+
ShouldWarn = true;
8706+
}
86978707
InvalidTypeKind = CountedByInvalidPointeeTypeKind::FLEXIBLE_ARRAY_MEMBER;
86988708
}
86998709

87008710
if (InvalidTypeKind != CountedByInvalidPointeeTypeKind::VALID) {
8701-
S.Diag(FD->getBeginLoc(), diag::err_counted_by_attr_pointee_unknown_size)
8711+
unsigned DiagID = ShouldWarn
8712+
? diag::warn_counted_by_attr_elt_type_unknown_size
8713+
: diag::err_counted_by_attr_pointee_unknown_size;
8714+
S.Diag(FD->getBeginLoc(), DiagID)
87028715
<< SelectPtrOrArr << PointeeTy << (int)InvalidTypeKind
8703-
<< FD->getSourceRange();
8716+
<< (ShouldWarn ? 1 : 0) << FD->getSourceRange();
87048717
return true;
87058718
}
87068719

clang/test/Sema/attr-counted-by-vla.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -173,21 +173,24 @@ struct has_annotated_VLA {
173173

174174
struct buffer_of_structs_with_unnannotated_vla {
175175
int count;
176-
// expected-error@+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}}
176+
// Treating this as a warning is a temporary fix for existing attribute adopters. It **SHOULD BE AN ERROR**.
177+
// expected-warning@+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}}
177178
struct has_unannotated_VLA Arr[] __counted_by(count);
178179
};
179180

180181

181182
struct buffer_of_structs_with_annotated_vla {
182183
int count;
183-
// expected-error@+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}}
184+
// Treating this as a warning is a temporary fix for existing attribute adopters. It **SHOULD BE AN ERROR**.
185+
// expected-warning@+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}}
184186
struct has_annotated_VLA Arr[] __counted_by(count);
185187
};
186188

187189
struct buffer_of_const_structs_with_annotated_vla {
188190
int count;
191+
// Treating this as a warning is a temporary fix for existing attribute adopters. It **SHOULD BE AN ERROR**.
189192
// Make sure the `const` qualifier is printed when printing the element type.
190-
// expected-error@+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}}
193+
// expected-warning@+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}}
191194
const struct has_annotated_VLA Arr[] __counted_by(count);
192195
};
193196

0 commit comments

Comments
 (0)