Skip to content

Commit 7b05e4c

Browse files
committed
[-Wunsafe-buffer-usage] Teach -Wunsafe-buffer-usage-in-container to be quiet on some benign cases
For some cases where the analyzer can tell it's safe to use two-parameter std::span constructors, the analyzer will not report false alarms on them. For example, ``` std::span<T>{&var, 1} or std::span<T>{new T, 1} ``` rdar://115817781
1 parent 6ba9576 commit 7b05e4c

File tree

2 files changed

+90
-1
lines changed

2 files changed

+90
-1
lines changed

clang/lib/Analysis/UnsafeBufferUsage.cpp

Lines changed: 70 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "clang/ASTMatchers/ASTMatchFinder.h"
1515
#include "clang/Lex/Lexer.h"
1616
#include "clang/Lex/Preprocessor.h"
17+
#include "llvm/ADT/APSInt.h"
1718
#include "llvm/ADT/SmallVector.h"
1819
#include <memory>
1920
#include <optional>
@@ -330,6 +331,73 @@ isInUnspecifiedUntypedContext(internal::Matcher<Stmt> InnerMatcher) {
330331
// FIXME: Handle loop bodies.
331332
return stmt(anyOf(CompStmt, IfStmtThen, IfStmtElse));
332333
}
334+
335+
// Given a two-param std::span construct call, matches iff the call has the
336+
// following forms:
337+
// 1. `std::span<T>{new T[n], n}`, where `n` is a literal or a DRE
338+
// 2. `std::span<T>{new T, 1}`
339+
// 3. `std::span<T>{&var, 1}`
340+
// 4. `std::span<T>{a, n}`, where `a` is of an array-of-T with constant size
341+
// `n`
342+
// 5. `std::span<T>{any, 0}`
343+
AST_MATCHER(CXXConstructExpr, isSafeSpanTwoParamConstruct) {
344+
assert(Node.getNumArgs() == 2 &&
345+
"expecting a two-parameter std::span constructor");
346+
const Expr *Arg0 = Node.getArg(0)->IgnoreImplicit();
347+
const Expr *Arg1 = Node.getArg(1)->IgnoreImplicit();
348+
auto HaveEqualConstantValues = [&Finder](const Expr *E0, const Expr *E1) {
349+
if (auto E0CV = E0->getIntegerConstantExpr(Finder->getASTContext()))
350+
if (auto E1CV = E1->getIntegerConstantExpr(Finder->getASTContext())) {
351+
return APSInt::compareValues(*E0CV, *E1CV) == 0;
352+
}
353+
return false;
354+
};
355+
auto AreSameDRE = [](const Expr *E0, const Expr *E1) {
356+
if (auto *DRE0 = dyn_cast<DeclRefExpr>(E0))
357+
if (auto *DRE1 = dyn_cast<DeclRefExpr>(E1)) {
358+
return DRE0->getDecl() == DRE1->getDecl();
359+
}
360+
return false;
361+
};
362+
std::optional<APSInt> Arg1CV =
363+
Arg1->getIntegerConstantExpr(Finder->getASTContext());
364+
365+
if (Arg1CV && Arg1CV->isZero())
366+
// Check form 5:
367+
return true;
368+
switch (Arg0->IgnoreImplicit()->getStmtClass()) {
369+
case Stmt::CXXNewExprClass:
370+
if (auto Size = cast<CXXNewExpr>(Arg0)->getArraySize()) {
371+
// Check form 1:
372+
return AreSameDRE((*Size)->IgnoreImplicit(), Arg1) ||
373+
HaveEqualConstantValues(*Size, Arg1);
374+
}
375+
// TODO: what's placeholder type? avoid it for now.
376+
if (!cast<CXXNewExpr>(Arg0)->hasPlaceholderType()) {
377+
// Check form 2:
378+
return Arg1CV && Arg1CV->isOne();
379+
}
380+
break;
381+
case Stmt::UnaryOperatorClass:
382+
if (cast<UnaryOperator>(Arg0)->getOpcode() ==
383+
UnaryOperator::Opcode::UO_AddrOf)
384+
// Check form 3:
385+
return Arg1CV && Arg1CV->isOne();
386+
break;
387+
default:
388+
break;
389+
}
390+
391+
QualType Arg0Ty = Arg0->IgnoreImplicit()->getType();
392+
393+
if (Arg0Ty->isConstantArrayType()) {
394+
const APInt &ConstArrSize = cast<ConstantArrayType>(Arg0Ty)->getSize();
395+
396+
// Check form 4:
397+
return Arg1CV && APSInt::compareValues(APSInt(ConstArrSize), *Arg1CV) == 0;
398+
}
399+
return false;
400+
}
333401
} // namespace clang::ast_matchers
334402

335403
namespace {
@@ -619,7 +687,8 @@ class SpanTwoParamConstructorGadget : public WarningGadget {
619687
cxxConstructorDecl(hasDeclContext(isInStdNamespace()), hasName("span"),
620688
parameterCountIs(2)));
621689

622-
return stmt(cxxConstructExpr(HasTwoParamSpanCtorDecl)
690+
return stmt(cxxConstructExpr(HasTwoParamSpanCtorDecl,
691+
unless(isSafeSpanTwoParamConstruct()))
623692
.bind(SpanTwoParamConstructorTag));
624693
}
625694

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,26 @@ namespace construct_wt_ptr_size {
7373
}
7474
return std::span<int>{p, 10}; // expected-warning{{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}}
7575
}
76+
77+
void notWarnSafeCases(unsigned n, int *p) {
78+
int X;
79+
unsigned Y = 10;
80+
std::span<int> S = std::span{&X, 1}; // no-warning
81+
int Arr[10];
82+
83+
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}}
84+
S = std::span{new int[10], 10}; // no-warning
85+
S = std::span{new int[n], n}; // no-warning
86+
S = std::span{new int, 1}; // no-warning
87+
S = std::span{new int, X}; // expected-warning{{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}}
88+
S = std::span{new int[n--], n--}; // expected-warning{{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}}
89+
S = std::span{new int[10], 11}; // expected-warning{{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}}
90+
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
91+
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
92+
S = std::span{Arr, 10}; // no-warning
93+
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
94+
S = std::span{p, 0}; // no-warning
95+
}
7696
} // namespace construct_wt_ptr_size
7797

7898
namespace construct_wt_begin_end {

0 commit comments

Comments
 (0)