[BoundsSafety] make alloc_size imply __sized_by_or_null return type

When -fbounds-safety is enabled, the semantics of the return value of
functions is that of __sized_by_or_null. Unlike return types annotated
with __sized_by_or_null, it is never included in the type system.
Instead every relevant analysis and transformation has to specifically
check for the alloc_size attribute.

This patch infers a __sized_by_or_null return type for functions
annotated with alloc_size. This enables us to remove those special
cases and reduce complexity.

rdar://118338657
rdar://91017404
rdar://11833865

Author: hnrklssn

clang/docs/InternalsManual.rst

@@ -427,6 +427,17 @@ Description:
   This is useful when the argument could be a string in some cases, but
   another class in other cases, and it needs to be quoted consistently.
 
+**"unquoted" format**
+
+Example:
+  ``"conflicting attributes were '%select{__counted_by|__sized_by|__counted_by_or_null|__sized_by_or_null}0(%unquoted1)' and '%select{__counted_by|__sized_by|__counted_by_or_null|__sized_by_or_null}2(%unquoted3)'"``
+Class:
+  ``Expr, Decl``
+Description:
+  This is a simple formatter which omits the single quotes from a class
+  that would normally be printed quoted. This is useful when the diagnostic
+  uses the argument to construct a larger type or expression.
+
 .. _internals-producing-diag:
 
 Producing the Diagnostic

clang/include/clang/AST/Attr.h

@@ -354,6 +354,22 @@ class ParamIdx {
     assertComparable(I);
     return Idx == I.Idx;
   }
