[-Wunsafe-buffer-usage] Handle pointer initializations for grouping related variables

Differential Revision: https://reviews.llvm.org/D150489
This commit is contained in:
Rashmi Mudduluru 2023-06-15 14:36:23 -07:00
parent 1df10f1580
commit db3dcedb9c
7 changed files with 252 additions and 29 deletions

View File

@ -37,6 +37,7 @@ FIXABLE_GADGET(UPCAddressofArraySubscript) // '&DRE[any]' in an Unspecified Poin
FIXABLE_GADGET(UPCStandalonePointer)
FIXABLE_GADGET(UPCPreIncrement) // '++Ptr' in an Unspecified Pointer Context
FIXABLE_GADGET(PointerAssignment)
FIXABLE_GADGET(PointerInit)
#undef FIXABLE_GADGET
#undef WARNING_GADGET

View File

@ -533,23 +533,66 @@ public:
// FIXME: this gadge will need a fix-it
};
/// A pointer initialization expression of the form:
/// \code
/// int *p = q;
/// \endcode
class PointerInitGadget : public FixableGadget {
private:
static constexpr const char *const PointerInitLHSTag = "ptrInitLHS";
static constexpr const char *const PointerInitRHSTag = "ptrInitRHS";
const VarDecl * PtrInitLHS; // the LHS pointer expression in `PI`
const DeclRefExpr * PtrInitRHS; // the RHS pointer expression in `PI`
public:
PointerInitGadget(const MatchFinder::MatchResult &Result)
: FixableGadget(Kind::PointerInit),
PtrInitLHS(Result.Nodes.getNodeAs<VarDecl>(PointerInitLHSTag)),
PtrInitRHS(Result.Nodes.getNodeAs<DeclRefExpr>(PointerInitRHSTag)) {}
static bool classof(const Gadget *G) {
return G->getKind() == Kind::PointerInit;
}
static Matcher matcher() {
auto PtrInitStmt = declStmt(hasSingleDecl(varDecl(
hasInitializer(ignoringImpCasts(declRefExpr(
hasPointerType()).
bind(PointerInitRHSTag)))).
bind(PointerInitLHSTag)));
return stmt(PtrInitStmt);
}
virtual std::optional<FixItList> getFixits(const Strategy &S) const override;
virtual const Stmt *getBaseStmt() const override { return nullptr; }
virtual DeclUseList getClaimedVarUseSites() const override {
return DeclUseList{PtrInitRHS};
}
virtual std::optional<std::pair<const VarDecl *, const VarDecl *>>
getStrategyImplications() const override {
return std::make_pair(PtrInitLHS,
cast<VarDecl>(PtrInitRHS->getDecl()));
}
};
/// A pointer assignment expression of the form:
/// \code
/// p = q;
/// \endcode
class PointerAssignmentGadget : public FixableGadget {
private:
static constexpr const char *const PointerAssignmentTag = "ptrAssign";
static constexpr const char *const PointerAssignLHSTag = "ptrLHS";
static constexpr const char *const PointerAssignRHSTag = "ptrRHS";
const BinaryOperator *PA; // pointer arithmetic expression
const DeclRefExpr * PtrLHS; // the LHS pointer expression in `PA`
const DeclRefExpr * PtrRHS; // the RHS pointer expression in `PA`
public:
PointerAssignmentGadget(const MatchFinder::MatchResult &Result)
: FixableGadget(Kind::PointerAssignment),
PA(Result.Nodes.getNodeAs<BinaryOperator>(PointerAssignmentTag)),
PtrLHS(Result.Nodes.getNodeAs<DeclRefExpr>(PointerAssignLHSTag)),
PtrRHS(Result.Nodes.getNodeAs<DeclRefExpr>(PointerAssignRHSTag)) {}
@ -566,13 +609,12 @@ public:
to(varDecl())).
bind(PointerAssignLHSTag))));
//FIXME: Handle declarations at assignments
return stmt(isInUnspecifiedUntypedContext(PtrAssignExpr));
}
virtual std::optional<FixItList> getFixits(const Strategy &S) const override;
virtual const Stmt *getBaseStmt() const override { return PA; }
virtual const Stmt *getBaseStmt() const override { return nullptr; }
virtual DeclUseList getClaimedVarUseSites() const override {
return DeclUseList{PtrLHS, PtrRHS};
@ -769,8 +811,8 @@ public:
namespace {
// An auxiliary tracking facility for the fixit analysis. It helps connect
// declarations to its and make sure we've covered all uses with our analysis
// before we try to fix the declaration.
// declarations to its uses and make sure we've covered all uses with our
// analysis before we try to fix the declaration.
class DeclUseTracker {
using UseSetTy = SmallSet<const DeclRefExpr *, 16>;
using DefMapTy = DenseMap<const VarDecl *, const DeclStmt *>;
@ -1174,6 +1216,24 @@ PointerAssignmentGadget::getFixits(const Strategy &S) const {
return std::nullopt;
}
std::optional<FixItList>
PointerInitGadget::getFixits(const Strategy &S) const {
const auto *LeftVD = PtrInitLHS;
const auto *RightVD = cast<VarDecl>(PtrInitRHS->getDecl());
switch (S.lookup(LeftVD)) {
case Strategy::Kind::Span:
if (S.lookup(RightVD) == Strategy::Kind::Span)
return FixItList{};
return std::nullopt;
case Strategy::Kind::Wontfix:
return std::nullopt;
case Strategy::Kind::Iterator:
case Strategy::Kind::Array:
case Strategy::Kind::Vector:
llvm_unreachable("unsupported strategies for FixableGadgets");
}
return std::nullopt;
}
std::optional<FixItList>
ULCArraySubscriptGadget::getFixits(const Strategy &S) const {
@ -2020,10 +2080,10 @@ static bool overlapWithMacro(const FixItList &FixIts) {
});
}
static bool impossibleToFixForVar(const FixableGadgetSets &FixablesForUnsafeVars,
static bool impossibleToFixForVar(const FixableGadgetSets &FixablesForAllVars,
const Strategy &S,
const VarDecl * Var) {
for (const auto &F : FixablesForUnsafeVars.byVar.find(Var)->second) {
for (const auto &F : FixablesForAllVars.byVar.find(Var)->second) {
std::optional<FixItList> Fixits = F->getFixits(S);
if (!Fixits) {
return true;
@ -2033,13 +2093,13 @@ static bool impossibleToFixForVar(const FixableGadgetSets &FixablesForUnsafeVars
}
static std::map<const VarDecl *, FixItList>
getFixIts(FixableGadgetSets &FixablesForUnsafeVars, const Strategy &S,
getFixIts(FixableGadgetSets &FixablesForAllVars, const Strategy &S,
ASTContext &Ctx,
/* The function decl under analysis */ const Decl *D,
const DeclUseTracker &Tracker, UnsafeBufferUsageHandler &Handler,
const DefMapTy &VarGrpMap) {
std::map<const VarDecl *, FixItList> FixItsForVariable;
for (const auto &[VD, Fixables] : FixablesForUnsafeVars.byVar) {
for (const auto &[VD, Fixables] : FixablesForAllVars.byVar) {
FixItsForVariable[VD] =
fixVariable(VD, S.lookup(VD), D, Tracker, Ctx, Handler);
// If we fail to produce Fix-It for the declaration we have to skip the
@ -2073,7 +2133,7 @@ getFixIts(FixableGadgetSets &FixablesForUnsafeVars, const Strategy &S,
if (V == VD) {
continue;
}
if (impossibleToFixForVar(FixablesForUnsafeVars, S, V)) {
if (impossibleToFixForVar(FixablesForAllVars, S, V)) {
ImpossibleToFix = true;
break;
}

View File

@ -0,0 +1,89 @@
// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \
// RUN: -fsafe-buffer-usage-suggestions \
// RUN: -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
void lhs_span_multi_assign() {
int *a = new int[2];
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> a"
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:22-[[@LINE-3]]:22}:", 2}"
int *b = a;
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> b"
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:13-[[@LINE-3]]:13}:", <# placeholder #>}"
int *c = b;
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> c"
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:13-[[@LINE-3]]:13}:", <# placeholder #>}"
int *d = c;
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> d"
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:13-[[@LINE-3]]:13}:", <# placeholder #>}"
int tmp = d[2]; // expected-note{{used in buffer access here}}
}
void rhs_span1() {
int *q = new int[12];
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> q"
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 12}"
int *p = q;
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> p"
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:13-[[@LINE-3]]:13}:", <# placeholder #>}"
p[5] = 10;
int *r = q;
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> r"
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:13-[[@LINE-3]]:13}:", <# placeholder #>}"
r[10] = 5; // expected-note{{used in buffer access here}}
}
void rhs_span2() {
int *q = new int[6];
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> q"
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:22-[[@LINE-3]]:22}:", 6}"
int *p = q;
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> p"
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:13-[[@LINE-3]]:13}:", <# placeholder #>}"
p[5] = 10; // expected-note{{used in buffer access here}}
}
void rhs_span3() {
int *q = new int[6];
int *p = q; // expected-warning{{'p' is an unsafe pointer used for buffer access}}
p[5] = 10; // expected-note{{used in buffer access here}}
int *r = q;
}
void test_grouping() {
int *z = new int[8];
int tmp;
int *y = new int[10];
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> y"
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}"
tmp = y[5];
int *x = new int[10];
x = y;
int *w = z;
}
void test_crash() {
int *r = new int[8];
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> r"
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:22-[[@LINE-3]]:22}:", 8}"
int *q = r;
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> q"
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:13-[[@LINE-3]]:13}:", <# placeholder #>}"
int *p;
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:9}:"std::span<int> p"
p = q;
int tmp = p[9]; // expected-note{{used in buffer access here}}
}

