Skip to content

Commit ca8ecf8

Browse files
committed
[Cherry-pick][BoundsSafety][Sema] Allow counted_by and counted_by_or_null on pointers where the pointee type is incomplete but potentially completable
--- This is a cherry-pick of the change in llvm#106321. It is being cherry-pick now so that the merge conflict is handled upfront to minimize the work when the conflict from the upstream change reaches the automerger. Conflicts: clang/include/clang/AST/Type.h clang/include/clang/Sema/Sema.h clang/lib/AST/Type.cpp clang/lib/Sema/SemaBoundsSafety.cpp clang/lib/Sema/SemaExpr.cpp clang/lib/Sema/SemaInit.cpp --- Previously using the `counted_by` or `counted_by_or_null` attribute on a pointer with an incomplete pointee type was forbidden. Unfortunately this prevented a situation like the following from being allowed. Header file: ``` struct EltTy; // Incomplete type struct Buffer { size_t count; struct EltTy* __counted_by(count) buffer; // error before this patch }; ``` Implementation file: ``` struct EltTy { // definition }; void allocBuffer(struct Buffer* b) { b->buffer = malloc(sizeof(EltTy)* b->count); } ``` To allow code like the above but still enforce that the pointee type size is known in locations where `-fbounds-safety` needs to emit bounds checks the following scheme is used. * For incomplete pointee types that can never be completed (e.g. `void`) these are treated as error where the attribute is written (just like before this patch). * For incomplete pointee types that might be completable later on (struct, union, and enum forward declarations) in the translation unit, writing the attribute on the incomplete pointee type is allowed on the FieldDecl declaration but "uses" of the declared pointer are forbidden if at the point of "use" the pointee type is still incomplete. For this patch a "use" of a FieldDecl covers: * Explicit and Implicit initialization (note see **Tentative Definition Initialization** for an exception to this) * Assignment * Conversion to an lvalue (e.g. for use in an expression) In the swift lang fork of Clang the `counted_by` and `counted_by_or_null` attribute are allowed in many more contexts. That isn't the case for upstream Clang so the "use" checks for the attribute on VarDecl, ParamVarDecl, and function return type have been omitted from this patch because they can't be tested. However, the `BoundsSafetyCheckAssignmentToCountAttrPtrWithIncompletePointeeTy` and `BoundsSafetyCheckUseOfCountAttrPtrWithIncompletePointeeTy` functions retain the ability to emit diagnostics for these other contexts to avoid unnecessary divergence between upstream Clang and Apple's internal fork. Support for checking "uses" will be upstreamed when upstream Clang allows the `counted_by` and `counted_by_or_null` attribute in additional contexts. This patch has a few limitations: ** 1. Tentative Defition Initialization ** This patch currently allows something like: ``` struct IncompleteTy; struct Buffer { int count; struct IncompleteTy* __counted_by(count) buf; }; // Tentative definition struct Buffer GlobalBuf; ``` The Tentative definition in this example becomes an actual definition whose initialization **should be checked** but it currently isn't. Addressing this problem will be done in a subseqent patch. ** 2. When the incomplete pointee type is a typedef diagnostics are slightly misleading ** For this situation: ``` struct IncompleteTy; typedef struct IncompleteTy Incomplete_t; struct Buffer { int count; struct IncompleteTy* __counted_by(count) buf; }; void use(struct Buffer b) { b.buf = 0x0; } ``` This code emits `note: forward declaration of 'Incomplete_t' (aka 'struct IncompleteTy')` but the location is on the `struct IncompleteTy;` forward declaration. This is misleading because `Incomplete_t` isn't actually forward declared there (instead the underlying type is). This could be resolved by additional diagnostics that walk the chain of typedefs and explain each step of the walk. However, that would be very verbose and didn't seem like a direction worth pursuing. rdar://133600117
1 parent 8f6856f commit ca8ecf8

22 files changed

+1945
-701
lines changed