+  /* TO_UPSTREAM(BoundsSafety) ON */
+  /// Unlike operator== which requires isValid to be true for both objects,
+  /// this relaxes that constraint and returns true when comparing two
+  /// invalid objects. An invalid object does not equal any valid object.
+  bool equals(const ParamIdx &I) const {
+    assert(HasThis == I.HasThis &&
+           "ParamIdx must be for the same function to be compared");
+    if (isValid() != I.isValid())
+      return false;
+    if (!isValid()) {
+      assert(!I.isValid());
+      return true;
+    }
+    return Idx == I.Idx;
+  }
+  /* TO_UPSTREAM(BoundsSafety) OFF */
   bool operator!=(const ParamIdx &I) const {
     assertComparable(I);
     return Idx != I.Idx;

clang/include/clang/AST/Type.h

@@ -2605,6 +2605,7 @@ class alignas(TypeAlignment) Type : public ExtQualsTypeCommonBase {
   bool isBidiIndexablePointerType() const;
   bool isUnspecifiedPointerType() const;
   bool isSafePointerType() const;
+  bool isImplicitSafePointerType() const;
   /* TO_UPSTREAM(BoundsSafety) OFF */
   bool isSignableType(const ASTContext &Ctx) const;
   bool isSignablePointerType() const;
@@ -8839,6 +8840,19 @@ inline bool Type::isSafePointerType() const {
           isValueTerminatedType());
 }
 
+/* TO_UPSTREAM(BoundsSafety) ON */
+inline bool Type::isImplicitSafePointerType() const {
+  if (auto AT = this->getAs<AttributedType>()) {
+    if (AT->getAttrKind() == attr::PtrAutoAttr) {
+      assert(isSafePointerType());
+      return true;
+    }
+    return AT->getEquivalentType()->isImplicitSafePointerType();
+  }
+  return false;
+}
+/* TO_UPSTREAM(BoundsSafety) OFF */
+
 inline bool Type::isPointerTypeWithBounds() const {
   const auto *PT = dyn_cast<PointerType>(CanonicalType);
   return PT && PT->getPointerAttributes().hasUpperBound();

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -3439,6 +3439,9 @@ def err_attribute_only_once_per_parameter : Error<
   "%0 attribute can only be applied once per parameter">;
 def err_mismatched_uuid : Error<"uuid does not match previous declaration">;
 def note_previous_uuid : Note<"previous uuid specified here">;
+/* TO_UPSTREAM(BoundsSafety) ON */
+def err_mismatched_alloc_size : Error<"'alloc_size' attribute does not match previous declaration">;
+/* TO_UPSTREAM(BoundsSafety) OFF */
 def warn_attribute_pointers_only : Warning<
   "%0 attribute only applies to%select{| constant}1 pointer arguments">,
   InGroup<IgnoredAttributes>;
@@ -12789,7 +12792,19 @@ let CategoryName = "BoundsSafety Pointer Attributes Issue" in {
 def err_bounds_safety_conflicting_pointer_attributes : Error<
   "%select{array|pointer}0 cannot have more than one %select{bound|type|count|end|terminator}1 attribute">;
 def note_bounds_safety_conflicting_pointer_attribute_args : Note<
-  "conflicting arguments for %select{count|end|terminator}0 were %1 and %2">;
+  "conflicting arguments for %select{end|terminator}0 were %1 and %2">;
+def warn_bounds_safety_ignored_implicit_sized_by_or_null : Warning<
+  "implicit __sized_by_or_null attribute ignored because of explicit __sized_by">;
+def note_bounds_safety_overriding_implicit_sized_by_or_null_silence : Note<
+  "add _Nonnull qualifier to return type to silence this warning">;
+def note_bounds_safety_conflicting_count_attribute : Note<
+  "conflicting attributes were '%select{__counted_by|__sized_by|__counted_by_or_null|__sized_by_or_null}0(%unquoted1)' and '%select{__counted_by|__sized_by|__counted_by_or_null|__sized_by_or_null}2(%unquoted3)'">;
+def note_bounds_safety_implicit_size_from_alloc_size_attr : Note<
+  "function return type implicitly __sized_by%select{|_or_null}0 because of the function's 'alloc_size' %select{and 'returns_nonnull' attributes|attribute}0">;
+def err_invalid_return_type_for_alloc_size : Error<
+  "invalid return type %0 for function with alloc_size attribute; '__sized_by_or_null(%unquoted1)' or '__sized_by(%unquoted1)' required">;
+def note_attribute_inherited : Note<
+  "attribute inherited from previous declaration here">;
 def warn_bounds_safety_duplicate_pointer_attributes : Warning<
   "%select{array|pointer}0 annotated with %select{__unsafe_indexable|__bidi_indexable|__indexable|__single|__terminated_by}1 "
   "multiple times. Annotate only once to remove this warning">, InGroup<BoundsSafetyRedundantAttribute>;

clang/include/clang/Sema/Attr.h

@@ -80,6 +80,13 @@ inline const ParmVarDecl *getFunctionOrMethodParam(const Decl *D,
   return nullptr;
 }
 
+/* TO_UPSTREAM(BoundsSafety) ON */
+inline ParmVarDecl *getFunctionOrMethodParam(Decl *D, unsigned Idx) {
+  return const_cast<ParmVarDecl *>(
+      getFunctionOrMethodParam(const_cast<const Decl *>(D), Idx));
+}
+/* TO_UPSTREAM(BoundsSafety) OFF */
+
 inline QualType getFunctionOrMethodParamType(const Decl *D, unsigned Idx) {
   if (const FunctionType *FnTy = D->getFunctionType())
     return cast<FunctionProtoType>(FnTy)->getParamType(Idx);

clang/include/clang/Sema/ParsedAttr.h

@@ -962,7 +962,10 @@ class ParsedAttributes : public ParsedAttributesView {
   void takeAllFrom(ParsedAttributes &Other) {
     assert(&Other != this &&
            "ParsedAttributes can't take attributes from itself");
-    addAll(Other.begin(), Other.end());
+    /* TO_UPSTREAM(BoundsSafety) ON
+     * Upstream uses addAll() instead, but that changes the attribute order. */
+    addAllAtEnd(Other.begin(), Other.end());
+    /* TO_UPSTREAM(BoundsSafety) OFF */
     Other.clearListOnly();
     pool.takeAllFrom(Other.pool);
   }

clang/include/clang/Sema/Sema.h

@@ -5380,6 +5380,9 @@ class Sema final : public SemaBase {
   MinSizeAttr *mergeMinSizeAttr(Decl *D, const AttributeCommonInfo &CI);
   OptimizeNoneAttr *mergeOptimizeNoneAttr(Decl *D,
                                           const AttributeCommonInfo &CI);
+  /* TO_UPSTREAM(BoundsSafety) ON */
+  AllocSizeAttr *mergeAllocSizeAttr(NamedDecl *D, const AllocSizeAttr &ASA);
+  /* TO_UPSTREAM(BoundsSafety) OFF */
   InternalLinkageAttr *mergeInternalLinkageAttr(Decl *D, const ParsedAttr &AL);
   InternalLinkageAttr *mergeInternalLinkageAttr(Decl *D,
                                                 const InternalLinkageAttr &AL);
@@ -5475,6 +5478,12 @@ class Sema final : public SemaBase {
 
   void DiagnoseUnknownAttribute(const ParsedAttr &AL);
 
+  /* TO_UPSTREAM(BoundsSafety) ON */
+  QualType
+  PostProcessBoundsSafetyAllocSizeAttribute(FunctionDecl *EnclosingDecl,
+                                            QualType FTy);
+  /* TO_UPSTREAM(BoundsSafety) OFF */
+
   /// DeclClonePragmaWeak - clone existing decl (maybe definition),
   /// \#pragma weak needs a non-definition decl and source may not have one.
   NamedDecl *DeclClonePragmaWeak(NamedDecl *ND, const IdentifierInfo *II,

clang/lib/AST/ASTDiagnostic.cpp

@@ -519,6 +519,11 @@ void clang::FormatASTNodeDiagnosticArgument(
     }
   }
 
+  /* TO_UPSTREAM(BoundsSafety) ON */
+  if (Modifier == "unquoted")
+    NeedQuotes = false;
+  /* TO_UPSTREAM(BoundsSafety) OFF */
+
   if (NeedQuotes) {
     Output.insert(Output.begin()+OldEnd, '\'');
     Output.push_back('\'');

clang/lib/Sema/BoundsSafetySuggestions.cpp

@@ -490,22 +490,6 @@ bool UnsafeOperationVisitor::FindSingleEntity(
       // Don't support indirect calls for now.
       return false;
     }
-    if (DirectCallDecl->hasAttr<AllocSizeAttr>() &&
-        IsReallySinglePtr(DirectCallDecl->getReturnType())) {
-      // Functions declared like
-      // void * custom_malloc(size_t s) __attribute__((alloc_size(1)))
-      //
-      // are currently are annotated as returning `void *__single` rather
-      // than `void *__sized_by(s)`. To make the right thing happen at call
-      // sites `BoundsSafetyPointerPromotionExpr` is used to generate a pointer
-      // with the appropriate bounds from the `void *__single`. For functions
-      // like this the warning needs to be suppressed because from the user's
-      // perspective the returned value is not actually __single.
-      //
-      // This code path can be deleted once allocating functions are properly
-      // annotated with __sized_by_or_null. rdar://117114186
-      return false;
-    }
 
     assert(IsReallySinglePtr(DirectCallDecl->getReturnType()));