View File

@ -0,0 +1,51 @@
// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fsafe-buffer-usage-suggestions -verify %s
void lhs_span_multi_assign() {
int *a = new int[2];
int *b = a;
int *c = b;
int *d = c; // expected-warning{{'d' is an unsafe pointer used for buffer access}} expected-note-re{{{{^change type of 'd' to 'std::span' to preserve bounds information, and change ('a', 'b', and 'c'|'a', 'c', and 'b'|'b', 'a', and 'c'|'b', 'c', and 'a'|'c', 'a', and 'b'|'c', 'b', and 'a') to 'std::span' to propagate bounds information between them$}}}}
int tmp = d[2]; // expected-note{{used in buffer access here}}
}
void rhs_span1() {
int *q = new int[12];
int *p = q; // expected-warning{{'p' is an unsafe pointer used for buffer access}} expected-note-re{{{{^change type of 'p' to 'std::span' to preserve bounds information, and change ('q' and 'r'|'r' and 'q') to 'std::span' to propagate bounds information between them$}}}}
p[5] = 10; // expected-note{{used in buffer access here}}
int *r = q; // expected-warning{{'r' is an unsafe pointer used for buffer access}} expected-note-re{{{{^change type of 'r' to 'std::span' to preserve bounds information, and change ('p' and 'q'|'q' and 'p') to 'std::span' to propagate bounds information between them$}}}}
r[10] = 5; // expected-note{{used in buffer access here}}
}
void rhs_span2() {
int *q = new int[6];
int *p = q; // expected-warning{{'p' is an unsafe pointer used for buffer access}} expected-note-re{{{{^change type of 'p' to 'std::span' to preserve bounds information, and change 'q' to 'std::span' to propagate bounds information between them$}}}}
p[5] = 10; // expected-note{{used in buffer access here}}
}
// FIXME: Suggest fixits for p, q, and r since span a valid fixit for r.
void rhs_span3() {
int *q = new int[6];
int *p = q; // expected-warning{{'p' is an unsafe pointer used for buffer access}}
p[5] = 10; // expected-note{{used in buffer access here}}
int *r = q;
}
void test_grouping() {
int *z = new int[8];
int tmp;
int *y = new int[10]; // expected-warning{{'y' is an unsafe pointer used for buffer access}} expected-note-re{{{{^change type of 'y' to 'std::span' to preserve bounds information$}}}}
tmp = y[5]; // expected-note{{used in buffer access here}}
int *x = new int[10];
x = y;
int *w = z;
}
void test_crash() {
int *r = new int[8];
int *q = r;
int *p; // expected-warning{{'p' is an unsafe pointer used for buffer access}} expected-note-re{{{{^change type of 'p' to 'std::span' to preserve bounds information, and change ('r' and 'q'|'q' and 'r') to 'std::span' to propagate bounds information between them$}}}}
p = q;
int tmp = p[9]; // expected-note{{used in buffer access here}}
}