clang/include/clang/AST/Type.h

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2480,6 +2480,7 @@ class alignas(TypeAlignment) Type : public ExtQualsTypeCommonBase {
24802480
bool isIncompleteOrSizelessType() const {
24812481
return isIncompleteType() || isSizelessType() || isFunctionType();
24822482
}
2483+
/* TO_UPSTREAM(BoundsSafety) OFF*/
24832484

24842485
/// \returns True if the type is incomplete and it is also a type that
24852486
/// cannot be completed by a later type definition.
@@ -2496,11 +2497,10 @@ class alignas(TypeAlignment) Type : public ExtQualsTypeCommonBase {
24962497
/// // This decl has type 'char[]' which is incomplete and cannot be later
24972498
/// // completed by another by another type declaration.
24982499
/// extern char foo[];
2499-
/// // This decl how has complete type 'char[5]'.
2500+
/// // This decl now has complete type 'char[5]'.
25002501
/// char foo[5]; // foo has a complete type
25012502
/// \endcode
2502-
bool isIncompletableIncompleteType() const;
2503-
/* TO_UPSTREAM(BoundsSafety) OFF*/
2503+
bool isAlwaysIncompleteType() const;
25042504

25052505
/// Determine whether this type is an object type.
25062506
bool isObjectType() const {
@@ -3705,9 +3705,7 @@ class CountAttributedType final
37053705
return T->getTypeClass() == CountAttributed;
37063706
}
37073707

3708-
/* TO_UPSTREAM(BoundsSafety) ON*/
3709-
StringRef GetAttributeName(bool WithMacroPrefix) const;
3710-
/* TO_UPSTREAM(BoundsSafety) OFF*/
3708+
StringRef getAttributeName(bool WithMacroPrefix) const;
37113709
};
37123710

37133711
/* TO_UPSTREAM(BoundsSafety) ON */

clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6800,6 +6800,30 @@ def err_counted_by_attr_pointee_unknown_size : Error<
68006800
"%select{|. This will be an error in a future compiler version}3"
68016801
""
68026802
"}2">;
6803+
def err_counted_by_on_incomplete_type_on_assign : Error <
6804+
"cannot %select{"
6805+
"assign to %select{object|'%1'}2 with|" // AA_Assigning,
6806+
"pass argument to %select{parameter|parameter '%1'}2 with|" // AA_Passing,
6807+
"return|" // AA_Returning,
6808+
"convert to|" // AA_Converting (UNUSED)
6809+
"%select{|implicitly }3initialize %select{object|'%1'}2 with|" // AA_Initializing,
6810+
"pass argument to parameter with|" // AA_Sending (UNUSED)
6811+
"cast to|" // AA_Casting (UNUSED)
6812+
"pass argument to parameter with" // AA_Passing_CFAudited (UNUSED)
6813+
"}0 '%5' attributed type %4 because the pointee type %6 is incomplete">;
6814+
6815+
def err_counted_by_on_incomplete_type_on_use : Error <
6816+
"cannot %select{"
6817+
"use '%1' with '%4' attributed|" // Generic expr
6818+
"call '%1' with '%4' attributed return" // CallExpr
6819+
"}0 type %2 because the pointee type %3 is incomplete">;
6820+
6821+
def note_counted_by_consider_completing_pointee_ty : Note<
6822+
"consider providing a complete definition for %0">;
6823+
6824+
def note_counted_by_consider_using_sized_by : Note<
6825+
"consider using '__sized_by%select{|_or_null}0' instead of "
6826+
"'__counted_by%select{|_or_null}0'">;
68036827

68046828
def warn_counted_by_attr_elt_type_unknown_size :
68056829
Warning<err_counted_by_attr_pointee_unknown_size.Summary>,
@@ -12912,8 +12936,7 @@ def err_bounds_safety_counted_by_on_incomplete_type_on_func_def : Error<
1291212936
" '%3'" // named parameter
1291312937
"}2 with"
1291412938
"}1 type %4 on a function definition because the pointee type %5 is "
12915-
"incomplete; consider providing a complete definition for %5 before the "
12916-
"function body or using the '__sized_by%select{|_or_null}6' attribute"
12939+
"incomplete"
1291712940
>;
1291812941

1291912942
def err_bounds_safety_counted_by_on_incomplete_type_on_var_decl : Error<

clang/include/clang/Sema/Sema.h

Lines changed: 9 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2263,7 +2263,6 @@ class Sema final : public SemaBase {
22632263
bool CheckCountedByAttrOnField(FieldDecl *FD, Expr *E, bool CountInBytes,
22642264
bool OrNull);
22652265

2266-
/* TO_UPSTREAM(BoundsSafety) ON*/
22672266
/// Perform Bounds Safety Semantic checks for assigning to a `__counted_by` or
22682267
/// `__counted_by_or_null` pointer type \param LHSTy.
22692268
///
@@ -2285,28 +2284,6 @@ class Sema final : public SemaBase {
22852284
SourceLocation Loc, const ValueDecl *Assignee,
22862285
bool ShowFullyQualifiedAssigneeName);
22872286

2288-
/// Perform Checks for assigning to a `__counted_by` or
2289-
/// `__counted_by_or_null` pointer type \param LHSTy where the pointee type
2290-
/// is incomplete which is invalid.
2291-
///
2292-
/// \param LHSTy The type being assigned to. Checks will only be performed if
2293-
/// the type is a `counted_by` or `counted_by_or_null ` pointer.
2294-
/// \param RHSExpr The expression being assigned from.
2295-
/// \param Action The type assignment being performed
2296-
/// \param Loc The SourceLocation to use for error diagnostics
2297-
/// \param Assignee The ValueDecl being assigned. This is used to compute
2298-
/// the name of the assignee. If the assignee isn't known this can
2299-
/// be set to nullptr.
2300-
/// \param ShowFullyQualifiedAssigneeName If set to true when using \p
2301-
/// Assignee to compute the name of the assignee use the fully
2302-
/// qualified name, otherwise use the unqualified name.
2303-
///
2304-
/// \returns True iff no diagnostic where emitted, false otherwise.
2305-
bool BoundsSafetyCheckAssignmentToCountAttrPtrWithIncompletePointeeTy(
2306-
QualType LHSTy, Expr *RHSExpr, AssignmentAction Action,
2307-
SourceLocation Loc, const ValueDecl *Assignee,
2308-
bool ShowFullyQualifiedAssigneeName);
2309-
23102287
/// Perform Bounds Safety Semantic checks for initializing a Bounds Safety
23112288
/// pointer.
23122289
///
@@ -2323,6 +2300,15 @@ class Sema final : public SemaBase {
23232300
AssignmentAction Action,
23242301
QualType LHSType, Expr *RHSExpr);
23252302

2303+
/// Perform Bounds Safety semantic checks for uses of invalid uses counted_by
2304+
/// or counted_by_or_null pointers in \param E.
2305+
///
2306+
/// \param E the expression to check
2307+
///
2308+
/// \returns True iff no diagnostic where emitted, false otherwise.
2309+
bool BoundsSafetyCheckUseOfCountAttrPtr(const Expr *E);
2310+
2311+
/* TO_UPSTREAM(BoundsSafety) ON*/
23262312
/// Perform Bounds Safety semantic checks on function parameters on a function
23272313
/// definition. This only performs checks that can be made by looking at
23282314
/// \param PVD in isolation (i.e. not looking at other parameters in the
@@ -2351,14 +2337,6 @@ class Sema final : public SemaBase {
23512337
/// \returns True iff no diagnostic where emitted, false otherwise.
23522338
bool BoundsSafetyCheckReturnTyForFunctionDef(FunctionDecl *FD);
23532339

2354-
/// Perform Bounds Safety semantic checks for uses of invalid uses counted_by
2355-
/// or counted_by_or_null pointers in \param E.
2356-
///
2357-
/// \param E the expression to check
2358-
///
2359-
/// \returns True iff no diagnostic where emitted, false otherwise.
2360-
bool BoundsSafetyCheckUseOfCountAttrPtr(Expr *E);
2361-
23622340
/// Perform Bounds Safety semantic checks on variable declaration \param VD.
23632341
///
23642342
/// \param VD The VarDecl to check

clang/lib/AST/Type.cpp

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2599,6 +2599,22 @@ bool Type::isIncompleteType(NamedDecl **Def) const {
25992599
}
26002600
}
26012601

2602+
bool Type::isAlwaysIncompleteType() const {
2603+
if (!isIncompleteType())
2604+
return false;
2605+
2606+
// Forward declarations of structs, classes, enums, and unions could be later
2607+
// completed in a compilation unit by providing a type definition.
2608+
if (getAsTagDecl())
2609+
return false;
2610+
2611+
// Other types are incompletable.
2612+
//
2613+
// E.g. `char[]` and `void`. The type is incomplete and no future
2614+
// type declarations can make the type complete.
2615+
return true;
2616+
}
2617+
26022618
bool Type::isSizelessBuiltinType() const {
26032619
if (isSizelessVectorType())
26042620
return true;
@@ -2716,22 +2732,6 @@ bool Type::isSizeMeaningless() const {
27162732
return true;
27172733
return false;
27182734
}
2719-
2720-
bool Type::isIncompletableIncompleteType() const {
2721-
if (!isIncompleteType())
2722-
return false;
2723-
2724-
// Forward declarations of structs, classes, enums, and unions could be later
2725-
// completed in a compilation unit by providing a definition.
2726-
if (isStructureOrClassType() || isEnumeralType() || isUnionType())
2727-
return false;
2728-
2729-
// Other types are incompletable.
2730-
//
2731-
// E.g. `char[]` and `void`. The type is incomplete and no future
2732-
// type declarations can make the type complete.
2733-
return true;
2734-
}
27352735
/* TO_UPSTREAM(BoundsSafety) OFF*/
27362736

27372737
QualType Type::getSizelessVectorEltType(const ASTContext &Ctx) const {
@@ -4080,8 +4080,11 @@ CountAttributedType::CountAttributedType(
40804080
DeclSlot[i] = CoupledDecls[i];
40814081
}
40824082

4083-
/* TO_UPSTREAM(BoundsSafety) ON*/
4084-
StringRef CountAttributedType::GetAttributeName(bool WithMacroPrefix) const {
4083+
StringRef CountAttributedType::getAttributeName(bool WithMacroPrefix) const {
4084+
// TODO: This method isn't really ideal because it doesn't return the spelling
4085+
// of the attribute that was used in the user's code. This method is used for
4086+
// diagnostics so the fact it doesn't use the spelling of the attribute in
4087+
// the user's code could be confusing (#113585).
40854088
#define ENUMERATE_ATTRS(PREFIX) \
40864089
do { \
40874090
if (isCountInBytes()) { \
@@ -4102,6 +4105,7 @@ StringRef CountAttributedType::GetAttributeName(bool WithMacroPrefix) const {
41024105
#undef ENUMERATE_ATTRS
41034106
}
41044107

4108+
/* TO_UPSTREAM(BoundsSafety) ON*/
41054109
DynamicRangePointerType::DynamicRangePointerType(
41064110
QualType PointerTy, QualType CanPointerTy, Expr *StartPtr, Expr *EndPtr,
41074111
ArrayRef<TypeCoupledDeclRefInfo> StartPtrDecls,

0 commit comments

Comments
 (0)