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@+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); }; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