View File

@ -48,6 +48,21 @@ void uuc_if_body1(bool flag) {
p[5] = 4;
}
void uuc_if_body2_ptr_init(bool flag) {
int *r = new int[7];
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> r"
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:22-[[@LINE-3]]:22}:", 7}"
if (flag) {
} else {
int* p = r;
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:13}:"std::span<int> p"
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:14-[[@LINE-2]]:14}:"{"
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:15-[[@LINE-3]]:15}:", <# placeholder #>}"
p[5] = 4;
}
}
void uuc_if_cond_no_unsafe_op() {
int *r = new int[7];
int *p = new int[4];
@ -58,32 +73,32 @@ void uuc_if_cond_no_unsafe_op() {
void uuc_if_cond_unsafe_op() {
int *r = new int[7];
int *p = new int[4]; //expected-warning{{'p' is an unsafe pointer used for buffer access}}
int *p = new int[4];
if ((p = r)) {
p[3] = 2; // expected-note{{used in buffer access here}}
p[3] = 2;
}
}
void uuc_if_cond_unsafe_op1() {
int *r = new int[7]; // expected-warning{{'r' is an unsafe pointer used for buffer access}}
int *r = new int[7];
int *p = new int[4];
if ((p = r)) {
r[3] = 2; // expected-note{{used in buffer access here}}
r[3] = 2;
}
}
void uuc_if_cond_unsafe_op2() {
int *r = new int[7]; // expected-warning{{'r' is an unsafe pointer used for buffer access}}
int *p = new int[4]; // expected-warning{{'p' is an unsafe pointer used for buffer access}}
int *r = new int[7];
int *p = new int[4];
if ((p = r)) {
r[3] = 2; // expected-note{{used in buffer access here}}
r[3] = 2;
}
p[4] = 6; // expected-note{{used in buffer access here}}
p[4] = 6;
}
void uuc_call1() {
int *w = new int[4]; // expected-warning{{'w' is an unsafe pointer used for buffer access}}
int *w = new int[4];
int *y = new int[4];
bar(w = y);
w[5] = 0; // expected-note{{used in buffer access here}}
w[5] = 0;
}

View File

@ -38,6 +38,15 @@ void uuc_if_body2(bool flag) {
p[5] = 4; // expected-note{{used in buffer access here}}
}
void uuc_if_body2_ptr_init(bool flag) {
int *r = new int[7];
if (flag) {
} else {
int* p = r; // expected-warning{{'p' is an unsafe pointer used for buffer access}} // expected-note-re{{{{^change type of 'p' to 'std::span' to preserve bounds information, and change 'r' to 'std::span' to propagate bounds information between them$}}}}
p[5] = 4; // expected-note{{used in buffer access here}}
}
}
void uuc_if_cond_no_unsafe_op() {
int *r = new int[7];
int *p = new int[4];

View File

@ -39,13 +39,11 @@ void local_assign_lhs_span() {
p = q;
}
// FIXME: Support initializations at declarations.
void lhs_span_multi_assign() {
int *a = new int[2];
int *b = a;
int *c = b;
int *d = c; // expected-warning{{'d' is an unsafe pointer used for buffer access}} expected-note-re{{{{^change type of 'd' to 'std::span' to preserve bounds information$}}}}
int *d = c; // expected-warning{{'d' is an unsafe pointer used for buffer access}} expected-note-re{{{{^change type of 'd' to 'std::span' to preserve bounds information, and change ('a', 'b', and 'c'|'a', 'c', and 'b'|'b', 'a', and 'c'|'b', 'c', and 'a'|'c', 'a', and 'b'|'c', 'b', and 'a') to 'std::span' to propagate bounds information between them$}}}}
int tmp = d[2]; // expected-note{{used in buffer access here}}
}
@ -59,15 +57,15 @@ void rhs_span() {
void rhs_span1() {
int *q = new int[12];
int *p = q; // expected-warning{{'p' is an unsafe pointer used for buffer access}} expected-note-re{{{{^change type of 'p' to 'std::span' to preserve bounds information$}}}}
int *p = q; // expected-warning{{'p' is an unsafe pointer used for buffer access}} expected-note-re{{{{^change type of 'p' to 'std::span' to preserve bounds information, and change ('q' and 'r'|'r' and 'q') to 'std::span' to propagate bounds information between them$}}}}
p[5] = 10; // expected-note{{used in buffer access here}}
int *r = q; // expected-warning{{'r' is an unsafe pointer used for buffer access}} expected-note-re{{{{^change type of 'r' to 'std::span' to preserve bounds information$}}}}
int *r = q; // expected-warning{{'r' is an unsafe pointer used for buffer access}} expected-note-re{{{{^change type of 'r' to 'std::span' to preserve bounds information, and change ('p' and 'q'|'q' and 'p') to 'std::span' to propagate bounds information between them$}}}}
r[10] = 5; // expected-note{{used in buffer access here}}
}
void rhs_span2() {
int *q = new int[6];
int *p = q; // expected-warning{{'p' is an unsafe pointer used for buffer access}} expected-note-re{{{{^change type of 'p' to 'std::span' to preserve bounds information$}}}}
int *p = q; // expected-warning{{'p' is an unsafe pointer used for buffer access}}
p[5] = 10; // expected-note{{used in buffer access here}}
int *r = q;
}
@ -175,7 +173,7 @@ void foo3b() {
void test_crash() {
int *r = new int[8];
int *q = r;
int *p; // expected-warning{{'p' is an unsafe pointer used for buffer access}} expected-note-re{{{{^change type of 'p' to 'std::span' to preserve bounds information, and change 'q' to 'std::span' to propagate bounds information between them$}}}}
int *p; // expected-warning{{'p' is an unsafe pointer used for buffer access}} expected-note-re{{{{^change type of 'p' to 'std::span' to preserve bounds information, and change ('r' and 'q'|'q' and 'r') to 'std::span' to propagate bounds information between them$}}}}
p = q;
int tmp = p[9]; // expected-note{{used in buffer access here}}
}