fixup: rework malloc_span validation and error reporting

Author: a-nogikh

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -3450,10 +3450,17 @@ def warn_attribute_return_pointers_only : Warning<
   "%0 attribute only applies to return values that are pointers">,
   InGroup<IgnoredAttributes>;
 def warn_attribute_return_span_only
-    : Warning<"%0 attribute only applies to functions that return span-like structures: "
-    "one field is a pointer to the allocated memory and another field is an integer with "
-    "the size of the allocated memory">,
+    : Warning<"%0 attribute only applies to functions that return span-like "
+              "structures">,
       InGroup<IgnoredAttributes>;
+def note_span_must_be_struct : Note<"span-like type must be a struct">;
+def note_span_must_be_complete : Note<"span-like type must be complete">;
+def note_wrong_span_field_count : Note<"span-like type must have 2 fields">;
+def note_wrong_span_field_types
+    : Note<"span-like type must have a pointer and an integer field or two "
+           "pointer fields">;
+def note_span_invalid_integer : Note<"the integer field must be an actual "
+                                     "integer that is at least as big as int">;
 def warn_attribute_return_pointers_refs_only : Warning<
   "%0 attribute only applies to return values that are pointers or references">,
   InGroup<IgnoredAttributes>;

clang/lib/Sema/SemaDeclAttr.cpp

@@ -1839,43 +1839,72 @@ static void handleRestrictAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
                  RestrictAttr(S.Context, AL, DeallocE, DeallocPtrIdx));
 }
 
-static bool checkSpanLikeType(const QualType &Ty) {
+static bool checkSpanLikeType(Sema &S, const ParsedAttr &AL,
+                              const QualType &Ty) {
   // Check that the type is a plain record with one field being a pointer
   // type and the other field being an integer. This matches the common
   // implementation of std::span or sized_allocation_t in P0901R11.
-  // Note that there may also be numerous cases of pointer+integer structures
-  // not actually exhibiting a span-like semantics, so sometimes
-  // this heuristic expectedly leads to false positive results.
+  // Note that there may also be numerous cases of pointer + integer /
+  // pointer + pointer / integer + pointer structures not actually exhibiting
+  // a span-like semantics, so sometimes these heuristics expectedly
+  // lead to false positive results.
+  auto emitWarning = [&S, &AL](unsigned NoteDiagID) {
+    S.Diag(AL.getLoc(), diag::warn_attribute_return_span_only) << AL;
+    S.Diag(AL.getLoc(), NoteDiagID);
+  };
   const RecordDecl *RD = Ty->getAsRecordDecl();
-  if (!RD || RD->isUnion())
+  if (!RD || RD->isUnion()) {
+    emitWarning(diag::note_span_must_be_struct);
     return false;
+  }
   const RecordDecl *Def = RD->getDefinition();
-  if (!Def)
+  if (!Def) {
+    emitWarning(diag::note_span_must_be_complete);
     return false; // This is an incomplete type.
+  }
   auto FieldsBegin = Def->field_begin();
-  if (std::distance(FieldsBegin, Def->field_end()) != 2)
+  if (std::distance(FieldsBegin, Def->field_end()) != 2) {
+    emitWarning(diag::note_wrong_span_field_count);
     return false;
+  }
   const QualType FirstFieldType = FieldsBegin->getType();
   const QualType SecondFieldType = std::next(FieldsBegin)->getType();
   auto validatePointerType = [](const QualType &T) {
     // It must not point to functions.
     return T->isPointerType() && !T->isFunctionPointerType();
   };
-  // Verify two possible orderings.
-  return (validatePointerType(FirstFieldType) &&
-          SecondFieldType->isIntegerType()) ||
-         (FirstFieldType->isIntegerType() &&
-          validatePointerType(SecondFieldType));
+  auto checkIntegerType = [&S, emitWarning](const QualType &T) {
+    bool valid = false;
+    // Must be an actual integer and at least as bit as int.
+    if (const auto *BT = dyn_cast<BuiltinType>(T.getCanonicalType())) {
+      const auto IntSize = S.Context.getTypeSize(S.Context.IntTy);
+      valid = BT->isInteger() && S.Context.getTypeSize(BT) >= IntSize;
+    }
+    if (!valid) {
+      emitWarning(diag::note_span_invalid_integer);
+    }
+    return valid;
+  };
+  if (validatePointerType(FirstFieldType) &&
+      validatePointerType(SecondFieldType)) {
+    // Pointer + pointer.
+    return true;
+  } else if (validatePointerType(FirstFieldType)) {
+    // Pointer + integer?
+    return checkIntegerType(SecondFieldType);
+  } else if (validatePointerType(SecondFieldType)) {
+    // Integer + pointer?
+    return checkIntegerType(FirstFieldType);
+  }
+  emitWarning(diag::note_wrong_span_field_types);
+  return false;
 }
 
 static void handleMallocSpanAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   QualType ResultType = getFunctionOrMethodResultType(D);
-  if (!checkSpanLikeType(ResultType)) {
-    S.Diag(AL.getLoc(), diag::warn_attribute_return_span_only)
-        << AL << getFunctionOrMethodResultSourceRange(D);
-    return;
+  if (checkSpanLikeType(S, AL, ResultType)) {
+    D->addAttr(::new (S.Context) MallocSpanAttr(S.Context, AL));
   }
-  D->addAttr(::new (S.Context) MallocSpanAttr(S.Context, AL));
 }
 
 static void handleCPUSpecificAttr(Sema &S, Decl *D, const ParsedAttr &AL) {

clang/test/Sema/attr-malloc_span.c

@@ -16,23 +16,57 @@ typedef struct {
 } span2;
 span2  returns_span2  (void) __attribute((malloc_span)); // no-warning
 
-// Ensure that a warning is produced on malloc_span precondition violation.
 typedef struct {
   void *ptr;
   void *ptr2;
-} invalid_span1;
-invalid_span1  returns_non_std_span1  (void) __attribute((malloc_span)); // expected-warning {{attribute only applies to functions that return span-like structures}}
+} span3;
+span3  returns_span3  (void) __attribute((malloc_span)); // no-warning
+// expected-warning@+2 {{attribute only applies to functions that return span-like structures}}
+// expected-note@+1 {{span-like type must be a struct}}
+int *returns_int_ptr  (void) __attribute((malloc_span));
 
 typedef struct {
   void *ptr;
   size_t n;
   size_t n2;
-} invalid_span2;
-invalid_span2  returns_non_std_span2  (void) __attribute((malloc_span)); // expected-warning {{attribute only applies to functions that return span-like structures}}
+} too_long_span;
+// expected-warning@+2 {{attribute only applies to functions that return span-like structures}}
+// expected-note@+1 {{span-like type must have 2 fields}}
+too_long_span  returns_too_long_span  (void) __attribute((malloc_span));
 
 // Function pointers are not allowed.
 typedef struct {
   int (*func_ptr)(void);
   size_t n;
 } func_span;
