[Bounds-Safety] Address review comments

- use enum to index into diagnostics
- rename err_counted_by* diagnostics to err_count_attr* when the
  diagnostic applies to other attributes than only counted_by (sized_by,
counted_by_or_null, sized_by_or_null)
- suggest using counted_by when arrays are annotated with sized_by,
counted_by_or_null or  sized_by_or_null
- update release notes with example use case for sized_by_or_null

Author: hnrklssn

clang/docs/ReleaseNotes.rst

@@ -559,7 +559,9 @@ Attribute Changes in Clang
   ``sized_by`` takes a byte size parameter instead of an element count, allowing pointees
   with unknown size. The ``counted_by_or_null`` and ``sized_by_or_null`` variants are equivalent
   to their base variants, except the pointer can be null regardless of count/size value.
-  If the pointer is null the size is effectively 0.
+  If the pointer is null the size is effectively 0. ``sized_by_or_null`` is needed to properly
+  annotate allocator functions like ``malloc`` that return a buffer of a given byte size, but can
+  also return null.
 
 - The ``guarded_by``, ``pt_guarded_by``, ``acquired_after``, ``acquired_before``
   attributes now support referencing struct members in C. The arguments are also

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -6567,23 +6567,23 @@ def warn_superclass_variable_sized_type_not_at_end : Warning<
   "field %0 can overwrite instance variable %1 with variable sized type %2"
   " in superclass %3">, InGroup<ObjCFlexibleArray>;
 
-def err_flexible_array_count_not_in_same_struct : Error<
-  "'%select{counted_by|sized_by|counted_by_or_null|sized_by_or_null}1' field %0 isn't within the same struct as the flexible array">;
-def err_counted_by_attr_not_on_ptr_or_flexible_array_member : Error<
-  "'%select{counted_by|sized_by|counted_by_or_null|sized_by_or_null}0' only applies to pointers%select{ or C99 flexible array members|||}0">;
+def err_count_attr_param_not_in_same_struct : Error<
+  "'%select{counted_by|sized_by|counted_by_or_null|sized_by_or_null}1' field %0 isn't within the same struct as the annotated %select{pointer|flexible array}2">;
+def err_count_attr_not_on_ptr_or_flexible_array_member : Error<
+  "'%select{counted_by|sized_by|counted_by_or_null|sized_by_or_null}0' only applies to pointers%select{ or C99 flexible array members|||}0%select{|; did you mean to use 'counted_by'?}1">;
 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<
+def err_count_attr_must_be_in_structure : Error<
   "field %0 in '%select{counted_by|sized_by|counted_by_or_null|sized_by_or_null}1' not inside structure">;
-def err_counted_by_attr_argument_not_integer : Error<
+def err_count_attr_argument_not_integer : Error<
   "'%select{counted_by|sized_by|counted_by_or_null|sized_by_or_null}0' requires a non-boolean integer type argument">;
-def err_counted_by_attr_only_support_simple_decl_reference : Error<
+def err_count_attr_only_support_simple_decl_reference : Error<
   "'%select{counted_by|sized_by|counted_by_or_null|sized_by_or_null}0' argument must be a simple declaration reference">;
-def err_counted_by_attr_in_union : Error<
+def err_count_attr_in_union : Error<
   "'%select{counted_by|sized_by|counted_by_or_null|sized_by_or_null}0' cannot be applied to a union member">;
-def err_counted_by_attr_refer_to_union : Error<
+def err_count_attr_refer_to_union : Error<
   "'%select{counted_by|sized_by|counted_by_or_null|sized_by_or_null}0' argument cannot refer to a union member">;
 def note_flexible_array_counted_by_attr_field : Note<
   "field %0 declared here">;

clang/lib/Parse/ParseDecl.cpp

