https://github.com/vvuksanovic created https://github.com/llvm/llvm-project/pull/150028
Warns about calls to functions decorated with attribute `alloc_size` that specify insufficient size for the type they are cast to. Matches the behavior of the GCC option of the same name. Closes #138973 >From 1f03a09bac08c70183d2a04fcbced83179cad414 Mon Sep 17 00:00:00 2001 From: Vladimir Vuksanovic <vladimir.vuksano...@htecgroup.com> Date: Mon, 7 Jul 2025 06:17:19 -0700 Subject: [PATCH] [clang] Implement -Walloc-size diagnostic option Warns about calls to functions decorated with attribute alloc_size that specify insufficient size for the type they are cast to. Matches the behavior of the GCC option of the same name. --- clang/docs/ReleaseNotes.rst | 4 ++ clang/include/clang/AST/Expr.h | 43 +++++++----- .../clang/Basic/DiagnosticSemaKinds.td | 6 ++ clang/lib/AST/Expr.cpp | 53 +++++++++++++++ clang/lib/AST/ExprConstant.cpp | 66 +------------------ clang/lib/Sema/SemaExpr.cpp | 36 ++++++++++ clang/test/Sema/warn-alloc-size.c | 42 ++++++++++++ 7 files changed, 170 insertions(+), 80 deletions(-) create mode 100644 clang/test/Sema/warn-alloc-size.c diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 46a77673919d3..bf4615129f357 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/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 ---------------------------------- diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h index 523c0326d47ef..8bdd0d45dcbb1 100644 --- a/clang/include/clang/AST/Expr.h +++ b/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) { diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index b2ea65ae111be..67bd44f1e5dc4 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/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< diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp index 2e1a9a3d9ad63..eb5f5875fcc76 100644 --- a/clang/lib/AST/Expr.cpp +++ b/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; } diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index 0d12161756467..a295d5e38b332 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/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); diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 45c7178c6965d..24088a7ca640c 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/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, diff --git a/clang/test/Sema/warn-alloc-size.c b/clang/test/Sema/warn-alloc-size.c new file mode 100644 index 0000000000000..e1ce341b79678 --- /dev/null +++ b/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'}} +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits