[BoundsSafety][Sema] Allow counted_by and counted_by_or_null on pointers where the pointee type is incomplete but potentially completable (#106321)

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

Author: delcypher

clang/include/clang/AST/Type.h

@@ -2446,6 +2446,26 @@ class alignas(TypeAlignment) Type : public ExtQualsTypeCommonBase {
     return !isFunctionType();
   }
 
+  /// \returns True if the type is incomplete and it is also a type that
+  /// cannot be completed by a later type definition.
+  ///
+  /// E.g. For `void` this is true but for `struct ForwardDecl;` this is false
+  /// because a definition for `ForwardDecl` could be provided later on in the
+  /// translation unit.
+  ///
+  /// Note even for types that this function returns true for it is still
+  /// possible for the declarations that contain this type to later have a
+  /// complete type in a translation unit. E.g.:
+  ///
+  /// \code{.c}
+  /// // This decl has type 'char[]' which is incomplete and cannot be later
+  /// // completed by another by another type declaration.
+  /// extern char foo[];
+  /// // This decl now has complete type 'char[5]'.
+  /// char foo[5]; // foo has a complete type
+  /// \endcode
+  bool isAlwaysIncompleteType() const;
+
   /// Determine whether this type is an object type.
   bool isObjectType() const {
     // C++ [basic.types]p8:
@@ -3376,6 +3396,8 @@ class CountAttributedType final
   static bool classof(const Type *T) {
     return T->getTypeClass() == CountAttributed;
   }
+
+  StringRef getAttributeName(bool WithMacroPrefix) const;
 };
 
 /// Represents a type which was implicitly adjusted by the semantic

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -6738,6 +6738,30 @@ def err_counted_by_attr_pointee_unknown_size : Error<
     "%select{|. This will be an error in a future compiler version}3"
     ""
   "}2">;
+def err_counted_by_on_incomplete_type_on_assign : Error <
+  "cannot %select{"
+    "assign to %select{object|'%1'}2 with|" // AA_Assigning,
+    "pass argument to %select{parameter|parameter '%1'}2 with|" // AA_Passing,
+    "return|" // AA_Returning,
+    "convert to|" // AA_Converting (UNUSED)
+    "%select{|implicitly }3initialize %select{object|'%1'}2 with|" // AA_Initializing,
+    "pass argument to parameter with|" // AA_Sending (UNUSED)
+    "cast to|" // AA_Casting (UNUSED)
+    "pass argument to parameter with" // AA_Passing_CFAudited (UNUSED)
+  "}0 '%5' attributed type %4 because the pointee type %6 is incomplete">;
+
+def err_counted_by_on_incomplete_type_on_use : Error <
+  "cannot %select{"
+    "use '%1' with '%4' attributed|" // Generic expr
+    "call '%1' with '%4' attributed return" // CallExpr
+  "}0 type %2 because the pointee type %3 is incomplete">;
+
+def note_counted_by_consider_completing_pointee_ty : Note<
+  "consider providing a complete definition for %0">;
+  
+def note_counted_by_consider_using_sized_by : Note<
+  "consider using '__sized_by%select{|_or_null}0' instead of "
+  "'__counted_by%select{|_or_null}0'">;
 
 def warn_counted_by_attr_elt_type_unknown_size :
   Warning<err_counted_by_attr_pointee_unknown_size.Summary>,

clang/include/clang/Sema/Sema.h

@@ -2100,6 +2100,50 @@ class Sema final : public SemaBase {
   bool CheckCountedByAttrOnField(FieldDecl *FD, Expr *E, bool CountInBytes,
                                  bool OrNull);
 
+  /// Perform Bounds Safety Semantic checks for assigning to a `__counted_by` or
+  /// `__counted_by_or_null` pointer type \param LHSTy.
+  ///
+  /// \param LHSTy The type being assigned to. Checks will only be performed if
+  ///              the type is a `counted_by` or `counted_by_or_null ` pointer.
+  /// \param RHSExpr The expression being assigned from.
+  /// \param Action The type assignment being performed
+  /// \param Loc The SourceLocation to use for error diagnostics
+  /// \param Assignee The ValueDecl being assigned. This is used to compute
+  ///        the name of the assignee. If the assignee isn't known this can
+  ///        be set to nullptr.
+  /// \param ShowFullyQualifiedAssigneeName If set to true when using \p
+  ///        Assignee to compute the name of the assignee use the fully
+  ///        qualified name, otherwise use the unqualified name.
+  ///
+  /// \returns True iff no diagnostic where emitted, false otherwise.
+  bool BoundsSafetyCheckAssignmentToCountAttrPtr(
+      QualType LHSTy, Expr *RHSExpr, AssignmentAction Action,
+      SourceLocation Loc, const ValueDecl *Assignee,
+      bool ShowFullyQualifiedAssigneeName);
+
+  /// Perform Bounds Safety Semantic checks for initializing a Bounds Safety
+  /// pointer.
+  ///
+  /// \param Entity The entity being initialized
+  /// \param Kind The kind of initialization being performed
+  /// \param Action The type assignment being performed
+  /// \param LHSTy The type being assigned to. Checks will only be performed if
+  ///              the type is a `counted_by` or `counted_by_or_null ` pointer.
+  /// \param RHSExpr The expression being used for initialization.
+  ///
+  /// \returns True iff no diagnostic where emitted, false otherwise.
+  bool BoundsSafetyCheckInitialization(const InitializedEntity &Entity,
+                                       const InitializationKind &Kind,
+                                       AssignmentAction Action,
+                                       QualType LHSType, Expr *RHSExpr);
+
+  /// Perform Bounds Safety semantic checks for uses of invalid uses counted_by
+  /// or counted_by_or_null pointers in \param E.
+  ///
+  /// \param E the expression to check
+  ///
+  /// \returns True iff no diagnostic where emitted, false otherwise.
+  bool BoundsSafetyCheckUseOfCountAttrPtr(const Expr *E);
   ///@}
 
   //

clang/lib/AST/Type.cpp

@@ -2499,6 +2499,22 @@ bool Type::isIncompleteType(NamedDecl **Def) const {
   }
 }
 
+bool Type::isAlwaysIncompleteType() const {
+  if (!isIncompleteType())
+    return false;
+
+  // Forward declarations of structs, classes, enums, and unions could be later
+  // completed in a compilation unit by providing a type definition.
+  if (getAsTagDecl())
+    return false;
+
+  // Other types are incompletable.
+  //
+  // E.g. `char[]` and `void`. The type is incomplete and no future
+  // type declarations can make the type complete.
+  return true;
+}
+
 bool Type::isSizelessBuiltinType() const {
   if (isSizelessVectorType())
     return true;
@@ -3940,6 +3956,31 @@ CountAttributedType::CountAttributedType(
     DeclSlot[i] = CoupledDecls[i];
 }
 
+StringRef CountAttributedType::getAttributeName(bool WithMacroPrefix) const {
+// TODO: This method isn't really ideal because it doesn't return the spelling
+// of the attribute that was used in the user's code. This method is used for
+// diagnostics so the fact it doesn't use the spelling of the attribute in
+// the user's code could be confusing (#113585).
+#define ENUMERATE_ATTRS(PREFIX)                                                \
+  do {                                                                         \
+    if (isCountInBytes()) {                                                    \
+      if (isOrNull())                                                          \
+        return PREFIX "sized_by_or_null";                                      \
+      return PREFIX "sized_by";                                                \
+    }                                                                          \
+    if (isOrNull())                                                            \
+      return PREFIX "counted_by_or_null";                                      \
+    return PREFIX "counted_by";                                                \
+  } while (0)
+
+  if (WithMacroPrefix)
+    ENUMERATE_ATTRS("__");
+  else
+    ENUMERATE_ATTRS("");
+
+#undef ENUMERATE_ATTRS
+}
+
 TypedefType::TypedefType(TypeClass tc, const TypedefNameDecl *D,
                          QualType Underlying, QualType can)
     : Type(tc, can, toSemanticDependence(can->getDependence())),