[clang] [BoundsSafety] WIP: Make 'counted_by' work for pointer fields; late parsing for 'counted_by' on decl attr position (PR #87596)

Dan Liew via cfe-commits cfe-commits at lists.llvm.org
Wed May 1 14:26:11 PDT 2024


https://github.com/delcypher updated https://github.com/llvm/llvm-project/pull/87596

>From 50e628d2d6fb496c1cb2fa188b653529508e383c Mon Sep 17 00:00:00 2001
From: Dan Liew <dan at su-root.co.uk>
Date: Wed, 1 May 2024 13:56:52 -0700
Subject: [PATCH] [BoundsSafety] Allow 'counted_by' attribute on pointers in
 structs in C

Previously the attribute was only allowed on flexible array members.
This patch patch changes this to also allow the attribute on pointer
fields in structs and also allows late parsing of the attribute in some
contexts.

For example this previously wasn't allowed:

```
struct BufferTypeDeclAttributePosition {
  size_t count;
  char* buffer __counted_by(count); // Now allowed
}
```

Note the attribute is prevented on pointee types where the size isn't
known at compile time. In particular pointee types that are:

* Incomplete (e.g. `void`) and sizeless types
* Function types (e.g. the pointee of a function pointer)
* Struct types with a flexible array member

This patch also introduces late parsing of the attribute when used
in the declaration attribute position. For example

```
struct BufferTypeDeclAttributePosition {
  char* buffer __counted_by(count); // Now allowed
  size_t count;
}
```

is now allowed but **only** when passing
`-fexperimental-late-parse-attributes`.  The motivation for using late
parsing here is to avoid breaking the data layout of structs in existing
code that want to use the `counted_by` attribute. This patch is the
first use of `LateAttrParseExperimentalExt` in `Attr.td` that was
introduced in a previous patch.

Note by allowing the attribute on struct member pointers this now allows
the possiblity of writing the attribute in the type attribute position.
For example:

```
struct BufferTypeAttributePosition {
  size_t count;
  char *__counted_by(count) buffer; // Now allowed
}
```

However, the attribute in this position is still currently parsed
immediately rather than late parsed. So this will not parse currently:

```
struct BufferTypeAttributePosition {
  char *__counted_by(count) buffer; // Fails to parse
  size_t count;
}
```

The intention is to lift this restriction in future patches. It has not
been done in this patch to keep this size of this commit small.

Note this patch only allows parsing of semantic analysis of the
attribute on pointers. It does not make use of this on the codegen side
(e.g. for _builtin_dynamic_object_size and UBSan's array-bounds checks).
Making use of the attribute on pointers will be handled in separate
patches.

This work is heavily based on a patch originally written by Yeoul Na.

rdar://125400257
---
 clang/docs/ReleaseNotes.rst                   |  19 +-
 clang/include/clang/AST/Type.h                |   1 +
 clang/include/clang/Basic/Attr.td             |   3 +-
 .../clang/Basic/DiagnosticSemaKinds.td        |  15 +-
 clang/include/clang/Parse/Parser.h            |   7 +-
 clang/include/clang/Sema/Sema.h               |   3 +-
 clang/lib/AST/Type.cpp                        |  12 ++
 clang/lib/Parse/ParseDecl.cpp                 | 104 ++++++++++-
 clang/lib/Parse/ParseObjc.cpp                 |  10 +-
 clang/lib/Sema/SemaDeclAttr.cpp               |  71 ++++++--
 clang/lib/Sema/SemaType.cpp                   |   6 +-
 clang/lib/Sema/TreeTransform.h                |   2 +-
 clang/test/AST/attr-counted-by-late-parsed.c  |  31 ++++
 .../Sema/attr-counted-by-late-parsed-off.c    |  26 +++
 .../attr-counted-by-late-parsed-struct-ptrs.c | 169 ++++++++++++++++++
 ...tr-counted-by-struct-ptrs-sizeless-types.c |  17 ++
 clang/test/Sema/attr-counted-by-struct-ptrs.c | 163 +++++++++++++++++
 clang/test/Sema/attr-counted-by.c             |   5 +-
 18 files changed, 626 insertions(+), 38 deletions(-)
 create mode 100644 clang/test/AST/attr-counted-by-late-parsed.c
 create mode 100644 clang/test/Sema/attr-counted-by-late-parsed-off.c
 create mode 100644 clang/test/Sema/attr-counted-by-late-parsed-struct-ptrs.c
 create mode 100644 clang/test/Sema/attr-counted-by-struct-ptrs-sizeless-types.c
 create mode 100644 clang/test/Sema/attr-counted-by-struct-ptrs.c

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 4c0fe5bcf6b122..81d0a89b294878 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -258,7 +258,8 @@ New Compiler Flags
 
 - ``-fexperimental-late-parse-attributes`` enables an experimental feature to
   allow late parsing certain attributes in specific contexts where they would
-  not normally be late parsed.
+  not normally be late parsed. Currently this allows late parsing the
+  `counted_by` attribute in C. See `Attribute Changes in Clang`_.
 
 Deprecated Compiler Flags
 -------------------------
@@ -335,6 +336,22 @@ Attribute Changes in Clang
 - Clang now warns that the ``exclude_from_explicit_instantiation`` attribute
   is ignored when applied to a local class or a member thereof.
 
+- The ``counted_by`` attribute can now be late parsed in C when
+  ``-fexperimental-late-parse-attributes`` is passed but only when attribute is
+  used in the declaration attribute position. This allows using the
+  attribute on existing code where it previously impossible to do so without
+  re-ordering struct field declarations would break ABI as shown below.
+
+  .. code-block:: c
+
+     struct BufferTy {
+       /* Refering to `count` requires late parsing */
+       char* buffer __counted_by(count);
+       /* Swapping `buffer` and `count` to avoid late parsing would break ABI */
+       size_t count;
+     };
+
+
 Improvements to Clang's diagnostics
 -----------------------------------
 - Clang now applies syntax highlighting to the code snippets it
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index e6643469e0b334..e982a565413751 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -2515,6 +2515,7 @@ class alignas(TypeAlignment) Type : public ExtQualsTypeCommonBase {
   bool isRecordType() const;
   bool isClassType() const;
   bool isStructureType() const;
+  bool isStructureTypeWithFlexibleArrayMember() const;
   bool isObjCBoxableRecordType() const;
   bool isInterfaceType() const;
   bool isStructureOrClassType() const;
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 0225598cbbe8ad..ef08d2a274a907 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -2229,7 +2229,8 @@ def TypeNullUnspecified : TypeAttr {
 def CountedBy : DeclOrTypeAttr {
   let Spellings = [Clang<"counted_by">];
   let Subjects = SubjectList<[Field], ErrorDiag>;
-  let Args = [ExprArgument<"Count">, IntArgument<"NestedLevel">];
+  let Args = [ExprArgument<"Count">, IntArgument<"NestedLevel", 1>];
+  let LateParsed = LateAttrParseExperimentalExt;
   let ParseArgumentsAsUnevaluated = 1;
   let Documentation = [CountedByDocs];
   let LangOpts = [COnly];
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index f72d5c252b863e..0ab2a96c7b464b 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6518,8 +6518,10 @@ def warn_superclass_variable_sized_type_not_at_end : Warning<
 
 def err_flexible_array_count_not_in_same_struct : Error<
   "'counted_by' field %0 isn't within the same struct as the flexible array">;
-def err_counted_by_attr_not_on_flexible_array_member : Error<
-  "'counted_by' only applies to C99 flexible array members">;
+def err_counted_by_attr_not_on_ptr_or_flexible_array_member : Error<
+  "'counted_by' only applies to pointers or C99 flexible array members">;
+def err_counted_by_attr_on_array_not_flexible_array_member : Error<
+  "'counted_by' on arrays only applies to C99 flexible array members">;
 def err_counted_by_attr_refer_to_itself : Error<
   "'counted_by' cannot refer to the flexible array member %0">;
 def err_counted_by_must_be_in_structure : Error<
@@ -6534,6 +6536,15 @@ def err_counted_by_attr_refer_to_union : Error<
   "'counted_by' argument cannot refer to a union member">;
 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 a pointer with pointee with unknown size "
+  "because %0 is %select{"
+    "an incomplete type|"  // CountedByInvalidPointeeTypeKind::INCOMPLETE
+    "a sizeless type|"     // CountedByInvalidPointeeTypeKind::SIZELESS
+    "a function type|"     // CountedByInvalidPointeeTypeKind::FUNCTION
+    // CountedByInvalidPointeeTypeKind::FLEXIBLE_ARRAY_MEMBER
+    "a struct type with a flexible array member"
+  "}1">;
 
 let CategoryName = "ARC Semantic Issue" in {
 
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 81aab8c888ab65..0f2ba17aadaf2d 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -1643,6 +1643,8 @@ class Parser : public CodeCompletionHandler {
                                bool EnterScope, bool OnDefinition);
   void ParseLexedAttribute(LateParsedAttribute &LA,
                            bool EnterScope, bool OnDefinition);
+  void ParseLexedCAttribute(LateParsedAttribute &LA,
+                            ParsedAttributes *OutAttrs = nullptr);
   void ParseLexedMethodDeclarations(ParsingClass &Class);
   void ParseLexedMethodDeclaration(LateParsedMethodDeclaration &LM);
   void ParseLexedMethodDefs(ParsingClass &Class);
@@ -2530,7 +2532,8 @@ class Parser : public CodeCompletionHandler {
 
   void ParseStructDeclaration(
       ParsingDeclSpec &DS,
-      llvm::function_ref<void(ParsingFieldDeclarator &)> FieldsCallback);
+      llvm::function_ref<Decl *(ParsingFieldDeclarator &)> FieldsCallback,
+      LateParsedAttrList *LateFieldAttrs = nullptr);
 
   DeclGroupPtrTy ParseTopLevelStmtDecl();
 
@@ -3107,6 +3110,8 @@ class Parser : public CodeCompletionHandler {
                                  SourceLocation ScopeLoc,
                                  ParsedAttr::Form Form);
 
+  void DistributeCLateParsedAttrs(Decl *Dcl, LateParsedAttrList *LateAttrs);
+
   void ParseBoundsAttribute(IdentifierInfo &AttrName,
                             SourceLocation AttrNameLoc, ParsedAttributes &Attrs,
                             IdentifierInfo *ScopeName, SourceLocation ScopeLoc,
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 1ca523ec88c2f9..f1e1be24f4fd2f 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -11509,7 +11509,8 @@ class Sema final : public SemaBase {
   QualType BuildMatrixType(QualType T, Expr *NumRows, Expr *NumColumns,
                            SourceLocation AttrLoc);
 
-  QualType BuildCountAttributedArrayType(QualType WrappedTy, Expr *CountExpr);
+  QualType BuildCountAttributedArrayOrPointerType(QualType WrappedTy,
+                                                  Expr *CountExpr);
 
   QualType BuildAddressSpaceAttr(QualType &T, LangAS ASIdx, Expr *AddrSpace,
                                  SourceLocation AttrLoc);
diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index 68e81f45b4c28e..316d70e0b79ecf 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -631,6 +631,18 @@ bool Type::isStructureType() const {
   return false;
 }
 
+bool Type::isStructureTypeWithFlexibleArrayMember() const {
+  const auto *RT = getAs<RecordType>();
+  if (!RT)
+    return false;
+  const auto *Decl = RT->getDecl();
+  if (!Decl->isStruct())
+    return false;
+  if (!Decl->hasFlexibleArrayMember())
+    return false;
+  return true;
+}
+
 bool Type::isObjCBoxableRecordType() const {
   if (const auto *RT = getAs<RecordType>())
     return RT->getDecl()->hasAttr<ObjCBoxableAttr>();
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 7431c256d2c135..470d26fb3d73be 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -3282,6 +3282,19 @@ void Parser::ParseAlignmentSpecifier(ParsedAttributes &Attrs,
   }
 }
 
+void Parser::DistributeCLateParsedAttrs(Decl *Dcl,
+                                        LateParsedAttrList *LateAttrs) {
+  assert(Dcl);
+
+  if (!LateAttrs)
+    return;
+
+  for (auto *LateAttr : *LateAttrs) {
+    if (LateAttr->Decls.empty())
+      LateAttr->addDecl(Dcl);
+  }
+}
+
 /// Bounds attributes (e.g., counted_by):
 ///   AttrName '(' expression ')'
 void Parser::ParseBoundsAttribute(IdentifierInfo &AttrName,
@@ -4815,13 +4828,14 @@ static void DiagnoseCountAttributedTypeInUnnamedAnon(ParsingDeclSpec &DS,
 ///
 void Parser::ParseStructDeclaration(
     ParsingDeclSpec &DS,
-    llvm::function_ref<void(ParsingFieldDeclarator &)> FieldsCallback) {
+    llvm::function_ref<Decl *(ParsingFieldDeclarator &)> FieldsCallback,
+    LateParsedAttrList *LateFieldAttrs) {
 
   if (Tok.is(tok::kw___extension__)) {
     // __extension__ silences extension warnings in the subexpression.
     ExtensionRAIIObject O(Diags);  // Use RAII to do this.
     ConsumeToken();
-    return ParseStructDeclaration(DS, FieldsCallback);
+    return ParseStructDeclaration(DS, FieldsCallback, LateFieldAttrs);
   }
 
   // Parse leading attributes.
@@ -4886,10 +4900,12 @@ void Parser::ParseStructDeclaration(
     }
 
     // If attributes exist after the declarator, parse them.
-    MaybeParseGNUAttributes(DeclaratorInfo.D);
+    MaybeParseGNUAttributes(DeclaratorInfo.D, LateFieldAttrs);
 
     // We're done with this declarator;  invoke the callback.
-    FieldsCallback(DeclaratorInfo);
+    Decl *Field = FieldsCallback(DeclaratorInfo);
+    if (Field)
+      DistributeCLateParsedAttrs(Field, LateFieldAttrs);
 
     // If we don't have a comma, it is either the end of the list (a ';')
     // or an error, bail out.
@@ -4900,6 +4916,69 @@ void Parser::ParseStructDeclaration(
   }
 }
 
+/// Finish parsing an attribute for which parsing was delayed.
+/// This will be called at the end of parsing a class declaration
+/// for each LateParsedAttribute. We consume the saved tokens and
+/// create an attribute with the arguments filled in. We add this
+/// to the Attribute list for the decl.
+void Parser::ParseLexedCAttribute(LateParsedAttribute &LA,
+                                  ParsedAttributes *OutAttrs) {
+  // Create a fake EOF so that attribute parsing won't go off the end of the
+  // attribute.
+  Token AttrEnd;
+  AttrEnd.startToken();
+  AttrEnd.setKind(tok::eof);
+  AttrEnd.setLocation(Tok.getLocation());
+  AttrEnd.setEofData(LA.Toks.data());
+  LA.Toks.push_back(AttrEnd);
+
+  // Append the current token at the end of the new token stream so that it
+  // doesn't get lost.
+  LA.Toks.push_back(Tok);
+  PP.EnterTokenStream(LA.Toks, /*DisableMacroExpansion=*/true,
+                      /*IsReinject=*/true);
+  // Drop the current token and bring the first cached one. It's the same token
+  // as when we entered this function.
+  ConsumeAnyToken(/*ConsumeCodeCompletionTok=*/true);
+
+  ParsedAttributes Attrs(AttrFactory);
+
+  assert(LA.Decls.size() <= 1 &&
+         "late field attribute expects to have at most one declaration.");
+
+  // Dispatch based on the attribute and parse it
+  const AttributeCommonInfo::Form ParsedForm = ParsedAttr::Form::GNU();
+  IdentifierInfo *ScopeName = nullptr;
+  const ParsedAttr::Kind AttrKind =
+      ParsedAttr::getParsedKind(&LA.AttrName, /*ScopeName=*/ScopeName,
+                                /*SyntaxUsed=*/ParsedForm.getSyntax());
+  switch (AttrKind) {
+  case ParsedAttr::Kind::AT_CountedBy:
+    ParseBoundsAttribute(LA.AttrName, LA.AttrNameLoc, Attrs,
+                         /*ScopeName=*/ScopeName, SourceLocation(),
+                         /*Form=*/ParsedForm);
+    break;
+  default:
+    llvm_unreachable("Unhandled late parsed attribute");
+  }
+
+  for (auto *D : LA.Decls)
+    Actions.ActOnFinishDelayedAttribute(getCurScope(), D, Attrs);
+
+  // Due to a parsing error, we either went over the cached tokens or
+  // there are still cached tokens left, so we skip the leftover tokens.
+  while (Tok.isNot(tok::eof))
+    ConsumeAnyToken();
+
+  // Consume the fake EOF token if it's there
+  if (Tok.is(tok::eof) && Tok.getEofData() == AttrEnd.getEofData())
+    ConsumeAnyToken();
+
+  if (OutAttrs) {
+    OutAttrs->takeAllFrom(Attrs);
+  }
+}
+
 /// ParseStructUnionBody
 ///       struct-contents:
 ///         struct-declaration-list
@@ -4923,6 +5002,11 @@ void Parser::ParseStructUnionBody(SourceLocation RecordLoc,
   ParseScope StructScope(this, Scope::ClassScope|Scope::DeclScope);
   Actions.ActOnTagStartDefinition(getCurScope(), TagDecl);
 
+  // `LateAttrParseExperimentalExtOnly=true` requests that only attributes
+  // marked with `LateAttrParseExperimentalExt` are late parsed.
+  LateParsedAttrList LateFieldAttrs(/*PSoon=*/false,
+                                    /*LateAttrParseExperimentalExtOnly=*/true);
+
   // While we still have something to read, read the declarations in the struct.
   while (!tryParseMisplacedModuleImport() && Tok.isNot(tok::r_brace) &&
          Tok.isNot(tok::eof)) {
@@ -4973,18 +5057,19 @@ void Parser::ParseStructUnionBody(SourceLocation RecordLoc,
     }
 
     if (!Tok.is(tok::at)) {
-      auto CFieldCallback = [&](ParsingFieldDeclarator &FD) {
+      auto CFieldCallback = [&](ParsingFieldDeclarator &FD) -> Decl * {
         // Install the declarator into the current TagDecl.
         Decl *Field =
             Actions.ActOnField(getCurScope(), TagDecl,
                                FD.D.getDeclSpec().getSourceRange().getBegin(),
                                FD.D, FD.BitfieldSize);
         FD.complete(Field);
+        return Field;
       };
 
       // Parse all the comma separated declarators.
       ParsingDeclSpec DS(*this);
-      ParseStructDeclaration(DS, CFieldCallback);
+      ParseStructDeclaration(DS, CFieldCallback, &LateFieldAttrs);
     } else { // Handle @defs
       ConsumeToken();
       if (!Tok.isObjCAtKeyword(tok::objc_defs)) {
@@ -5025,7 +5110,12 @@ void Parser::ParseStructUnionBody(SourceLocation RecordLoc,
 
   ParsedAttributes attrs(AttrFactory);
   // If attributes exist after struct contents, parse them.
-  MaybeParseGNUAttributes(attrs);
+  MaybeParseGNUAttributes(attrs, &LateFieldAttrs);
+
+  // Late parse field attributes if necessary.
+  assert(!getLangOpts().CPlusPlus);
+  for (auto *LateAttr : LateFieldAttrs)
+    ParseLexedCAttribute(*LateAttr);
 
   SmallVector<Decl *, 32> FieldDecls(TagDecl->fields());
 
diff --git a/clang/lib/Parse/ParseObjc.cpp b/clang/lib/Parse/ParseObjc.cpp
index 8e54fe012c55d7..2eaeff38a24a6f 100644
--- a/clang/lib/Parse/ParseObjc.cpp
+++ b/clang/lib/Parse/ParseObjc.cpp
@@ -778,16 +778,16 @@ void Parser::ParseObjCInterfaceDeclList(tok::ObjCKeywordKind contextKey,
       }
 
       bool addedToDeclSpec = false;
-      auto ObjCPropertyCallback = [&](ParsingFieldDeclarator &FD) {
+      auto ObjCPropertyCallback = [&](ParsingFieldDeclarator &FD) -> Decl * {
         if (FD.D.getIdentifier() == nullptr) {
           Diag(AtLoc, diag::err_objc_property_requires_field_name)
               << FD.D.getSourceRange();
-          return;
+          return nullptr;
         }
         if (FD.BitfieldSize) {
           Diag(AtLoc, diag::err_objc_property_bitfield)
               << FD.D.getSourceRange();
-          return;
+          return nullptr;
         }
 
         // Map a nullability property attribute to a context-sensitive keyword
@@ -816,6 +816,7 @@ void Parser::ParseObjCInterfaceDeclList(tok::ObjCKeywordKind contextKey,
             MethodImplKind);
 
         FD.complete(Property);
+        return Property;
       };
 
       // Parse all the comma separated declarators.
@@ -2024,7 +2025,7 @@ void Parser::ParseObjCClassInstanceVariables(ObjCContainerDecl *interfaceDecl,
       continue;
     }
 
-    auto ObjCIvarCallback = [&](ParsingFieldDeclarator &FD) {
+    auto ObjCIvarCallback = [&](ParsingFieldDeclarator &FD) -> Decl * {
       assert(getObjCDeclContext() == interfaceDecl &&
              "Ivar should have interfaceDecl as its decl context");
       // Install the declarator into the interface decl.
@@ -2035,6 +2036,7 @@ void Parser::ParseObjCClassInstanceVariables(ObjCContainerDecl *interfaceDecl,
       if (Field)
         AllIvarDecls.push_back(Field);
       FD.complete(Field);
+      return Field;
     };
 
     // Parse all the comma separated declarators.
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 363ae93cb62df1..7ae1e09c7a5dcd 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -8588,31 +8588,71 @@ static const RecordDecl *GetEnclosingNamedOrTopAnonRecord(const FieldDecl *FD) {
   return RD;
 }
 
-static bool
-CheckCountExpr(Sema &S, FieldDecl *FD, Expr *E,
-               llvm::SmallVectorImpl<TypeCoupledDeclRefInfo> &Decls) {
+enum class CountedByInvalidPointeeTypeKind {
+  INCOMPLETE,
+  SIZELESS,
+  FUNCTION,
+  FLEXIBLE_ARRAY_MEMBER,
+  VALID,
+};
+
+static bool CheckCountedByAttrOnField(
+    Sema &S, FieldDecl *FD, Expr *E,
+    llvm::SmallVectorImpl<TypeCoupledDeclRefInfo> &Decls) {
+  // Check the context the attribute is used in
+
   if (FD->getParent()->isUnion()) {
     S.Diag(FD->getBeginLoc(), diag::err_counted_by_attr_in_union)
         << FD->getSourceRange();
     return true;
   }
 
-  if (!E->getType()->isIntegerType() || E->getType()->isBooleanType()) {
-    S.Diag(E->getBeginLoc(), diag::err_counted_by_attr_argument_not_integer)
-        << E->getSourceRange();
+  const auto FieldTy = FD->getType();
+  if (!FieldTy->isArrayType() && !FieldTy->isPointerType()) {
+    S.Diag(FD->getBeginLoc(),
+           diag::err_counted_by_attr_not_on_ptr_or_flexible_array_member)
+        << FD->getLocation();
     return true;
   }
 
   LangOptions::StrictFlexArraysLevelKind StrictFlexArraysLevel =
       LangOptions::StrictFlexArraysLevelKind::IncompleteOnly;
-
-  if (!Decl::isFlexibleArrayMemberLike(S.getASTContext(), FD, FD->getType(),
+  if (FieldTy->isArrayType() &&
+      !Decl::isFlexibleArrayMemberLike(S.getASTContext(), FD, FieldTy,
                                        StrictFlexArraysLevel, true)) {
-    // The "counted_by" attribute must be on a flexible array member.
-    SourceRange SR = FD->getLocation();
-    S.Diag(SR.getBegin(),
-           diag::err_counted_by_attr_not_on_flexible_array_member)
-        << SR;
+    S.Diag(FD->getBeginLoc(),
+           diag::err_counted_by_attr_on_array_not_flexible_array_member)
+        << FD->getLocation();
+    return true;
+  }
+
+  if (FieldTy->isPointerType()) {
+    CountedByInvalidPointeeTypeKind InvalidTypeKind =
+        CountedByInvalidPointeeTypeKind::VALID;
+    const auto PointeeTy = FieldTy->getPointeeType();
+    if (PointeeTy->isIncompleteType()) {
+      InvalidTypeKind = CountedByInvalidPointeeTypeKind::INCOMPLETE;
+    } else if (PointeeTy->isSizelessType()) {
+      InvalidTypeKind = CountedByInvalidPointeeTypeKind::SIZELESS;
+    } else if (PointeeTy->isFunctionType()) {
+      InvalidTypeKind = CountedByInvalidPointeeTypeKind::FUNCTION;
+    } else if (PointeeTy->isStructureTypeWithFlexibleArrayMember()) {
+      InvalidTypeKind = CountedByInvalidPointeeTypeKind::FLEXIBLE_ARRAY_MEMBER;
+    }
+
+    if (InvalidTypeKind != CountedByInvalidPointeeTypeKind::VALID) {
+      S.Diag(FD->getBeginLoc(), diag::err_counted_by_attr_pointee_unknown_size)
+          << FieldTy->getPointeeType() << (int)InvalidTypeKind
+          << 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();
     return true;
   }
 
@@ -8675,10 +8715,11 @@ static void handleCountedByAttrField(Sema &S, Decl *D, const ParsedAttr &AL) {
     return;
 
   llvm::SmallVector<TypeCoupledDeclRefInfo, 1> Decls;
-  if (CheckCountExpr(S, FD, CountExpr, Decls))
+  if (CheckCountedByAttrOnField(S, FD, CountExpr, Decls))
     return;
 
-  QualType CAT = S.BuildCountAttributedArrayType(FD->getType(), CountExpr);
+  QualType CAT =
+      S.BuildCountAttributedArrayOrPointerType(FD->getType(), CountExpr);
   FD->setType(CAT);
 }
 
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index fddc3545ecb61c..29e9700959cf51 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -9790,9 +9790,9 @@ BuildTypeCoupledDecls(Expr *E,
   Decls.push_back(TypeCoupledDeclRefInfo(CountDecl, /*IsDref*/ false));
 }
 
-QualType Sema::BuildCountAttributedArrayType(QualType WrappedTy,
-                                             Expr *CountExpr) {
-  assert(WrappedTy->isIncompleteArrayType());
+QualType Sema::BuildCountAttributedArrayOrPointerType(QualType WrappedTy,
+                                                      Expr *CountExpr) {
+  assert(WrappedTy->isIncompleteArrayType() || WrappedTy->isPointerType());
 
   llvm::SmallVector<TypeCoupledDeclRefInfo, 1> Decls;
   BuildTypeCoupledDecls(CountExpr, Decls);
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index f47bc219e6fa32..7c37066e6b6152 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -7337,7 +7337,7 @@ QualType TreeTransform<Derived>::TransformCountAttributedType(
   if (getDerived().AlwaysRebuild() || InnerTy != OldTy->desugar() ||
       OldCount != NewCount) {
     // Currently, CountAttributedType can only wrap incomplete array types.
-    Result = SemaRef.BuildCountAttributedArrayType(InnerTy, NewCount);
+    Result = SemaRef.BuildCountAttributedArrayOrPointerType(InnerTy, NewCount);
   }
 
   TLB.push<CountAttributedTypeLoc>(Result);
diff --git a/clang/test/AST/attr-counted-by-late-parsed.c b/clang/test/AST/attr-counted-by-late-parsed.c
new file mode 100644
index 00000000000000..1f37747dd7e3e3
--- /dev/null
+++ b/clang/test/AST/attr-counted-by-late-parsed.c
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -fexperimental-late-parse-attributes %s -ast-dump | FileCheck %s
+
+#define __counted_by(f)  __attribute__((counted_by(f)))
+
+struct size_known {
+  int field;
+};
+
+struct at_decl {
+  struct size_known *buf __counted_by(count);
+  int count;
+};
+// CHECK-LABEL: struct at_decl definition
+// CHECK-NEXT: |-FieldDecl {{.*}} buf 'struct size_known * __counted_by(count)':'struct size_known *'
+// CHECK-NEXT: `-FieldDecl {{.*}} referenced count 'int'
+
+struct at_decl_anon_count {
+  struct size_known *buf __counted_by(count);
+  struct {
+    int count;
+  };
+};
+
+// CHECK-LABEL: struct at_decl_anon_count definition
+// CHECK-NEXT:  |-FieldDecl {{.*}} buf 'struct size_known * __counted_by(count)':'struct size_known *'
+// CHECK-NEXT:  |-RecordDecl {{.*}} struct definition
+// CHECK-NEXT:  | `-FieldDecl {{.*}} count 'int'
+// CHECK-NEXT:  |-FieldDecl {{.*}} implicit 'struct at_decl_anon_count::(anonymous at {{.*}})'
+// CHECK-NEXT:  `-IndirectFieldDecl {{.*}} implicit referenced count 'int'
+// CHECK-NEXT:    |-Field {{.*}} '' 'struct at_decl_anon_count::(anonymous at {{.*}})'
+// CHECK-NEXT:    `-Field {{.*}} 'count' 'int'
diff --git a/clang/test/Sema/attr-counted-by-late-parsed-off.c b/clang/test/Sema/attr-counted-by-late-parsed-off.c
new file mode 100644
index 00000000000000..34f51d10c08389
--- /dev/null
+++ b/clang/test/Sema/attr-counted-by-late-parsed-off.c
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -DNEEDS_LATE_PARSING -fno-experimental-late-parse-attributes -fsyntax-only -verify %s
+// RUN: %clang_cc1 -DNEEDS_LATE_PARSING -fsyntax-only -verify %s
+
+// RUN: %clang_cc1 -UNEEDS_LATE_PARSING -fno-experimental-late-parse-attributes -fsyntax-only -verify=ok %s
+// RUN: %clang_cc1 -UNEEDS_LATE_PARSING -fsyntax-only -verify=ok %s
+
+#define __counted_by(f)  __attribute__((counted_by(f)))
+
+struct size_known { int dummy; };
+
+#ifdef NEEDS_LATE_PARSING
+struct on_decl {
+  // expected-error at +1{{use of undeclared identifier 'count'}}
+  struct size_known *buf __counted_by(count);
+  int count;
+};
+
+#else
+
+// ok-no-diagnostics
+struct on_decl {
+  int count;
+  struct size_known *buf __counted_by(count);
+};
+
+#endif
diff --git a/clang/test/Sema/attr-counted-by-late-parsed-struct-ptrs.c b/clang/test/Sema/attr-counted-by-late-parsed-struct-ptrs.c
new file mode 100644
index 00000000000000..b2fc625c73eb9f
--- /dev/null
+++ b/clang/test/Sema/attr-counted-by-late-parsed-struct-ptrs.c
@@ -0,0 +1,169 @@
+// RUN: %clang_cc1 -fexperimental-late-parse-attributes -fsyntax-only -verify %s
+
+#define __counted_by(f)  __attribute__((counted_by(f)))
+
+struct size_unknown;
+struct size_known {
+  int field;
+};
+
+//==============================================================================
+// __counted_by on struct member pointer in decl attribute position
+//==============================================================================
+
+struct on_member_pointer_complete_ty {
+  struct size_known * buf __counted_by(count);
+  int count;
+};
+
+struct on_member_pointer_incomplete_ty {
+  struct size_unknown * buf __counted_by(count); // expected-error{{'counted_by' cannot be applied a pointer with pointee with unknown size because 'struct size_unknown' is an incomplete type}}
+  int count;
+};
+
+struct on_member_pointer_void_ty {
+  void* buf __counted_by(count); // expected-error{{'counted_by' cannot be applied a pointer with pointee with unknown size because 'void' is an incomplete type}}
+  int count;
+};
+
+struct on_member_pointer_fn_ty {
+  // expected-error at +1{{'counted_by' cannot be applied a pointer with pointee with unknown size because 'void (void)' is a function type}}
+  void (*fn_ptr)(void) __counted_by(count);
+  int count;
+};
+
+struct has_unannotated_vla {
+  int count;
+  int buffer[];
+};
+
+struct on_member_pointer_struct_with_vla {
+  // expected-error at +1{{'counted_by' cannot be applied a pointer with pointee with unknown size because 'struct has_unannotated_vla' is a struct type with a flexible array member}}
+  struct has_unannotated_vla* objects __counted_by(count);
+  int count;
+};
+
+struct has_annotated_vla {
+  int count;
+  int buffer[] __counted_by(count);
+};
+
+// Currently prevented because computing the size of `objects` at runtime would
+// require an O(N) walk of `objects` to take into account the length of the VLA
+// in each struct instance.
+struct on_member_pointer_struct_with_annotated_vla {
+  // expected-error at +1{{'counted_by' cannot be applied a pointer with pointee with unknown size because 'struct has_annotated_vla' is a struct type with a flexible array member}}
+  struct has_annotated_vla* objects __counted_by(count);
+  int count;
+};
+
+struct on_pointer_anon_buf {
+  // TODO: Support referring to parent scope
+  struct {
+    // expected-error at +1{{use of undeclared identifier 'count'}}
+    struct size_known *buf __counted_by(count);
+  };
+  int count;
+};
+
+struct on_pointer_anon_count {
+  struct size_known *buf __counted_by(count);
+  struct {
+    int count;
+  };
+};
+
+//==============================================================================
+// __counted_by on struct member pointer in type attribute position
+//==============================================================================
+// TODO: Correctly parse counted_by as a type attribute. Currently it is parsed
+// as a declaration attribute and is **not** late parsed resulting in the `count`
+// field being unavailable.
+
+struct on_member_pointer_complete_ty_ty_pos {
+  // expected-error at +1{{use of undeclared identifier 'count'}}
+  struct size_known *__counted_by(count) buf;
+  int count;
+};
+
+struct on_member_pointer_incomplete_ty_ty_pos {
+  // expected-error at +1{{use of undeclared identifier 'count'}}
+  struct size_unknown * __counted_by(count) buf;
+  int count;
+};
+
+struct on_member_pointer_void_ty_ty_pos {
+  // expected-error at +1{{use of undeclared identifier 'count'}}
+  void *__counted_by(count) buf;
+  int count;
+};
+
+struct on_member_pointer_fn_ty_ty_pos {
+  // expected-error at +1{{use of undeclared identifier 'count'}}
+  void (* __counted_by(count) fn_ptr)(void);
+  int count;
+};
+
+struct on_member_pointer_fn_ptr_ty_ty_pos {
+  // expected-error at +1{{use of undeclared identifier 'count'}}
+  void (** __counted_by(count) fn_ptr)(void);
+  int count;
+};
+
+struct on_member_pointer_fn_ptr_ty_ty_pos_inner {
+  // expected-error at +1{{use of undeclared identifier 'count'}}
+  void (* __counted_by(count) * fn_ptr)(void);
+  int count;
+};
+
+struct on_member_pointer_struct_with_vla_ty_pos {
+  // expected-error at +1{{use of undeclared identifier 'count'}}
+  struct has_unannotated_vla *__counted_by(count) objects;
+  int count;
+};
+
+struct on_member_pointer_struct_with_annotated_vla_ty_pos {
+  // expected-error at +1{{use of undeclared identifier 'count'}}
+  struct has_annotated_vla* __counted_by(count) objects;
+  int count;
+};
+
+struct on_nested_pointer {
+  // expected-error at +1{{use of undeclared identifier 'count'}}
+  struct size_known *__counted_by(count) *buf;
+  int count;
+};
+
+struct on_pointer_anon_buf_ty_pos {
+  struct {
+    // TODO: Support referring to parent scope
+    // expected-error at +1{{use of undeclared identifier 'count'}}
+    struct size_known * __counted_by(count) buf;
+  };
+  int count;
+};
+
+struct on_pointer_anon_count_ty_pos {
+  // expected-error at +1{{use of undeclared identifier 'count'}}
+  struct size_known *__counted_by(count) buf;
+  struct {
+    int count;
+  };
+};
+
+//==============================================================================
+// __counted_by on struct non-pointer members
+//==============================================================================
+
+struct on_pod_ty {
+  // expected-error at +1{{'counted_by' only applies to pointers or C99 flexible array members}}
+  int wrong_ty __counted_by(count);
+  int count;
+};
+
+struct on_void_ty {
+  // expected-error at +2{{'counted_by' only applies to pointers or C99 flexible array members}}
+  // expected-error at +1{{field has incomplete type 'void'}}
+  void wrong_ty __counted_by(count);
+  int count;
+};
diff --git a/clang/test/Sema/attr-counted-by-struct-ptrs-sizeless-types.c b/clang/test/Sema/attr-counted-by-struct-ptrs-sizeless-types.c
new file mode 100644
index 00000000000000..fa76f236f90e57
--- /dev/null
+++ b/clang/test/Sema/attr-counted-by-struct-ptrs-sizeless-types.c
@@ -0,0 +1,17 @@
+// __SVInt8_t is specific to ARM64 so specify that in the target triple
+// RUN: %clang_cc1 -triple arm64-apple-darwin -fsyntax-only -verify %s
+
+#define __counted_by(f)  __attribute__((counted_by(f)))
+
+struct on_sizeless_pointee_ty {
+    int count;
+    // expected-error at +1{{'counted_by' cannot be applied a pointer with pointee with unknown size because '__SVInt8_t' is a sizeless type}}
+    __SVInt8_t* member __counted_by(count);
+};
+
+struct on_sizeless_ty {
+    int count;
+    // expected-error at +2{{'counted_by' only applies to pointers or C99 flexible array members}}
+    // expected-error at +1{{field has sizeless type '__SVInt8_t'}}
+    __SVInt8_t member __counted_by(count);
+};
diff --git a/clang/test/Sema/attr-counted-by-struct-ptrs.c b/clang/test/Sema/attr-counted-by-struct-ptrs.c
new file mode 100644
index 00000000000000..f139cc7a6e2d6c
--- /dev/null
+++ b/clang/test/Sema/attr-counted-by-struct-ptrs.c
@@ -0,0 +1,163 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+#define __counted_by(f)  __attribute__((counted_by(f)))
+
+struct size_unknown;
+struct size_known {
+  int field;
+};
+
+//==============================================================================
+// __counted_by on struct member pointer in decl attribute position
+//==============================================================================
+
+struct on_member_pointer_complete_ty {
+  int count;
+  struct size_known * buf __counted_by(count);
+};
+
+struct on_member_pointer_incomplete_ty {
+  int count;
+  struct size_unknown * buf __counted_by(count); // expected-error{{'counted_by' cannot be applied a pointer with pointee with unknown size because 'struct size_unknown' is an incomplete type}}
+};
+
+struct on_member_pointer_void_ty {
+  int count;
+  void* buf __counted_by(count); // expected-error{{'counted_by' cannot be applied a pointer with pointee with unknown size because 'void' is an incomplete type}}
+};
+
+struct on_member_pointer_fn_ty {
+  int count;
+  // expected-error at +1{{'counted_by' cannot be applied a pointer with pointee with unknown size because 'void (void)' is a function type}}
+  void (*fn_ptr)(void) __counted_by(count);
+};
+
+struct has_unannotated_vla {
+  int count;
+  int buffer[];
+};
+
+struct on_member_pointer_struct_with_vla {
+  int count;
+  // expected-error at +1{{'counted_by' cannot be applied a pointer with pointee with unknown size because 'struct has_unannotated_vla' is a struct type with a flexible array member}}
+  struct has_unannotated_vla* objects __counted_by(count);
+};
+
+struct has_annotated_vla {
+  int count;
+  int buffer[] __counted_by(count);
+};
+
+// Currently prevented because computing the size of `objects` at runtime would
+// require an O(N) walk of `objects` to take into account the length of the VLA
+// in each struct instance.
+struct on_member_pointer_struct_with_annotated_vla {
+  int count;
+  // expected-error at +1{{'counted_by' cannot be applied a pointer with pointee with unknown size because 'struct has_annotated_vla' is a struct type with a flexible array member}}
+  struct has_annotated_vla* objects __counted_by(count);
+};
+
+struct on_pointer_anon_buf {
+  int count;
+  struct {
+    struct size_known *buf __counted_by(count);
+  };
+};
+
+struct on_pointer_anon_count {
+  struct {
+    int count;
+  };
+  struct size_known *buf __counted_by(count);
+};
+
+//==============================================================================
+// __counted_by on struct member pointer in type attribute position
+//==============================================================================
+// TODO: Correctly parse counted_by as a type attribute. Currently it is parsed
+// as a declaration attribute
+
+struct on_member_pointer_complete_ty_ty_pos {
+  int count;
+  struct size_known *__counted_by(count) buf ;
+};
+
+struct on_member_pointer_incomplete_ty_ty_pos {
+  int count;
+  struct size_unknown * __counted_by(count) buf; // expected-error{{'counted_by' cannot be applied a pointer with pointee with unknown size because 'struct size_unknown' is an incomplete type}}
+};
+
+struct on_member_pointer_void_ty_ty_pos {
+  int count;
+  void *__counted_by(count) buf; // expected-error{{'counted_by' cannot be applied a pointer with pointee with unknown size because 'void' is an incomplete type}}
+};
+
+struct on_member_pointer_fn_ty_ty_pos {
+  int count;
+  // expected-error at +1{{'counted_by' cannot be applied a pointer with pointee with unknown size because 'void (void)' is a function type}}
+  void (* __counted_by(count) fn_ptr)(void);
+};
+
+struct on_member_pointer_fn_ptr_ty_ty_pos {
+  int count;
+  // A buffer of function pointers that is counted by `count` is ok.
+  void (** __counted_by(count) fn_ptr)(void);
+};
+
+// TODO: This should be forbidden but isn't due to counted_by being treated
+// as a declaration attribute.
+struct on_member_pointer_fn_ptr_ty_ty_pos_inner {
+  int count;
+  void (* __counted_by(count) * fn_ptr)(void);
+};
+
+struct on_member_pointer_struct_with_vla_ty_pos {
+  int count;
+  // expected-error at +1{{'counted_by' cannot be applied a pointer with pointee with unknown size because 'struct has_unannotated_vla' is a struct type with a flexible array member}}
+  struct has_unannotated_vla *__counted_by(count) objects;
+};
+
+// Currently prevented because computing the size of `objects` at runtime would
+// require an O(N) walk of `objects` to take into account the length of the VLA
+// in each struct instance.
+struct on_member_pointer_struct_with_annotated_vla_ty_pos {
+  int count;
+  // expected-error at +1{{'counted_by' cannot be applied a pointer with pointee with unknown size because 'struct has_annotated_vla' is a struct type with a flexible array member}}
+  struct has_annotated_vla* __counted_by(count) objects;
+};
+
+struct on_nested_pointer {
+  int count;
+  struct size_known *__counted_by(count) *buf;
+};
+
+struct on_pointer_anon_buf_ty_pos {
+  int count;
+  struct {
+    struct size_known * __counted_by(count) buf;
+  };
+};
+
+struct on_pointer_anon_count_ty_pos {
+  struct {
+    int count;
+  };
+  struct size_known *__counted_by(count) buf;
+};
+
+//==============================================================================
+// __counted_by on struct non-pointer members
+//==============================================================================
+
+struct on_pod_ty {
+  int count;
+  // expected-error at +1{{'counted_by' only applies to pointers or C99 flexible array members}}
+  int wrong_ty __counted_by(count);
+};
+
+struct on_void_ty {
+  int count;
+  // expected-error at +2{{'counted_by' only applies to pointers or C99 flexible array members}}
+  // expected-error at +1{{field has incomplete type 'void'}}
+  void wrong_ty __counted_by(count);
+};
diff --git a/clang/test/Sema/attr-counted-by.c b/clang/test/Sema/attr-counted-by.c
index d5d4ebf5573922..54faf9b888087c 100644
--- a/clang/test/Sema/attr-counted-by.c
+++ b/clang/test/Sema/attr-counted-by.c
@@ -95,12 +95,13 @@ struct array_of_ints_count {
 
 struct not_a_fam {
   int count;
-  struct bar *non_fam __counted_by(count); // expected-error {{'counted_by' only applies to C99 flexible array members}}
+  // expected-error at +1{{'counted_by' cannot be applied a pointer with pointee with unknown size because 'struct bar' is an incomplete type}}
+  struct bar *non_fam __counted_by(count);
 };
 
 struct not_a_c99_fam {
   int count;
-  struct bar *non_c99_fam[0] __counted_by(count); // expected-error {{'counted_by' only applies to C99 flexible array members}}
+  struct bar *non_c99_fam[0] __counted_by(count); // expected-error {{'counted_by' on arrays only applies to C99 flexible array members}}
 };
 
 struct annotated_with_anon_struct {



More information about the cfe-commits mailing list