[clang] Implement -Walloc-size diagnostic option

clang/docs/ReleaseNotes.rst

@@ -717,6 +717,10 @@ Improvements to Clang's diagnostics
   Added a new warning in this group for the case where the attribute is missing/implicit on
   an override of a virtual method.
 
+- A new warning ``-Walloc-size`` has been added to detect calls to functions
+  decorated with the ``alloc_size`` attribute don't allocate enough space for
+  the target pointer type.
+
 Improvements to Clang's time-trace
 ----------------------------------
 

clang/include/clang/AST/Expr.h

@@ -40,23 +40,24 @@
 #include <optional>
 
 namespace clang {
-  class APValue;
-  class ASTContext;
-  class BlockDecl;
-  class CXXBaseSpecifier;
-  class CXXMemberCallExpr;
-  class CXXOperatorCallExpr;
-  class CastExpr;
-  class Decl;
-  class IdentifierInfo;
-  class MaterializeTemporaryExpr;
-  class NamedDecl;
-  class ObjCPropertyRefExpr;
-  class OpaqueValueExpr;
-  class ParmVarDecl;
-  class StringLiteral;
-  class TargetInfo;
-  class ValueDecl;
+class AllocSizeAttr;
+class APValue;
+class ASTContext;
+class BlockDecl;
+class CXXBaseSpecifier;
+class CXXMemberCallExpr;
+class CXXOperatorCallExpr;
+class CastExpr;
+class Decl;
+class IdentifierInfo;
+class MaterializeTemporaryExpr;
+class NamedDecl;
+class ObjCPropertyRefExpr;
+class OpaqueValueExpr;
+class ParmVarDecl;
+class StringLiteral;
+class TargetInfo;
+class ValueDecl;
 
 /// A simple array of base specifiers.
 typedef SmallVector<CXXBaseSpecifier*, 4> CXXCastPath;
@@ -3256,6 +3257,14 @@ class CallExpr : public Expr {
     setDependence(getDependence() | ExprDependence::TypeValueInstantiation);
   }
 
+  /// Try to get the alloc_size attribute of the callee. May return null.
+  const AllocSizeAttr *getAllocSizeAttr() const;
+
+  /// Get the total size in bytes allocated by calling a function decorated with
+  /// alloc_size. Returns true if the the result was successfully evaluated.
+  bool getBytesReturnedByAllocSizeCall(const ASTContext &Ctx,
+                                       llvm::APInt &Result) const;
+
   bool isCallToStdMove() const;
 
   static bool classof(const Stmt *T) {

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -3684,6 +3684,12 @@ def warn_alloca_align_alignof : Warning<
   "second argument to __builtin_alloca_with_align is supposed to be in bits">,
   InGroup<DiagGroup<"alloca-with-align-alignof">>;
 
+def warn_alloc_size
+    : Warning<
+          "allocation of insufficient size '%0' for type %1 with size '%2'">,
+      InGroup<DiagGroup<"alloc-size">>,
+      DefaultIgnore;
+
 def err_alignment_too_small : Error<
   "requested alignment must be %0 or greater">;
 def err_alignment_too_big : Error<

clang/lib/AST/Expr.cpp

@@ -3542,6 +3542,59 @@ bool CallExpr::isBuiltinAssumeFalse(const ASTContext &Ctx) const {
          Arg->EvaluateAsBooleanCondition(ArgVal, Ctx) && !ArgVal;
 }
 
+const AllocSizeAttr *CallExpr::getAllocSizeAttr() const {
+  if (const FunctionDecl *DirectCallee = getDirectCallee())
+    return DirectCallee->getAttr<AllocSizeAttr>();
+  if (const Decl *IndirectCallee = getCalleeDecl())
+    return IndirectCallee->getAttr<AllocSizeAttr>();
+  return nullptr;
+}
+
+bool CallExpr::getBytesReturnedByAllocSizeCall(const ASTContext &Ctx,
+                                               llvm::APInt &Result) const {
+  const AllocSizeAttr *AllocSize = getAllocSizeAttr();
+
+  assert(AllocSize && AllocSize->getElemSizeParam().isValid());
+  unsigned SizeArgNo = AllocSize->getElemSizeParam().getASTIndex();
+  unsigned BitsInSizeT = Ctx.getTypeSize(Ctx.getSizeType());
+  if (getNumArgs() <= SizeArgNo)
+    return false;
+
+  auto EvaluateAsSizeT = [&](const Expr *E, llvm::APSInt &Into) {
+    Expr::EvalResult ExprResult;
+    if (E->isValueDependent() ||
+        !E->EvaluateAsInt(ExprResult, Ctx, Expr::SE_AllowSideEffects))
+      return false;
+    Into = ExprResult.Val.getInt();
+    if (Into.isNegative() || !Into.isIntN(BitsInSizeT))
+      return false;
+    Into = Into.zext(BitsInSizeT);
+    return true;
+  };
+
+  llvm::APSInt SizeOfElem;
+  if (!EvaluateAsSizeT(getArg(SizeArgNo), SizeOfElem))
+    return false;
+
+  if (!AllocSize->getNumElemsParam().isValid()) {
+    Result = std::move(SizeOfElem);
+    return true;
+  }
+
+  llvm::APSInt NumberOfElems;
+  unsigned NumArgNo = AllocSize->getNumElemsParam().getASTIndex();
+  if (!EvaluateAsSizeT(getArg(NumArgNo), NumberOfElems))
+    return false;
+
+  bool Overflow;
+  llvm::APInt BytesAvailable = SizeOfElem.umul_ov(NumberOfElems, Overflow);
+  if (Overflow)
+    return false;
+
+  Result = std::move(BytesAvailable);
+  return true;
+}
+
 bool CallExpr::isCallToStdMove() const {
   return getBuiltinCallee() == Builtin::BImove;
 }

clang/lib/AST/ExprConstant.cpp

@@ -114,15 +114,6 @@ namespace {
     return Ctx.getLValueReferenceType(E->getType());
   }
 
-  /// Given a CallExpr, try to get the alloc_size attribute. May return null.
-  static const AllocSizeAttr *getAllocSizeAttr(const CallExpr *CE) {
-    if (const FunctionDecl *DirectCallee = CE->getDirectCallee())
-      return DirectCallee->getAttr<AllocSizeAttr>();
-    if (const Decl *IndirectCallee = CE->getCalleeDecl())
-      return IndirectCallee->getAttr<AllocSizeAttr>();
-    return nullptr;
-  }
-
   /// Attempts to unwrap a CallExpr (with an alloc_size attribute) from an Expr.
   /// This will look through a single cast.
   ///
@@ -142,7 +133,7 @@ namespace {
       E = Cast->getSubExpr()->IgnoreParens();
 
     if (const auto *CE = dyn_cast<CallExpr>(E))
-      return getAllocSizeAttr(CE) ? CE : nullptr;
+      return CE->getAllocSizeAttr() ? CE : nullptr;
     return nullptr;
   }
 
@@ -9439,57 +9430,6 @@ bool LValueExprEvaluator::VisitBinAssign(const BinaryOperator *E) {
 // Pointer Evaluation
 //===----------------------------------------------------------------------===//
 
-/// Attempts to compute the number of bytes available at the pointer
-/// returned by a function with the alloc_size attribute. Returns true if we
-/// were successful. Places an unsigned number into `Result`.
-///
-/// This expects the given CallExpr to be a call to a function with an
-/// alloc_size attribute.
-static bool getBytesReturnedByAllocSizeCall(const ASTContext &Ctx,
-                                            const CallExpr *Call,
-                                            llvm::APInt &Result) {
-  const AllocSizeAttr *AllocSize = getAllocSizeAttr(Call);
-
-  assert(AllocSize && AllocSize->getElemSizeParam().isValid());
-  unsigned SizeArgNo = AllocSize->getElemSizeParam().getASTIndex();
-  unsigned BitsInSizeT = Ctx.getTypeSize(Ctx.getSizeType());
-  if (Call->getNumArgs() <= SizeArgNo)
-    return false;
-
-  auto EvaluateAsSizeT = [&](const Expr *E, APSInt &Into) {
-    Expr::EvalResult ExprResult;
-    if (!E->EvaluateAsInt(ExprResult, Ctx, Expr::SE_AllowSideEffects))
-      return false;
-    Into = ExprResult.Val.getInt();
-    if (Into.isNegative() || !Into.isIntN(BitsInSizeT))
-      return false;
-    Into = Into.zext(BitsInSizeT);
-    return true;
-  };
-
-  APSInt SizeOfElem;
-  if (!EvaluateAsSizeT(Call->getArg(SizeArgNo), SizeOfElem))
-    return false;
-
-  if (!AllocSize->getNumElemsParam().isValid()) {
-    Result = std::move(SizeOfElem);
-    return true;
-  }
-
-  APSInt NumberOfElems;
-  unsigned NumArgNo = AllocSize->getNumElemsParam().getASTIndex();
-  if (!EvaluateAsSizeT(Call->getArg(NumArgNo), NumberOfElems))
-    return false;
-
-  bool Overflow;
-  llvm::APInt BytesAvailable = SizeOfElem.umul_ov(NumberOfElems, Overflow);
-  if (Overflow)
-    return false;
-
-  Result = std::move(BytesAvailable);
-  return true;
-}
-
 /// Convenience function. LVal's base must be a call to an alloc_size
 /// function.
 static bool getBytesReturnedByAllocSizeCall(const ASTContext &Ctx,
@@ -9499,7 +9439,7 @@ static bool getBytesReturnedByAllocSizeCall(const ASTContext &Ctx,
          "Can't get the size of a non alloc_size function");
   const auto *Base = LVal.getLValueBase().get<const Expr *>();
   const CallExpr *CE = tryUnwrapAllocSizeCall(Base);
-  return getBytesReturnedByAllocSizeCall(Ctx, CE, Result);
+  return CE->getBytesReturnedByAllocSizeCall(Ctx, Result);
 }
 
 /// Attempts to evaluate the given LValueBase as the result of a call to
@@ -9977,7 +9917,7 @@ bool PointerExprEvaluator::visitNonBuiltinCallExpr(const CallExpr *E) {
   if (ExprEvaluatorBaseTy::VisitCallExpr(E))
     return true;
 
-  if (!(InvalidBaseOK && getAllocSizeAttr(E)))
+  if (!(InvalidBaseOK && E->getAllocSizeAttr()))
     return false;
 
   Result.setInvalid(E);

clang/lib/Sema/SemaExpr.cpp

@@ -18,6 +18,7 @@
 #include "clang/AST/ASTDiagnostic.h"
 #include "clang/AST/ASTLambda.h"
 #include "clang/AST/ASTMutationListener.h"
+#include "clang/AST/Attrs.inc"
 #include "clang/AST/CXXInheritance.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclObjC.h"
@@ -7818,6 +7819,33 @@ ExprResult Sema::CheckExtVectorCast(SourceRange R, QualType DestTy,
   return prepareVectorSplat(DestTy, CastExpr);
 }
 
+/// Check that a call to alloc_size function specifies sufficient space for the
+/// destination type.
+static void CheckSufficientAllocSize(Sema &S, QualType DestType,
+                                     const Expr *E) {
+  QualType SourceType = E->getType();
+  if (!DestType->isPointerType() || !SourceType->isPointerType() ||
+      DestType == SourceType)
+    return;
+
+  const auto *CE = dyn_cast<CallExpr>(E->IgnoreCasts());
+  if (!CE)
+    return;
+
+  // Find the total size allocated by the function call.
+  llvm::APInt AllocSize;
+  if (!CE->getAllocSizeAttr() ||
+      !CE->getBytesReturnedByAllocSizeCall(S.Context, AllocSize))
+    return;
+  auto Size = CharUnits::fromQuantity(AllocSize.getZExtValue());
+
+  QualType TargetType = DestType->getPointeeType();
+  auto LhsSize = S.Context.getTypeSizeInCharsIfKnown(TargetType);
+  if (LhsSize && Size < LhsSize)
+    S.Diag(E->getExprLoc(), diag::warn_alloc_size)
+        << Size.getQuantity() << TargetType << LhsSize->getQuantity();
+}
+
 ExprResult
 Sema::ActOnCastExpr(Scope *S, SourceLocation LParenLoc,
                     Declarator &D, ParsedType &Ty,
@@ -7883,6 +7911,8 @@ Sema::ActOnCastExpr(Scope *S, SourceLocation LParenLoc,
 
   DiscardMisalignedMemberAddress(castType.getTypePtr(), CastExpr);
 
+  CheckSufficientAllocSize(*this, castType, CastExpr);
+
   return BuildCStyleCastExpr(LParenLoc, castTInfo, RParenLoc, CastExpr);
 }
 
@@ -9887,6 +9917,12 @@ AssignConvertType Sema::CheckSingleAssignmentConstraints(QualType LHSType,
   AssignConvertType result =
       CheckAssignmentConstraints(LHSType, RHS, Kind, ConvertRHS);
 
+  // If assigning a void * created by an allocation function call to some other
+  // type, check that the allocated size is sufficient for that type.
+  if (result != AssignConvertType::Incompatible &&
+      RHS.get()->getType()->isVoidPointerType())
+    CheckSufficientAllocSize(*this, LHSType, RHS.get());
+
   // C99 6.5.16.1p2: The value of the right operand is converted to the
   // type of the assignment expression.
   // CheckAssignmentConstraints allows the left-hand side to be a reference,

clang/test/Sema/warn-alloc-size.c

@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Walloc-size %s
+struct Foo { int x[10]; };
+
+void *malloc(unsigned long) __attribute__((alloc_size(1)));
+void *alloca(unsigned long) __attribute__((alloc_size(1)));
+void *calloc(unsigned long, unsigned long) __attribute__((alloc_size(2, 1)));
+
+void foo_consumer(struct Foo* p);
+
+void alloc_foo(void) {
+  struct Foo *ptr1 = malloc(sizeof(struct Foo));
+  struct Foo *ptr2 = malloc(sizeof(*ptr2));
+  struct Foo *ptr3 = calloc(1, sizeof(*ptr3));
+  struct Foo *ptr4 = calloc(sizeof(*ptr4), 1);
+  struct Foo (*ptr5)[5] = malloc(sizeof(*ptr5));
+  void *ptr6 = malloc(4);
+
+  // Test insufficient size with different allocation functions.
+  struct Foo *ptr7 = malloc(sizeof(ptr7));      // expected-warning {{allocation of insufficient size '8' for type 'struct Foo' with size '40'}}
+  struct Foo *ptr8 = alloca(sizeof(ptr8));      // expected-warning {{allocation of insufficient size '8' for type 'struct Foo' with size '40'}}
+  struct Foo *ptr9 = calloc(1, sizeof(ptr9));   // expected-warning {{allocation of insufficient size '8' for type 'struct Foo' with size '40'}}
+  struct Foo *ptr10 = calloc(sizeof(ptr10), 1); // expected-warning {{allocation of insufficient size '8' for type 'struct Foo' with size '40'}}
+
+  // Test function arguments.
+  foo_consumer(malloc(4)); // expected-warning {{allocation of insufficient size '4' for type 'struct Foo' with size '40'}}
+
+  // Test explicit cast.
+  struct Foo *ptr11 = (struct Foo *)malloc(sizeof(*ptr11));
+  struct Foo *ptr12 = (struct Foo *)malloc(sizeof(ptr12));    // expected-warning {{allocation of insufficient size '8' for type 'struct Foo' with size '40'}}
+  struct Foo *ptr13 = (struct Foo *)alloca(sizeof(ptr13));    // expected-warning {{allocation of insufficient size '8' for type 'struct Foo' with size '40'}}
+  struct Foo *ptr14 = (struct Foo *)calloc(1, sizeof(ptr14)); // expected-warning {{allocation of insufficient size '8' for type 'struct Foo' with size '40'}}
+  struct Foo *ptr15 = (struct Foo *)malloc(4);                // expected-warning {{allocation of insufficient size '4' for type 'struct Foo' with size '40'}}
+  void *ptr16 = (struct Foo *)malloc(4);                      // expected-warning {{allocation of insufficient size '4' for type 'struct Foo' with size '40'}}
+
+  struct Foo *ptr17 = (void *)(struct Foo *)malloc(4); // expected-warning 2 {{allocation of insufficient size '4' for type 'struct Foo' with size '40'}}
+  int *ptr18 = (unsigned *)(void *)(int *)malloc(1);   // expected-warning {{initializing 'int *' with an expression of type 'unsigned int *' converts between pointers to integer types with different sign}}
+                                                       // expected-warning@-1 {{allocation of insufficient size '1' for type 'int' with size '4'}}
+                                                       // expected-warning@-2 {{allocation of insufficient size '1' for type 'unsigned int' with size '4'}}
+  int *ptr19 = (void *)(int *)malloc(1);               // expected-warning {{allocation of insufficient size '1' for type 'int' with size '4'}}
+                                                       // expected-warning@-1 {{allocation of insufficient size '1' for type 'int' with size '4'}}
+  (void)(int *)malloc(1);                              // expected-warning {{allocation of insufficient size '1' for type 'int' with size '4'}}
+}