Skip to content

Commit 755bbe3

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 755bbe3

File tree

4 files changed

+92
-4
lines changed

4 files changed

+92
-4
lines changed

clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12076,7 +12076,7 @@ def note_unsafe_buffer_variable_fixit_together : Note<
1207612076
def note_safe_buffer_usage_suggestions_disabled : Note<
1207712077
"pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions">;
1207812078
def warn_unsafe_buffer_usage_in_container : Warning<
12079-
"%select{the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information}0">,
12079+
"the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information">,
1208012080
InGroup<UnsafeBufferUsageInContainer>, DefaultIgnore;
1208112081
#ifndef NDEBUG
1208212082
// Not a user-facing diagnostic. Useful for debugging false negatives in

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/lib/Sema/AnalysisBasedWarnings.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2258,8 +2258,7 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler {
22582258
}
22592259
} else if (const auto *CtorExpr = dyn_cast<CXXConstructExpr>(Operation)) {
22602260
S.Diag(CtorExpr->getLocation(),
2261-
diag::warn_unsafe_buffer_usage_in_container)
2262-
<< 0;
2261+
diag::warn_unsafe_buffer_usage_in_container);
22632262
} else {
22642263
if (isa<CallExpr>(Operation)) {
22652264
// note_unsafe_buffer_operation doesn't have this mode yet.

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)