Skip to content

Commit d6c860d

Browse files
committed
[-Wunsafe-buffer-usage] Fix a bug in the ASTMatcher for span constructors
`QualType::isConstantArrayType()` checks canonical type. So a following cast should be applied to canonical type as well. And, a better option is to use `ASTContext::getAsArrayType()`. ``` if (Ty->isConstantArrayType()) cast<ConstantArrayType>(Ty); // this is incorrect if (auto *CAT = Context.getAsConstantArrayType(Ty)) ... // this is good ```
1 parent ee5d572 commit d6c860d

File tree

2 files changed

+10
-7
lines changed

2 files changed

+10
-7
lines changed

clang/lib/Analysis/UnsafeBufferUsage.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -402,9 +402,9 @@ AST_MATCHER(CXXConstructExpr, isSafeSpanTwoParamConstruct) {
402402

403403
QualType Arg0Ty = Arg0->IgnoreImplicit()->getType();
404404

405-
if (Arg0Ty->isConstantArrayType()) {
406-
const APSInt ConstArrSize =
407-
APSInt(cast<ConstantArrayType>(Arg0Ty)->getSize());
405+
if (auto *ConstArrTy =
406+
Finder->getASTContext().getAsConstantArrayType(Arg0Ty)) {
407+
const APSInt ConstArrSize = APSInt(ConstArrTy->getSize());
408408

409409
// Check form 4:
410410
return Arg1CV && APSInt::compareValues(ConstArrSize, *Arg1CV) == 0;
@@ -2659,7 +2659,7 @@ static FixItList fixVarDeclWithArray(const VarDecl *D, const ASTContext &Ctx,
26592659

26602660
// Note: the code below expects the declaration to not use any type sugar like
26612661
// typedef.
2662-
if (auto CAT = dyn_cast<clang::ConstantArrayType>(D->getType())) {
2662+
if (auto CAT = Ctx.getAsConstantArrayType(D->getType())) {
26632663
const QualType &ArrayEltT = CAT->getElementType();
26642664
assert(!ArrayEltT.isNull() && "Trying to fix a non-array type variable!");
26652665
// FIXME: support multi-dimensional arrays
@@ -2792,9 +2792,9 @@ fixVariable(const VarDecl *VD, FixitStrategy::Kind K,
27922792
return {};
27932793
}
27942794
case FixitStrategy::Kind::Array: {
2795-
if (VD->isLocalVarDecl() &&
2796-
isa<clang::ConstantArrayType>(VD->getType().getCanonicalType()))
2797-
return fixVariableWithArray(VD, Tracker, Ctx, Handler);
2795+
if (VD->isLocalVarDecl())
2796+
if (auto CAT = Ctx.getAsConstantArrayType(VD->getType()))
2797+
return fixVariableWithArray(VD, Tracker, Ctx, Handler);
27982798

27992799
DEBUG_NOTE_DECL_FAIL(VD, " : not a local const-size array");
28002800
return {};

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,8 @@ namespace construct_wt_ptr_size {
7979
unsigned Y = 10;
8080
std::span<int> S = std::span{&X, 1}; // no-warning
8181
int Arr[10];
82+
typedef int TenInts_t[10];
83+
TenInts_t Arr2;
8284

8385
S = std::span{&X, 2}; // expected-warning{{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}}
8486
S = std::span{new int[10], 10}; // no-warning
@@ -90,6 +92,7 @@ namespace construct_wt_ptr_size {
9092
S = std::span{new int[10], 9}; // expected-warning{{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}} // not smart enough to tell its safe
9193
S = std::span{new int[10], Y}; // expected-warning{{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}} // not smart enough to tell its safe
9294
S = std::span{Arr, 10}; // no-warning
95+
S = std::span{Arr2, 10}; // no-warning
9396
S = std::span{Arr, Y}; // expected-warning{{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}} // not smart enough to tell its safe
9497
S = std::span{p, 0}; // no-warning
9598
}

0 commit comments

Comments
 (0)