@@ -4869,9 +4869,8 @@ static void DiagnoseCountAttributedTypeInUnnamedAnon(ParsingDeclSpec &DS,
 
     for (const auto &DD : CAT->dependent_decls()) {
       if (!RD->containsDecl(DD.getDecl())) {
-        P.Diag(VD->getBeginLoc(),
-               diag::err_flexible_array_count_not_in_same_struct)
-            << DD.getDecl() << CAT->getKind();
+        P.Diag(VD->getBeginLoc(), diag::err_count_attr_param_not_in_same_struct)
+            << DD.getDecl() << CAT->getKind() << CAT->isArrayType();
         P.Diag(DD.getDecl()->getBeginLoc(),
                diag::note_flexible_array_counted_by_attr_field)
             << DD.getDecl();

clang/lib/Sema/SemaDeclAttr.cpp

@@ -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,
@@ -5882,27 +5891,25 @@ CheckCountedByAttrOnField(Sema &S, FieldDecl *FD, Expr *E,
                           bool CountInBytes, bool OrNull) {
   // Check the context the attribute is used in
 
-  unsigned Kind = CountInBytes;
-  if (OrNull)
-    Kind += 2;
+  unsigned Kind = getCountAttrKind(CountInBytes, OrNull);
 
   if (FD->getParent()->isUnion()) {
-    S.Diag(FD->getBeginLoc(), diag::err_counted_by_attr_in_union)
+    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_counted_by_attr_not_on_ptr_or_flexible_array_member)
-        << Kind << FD->getLocation();
+           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)
-        << Kind << FD->getLocation();
+           diag::err_count_attr_not_on_ptr_or_flexible_array_member)
+        << Kind << FD->getLocation() << /* do not suggest counted_by */ 0;
     return true;
   }
 
@@ -5967,15 +5974,15 @@ CheckCountedByAttrOnField(Sema &S, FieldDecl *FD, Expr *E,
   // Check the expression
 
   if (!E->getType()->isIntegerType() || E->getType()->isBooleanType()) {
-    S.Diag(E->getBeginLoc(), diag::err_counted_by_attr_argument_not_integer)
+    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)
+           diag::err_count_attr_only_support_simple_decl_reference)
         << Kind << E->getSourceRange();
     return true;
   }
