Skip to content

Commit 6dc188d

Browse files
vvuksanovicVladimir Vuksanovic
vvuksanovic
and
Vladimir Vuksanovic
authored
[clang] Implement -Walloc-size diagnostic option (#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 --------- Co-authored-by: Vladimir Vuksanovic <vvuksano@cisco.com>
1 parent 495adb3 commit 6dc188d

19 files changed

+206
-77
lines changed

‎clang/docs/ReleaseNotes.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,10 @@ Improvements to Clang's diagnostics
239239
specializations in th same way as it already did for other declarators.
240240
(#GH147333)
241241

242+
- A new warning ``-Walloc-size`` has been added to detect calls to functions
243+
decorated with the ``alloc_size`` attribute don't allocate enough space for
244+
the target pointer type.
245+
242246
Improvements to Clang's time-trace
243247
----------------------------------
244248

‎clang/include/clang/AST/Expr.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
#include <optional>
4141

4242
namespace clang {
43+
class AllocSizeAttr;
4344
class APValue;
4445
class ASTContext;
4546
class BlockDecl;
@@ -3261,6 +3262,14 @@ class CallExpr : public Expr {
32613262
setDependence(getDependence() | ExprDependence::TypeValueInstantiation);
32623263
}
32633264

3265+
/// Try to get the alloc_size attribute of the callee. May return null.
3266+
const AllocSizeAttr *getCalleeAllocSizeAttr() const;
3267+
3268+
/// Get the total size in bytes allocated by calling a function decorated with
3269+
/// alloc_size. Returns std::nullopt if the the result cannot be evaluated.
3270+
std::optional<llvm::APInt>
3271+
getBytesReturnedByAllocSizeCall(const ASTContext &Ctx) const;
3272+
32643273
bool isCallToStdMove() const;
32653274

32663275
static bool classof(const Stmt *T) {

‎clang/include/clang/Basic/AttrDocs.td

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -973,6 +973,21 @@ An example of how to use ``alloc_size``
973973
assert(__builtin_object_size(a, 0) == 100);
974974
}
975975

976+
When ``-Walloc-size`` is enabled, this attribute allows the compiler to
977+
diagnose cases when the allocated memory is insufficient for the size of the
978+
type the returned pointer is cast to.
979+
980+
.. code-block:: c
981+
982+
void *my_malloc(int a) __attribute__((alloc_size(1)));
983+
void consumer_func(int *);
984+
985+
int main() {
986+
int *ptr = my_malloc(sizeof(int)); // no warning
987+
int *w = my_malloc(1); // warning: allocation of insufficient size '1' for type 'int' with size '4'
988+
consumer_func(my_malloc(1)); // warning: allocation of insufficient size '1' for type 'int' with size '4'
989+
}
990+
976991
.. Note:: This attribute works differently in clang than it does in GCC.
977992
Specifically, clang will only trace ``const`` pointers (as above); we give up
978993
on pointers that are not marked as ``const``. In the vast majority of cases,

‎clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3687,6 +3687,10 @@ def warn_alloca_align_alignof : Warning<
36873687
"second argument to __builtin_alloca_with_align is supposed to be in bits">,
36883688
InGroup<DiagGroup<"alloca-with-align-alignof">>;
36893689

3690+
def warn_alloc_size : Warning<
3691+
"allocation of insufficient size '%0' for type %1 with size '%2'">,
3692+
InGroup<DiagGroup<"alloc-size">>;
3693+
36903694
def err_alignment_too_small : Error<
36913695
"requested alignment must be %0 or greater">;
36923696
def err_alignment_too_big : Error<

‎clang/lib/AST/Expr.cpp

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3533,6 +3533,56 @@ bool CallExpr::isBuiltinAssumeFalse(const ASTContext &Ctx) const {
35333533
Arg->EvaluateAsBooleanCondition(ArgVal, Ctx) && !ArgVal;
35343534
}
35353535

3536+
const AllocSizeAttr *CallExpr::getCalleeAllocSizeAttr() const {
3537+
if (const FunctionDecl *DirectCallee = getDirectCallee())
3538+
return DirectCallee->getAttr<AllocSizeAttr>();
3539+
if (const Decl *IndirectCallee = getCalleeDecl())
3540+
return IndirectCallee->getAttr<AllocSizeAttr>();
3541+
return nullptr;
3542+
}
3543+
3544+
std::optional<llvm::APInt>
3545+
CallExpr::getBytesReturnedByAllocSizeCall(const ASTContext &Ctx) const {
3546+
const AllocSizeAttr *AllocSize = getCalleeAllocSizeAttr();
3547+
3548+
assert(AllocSize && AllocSize->getElemSizeParam().isValid());
3549+
unsigned SizeArgNo = AllocSize->getElemSizeParam().getASTIndex();
3550+
unsigned BitsInSizeT = Ctx.getTypeSize(Ctx.getSizeType());
3551+
if (getNumArgs() <= SizeArgNo)
3552+
return {};
3553+
3554+
auto EvaluateAsSizeT = [&](const Expr *E, llvm::APSInt &Into) {
3555+
Expr::EvalResult ExprResult;
3556+
if (E->isValueDependent() ||
3557+
!E->EvaluateAsInt(ExprResult, Ctx, Expr::SE_AllowSideEffects))
3558+
return false;
3559+
Into = ExprResult.Val.getInt();
3560+
if (Into.isNegative() || !Into.isIntN(BitsInSizeT))
3561+
return false;
3562+
Into = Into.zext(BitsInSizeT);
3563+
return true;
3564+
};
3565+
3566+
llvm::APSInt SizeOfElem;
3567+
if (!EvaluateAsSizeT(getArg(SizeArgNo), SizeOfElem))
3568+
return {};
3569+
3570+
if (!AllocSize->getNumElemsParam().isValid())
3571+
return SizeOfElem;
3572+
3573+
llvm::APSInt NumberOfElems;
3574+
unsigned NumArgNo = AllocSize->getNumElemsParam().getASTIndex();
3575+
if (!EvaluateAsSizeT(getArg(NumArgNo), NumberOfElems))
3576+
return {};
3577+
3578+
bool Overflow;
3579+
llvm::APInt BytesAvailable = SizeOfElem.umul_ov(NumberOfElems, Overflow);
3580+
if (Overflow)
3581+
return {};
3582+
3583+
return BytesAvailable;
3584+
}
3585+
35363586
bool CallExpr::isCallToStdMove() const {
35373587
return getBuiltinCallee() == Builtin::BImove;
35383588
}

‎clang/lib/AST/ExprConstant.cpp

Lines changed: 8 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -114,15 +114,6 @@ namespace {
114114
return Ctx.getLValueReferenceType(E->getType());
115115
}
116116

117-
/// Given a CallExpr, try to get the alloc_size attribute. May return null.
118-
static const AllocSizeAttr *getAllocSizeAttr(const CallExpr *CE) {
119-
if (const FunctionDecl *DirectCallee = CE->getDirectCallee())
120-
return DirectCallee->getAttr<AllocSizeAttr>();
121-
if (const Decl *IndirectCallee = CE->getCalleeDecl())
122-
return IndirectCallee->getAttr<AllocSizeAttr>();
123-
return nullptr;
124-
}
125-
126117
/// Attempts to unwrap a CallExpr (with an alloc_size attribute) from an Expr.
127118
/// This will look through a single cast.
128119
///
@@ -142,7 +133,7 @@ namespace {
142133
E = Cast->getSubExpr()->IgnoreParens();
143134

144135
if (const auto *CE = dyn_cast<CallExpr>(E))
145-
return getAllocSizeAttr(CE) ? CE : nullptr;
136+
return CE->getCalleeAllocSizeAttr() ? CE : nullptr;
146137
return nullptr;
147138
}
148139

@@ -9466,57 +9457,6 @@ bool LValueExprEvaluator::VisitBinAssign(const BinaryOperator *E) {
94669457
// Pointer Evaluation
94679458
//===----------------------------------------------------------------------===//
94689459

9469-
/// Attempts to compute the number of bytes available at the pointer
9470-
/// returned by a function with the alloc_size attribute. Returns true if we
9471-
/// were successful. Places an unsigned number into `Result`.
9472-
///
9473-
/// This expects the given CallExpr to be a call to a function with an
9474-
/// alloc_size attribute.
9475-
static bool getBytesReturnedByAllocSizeCall(const ASTContext &Ctx,
9476-
const CallExpr *Call,
9477-
llvm::APInt &Result) {
9478-
const AllocSizeAttr *AllocSize = getAllocSizeAttr(Call);
9479-
9480-
assert(AllocSize && AllocSize->getElemSizeParam().isValid());
9481-
unsigned SizeArgNo = AllocSize->getElemSizeParam().getASTIndex();
9482-
unsigned BitsInSizeT = Ctx.getTypeSize(Ctx.getSizeType());
9483-
if (Call->getNumArgs() <= SizeArgNo)
9484-
return false;
9485-
9486-
auto EvaluateAsSizeT = [&](const Expr *E, APSInt &Into) {
9487-
Expr::EvalResult ExprResult;
9488-
if (!E->EvaluateAsInt(ExprResult, Ctx, Expr::SE_AllowSideEffects))
9489-
return false;
9490-
Into = ExprResult.Val.getInt();
9491-
if (Into.isNegative() || !Into.isIntN(BitsInSizeT))
9492-
return false;
9493-
Into = Into.zext(BitsInSizeT);
9494-
return true;
9495-
};
9496-
9497-
APSInt SizeOfElem;
9498-
if (!EvaluateAsSizeT(Call->getArg(SizeArgNo), SizeOfElem))
9499-
return false;
9500-
9501-
if (!AllocSize->getNumElemsParam().isValid()) {
9502-
Result = std::move(SizeOfElem);
9503-
return true;
9504-
}
9505-
9506-
APSInt NumberOfElems;
9507-
unsigned NumArgNo = AllocSize->getNumElemsParam().getASTIndex();
9508-
if (!EvaluateAsSizeT(Call->getArg(NumArgNo), NumberOfElems))
9509-
return false;
9510-
9511-
bool Overflow;
9512-
llvm::APInt BytesAvailable = SizeOfElem.umul_ov(NumberOfElems, Overflow);
9513-
if (Overflow)
9514-
return false;
9515-
9516-
Result = std::move(BytesAvailable);
9517-
return true;
9518-
}
9519-
95209460
/// Convenience function. LVal's base must be a call to an alloc_size
95219461
/// function.
95229462
static bool getBytesReturnedByAllocSizeCall(const ASTContext &Ctx,
@@ -9526,7 +9466,12 @@ static bool getBytesReturnedByAllocSizeCall(const ASTContext &Ctx,
95269466
"Can't get the size of a non alloc_size function");
95279467
const auto *Base = LVal.getLValueBase().get<const Expr *>();
95289468
const CallExpr *CE = tryUnwrapAllocSizeCall(Base);
9529-
return getBytesReturnedByAllocSizeCall(Ctx, CE, Result);
9469+
std::optional<llvm::APInt> Size = CE->getBytesReturnedByAllocSizeCall(Ctx);
9470+
if (!Size)
9471+
return false;
9472+
9473+
Result = std::move(*Size);
9474+
return true;
95309475
}
95319476

95329477
/// Attempts to evaluate the given LValueBase as the result of a call to
@@ -10017,7 +9962,7 @@ bool PointerExprEvaluator::visitNonBuiltinCallExpr(const CallExpr *E) {
100179962
if (ExprEvaluatorBaseTy::VisitCallExpr(E))
100189963
return true;
100199964

10020-
if (!(InvalidBaseOK && getAllocSizeAttr(E)))
9965+
if (!(InvalidBaseOK && E->getCalleeAllocSizeAttr()))
100219966
return false;
100229967

100239968
Result.setInvalid(E);

‎clang/lib/Sema/SemaExpr.cpp

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "clang/AST/ASTDiagnostic.h"
1919
#include "clang/AST/ASTLambda.h"
2020
#include "clang/AST/ASTMutationListener.h"
21+
#include "clang/AST/Attrs.inc"
2122
#include "clang/AST/CXXInheritance.h"
2223
#include "clang/AST/Decl.h"
2324
#include "clang/AST/DeclObjC.h"
@@ -7818,6 +7819,39 @@ ExprResult Sema::CheckExtVectorCast(SourceRange R, QualType DestTy,
78187819
return prepareVectorSplat(DestTy, CastExpr);
78197820
}
78207821

7822+
/// Check that a call to alloc_size function specifies sufficient space for the
7823+
/// destination type.
7824+
static void CheckSufficientAllocSize(Sema &S, QualType DestType,
7825+
const Expr *E) {
7826+
QualType SourceType = E->getType();
7827+
if (!DestType->isPointerType() || !SourceType->isPointerType() ||
7828+
DestType == SourceType)
7829+
return;
7830+
7831+
const auto *CE = dyn_cast<CallExpr>(E->IgnoreParenCasts());
7832+
if (!CE)
7833+
return;
7834+
7835+
// Find the total size allocated by the function call.
7836+
if (!CE->getCalleeAllocSizeAttr())
7837+
return;
7838+
std::optional<llvm::APInt> AllocSize =
7839+
CE->getBytesReturnedByAllocSizeCall(S.Context);
7840+
if (!AllocSize)
7841+
return;
7842+
auto Size = CharUnits::fromQuantity(AllocSize->getZExtValue());
7843+
7844+
QualType TargetType = DestType->getPointeeType();
7845+
// Find the destination size. As a special case function types have size of
7846+
// one byte to match the sizeof operator behavior.
7847+
auto LhsSize = TargetType->isFunctionType()
7848+
? CharUnits::One()
7849+
: S.Context.getTypeSizeInCharsIfKnown(TargetType);
7850+
if (LhsSize && Size < LhsSize)
7851+
S.Diag(E->getExprLoc(), diag::warn_alloc_size)
7852+
<< Size.getQuantity() << TargetType << LhsSize->getQuantity();
7853+
}
7854+
78217855
ExprResult
78227856
Sema::ActOnCastExpr(Scope *S, SourceLocation LParenLoc,
78237857
Declarator &D, ParsedType &Ty,
@@ -7883,6 +7917,8 @@ Sema::ActOnCastExpr(Scope *S, SourceLocation LParenLoc,
78837917

78847918
DiscardMisalignedMemberAddress(castType.getTypePtr(), CastExpr);
78857919

7920+
CheckSufficientAllocSize(*this, castType, CastExpr);
7921+
78867922
return BuildCStyleCastExpr(LParenLoc, castTInfo, RParenLoc, CastExpr);
78877923
}
78887924

@@ -9893,6 +9929,12 @@ AssignConvertType Sema::CheckSingleAssignmentConstraints(QualType LHSType,
98939929
AssignConvertType result =
98949930
CheckAssignmentConstraints(LHSType, RHS, Kind, ConvertRHS);
98959931

9932+
// If assigning a void * created by an allocation function call to some other
9933+
// type, check that the allocated size is sufficient for that type.
9934+
if (result != AssignConvertType::Incompatible &&
9935+
RHS.get()->getType()->isVoidPointerType())
9936+
CheckSufficientAllocSize(*this, LHSType, RHS.get());
9937+
98969938
// C99 6.5.16.1p2: The value of the right operand is converted to the
98979939
// type of the assignment expression.
98989940
// CheckAssignmentConstraints allows the left-hand side to be a reference,

‎clang/test/Analysis/Malloc+MismatchedDeallocator+NewDelete.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,unix.MismatchedDeallocator,cplusplus.NewDelete -std=c++11 -verify %s
2-
// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,unix.MismatchedDeallocator,cplusplus.NewDelete,cplusplus.NewDeleteLeaks -DLEAKS -std=c++11 -verify %s
1+
// RUN: %clang_analyze_cc1 -Wno-alloc-size -analyzer-checker=core,unix.Malloc,unix.MismatchedDeallocator,cplusplus.NewDelete -std=c++11 -verify %s
2+
// RUN: %clang_analyze_cc1 -Wno-alloc-size -analyzer-checker=core,unix.Malloc,unix.MismatchedDeallocator,cplusplus.NewDelete,cplusplus.NewDeleteLeaks -DLEAKS -std=c++11 -verify %s
33

44
#include "Inputs/system-header-simulator-for-malloc.h"
55

‎clang/test/Analysis/Malloc+MismatchedDeallocator_intersections.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,unix.MismatchedDeallocator -std=c++11 -verify %s
1+
// RUN: %clang_analyze_cc1 -Wno-alloc-size -analyzer-checker=core,unix.Malloc,unix.MismatchedDeallocator -std=c++11 -verify %s
22
// expected-no-diagnostics
33

44
typedef __typeof(sizeof(int)) size_t;

‎clang/test/Analysis/MismatchedDeallocator-checker-test.mm

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.MismatchedDeallocator -fblocks -verify %s
2-
// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.MismatchedDeallocator -fblocks -DTEST_INLINABLE_ALLOCATORS -verify %s
1+
// RUN: %clang_analyze_cc1 -Wno-alloc-size -analyzer-checker=core,unix.MismatchedDeallocator -fblocks -verify %s
2+
// RUN: %clang_analyze_cc1 -Wno-alloc-size -analyzer-checker=core,unix.MismatchedDeallocator -fblocks -DTEST_INLINABLE_ALLOCATORS -verify %s
33

44
#include "Inputs/system-header-simulator-objc.h"
55
#include "Inputs/system-header-simulator-cxx.h"

0 commit comments

Comments
 (0)