[Bounds-Safety] Temporarily relax a counted_by attribute restriction on flexible array members

In 0ec3b972e5 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.
This commit is contained in:
Dan Liew
2024-05-17 16:12:31 -07:00
parent b80e0fbcfb
commit cef6387e52
4 changed files with 32 additions and 6 deletions

View File

@@ -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">;

View File

@@ -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.

View File

@@ -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;
}

View File

@@ -173,21 +173,24 @@ struct has_annotated_VLA {
struct buffer_of_structs_with_unnannotated_vla {
int count;
// 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}}
// Treating this as a warning is a temporary fix for existing attribute adopters. It **SHOULD BE AN ERROR**.
// 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}}
struct has_unannotated_VLA Arr[] __counted_by(count);
};
struct buffer_of_structs_with_annotated_vla {
int count;
// 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}}
// Treating this as a warning is a temporary fix for existing attribute adopters. It **SHOULD BE AN ERROR**.
// 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}}
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@+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@+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);
};