Skip to content

Commit bcc39cf

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 (llvm#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 6b60f43 commit bcc39cf

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
@@ -6563,7 +6563,7 @@ def err_counted_by_attr_refer_to_union : Error<
65636563
def note_flexible_array_counted_by_attr_field : Note<
65646564
"field %0 declared here">;
65656565
def err_counted_by_attr_pointee_unknown_size : Error<
6566-
"'counted_by' cannot be applied to %select{"
6566+
"'counted_by' %select{cannot|should not}3 be applied to %select{"
65676567
"a pointer with pointee|" // pointer
65686568
"an array with element}0" // array
65696569
" of unknown size because %1 is %select{"
@@ -6572,8 +6572,14 @@ def err_counted_by_attr_pointee_unknown_size : Error<
65726572
"a function type|" // CountedByInvalidPointeeTypeKind::FUNCTION
65736573
// CountedByInvalidPointeeTypeKind::FLEXIBLE_ARRAY_MEMBER
65746574
"a struct type with a flexible array member"
6575+
"%select{|. This will be an error in a future compiler version}3"
6576+
""
65756577
"}2">;
65766578

6579+
def warn_counted_by_attr_elt_type_unknown_size :
6580+
Warning<err_counted_by_attr_pointee_unknown_size.Summary>,
6581+
InGroup<BoundsSafetyCountedByEltTyUnknownSize>;
6582+
65776583
let CategoryName = "ARC Semantic Issue" in {
65786584

65796585
// ARC-mode diagnostics.

clang/lib/Sema/SemaDeclAttr.cpp

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8717,20 +8717,33 @@ static bool CheckCountedByAttrOnField(
87178717
// Note: The `Decl::isFlexibleArrayMemberLike` check earlier on means
87188718
// only `PointeeTy->isStructureTypeWithFlexibleArrayMember()` is reachable
87198719
// when `FieldTy->isArrayType()`.
8720+
bool ShouldWarn = false;
87208721
if (PointeeTy->isIncompleteType()) {
87218722
InvalidTypeKind = CountedByInvalidPointeeTypeKind::INCOMPLETE;
87228723
} else if (PointeeTy->isSizelessType()) {
87238724
InvalidTypeKind = CountedByInvalidPointeeTypeKind::SIZELESS;
87248725
} else if (PointeeTy->isFunctionType()) {
87258726
InvalidTypeKind = CountedByInvalidPointeeTypeKind::FUNCTION;
87268727
} else if (PointeeTy->isStructureTypeWithFlexibleArrayMember()) {
8728+
if (FieldTy->isArrayType()) {
8729+
// This is a workaround for the Linux kernel that has already adopted
8730+
// `counted_by` on a FAM where the pointee is a struct with a FAM. This
8731+
// should be an error because computing the bounds of the array cannot be
8732+
// done correctly without manually traversing every struct object in the
8733+
// array at runtime. To allow the code to be built this error is
8734+
// downgraded to a warning.
8735+
ShouldWarn = true;
8736+
}
87278737
InvalidTypeKind = CountedByInvalidPointeeTypeKind::FLEXIBLE_ARRAY_MEMBER;
87288738
}
87298739

87308740
if (InvalidTypeKind != CountedByInvalidPointeeTypeKind::VALID) {
8731-
S.Diag(FD->getBeginLoc(), diag::err_counted_by_attr_pointee_unknown_size)
8741+
unsigned DiagID = ShouldWarn
8742+
? diag::warn_counted_by_attr_elt_type_unknown_size
8743+
: diag::err_counted_by_attr_pointee_unknown_size;
8744+
S.Diag(FD->getBeginLoc(), DiagID)
87328745
<< SelectPtrOrArr << PointeeTy << (int)InvalidTypeKind
8733-
<< FD->getSourceRange();
8746+
<< (ShouldWarn ? 1 : 0) << FD->getSourceRange();
87348747
return true;
87358748
}
87368749

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)