-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[Bounds-Safety] Add sized_by, counted_by_or_null & sized_by_or_null #93231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5868,6 +5868,15 @@ static const RecordDecl *GetEnclosingNamedOrTopAnonRecord(const FieldDecl *FD) { | |
| return RD; | ||
| } | ||
|
|
||
| static CountAttributedType::DynamicCountPointerKind | ||
| getCountAttrKind(bool CountInBytes, bool OrNull) { | ||
|
||
| if (CountInBytes) | ||
| return OrNull ? CountAttributedType::SizedByOrNull | ||
| : CountAttributedType::SizedBy; | ||
| return OrNull ? CountAttributedType::CountedByOrNull | ||
| : CountAttributedType::CountedBy; | ||
| } | ||
|
|
||
| enum class CountedByInvalidPointeeTypeKind { | ||
| INCOMPLETE, | ||
| SIZELESS, | ||
|
|
@@ -5876,22 +5885,31 @@ enum class CountedByInvalidPointeeTypeKind { | |
| VALID, | ||
| }; | ||
|
|
||
| static bool CheckCountedByAttrOnField( | ||
| Sema &S, FieldDecl *FD, Expr *E, | ||
| llvm::SmallVectorImpl<TypeCoupledDeclRefInfo> &Decls) { | ||
| static bool | ||
| CheckCountedByAttrOnField(Sema &S, FieldDecl *FD, Expr *E, | ||
| llvm::SmallVectorImpl<TypeCoupledDeclRefInfo> &Decls, | ||
| bool CountInBytes, bool OrNull) { | ||
| // Check the context the attribute is used in | ||
|
|
||
| unsigned Kind = getCountAttrKind(CountInBytes, OrNull); | ||
|
|
||
| if (FD->getParent()->isUnion()) { | ||
| S.Diag(FD->getBeginLoc(), diag::err_counted_by_attr_in_union) | ||
| << FD->getSourceRange(); | ||
| S.Diag(FD->getBeginLoc(), diag::err_count_attr_in_union) | ||
| << Kind << FD->getSourceRange(); | ||
| return true; | ||
| } | ||
|
|
||
| const auto FieldTy = FD->getType(); | ||
| if (FieldTy->isArrayType() && (CountInBytes || OrNull)) { | ||
| S.Diag(FD->getBeginLoc(), | ||
| diag::err_count_attr_not_on_ptr_or_flexible_array_member) | ||
| << Kind << FD->getLocation() << /* suggest counted_by */ 1; | ||
| return true; | ||
| } | ||
| if (!FieldTy->isArrayType() && !FieldTy->isPointerType()) { | ||
| S.Diag(FD->getBeginLoc(), | ||
| diag::err_counted_by_attr_not_on_ptr_or_flexible_array_member) | ||
| << FD->getLocation(); | ||
| diag::err_count_attr_not_on_ptr_or_flexible_array_member) | ||
| << Kind << FD->getLocation() << /* do not suggest counted_by */ 0; | ||
| return true; | ||
| } | ||
|
|
||
|
|
@@ -5902,7 +5920,7 @@ static bool CheckCountedByAttrOnField( | |
| StrictFlexArraysLevel, true)) { | ||
| S.Diag(FD->getBeginLoc(), | ||
| diag::err_counted_by_attr_on_array_not_flexible_array_member) | ||
| << FD->getLocation(); | ||
| << Kind << FD->getLocation(); | ||
| return true; | ||
| } | ||
|
|
||
|
|
@@ -5923,7 +5941,7 @@ static bool CheckCountedByAttrOnField( | |
| // only `PointeeTy->isStructureTypeWithFlexibleArrayMember()` is reachable | ||
| // when `FieldTy->isArrayType()`. | ||
| bool ShouldWarn = false; | ||
| if (PointeeTy->isIncompleteType()) { | ||
| if (PointeeTy->isIncompleteType() && !CountInBytes) { | ||
| InvalidTypeKind = CountedByInvalidPointeeTypeKind::INCOMPLETE; | ||
| } else if (PointeeTy->isSizelessType()) { | ||
| InvalidTypeKind = CountedByInvalidPointeeTypeKind::SIZELESS; | ||
|
|
@@ -5948,23 +5966,23 @@ static bool CheckCountedByAttrOnField( | |
| : diag::err_counted_by_attr_pointee_unknown_size; | ||
| S.Diag(FD->getBeginLoc(), DiagID) | ||
| << SelectPtrOrArr << PointeeTy << (int)InvalidTypeKind | ||
| << (ShouldWarn ? 1 : 0) << FD->getSourceRange(); | ||
| << (ShouldWarn ? 1 : 0) << Kind << FD->getSourceRange(); | ||
| return true; | ||
| } | ||
|
|
||
| // Check the expression | ||
|
|
||
| if (!E->getType()->isIntegerType() || E->getType()->isBooleanType()) { | ||
| S.Diag(E->getBeginLoc(), diag::err_counted_by_attr_argument_not_integer) | ||
| << E->getSourceRange(); | ||
| S.Diag(E->getBeginLoc(), diag::err_count_attr_argument_not_integer) | ||
| << Kind << E->getSourceRange(); | ||
| return true; | ||
| } | ||
|
|
||
| auto *DRE = dyn_cast<DeclRefExpr>(E); | ||
| if (!DRE) { | ||
| S.Diag(E->getBeginLoc(), | ||
| diag::err_counted_by_attr_only_support_simple_decl_reference) | ||
| << E->getSourceRange(); | ||
| diag::err_count_attr_only_support_simple_decl_reference) | ||
| << Kind << E->getSourceRange(); | ||
| return true; | ||
| } | ||
|
|
||
|
|
@@ -5974,8 +5992,8 @@ static bool CheckCountedByAttrOnField( | |
| CountFD = IFD->getAnonField(); | ||
| } | ||
| if (!CountFD) { | ||
| S.Diag(E->getBeginLoc(), diag::err_counted_by_must_be_in_structure) | ||
| << CountDecl << E->getSourceRange(); | ||
| S.Diag(E->getBeginLoc(), diag::err_count_attr_must_be_in_structure) | ||
| << CountDecl << Kind << E->getSourceRange(); | ||
|
|
||
| S.Diag(CountDecl->getBeginLoc(), | ||
| diag::note_flexible_array_counted_by_attr_field) | ||
|
|
@@ -5985,8 +6003,8 @@ static bool CheckCountedByAttrOnField( | |
|
|
||
| if (FD->getParent() != CountFD->getParent()) { | ||
| if (CountFD->getParent()->isUnion()) { | ||
| S.Diag(CountFD->getBeginLoc(), diag::err_counted_by_attr_refer_to_union) | ||
| << CountFD->getSourceRange(); | ||
| S.Diag(CountFD->getBeginLoc(), diag::err_count_attr_refer_to_union) | ||
| << Kind << CountFD->getSourceRange(); | ||
| return true; | ||
| } | ||
| // Whether CountRD is an anonymous struct is not determined at this | ||
|
|
@@ -5996,9 +6014,8 @@ static bool CheckCountedByAttrOnField( | |
| auto *CountRD = GetEnclosingNamedOrTopAnonRecord(CountFD); | ||
|
|
||
| if (RD != CountRD) { | ||
| S.Diag(E->getBeginLoc(), | ||
| diag::err_flexible_array_count_not_in_same_struct) | ||
| << CountFD << E->getSourceRange(); | ||
| S.Diag(E->getBeginLoc(), diag::err_count_attr_param_not_in_same_struct) | ||
| << CountFD << Kind << FieldTy->isArrayType() << E->getSourceRange(); | ||
| S.Diag(CountFD->getBeginLoc(), | ||
| diag::note_flexible_array_counted_by_attr_field) | ||
| << CountFD << CountFD->getSourceRange(); | ||
|
|
@@ -6018,12 +6035,35 @@ static void handleCountedByAttrField(Sema &S, Decl *D, const ParsedAttr &AL) { | |
| if (!CountExpr) | ||
| return; | ||
|
|
||
| bool CountInBytes; | ||
| bool OrNull; | ||
| switch (AL.getKind()) { | ||
| case ParsedAttr::AT_CountedBy: | ||
| CountInBytes = false; | ||
| OrNull = false; | ||
| break; | ||
| case ParsedAttr::AT_CountedByOrNull: | ||
| CountInBytes = false; | ||
| OrNull = true; | ||
| break; | ||
| case ParsedAttr::AT_SizedBy: | ||
| CountInBytes = true; | ||
| OrNull = false; | ||
| break; | ||
| case ParsedAttr::AT_SizedByOrNull: | ||
| CountInBytes = true; | ||
| OrNull = true; | ||
| break; | ||
| default: | ||
| llvm_unreachable("unexpected counted_by family attribute"); | ||
| } | ||
|
|
||
| llvm::SmallVector<TypeCoupledDeclRefInfo, 1> Decls; | ||
| if (CheckCountedByAttrOnField(S, FD, CountExpr, Decls)) | ||
| if (CheckCountedByAttrOnField(S, FD, CountExpr, Decls, CountInBytes, OrNull)) | ||
| return; | ||
|
|
||
| QualType CAT = | ||
| S.BuildCountAttributedArrayOrPointerType(FD->getType(), CountExpr); | ||
| QualType CAT = S.BuildCountAttributedArrayOrPointerType( | ||
| FD->getType(), CountExpr, CountInBytes, OrNull); | ||
| FD->setType(CAT); | ||
| } | ||
|
|
||
|
|
@@ -6971,6 +7011,9 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL, | |
| break; | ||
|
|
||
| case ParsedAttr::AT_CountedBy: | ||
| case ParsedAttr::AT_CountedByOrNull: | ||
| case ParsedAttr::AT_SizedBy: | ||
| case ParsedAttr::AT_SizedByOrNull: | ||
| handleCountedByAttrField(S, D, AL); | ||
| break; | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: The description of the behavior of
counted_bywith anullptris the-fbound-safetybehavior. I'm not sure if that restriction exists forcounted_byoutside that mode.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm yeah there's nothing preventing you from assigning null I guess, but I don't believe
__bdosCG has any support for returning 0 when the pointer is null.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to add a suggested use case for
sized_by_or_null. e.g., an allocator that returns either a buffer of size or nullptr.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Maybe it's worth calling out (because it's really not obvious) that the only valid count for
__sized_bywhen the pointer isNULLis0,__sized_by_or_nullallows any count value when the pointer isNULL. We could also note that this isn't currently enforced.