@@ -5986,7 +5993,7 @@ CheckCountedByAttrOnField(Sema &S, FieldDecl *FD, Expr *E,
     CountFD = IFD->getAnonField();
   }
   if (!CountFD) {
-    S.Diag(E->getBeginLoc(), diag::err_counted_by_must_be_in_structure)
+    S.Diag(E->getBeginLoc(), diag::err_count_attr_must_be_in_structure)
         << CountDecl << Kind << E->getSourceRange();
 
     S.Diag(CountDecl->getBeginLoc(),
@@ -5997,7 +6004,7 @@ CheckCountedByAttrOnField(Sema &S, FieldDecl *FD, Expr *E,
 
   if (FD->getParent() != CountFD->getParent()) {
     if (CountFD->getParent()->isUnion()) {
-      S.Diag(CountFD->getBeginLoc(), diag::err_counted_by_attr_refer_to_union)
+      S.Diag(CountFD->getBeginLoc(), diag::err_count_attr_refer_to_union)
           << Kind << CountFD->getSourceRange();
       return true;
     }
@@ -6008,9 +6015,8 @@ CheckCountedByAttrOnField(Sema &S, FieldDecl *FD, Expr *E,
     auto *CountRD = GetEnclosingNamedOrTopAnonRecord(CountFD);
 
     if (RD != CountRD) {
-      S.Diag(E->getBeginLoc(),
-             diag::err_flexible_array_count_not_in_same_struct)
-          << CountFD << Kind << 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();

clang/test/Sema/attr-counted-by-or-null-last-field.c

@@ -0,0 +1,141 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+#define __counted_by_or_null(f)  __attribute__((counted_by_or_null(f)))
+
+// This has been adapted from clang/test/Sema/attr-counted-by-vla.c, but with VLAs replaced with pointers
+
+struct bar;
+
+struct not_found {
+  int count;
+  struct bar *ptr __counted_by_or_null(bork); // expected-error {{use of undeclared identifier 'bork'}}
+};
+
+struct no_found_count_not_in_substruct {
+  unsigned long flags;
+  unsigned char count; // expected-note {{'count' declared here}}
+  struct A {
+    int dummy;
+    int * ptr __counted_by_or_null(count); // expected-error {{'counted_by_or_null' field 'count' isn't within the same struct as the annotated pointer}}
+  } a;
+};
+
+struct not_found_count_not_in_unnamed_substruct {
+  unsigned char count; // expected-note {{'count' declared here}}
+  struct {
+    int dummy;
+    int * ptr __counted_by_or_null(count); // expected-error {{'counted_by_or_null' field 'count' isn't within the same struct as the annotated pointer}}
+  } a;
+};
+
+struct not_found_count_not_in_unnamed_substruct_2 {
+  struct {
+    unsigned char count; // expected-note {{'count' declared here}}
+  };
+  struct {
+    int dummy;
+    int * ptr __counted_by_or_null(count); // expected-error {{'counted_by_or_null' field 'count' isn't within the same struct as the annotated pointer}}
+  } a;
+};
+
+struct not_found_count_in_other_unnamed_substruct {
+  struct {
+    unsigned char count;
+  } a1;
+
+  struct {
+    int dummy;
+    int * ptr __counted_by_or_null(count); // expected-error {{use of undeclared identifier 'count'}}
+  };
+};
+
+struct not_found_count_in_other_substruct {
+  struct _a1 {
+    unsigned char count;
+  } a1;
+
+  struct {
+    int dummy;
+    int * ptr __counted_by_or_null(count); // expected-error {{use of undeclared identifier 'count'}}
+  };
+};
+
+struct not_found_count_in_other_substruct_2 {
+  struct _a2 {
+    unsigned char count;
+  } a2;
+
+  int * ptr __counted_by_or_null(count); // expected-error {{use of undeclared identifier 'count'}}
+};
+
+struct not_found_suggest {
+  int bork;
+  struct bar **ptr __counted_by_or_null(blork); // expected-error {{use of undeclared identifier 'blork'}}
+};
+
+int global; // expected-note {{'global' declared here}}
+
+struct found_outside_of_struct {
+  int bork;
+  struct bar ** ptr __counted_by_or_null(global); // expected-error {{field 'global' in 'counted_by_or_null' not inside structure}}
+};
+
+struct self_referrential {
+  int bork;
+  struct bar *self[] __counted_by_or_null(self); // expected-error {{use of undeclared identifier 'self'}}
+};
+
+struct non_int_count {
+  double dbl_count;
+  struct bar ** ptr __counted_by_or_null(dbl_count); // expected-error {{'counted_by_or_null' requires a non-boolean integer type argument}}
+};
+
+struct array_of_ints_count {
+  int integers[2];
+  struct bar ** ptr __counted_by_or_null(integers); // expected-error {{'counted_by_or_null' requires a non-boolean integer type argument}}
+};
+
+struct not_a_c99_fam {
+  int count;
+  struct bar *non_c99_fam[0] __counted_by_or_null(count); // expected-error {{'counted_by_or_null' only applies to pointers; did you mean to use 'counted_by'?}}
+};
+
+struct annotated_with_anon_struct {
+  unsigned long flags;
+  struct {
+    unsigned char count;
+    int * ptr __counted_by_or_null(crount); // expected-error {{use of undeclared identifier 'crount'}}
+  };
+};
+
+//==============================================================================
+// __counted_by_or_null on a struct ptr with element type that has unknown count
+//==============================================================================
+
+struct count_unknown;
+struct on_member_ptr_incomplete_ty_ty_pos {
+  int count;
+  struct count_unknown * ptr __counted_by_or_null(count); // expected-error {{'counted_by_or_null' cannot be applied to a pointer with pointee of unknown size because 'struct count_unknown' is an incomplete type}}
+};
+
+struct on_member_ptr_incomplete_const_ty_ty_pos {
+  int count;
+  const struct count_unknown * ptr __counted_by_or_null(count); // expected-error {{'counted_by_or_null' cannot be applied to a pointer with pointee of unknown size because 'const struct count_unknown' is an incomplete type}}
+};
+
+struct on_member_ptr_void_ty_ty_pos {
+  int count;
+  void * ptr __counted_by_or_null(count); // expected-error {{'counted_by_or_null' cannot be applied to a pointer with pointee of unknown size because 'void' is an incomplete type}}
+};
+
+typedef void(fn_ty)(int);
+
+struct on_member_ptr_fn_ptr_ty {
+  int count;
+  fn_ty* * ptr __counted_by_or_null(count);
+};
+
+struct on_member_ptr_fn_ty {
+  int count;
+  fn_ty * ptr __counted_by_or_null(count); // expected-error {{'counted_by_or_null' cannot be applied to a pointer with pointee of unknown size because 'fn_ty' (aka 'void (int)') is a function type}}
+};

clang/test/Sema/attr-counted-by-or-null-late-parsed-struct-ptrs.c

@@ -242,13 +242,13 @@ struct on_pointer_anon_count_ty_pos {
 //==============================================================================
 
 struct on_pod_ty {
-  // expected-error@+1{{'counted_by_or_null' only applies to pointers}}
+  // expected-error-re@+1{{'counted_by_or_null' only applies to pointers{{$}}}}
   int wrong_ty __counted_by_or_null(count);
   int count;
 };
 
 struct on_void_ty {
-  // expected-error@+2{{'counted_by_or_null' only applies to pointers}}
+  // expected-error-re@+2{{'counted_by_or_null' only applies to pointers{{$}}}}
   // expected-error@+1{{field has incomplete type 'void'}}
   void wrong_ty __counted_by_or_null(count);
   int count;

clang/test/Sema/attr-counted-by-or-null-struct-ptrs-sizeless-types.c

@@ -11,7 +11,7 @@ struct on_sizeless_pointee_ty {
 
 struct on_sizeless_ty {
     int count;
-    // expected-error@+2{{'counted_by_or_null' only applies to pointers}}
+    // expected-error-re@+2{{'counted_by_or_null' only applies to pointers{{$}}}}
     // expected-error@+1{{field has sizeless type '__SVInt8_t'}}
     __SVInt8_t member __counted_by_or_null(count);
 };

clang/test/Sema/attr-counted-by-or-null-struct-ptrs.c

@@ -213,13 +213,13 @@ struct on_pointer_anon_count_ty_pos {
 
 struct on_pod_ty {
   int count;
-  // expected-error@+1{{'counted_by_or_null' only applies to pointers}}
+  // expected-error-re@+1{{'counted_by_or_null' only applies to pointers{{$}}}}
   int wrong_ty __counted_by_or_null(count);
 };
 
 struct on_void_ty {
   int count;
-  // expected-error@+2{{'counted_by_or_null' only applies to pointers}}
+  // expected-error-re@+2{{'counted_by_or_null' only applies to pointers{{$}}}}
   // expected-error@+1{{field has incomplete type 'void'}}
   void wrong_ty __counted_by_or_null(count);
 };

clang/test/Sema/attr-counted-by-or-null-vla-sizeless-types.c

@@ -5,7 +5,7 @@
 
 struct on_sizeless_elt_ty {
     int count;
-    // expected-error@+2{{'counted_by_or_null' only applies to pointers}}
+    // expected-error-re@+2{{'counted_by_or_null' only applies to pointers{{$}}}}
     // expected-error@+1{{array has sizeless element type '__SVInt8_t'}}
     __SVInt8_t arr[] __counted_by_or_null(count);
 };

clang/test/Sema/attr-counted-by-vla.c

@@ -14,15 +14,15 @@ struct no_found_count_not_in_substruct {
   unsigned char count; // expected-note {{'count' declared here}}
   struct A {
     int dummy;
-    int array[] __counted_by(count); // expected-error {{'counted_by' field 'count' isn't within the same struct as the flexible array}}
+    int array[] __counted_by(count); // expected-error {{'counted_by' field 'count' isn't within the same struct as the annotated flexible array}}
   } a;
 };
 
 struct not_found_count_not_in_unnamed_substruct {
   unsigned char count; // expected-note {{'count' declared here}}
   struct {
     int dummy;
-    int array[] __counted_by(count); // expected-error {{'counted_by' field 'count' isn't within the same struct as the flexible array}}
+    int array[] __counted_by(count); // expected-error {{'counted_by' field 'count' isn't within the same struct as the annotated flexible array}}
   } a;
 };
 
@@ -32,7 +32,7 @@ struct not_found_count_not_in_unnamed_substruct_2 {
   };
   struct {
     int dummy;
-    int array[] __counted_by(count); // expected-error {{'counted_by' field 'count' isn't within the same struct as the flexible array}}
+    int array[] __counted_by(count); // expected-error {{'counted_by' field 'count' isn't within the same struct as the annotated flexible array}}
   } a;
 };