diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def --- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def +++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def @@ -37,6 +37,7 @@ FIXABLE_GADGET(UPCStandalonePointer) FIXABLE_GADGET(UPCPreIncrement) // '++Ptr' in an Unspecified Pointer Context FIXABLE_GADGET(PointerAssignment) +FIXABLE_GADGET(PointerInit) #undef FIXABLE_GADGET #undef WARNING_GADGET diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -533,23 +533,66 @@ // 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(PointerInitLHSTag)), + PtrInitRHS(Result.Nodes.getNodeAs(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 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> + getStrategyImplications() const override { + return std::make_pair(PtrInitLHS, + cast(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(PointerAssignmentTag)), PtrLHS(Result.Nodes.getNodeAs(PointerAssignLHSTag)), PtrRHS(Result.Nodes.getNodeAs(PointerAssignRHSTag)) {} @@ -566,13 +609,12 @@ to(varDecl())). bind(PointerAssignLHSTag)))); - //FIXME: Handle declarations at assignments return stmt(isInUnspecifiedUntypedContext(PtrAssignExpr)); } virtual std::optional 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 @@ 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; using DefMapTy = DenseMap; @@ -1174,6 +1216,24 @@ return std::nullopt; } +std::optional +PointerInitGadget::getFixits(const Strategy &S) const { + const auto *LeftVD = PtrInitLHS; + const auto *RightVD = cast(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 ULCArraySubscriptGadget::getFixits(const Strategy &S) const { @@ -2020,10 +2080,10 @@ }); } -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 Fixits = F->getFixits(S); if (!Fixits) { return true; @@ -2033,13 +2093,13 @@ } static std::map -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 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 @@ if (V == VD) { continue; } - if (impossibleToFixForVar(FixablesForUnsafeVars, S, V)) { + if (impossibleToFixForVar(FixablesForAllVars, S, V)) { ImpossibleToFix = true; break; } diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-ptr-init-fixits.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-ptr-init-fixits.cpp new file mode 100644 --- /dev/null +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-ptr-init-fixits.cpp @@ -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 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 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 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 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 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 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 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 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 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 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 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 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 p" + p = q; + int tmp = p[9]; // expected-note{{used in buffer access here}} +} diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-ptr-init.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-ptr-init.cpp new file mode 100644 --- /dev/null +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-ptr-init.cpp @@ -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}} +} diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-uuc-fixits.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-uuc-fixits.cpp --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-uuc-fixits.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-uuc-fixits.cpp @@ -48,6 +48,21 @@ 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 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 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_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; } diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-uuc.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-uuc.cpp --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-uuc.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-uuc.cpp @@ -38,6 +38,15 @@ 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]; diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-warnings.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-warnings.cpp --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-warnings.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-warnings.cpp @@ -39,13 +39,11 @@ 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_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 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}} }