Skip to content

Commit bfcce44

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 bfcce44

File tree

2 files changed

+142
-10
lines changed

2 files changed

+142
-10
lines changed

‎clang/lib/Analysis/UnsafeBufferUsage.cpp

Lines changed: 104 additions & 10 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;
@@ -385,13 +450,14 @@ AST_MATCHER(CXXConstructExpr, isSafeSpanTwoParamConstruct) {
385450
}
386451
return false;
387452
};
388-
std::optional<APSInt> Arg1CV =
389-
Arg1->getIntegerConstantExpr(Finder->getASTContext());
453+
std::optional<APSInt> Arg1CV = Arg1->getIntegerConstantExpr(Ctx);
390454

391455
if (Arg1CV && Arg1CV->isZero())
392456
// Check form 5:
393457
return true;
394-
switch (Arg0->IgnoreImplicit()->getStmtClass()) {
458+
459+
// Check forms 1-3:
460+
switch (Arg0->getStmtClass()) {
395461
case Stmt::CXXNewExprClass:
396462
if (auto Size = cast<CXXNewExpr>(Arg0)->getArraySize()) {
397463
// Check form 1:
@@ -416,13 +482,41 @@ AST_MATCHER(CXXConstructExpr, isSafeSpanTwoParamConstruct) {
416482

417483
QualType Arg0Ty = Arg0->IgnoreImplicit()->getType();
418484

419-
if (auto *ConstArrTy =
420-
Finder->getASTContext().getAsConstantArrayType(Arg0Ty)) {
485+
if (auto *ConstArrTy = Ctx.getAsConstantArrayType(Arg0Ty)) {
421486
const APSInt ConstArrSize = APSInt(ConstArrTy->getSize());
422487

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

‎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)