Skip to content

Commit 3f4032e

Browse files
ziqingluo-90ziqingluo-90
committed
[-Wunsafe-buffer-usage] Add alloc_size attribute knowledge to the 2-param span ctor warning
rdar://136634730
1 parent bb9ff32 commit 3f4032e

File tree

2 files changed

+142
-8
lines changed

2 files changed

+142
-8
lines changed

‎clang/lib/Analysis/UnsafeBufferUsage.cpp

Lines changed: 104 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "clang/Analysis/Analyses/UnsafeBufferUsage.h"
10+
#include "clang/AST/APValue.h"
1011
#include "clang/AST/ASTContext.h"
12+
#include "clang/AST/Attr.h"
1113
#include "clang/AST/Decl.h"
1214
#include "clang/AST/Expr.h"
1315
#include "clang/AST/FormatString.h"
@@ -358,6 +360,63 @@ isInUnspecifiedUntypedContext(internal::Matcher<Stmt> InnerMatcher) {
358360
return stmt(anyOf(CompStmt, IfStmtThen, IfStmtElse));
359361
}
360362

363+
// Returns true iff integer E1 is equivalent to integer E2.
364+
//
365+
// For now we only support such expressions:
366+
// expr := DRE | const-value | expr BO expr
367+
// BO := '*' | '+'
368+
static bool areEqualIntegers(const Expr *E1, const Expr *E2, ASTContext &Ctx);
369+
static bool areEqualIntegralBinaryOperators(const BinaryOperator *E1,
370+
const Expr *E2_LHS,
371+
BinaryOperatorKind BOP,
372+
const Expr *E2_RHS,
373+
ASTContext &Ctx) {
374+
if (E1->getOpcode() == BOP) {
375+
switch (BOP) {
376+
// Commutative operators:
377+
case BO_Mul:
378+
case BO_Add:
379+
return (areEqualIntegers(E1->getLHS(), E2_LHS, Ctx) &&
380+
areEqualIntegers(E1->getRHS(), E2_RHS, Ctx)) ||
381+
(areEqualIntegers(E1->getLHS(), E2_RHS, Ctx) &&
382+
areEqualIntegers(E1->getRHS(), E2_LHS, Ctx));
383+
default:
384+
return false;
385+
}
386+
}
387+
return false;
388+
}
389+
390+
static bool areEqualIntegers(const Expr *E1, const Expr *E2, ASTContext &Ctx) {
391+
E1 = E1->IgnoreParenImpCasts();
392+
E2 = E2->IgnoreParenImpCasts();
393+
if (!E1->getType()->isIntegerType() || E1->getType() != E2->getType())
394+
return false;
395+
396+
Expr::EvalResult ER1, ER2;
397+
398+
// If both are constants:
399+
if (E1->EvaluateAsConstantExpr(ER1, Ctx) &&
400+
E2->EvaluateAsConstantExpr(ER2, Ctx))
401+
return ER1.Val.getInt() == ER2.Val.getInt();
402+
403+
// Otherwise, they should have identical stmt kind:
404+
if (E1->getStmtClass() != E2->getStmtClass())
405+
return false;
406+
switch (E1->getStmtClass()) {
407+
case Stmt::DeclRefExprClass:
408+
return cast<DeclRefExpr>(E1)->getDecl() == cast<DeclRefExpr>(E2)->getDecl();
409+
case Stmt::BinaryOperatorClass: {
410+
auto BO2 = cast<BinaryOperator>(E2);
411+
return areEqualIntegralBinaryOperators(cast<BinaryOperator>(E1),
412+
BO2->getLHS(), BO2->getOpcode(),
413+
BO2->getRHS(), Ctx);
414+
}
415+
default:
416+
return false;
417+
}
418+
}
419+
361420
// Given a two-param std::span construct call, matches iff the call has the
362421
// following forms:
363422
// 1. `std::span<T>{new T[n], n}`, where `n` is a literal or a DRE
@@ -366,14 +425,20 @@ isInUnspecifiedUntypedContext(internal::Matcher<Stmt> InnerMatcher) {
366425
// 4. `std::span<T>{a, n}`, where `a` is of an array-of-T with constant size
367426
// `n`
368427
// 5. `std::span<T>{any, 0}`
428+
// 6. `std::span<T>{ (char *)f(args), args[N] * arg*[M]}`, where
429+
// `f` is a function with attribute `alloc_size(N, M)`;
430+
// `args` represents the list of arguments;
431+
// `N, M` are parameter indexes to the allocating element number and size.
369432
AST_MATCHER(CXXConstructExpr, isSafeSpanTwoParamConstruct) {
370433
assert(Node.getNumArgs() == 2 &&
371434
"expecting a two-parameter std::span constructor");
372-
const Expr *Arg0 = Node.getArg(0)->IgnoreImplicit();
373-
const Expr *Arg1 = Node.getArg(1)->IgnoreImplicit();
374-
auto HaveEqualConstantValues = [&Finder](const Expr *E0, const Expr *E1) {
375-
if (auto E0CV = E0->getIntegerConstantExpr(Finder->getASTContext()))
376-
if (auto E1CV = E1->getIntegerConstantExpr(Finder->getASTContext())) {
435+
const Expr *Arg0 = Node.getArg(0)->IgnoreParenImpCasts();
436+
const Expr *Arg1 = Node.getArg(1)->IgnoreParenImpCasts();
437+
ASTContext &Ctx = Finder->getASTContext();
438+
439+
auto HaveEqualConstantValues = [&Ctx](const Expr *E0, const Expr *E1) {
440+
if (auto E0CV = E0->getIntegerConstantExpr(Ctx))
441+
if (auto E1CV = E1->getIntegerConstantExpr(Ctx)) {
377442
return APSInt::compareValues(*E0CV, *E1CV) == 0;
378443
}
379444
return false;
@@ -386,12 +451,14 @@ AST_MATCHER(CXXConstructExpr, isSafeSpanTwoParamConstruct) {
386451
return false;
387452
};
388453
std::optional<APSInt> Arg1CV =
389-
Arg1->getIntegerConstantExpr(Finder->getASTContext());
454+
Arg1->getIntegerConstantExpr(Ctx);
390455

391456
if (Arg1CV && Arg1CV->isZero())
392457
// Check form 5:
393458
return true;
394-
switch (Arg0->IgnoreImplicit()->getStmtClass()) {
459+
460+
// Check forms 1-3:
461+
switch (Arg0->getStmtClass()) {
395462
case Stmt::CXXNewExprClass:
396463
if (auto Size = cast<CXXNewExpr>(Arg0)->getArraySize()) {
397464
// Check form 1:
@@ -417,12 +484,41 @@ AST_MATCHER(CXXConstructExpr, isSafeSpanTwoParamConstruct) {
417484
QualType Arg0Ty = Arg0->IgnoreImplicit()->getType();
418485

419486
if (auto *ConstArrTy =
420-
Finder->getASTContext().getAsConstantArrayType(Arg0Ty)) {
487+
Ctx.getAsConstantArrayType(Arg0Ty)) {
421488
const APSInt ConstArrSize = APSInt(ConstArrTy->getSize());
422489

423490
// Check form 4:
424491
return Arg1CV && APSInt::compareValues(ConstArrSize, *Arg1CV) == 0;
425492
}
493+
// Check form 6:
494+
if (auto CCast = dyn_cast<CStyleCastExpr>(Arg0)) {
495+
if (!CCast->getType()->isPointerType())
496+
return false;
497+
498+
QualType PteTy = CCast->getType()->getPointeeType();
499+
500+
if (!(PteTy->isConstantSizeType() && Ctx.getTypeSizeInChars(PteTy).isOne()))
501+
return false;
502+
503+
if (auto Call = dyn_cast<CallExpr>(CCast->getSubExpr())) {
504+
if (const FunctionDecl *FD = Call->getDirectCallee())
505+
if (auto AllocAttr = FD->getAttr<AllocSizeAttr>()) {
506+
const Expr *EleSizeExpr =
507+
Call->getArg(AllocAttr->getElemSizeParam().getASTIndex());
508+
// NumElemIdx is invalid if AllocSizeAttr has 1 argument:
509+
ParamIdx NumElemIdx = AllocAttr->getNumElemsParam();
510+
511+
if (!NumElemIdx.isValid())
512+
return areEqualIntegers(Arg1, EleSizeExpr, Ctx);
513+
514+
const Expr *NumElesExpr = Call->getArg(NumElemIdx.getASTIndex());
515+
516+
if (auto BO = dyn_cast<BinaryOperator>(Arg1))
517+
return areEqualIntegralBinaryOperators(BO, NumElesExpr, BO_Mul,
518+
EleSizeExpr, Ctx);
519+
}
520+
}
521+
}
426522
return false;
427523
}
428524

‎clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct.cpp

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,44 @@ namespace construct_wt_begin_end {
137137
}
138138
} // namespace construct_wt_begin_end
139139

140+
namespace test_alloc_size_attr {
141+
void * my_alloc(unsigned size) __attribute__((alloc_size(1)));
142+
void * my_alloc2(unsigned count, unsigned size) __attribute__((alloc_size(1,2)));
143+
144+
template<typename T>
145+
void foo(std::span<T> S);
146+
147+
void safe(int x, unsigned y) {
148+
foo(std::span<char>{(char *)my_alloc(10), 10});
149+
foo(std::span<char>{(char *)my_alloc(x), x});
150+
foo(std::span<char>{(char *)my_alloc(x * y), x * y});
151+
foo(std::span<char>{(char *)my_alloc(x * y), y * x});
152+
foo(std::span<char>{(char *)my_alloc(x * y + x), x * y + x});
153+
foo(std::span<char>{(char *)my_alloc(x * y + x), x + y * x});
154+
155+
foo(std::span<char>{(char *)my_alloc2(x, y), x * y});
156+
foo(std::span<char>{(char *)my_alloc2(x, y), y * x});
157+
//foo(std::span<char>{(char *)my_alloc2(x, sizeof(char)), x}); // lets not worry about this case for now
158+
foo(std::span<char>{(char *)my_alloc2(x, sizeof(char)), x * sizeof(char)});
159+
//foo(std::span<char>{(char *)my_alloc2(10, sizeof(char)), 10});
160+
foo(std::span<char>{(char *)my_alloc2(10, sizeof(char)), 10 * sizeof(char)});
161+
}
162+
163+
void unsafe(int x, int y) {
164+
foo(std::span<char>{(char *)my_alloc(10), 11}); // expected-warning{{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}}
165+
foo(std::span<char>{(char *)my_alloc(x * y), x + y}); // expected-warning{{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}}
166+
foo(std::span<int>{(int *)my_alloc(x), x}); // expected-warning{{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}}
167+
foo(std::span<char>{(char *)my_alloc2(x, y), x + y}); // expected-warning{{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}}
168+
foo(std::span<int>{(int *)my_alloc2(x, y), x * y}); // expected-warning{{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}}
169+
}
170+
171+
void unsupport(int x, int y) {
172+
// Casting to `T*` where sizeof(T) > 1 is not supported yet:
173+
foo(std::span<long>{(long *)my_alloc(10 * sizeof(long)), 10}); // expected-warning{{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}}
174+
foo(std::span<long>{(long *)my_alloc2(x, sizeof(long)), x}); // expected-warning{{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}}
175+
}
176+
}
177+
140178
namespace test_flag {
141179
void f(int *p) {
142180
#pragma clang diagnostic push

0 commit comments

Comments
 (0)