[clang] [BoundsSafety] Allow 'counted_by' attribute on pointers in structs in C (PR #90786)

[Dan Liew via cfe-commits]

================
@@ -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()) {
----------------
delcypher wrote:

Good catch!

## Flexible array member

```
struct Has_VLA {
    int count;
    char buffer[] __counted_by(count);
};

struct Test {
    int count;
    struct Has_VLA Arr[] __counted_by(count);
};
```

There's no diagnostic here currently. I think the code above is something we should disallow (at least for `-fbounds-safety`). I'll adjust the patch to disallow this.

@isanbard Is the above code pattern something you actually want to support? In `-fbounds-safety` we disallow it because it means computing the bounds of `Arr` is not constant time because the size of `Has_VLA` is not a compile time constant. To compute the bounds of `Arr` at runtime it would require walking every `Has_VLA` object in `Arr` to get its `count` field. That seems expensive and is also at risk of running into a race condition (i.e. the size of a VLA changes while the bounds are being computed).

## Incomplete type

```
struct Test {
    int count;
    void Arr[] __counted_by(count);
};
```

We already have these diagnostics:

```
tmp/arr_sizeless_ty.c:5:13: error: array has incomplete element type 'void'
    5 |     void Arr[] __counted_by(count);
      |             ^
tmp/arr_sizeless_ty.c:5:5: error: 'counted_by' only applies to pointers or C99 flexible array members
    5 |     void Arr[] __counted_by(count);
      |     ^    ~~~
2 errors generated.
```

I don't think we need to add another.

## Sizeless types

```
struct Test {
    int count;
    __SVInt8_t Arr[] __counted_by(count);
};
```

We already have these diagnostics:

```
tmp/arr_sizeless_ty.c:5:19: error: array has sizeless element type '__SVInt8_t'
    5 |     __SVInt8_t Arr[] __counted_by(count);
      |                   ^
tmp/arr_sizeless_ty.c:5:5: error: 'counted_by' only applies to pointers or C99 flexible array members
    5 |     __SVInt8_t Arr[] __counted_by(count);
      |     ^          ~~~
2 errors generated.
```

I don't think we need to add another.

## Function Type

```
typedef void(foo_fn)(int);

struct Test {
    int count;
    foo_fn Arr[] __counted_by(count);
};
```

We already have these diagnostics:

```
tmp/arr_sizeless_ty.c:7:15: error: 'Arr' declared as array of functions of type 'foo_fn' (aka 'void (int)')
    7 |     foo_fn Arr[] __counted_by(count);
      |               ^
tmp/arr_sizeless_ty.c:7:5: error: 'counted_by' only applies to pointers or C99 flexible array members
    7 |     foo_fn Arr[] __counted_by(count);
      |     ^      ~~~
2 errors generated.
```

I don't think we need to add another.


https://github.com/llvm/llvm-project/pull/90786