-func_span  returns_func_span  (void) __attribute((malloc_span)); // expected-warning {{attribute only applies to functions that return span-like structures}}
+// expected-warning@+2 {{attribute only applies to functions that return span-like structures}}
+// expected-note@+1 {{span-like type must have a pointer and an integer field or two pointer fields}}
+func_span  returns_func_span  (void) __attribute((malloc_span));
+
+// Integer should not be an enum.
+enum some_enum { some_value, other_value };
+typedef struct {
+  void *ptr;
+  enum some_enum field;
+} enum_span;
+// expected-warning@+2 {{attribute only applies to functions that return span-like structures}}
+// expected-note@+1 {{the integer field must be an actual integer}}
+enum_span  returns_enum_span  (void) __attribute((malloc_span));
+
+// Bit integers are also not supported.
+typedef struct {
+  void *ptr;
+  _BitInt(16) n;
+} bit_span;
+// expected-warning@+2 {{attribute only applies to functions that return span-like structures}}
+// expected-note@+1 {{the integer field must be an actual integer}}
+bit_span  returns_bit_span  (void) __attribute((malloc_span));
+
+// Integer must be at least as big as int.
+typedef struct {
+  void *ptr;
+  short n;
+} short_span;
+// expected-warning@+2 {{attribute only applies to functions that return span-like structures}}
+// expected-note@+1 {{the integer field must be an actual integer}}
+short_span  returns_short_span  (void) __attribute((malloc_span));

clang/test/SemaCXX/attr-malloc_span.cpp

@@ -11,7 +11,9 @@ struct DataMemberSpan {
   int n;
 };
 
-DataMemberSpan returns_data_member_span(void) __attribute((malloc_span)) { // expected-warning {{attribute only applies to functions that return span-like structures}}
+// expected-warning@+2 {{attribute only applies to functions that return span-like structures}}
+// expected-note@+1 {{span-like type must have a pointer and an integer field or two pointer fields}}
+DataMemberSpan returns_data_member_span(void) __attribute((malloc_span)) {
   return DataMemberSpan{};
 }
 
@@ -21,7 +23,9 @@ struct MemberFuncSpan {
   int n;
 };
 
-MemberFuncSpan returns_member_func_span(void) __attribute((malloc_span)) { // expected-warning {{attribute only applies to functions that return span-like structures}}
+// expected-warning@+2 {{attribute only applies to functions that return span-like structures}}
+// expected-note@+1 {{span-like type must have a pointer and an integer field or two pointer fields}}
+MemberFuncSpan returns_member_func_span(void) __attribute((malloc_span)) {
   return MemberFuncSpan{};
 }