https://github.com/t-rasmud updated https://github.com/llvm/llvm-project/pull/71862
>From 6636745d1c444747a33c91b366a730d81ca5db5a Mon Sep 17 00:00:00 2001 From: Rashmi Mudduluru <r_muddul...@apple.com> Date: Wed, 1 Nov 2023 13:43:12 -0700 Subject: [PATCH 01/14] [-Wunsafe-buffer-usage][WIP] Fixable gadget for AddAssign --- .../Analyses/UnsafeBufferUsageGadgets.def | 1 + clang/lib/Analysis/UnsafeBufferUsage.cpp | 65 +++++++++++++++++++ ...-unsafe-buffer-usage-fixits-add-assign.cpp | 38 +++++++++++ 3 files changed, 104 insertions(+) create mode 100644 clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def index ff687a0d178bdea..757ee452ced7488 100644 --- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def +++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def @@ -36,6 +36,7 @@ FIXABLE_GADGET(PointerDereference) FIXABLE_GADGET(UPCAddressofArraySubscript) // '&DRE[any]' in an Unspecified Pointer Context FIXABLE_GADGET(UPCStandalonePointer) FIXABLE_GADGET(UPCPreIncrement) // '++Ptr' in an Unspecified Pointer Context +FIXABLE_GADGET(UUCAddAssign) // 'Ptr += n' in an Unspecified Untyped Context FIXABLE_GADGET(PointerAssignment) FIXABLE_GADGET(PointerInit) diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index e332a3609290aac..7b79f5360c79d6e 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -1028,6 +1028,42 @@ class UPCPreIncrementGadget : public FixableGadget { } }; +// Representing a pointer type expression of the form `Ptr += n` in an +// Unspecified Untyped Context (UUC): +class UUCAddAssignGadget : public FixableGadget { +private: + static constexpr const char *const UUCAddAssignTag = + "PointerAddAssignUnderUUC"; + const BinaryOperator *Node; // the `Ptr += n` node + +public: + UUCAddAssignGadget(const MatchFinder::MatchResult &Result) + : FixableGadget(Kind::UUCAddAssign), + Node(Result.Nodes.getNodeAs<BinaryOperator>(UUCAddAssignTag)) { + assert(Node != nullptr && "Expecting a non-null matching result"); + } + + static bool classof(const Gadget *G) { + return G->getKind() == Kind::UUCAddAssign; + } + + static Matcher matcher() { + return stmt(isInUnspecifiedUntypedContext(expr(ignoringImpCasts( + binaryOperator(hasOperatorName("+="), + hasLHS(declRefExpr( + toSupportedVariable())) + ).bind(UUCAddAssignTag))))); + } + + virtual std::optional<FixItList> getFixits(const Strategy &S) const override; + + virtual const Stmt *getBaseStmt() const override { return Node; } + + virtual DeclUseList getClaimedVarUseSites() const override { + return {dyn_cast<DeclRefExpr>(Node->getLHS())}; + } +}; + // Representing a fixable expression of the form `*(ptr + 123)` or `*(123 + // ptr)`: class DerefSimplePtrArithFixableGadget : public FixableGadget { @@ -1766,6 +1802,35 @@ fixUPCAddressofArraySubscriptWithSpan(const UnaryOperator *Node) { FixItHint::CreateReplacement(Node->getSourceRange(), SS.str())}; } +std::optional<FixItList> UUCAddAssignGadget::getFixits(const Strategy &S) const { + DeclUseList DREs = getClaimedVarUseSites(); + + if (DREs.size() != 1) + return std::nullopt; // In cases of `Ptr += n` where `Ptr` is not a DRE, we + // give up + if (const VarDecl *VD = dyn_cast<VarDecl>(DREs.front()->getDecl())) { + if (S.lookup(VD) == Strategy::Kind::Span) { + FixItList Fixes; + std::stringstream SS; + const Stmt *AddAssignNode = getBaseStmt(); + StringRef varName = VD->getName(); + const ASTContext &Ctx = VD->getASTContext(); + + // To transform UUC(p += n) to UUC((p = p.subspan(1)).data()): + SS << varName.data() << " = " << varName.data() + << ".subspan(" << getUserFillPlaceHolder() << ")"; + std::optional<SourceLocation> AddAssignLocation = + getEndCharLoc(AddAssignNode, Ctx.getSourceManager(), Ctx.getLangOpts()); + if (!AddAssignLocation) + return std::nullopt; + + Fixes.push_back(FixItHint::CreateReplacement( + SourceRange(AddAssignNode->getBeginLoc(), *AddAssignLocation), SS.str())); + return Fixes; + } + } + return std::nullopt; // Not in the cases that we can handle for now, give up. +} std::optional<FixItList> UPCPreIncrementGadget::getFixits(const Strategy &S) const { DeclUseList DREs = getClaimedVarUseSites(); diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp new file mode 100644 index 000000000000000..e2b9a43dee9b3c3 --- /dev/null +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp @@ -0,0 +1,38 @@ +// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \ +// RUN: -fsafe-buffer-usage-suggestions \ +// RUN: -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s +void foo(int * , int *); + +void add_assign_test() { + int *p = new int[10]; + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> p" + // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{" + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}" + p += 2; + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:9}:"p = p.subspan(<# placeholder #>)" + + int *r = p; + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> r" + // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{" + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:13-[[@LINE-3]]:13}:", <# placeholder #>}" + while (*r != 0) { + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:11}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"[0]" + r += 2; + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:11}:"r = r.subspan(<# placeholder #>)" + } + + if (*p == 0) { + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:8}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:9-[[@LINE-2]]:9}:"[0]" + p += 2; + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:11}:"p = p.subspan(<# placeholder #>)" + } + + if (*p == 1) + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:8}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:9-[[@LINE-2]]:9}:"[0]" + p += 3; + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:11}:"p = p.subspan(<# placeholder #>)" +} + >From d5f00a5978b8a1b3574d7a3d0a18bce956138bb3 Mon Sep 17 00:00:00 2001 From: Rashmi Mudduluru <r_muddul...@apple.com> Date: Tue, 7 Nov 2023 16:02:02 -0800 Subject: [PATCH 02/14] [-Wunsafe-buffer-usage][WIP] Fixable gadget for AddAssign: handle integer literal case --- clang/lib/Analysis/UnsafeBufferUsage.cpp | 34 ++++++++++++++++--- ...-unsafe-buffer-usage-fixits-add-assign.cpp | 11 +++--- 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index 7b79f5360c79d6e..dd8ccbdd1533aa4 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -1034,12 +1034,16 @@ class UUCAddAssignGadget : public FixableGadget { private: static constexpr const char *const UUCAddAssignTag = "PointerAddAssignUnderUUC"; + static constexpr const char *const IntOffsetTag = "IntOffset"; + const BinaryOperator *Node; // the `Ptr += n` node + const IntegerLiteral *IntOffset = nullptr; public: UUCAddAssignGadget(const MatchFinder::MatchResult &Result) : FixableGadget(Kind::UUCAddAssign), - Node(Result.Nodes.getNodeAs<BinaryOperator>(UUCAddAssignTag)) { + Node(Result.Nodes.getNodeAs<BinaryOperator>(UUCAddAssignTag)), + IntOffset(Result.Nodes.getNodeAs<IntegerLiteral>(IntOffsetTag)) { assert(Node != nullptr && "Expecting a non-null matching result"); } @@ -1051,7 +1055,10 @@ class UUCAddAssignGadget : public FixableGadget { return stmt(isInUnspecifiedUntypedContext(expr(ignoringImpCasts( binaryOperator(hasOperatorName("+="), hasLHS(declRefExpr( - toSupportedVariable())) + toSupportedVariable())), + hasRHS(expr(anyOf( + ignoringImpCasts(declRefExpr()), + integerLiteral().bind(IntOffsetTag)))) ).bind(UUCAddAssignTag))))); } @@ -1815,10 +1822,27 @@ std::optional<FixItList> UUCAddAssignGadget::getFixits(const Strategy &S) const const Stmt *AddAssignNode = getBaseStmt(); StringRef varName = VD->getName(); const ASTContext &Ctx = VD->getASTContext(); - - // To transform UUC(p += n) to UUC((p = p.subspan(1)).data()): + + std::string SubSpanOffset; + if (IntOffset) { + auto ConstVal = IntOffset->getIntegerConstantExpr(Ctx); + if (ConstVal->isNegative()) + return std::nullopt; + + SmallString<256> OffsetStr; + ConstVal->toString(OffsetStr); + SubSpanOffset = OffsetStr.c_str(); + // To transform UUC(p += IntegerLiteral) to UUC(p = p.subspan(IntegerLiteral)): + SubSpanOffset = OffsetStr.c_str(); + } + else { + SubSpanOffset = getUserFillPlaceHolder(); + } + + // To transform UUC(p += n) to UUC(p = p.subspan(..)): SS << varName.data() << " = " << varName.data() - << ".subspan(" << getUserFillPlaceHolder() << ")"; + << ".subspan(" << SubSpanOffset << ")"; + std::optional<SourceLocation> AddAssignLocation = getEndCharLoc(AddAssignNode, Ctx.getSourceManager(), Ctx.getLangOpts()); if (!AddAssignLocation) diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp index e2b9a43dee9b3c3..786335b5cfcef97 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp @@ -3,13 +3,13 @@ // RUN: -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s void foo(int * , int *); -void add_assign_test() { +void add_assign_test(int n) { int *p = new int[10]; // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> p" // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{" // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}" p += 2; - // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:9}:"p = p.subspan(<# placeholder #>)" + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:9}:"p = p.subspan(2)" int *r = p; // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> r" @@ -19,13 +19,13 @@ void add_assign_test() { // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:11}:"" // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"[0]" r += 2; - // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:11}:"r = r.subspan(<# placeholder #>)" + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:11}:"r = r.subspan(2)" } if (*p == 0) { // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:8}:"" // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:9-[[@LINE-2]]:9}:"[0]" - p += 2; + p += n; // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:11}:"p = p.subspan(<# placeholder #>)" } @@ -33,6 +33,5 @@ void add_assign_test() { // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:8}:"" // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:9-[[@LINE-2]]:9}:"[0]" p += 3; - // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:11}:"p = p.subspan(<# placeholder #>)" + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:11}:"p = p.subspan(3)" } - >From ec71495b80e209e7c2dffaaadb1f05814cfafaed Mon Sep 17 00:00:00 2001 From: Rashmi Mudduluru <r_muddul...@apple.com> Date: Thu, 9 Nov 2023 10:42:58 -0800 Subject: [PATCH 03/14] [-Wunsafe-buffer-usage][WIP] Fixable gadget for AddAssign: handle DeclRefExpr, add test --- clang/lib/Analysis/UnsafeBufferUsage.cpp | 9 ++++++--- .../warn-unsafe-buffer-usage-fixits-add-assign.cpp | 7 +++++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index dd8ccbdd1533aa4..54923620274c0d5 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -1035,15 +1035,18 @@ class UUCAddAssignGadget : public FixableGadget { static constexpr const char *const UUCAddAssignTag = "PointerAddAssignUnderUUC"; static constexpr const char *const IntOffsetTag = "IntOffset"; + static constexpr const char *const OffsetTag = "Offset"; const BinaryOperator *Node; // the `Ptr += n` node const IntegerLiteral *IntOffset = nullptr; + const DeclRefExpr *Offset = nullptr; public: UUCAddAssignGadget(const MatchFinder::MatchResult &Result) : FixableGadget(Kind::UUCAddAssign), Node(Result.Nodes.getNodeAs<BinaryOperator>(UUCAddAssignTag)), - IntOffset(Result.Nodes.getNodeAs<IntegerLiteral>(IntOffsetTag)) { + IntOffset(Result.Nodes.getNodeAs<IntegerLiteral>(IntOffsetTag)), + Offset(Result.Nodes.getNodeAs<DeclRefExpr>(OffsetTag)) { assert(Node != nullptr && "Expecting a non-null matching result"); } @@ -1057,7 +1060,7 @@ class UUCAddAssignGadget : public FixableGadget { hasLHS(declRefExpr( toSupportedVariable())), hasRHS(expr(anyOf( - ignoringImpCasts(declRefExpr()), + ignoringImpCasts(declRefExpr().bind(OffsetTag)), integerLiteral().bind(IntOffsetTag)))) ).bind(UUCAddAssignTag))))); } @@ -1836,7 +1839,7 @@ std::optional<FixItList> UUCAddAssignGadget::getFixits(const Strategy &S) const SubSpanOffset = OffsetStr.c_str(); } else { - SubSpanOffset = getUserFillPlaceHolder(); + SubSpanOffset = Offset->getDecl()->getName().str(); } // To transform UUC(p += n) to UUC(p = p.subspan(..)): diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp index 786335b5cfcef97..30c587b2110d19b 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp @@ -3,7 +3,7 @@ // RUN: -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s void foo(int * , int *); -void add_assign_test(int n) { +void add_assign_test(int n, int *a) { int *p = new int[10]; // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> p" // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{" @@ -26,7 +26,7 @@ void add_assign_test(int n) { // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:8}:"" // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:9-[[@LINE-2]]:9}:"[0]" p += n; - // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:11}:"p = p.subspan(<# placeholder #>)" + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:11}:"p = p.subspan(n)" } if (*p == 1) @@ -34,4 +34,7 @@ void add_assign_test(int n) { // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:9-[[@LINE-2]]:9}:"[0]" p += 3; // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:11}:"p = p.subspan(3)" + + a += -9; + // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:11}:"p = p.subspan(-9)" } >From b040dcb613f1fe74080d2900e7c51a554d93aa36 Mon Sep 17 00:00:00 2001 From: Rashmi Mudduluru <r_muddul...@apple.com> Date: Thu, 9 Nov 2023 13:26:10 -0800 Subject: [PATCH 04/14] [-Wunsafe-buffer-usage] Fixable gadget for AddAssign: handle DeclRefExpr, add test --- clang/lib/Analysis/UnsafeBufferUsage.cpp | 44 +++++++++++------------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index 54923620274c0d5..32d3f816b915020 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -1033,7 +1033,7 @@ class UPCPreIncrementGadget : public FixableGadget { class UUCAddAssignGadget : public FixableGadget { private: static constexpr const char *const UUCAddAssignTag = - "PointerAddAssignUnderUUC"; + "PointerAddAssignUnderUUC"; static constexpr const char *const IntOffsetTag = "IntOffset"; static constexpr const char *const OffsetTag = "Offset"; @@ -1043,10 +1043,10 @@ class UUCAddAssignGadget : public FixableGadget { public: UUCAddAssignGadget(const MatchFinder::MatchResult &Result) - : FixableGadget(Kind::UUCAddAssign), - Node(Result.Nodes.getNodeAs<BinaryOperator>(UUCAddAssignTag)), - IntOffset(Result.Nodes.getNodeAs<IntegerLiteral>(IntOffsetTag)), - Offset(Result.Nodes.getNodeAs<DeclRefExpr>(OffsetTag)) { + : FixableGadget(Kind::UUCAddAssign), + Node(Result.Nodes.getNodeAs<BinaryOperator>(UUCAddAssignTag)), + IntOffset(Result.Nodes.getNodeAs<IntegerLiteral>(IntOffsetTag)), + Offset(Result.Nodes.getNodeAs<DeclRefExpr>(OffsetTag)) { assert(Node != nullptr && "Expecting a non-null matching result"); } @@ -1056,13 +1056,11 @@ class UUCAddAssignGadget : public FixableGadget { static Matcher matcher() { return stmt(isInUnspecifiedUntypedContext(expr(ignoringImpCasts( - binaryOperator(hasOperatorName("+="), - hasLHS(declRefExpr( - toSupportedVariable())), - hasRHS(expr(anyOf( - ignoringImpCasts(declRefExpr().bind(OffsetTag)), - integerLiteral().bind(IntOffsetTag)))) - ).bind(UUCAddAssignTag))))); + binaryOperator( + hasOperatorName("+="), hasLHS(declRefExpr(toSupportedVariable())), + hasRHS(expr(anyOf(ignoringImpCasts(declRefExpr().bind(OffsetTag)), + integerLiteral().bind(IntOffsetTag))))) + .bind(UUCAddAssignTag))))); } virtual std::optional<FixItList> getFixits(const Strategy &S) const override; @@ -1812,7 +1810,8 @@ fixUPCAddressofArraySubscriptWithSpan(const UnaryOperator *Node) { FixItHint::CreateReplacement(Node->getSourceRange(), SS.str())}; } -std::optional<FixItList> UUCAddAssignGadget::getFixits(const Strategy &S) const { +std::optional<FixItList> +UUCAddAssignGadget::getFixits(const Strategy &S) const { DeclUseList DREs = getClaimedVarUseSites(); if (DREs.size() != 1) @@ -1825,34 +1824,33 @@ std::optional<FixItList> UUCAddAssignGadget::getFixits(const Strategy &S) const const Stmt *AddAssignNode = getBaseStmt(); StringRef varName = VD->getName(); const ASTContext &Ctx = VD->getASTContext(); - + std::string SubSpanOffset; if (IntOffset) { auto ConstVal = IntOffset->getIntegerConstantExpr(Ctx); if (ConstVal->isNegative()) return std::nullopt; - + SmallString<256> OffsetStr; ConstVal->toString(OffsetStr); SubSpanOffset = OffsetStr.c_str(); - // To transform UUC(p += IntegerLiteral) to UUC(p = p.subspan(IntegerLiteral)): SubSpanOffset = OffsetStr.c_str(); - } - else { + } else { SubSpanOffset = Offset->getDecl()->getName().str(); } // To transform UUC(p += n) to UUC(p = p.subspan(..)): - SS << varName.data() << " = " << varName.data() - << ".subspan(" << SubSpanOffset << ")"; + SS << varName.data() << " = " << varName.data() << ".subspan(" + << SubSpanOffset << ")"; - std::optional<SourceLocation> AddAssignLocation = - getEndCharLoc(AddAssignNode, Ctx.getSourceManager(), Ctx.getLangOpts()); + std::optional<SourceLocation> AddAssignLocation = getEndCharLoc( + AddAssignNode, Ctx.getSourceManager(), Ctx.getLangOpts()); if (!AddAssignLocation) return std::nullopt; Fixes.push_back(FixItHint::CreateReplacement( - SourceRange(AddAssignNode->getBeginLoc(), *AddAssignLocation), SS.str())); + SourceRange(AddAssignNode->getBeginLoc(), *AddAssignLocation), + SS.str())); return Fixes; } } >From 65ae8a823ae6c4826e2322bdf173f7611dd4e5b6 Mon Sep 17 00:00:00 2001 From: Rashmi Mudduluru <r_muddul...@apple.com> Date: Thu, 9 Nov 2023 15:21:02 -0800 Subject: [PATCH 05/14] [-Wunsafe-buffer-usage] Fixable gadget for AddAssign: fix formatting --- clang/lib/Analysis/UnsafeBufferUsage.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index 32d3f816b915020..7ce114e3dd5a334 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -1841,15 +1841,15 @@ UUCAddAssignGadget::getFixits(const Strategy &S) const { // To transform UUC(p += n) to UUC(p = p.subspan(..)): SS << varName.data() << " = " << varName.data() << ".subspan(" - << SubSpanOffset << ")"; - + << SubSpanOffset << ")"; + std::optional<SourceLocation> AddAssignLocation = getEndCharLoc( AddAssignNode, Ctx.getSourceManager(), Ctx.getLangOpts()); if (!AddAssignLocation) return std::nullopt; Fixes.push_back(FixItHint::CreateReplacement( - SourceRange(AddAssignNode->getBeginLoc(), *AddAssignLocation), + SourceRange(AddAssignNode->getBeginLoc(), *AddAssignLocation), SS.str())); return Fixes; } >From 4ccd7e12e1708b7df4d05b029cda26f6219d934c Mon Sep 17 00:00:00 2001 From: Rashmi Mudduluru <r_muddul...@apple.com> Date: Fri, 10 Nov 2023 12:01:46 -0800 Subject: [PATCH 06/14] [-Wunsafe-buffer-usage] Fixable gadget for AddAssign: fix formatting --- clang/lib/Analysis/UnsafeBufferUsage.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index 7ce114e3dd5a334..32e0868bd6c5ea8 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -1036,7 +1036,7 @@ class UUCAddAssignGadget : public FixableGadget { "PointerAddAssignUnderUUC"; static constexpr const char *const IntOffsetTag = "IntOffset"; static constexpr const char *const OffsetTag = "Offset"; - + const BinaryOperator *Node; // the `Ptr += n` node const IntegerLiteral *IntOffset = nullptr; const DeclRefExpr *Offset = nullptr; @@ -1838,7 +1838,7 @@ UUCAddAssignGadget::getFixits(const Strategy &S) const { } else { SubSpanOffset = Offset->getDecl()->getName().str(); } - + // To transform UUC(p += n) to UUC(p = p.subspan(..)): SS << varName.data() << " = " << varName.data() << ".subspan(" << SubSpanOffset << ")"; >From 19083c11752a5151ad556d6bc6858c8d704a1983 Mon Sep 17 00:00:00 2001 From: Rashmi Mudduluru <r_muddul...@apple.com> Date: Tue, 14 Nov 2023 15:17:04 -0800 Subject: [PATCH 07/14] [-Wunsafe-buffer-usage] Fixable gadget for AddAssign: Address comments --- clang/lib/Analysis/UnsafeBufferUsage.cpp | 49 ++++++++++--------- ...-unsafe-buffer-usage-fixits-add-assign.cpp | 17 ++++++- 2 files changed, 42 insertions(+), 24 deletions(-) diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index 32e0868bd6c5ea8..311e47e730ee280 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -1034,19 +1034,16 @@ class UUCAddAssignGadget : public FixableGadget { private: static constexpr const char *const UUCAddAssignTag = "PointerAddAssignUnderUUC"; - static constexpr const char *const IntOffsetTag = "IntOffset"; static constexpr const char *const OffsetTag = "Offset"; const BinaryOperator *Node; // the `Ptr += n` node - const IntegerLiteral *IntOffset = nullptr; - const DeclRefExpr *Offset = nullptr; + const Expr *Offset = nullptr; public: UUCAddAssignGadget(const MatchFinder::MatchResult &Result) : FixableGadget(Kind::UUCAddAssign), Node(Result.Nodes.getNodeAs<BinaryOperator>(UUCAddAssignTag)), - IntOffset(Result.Nodes.getNodeAs<IntegerLiteral>(IntOffsetTag)), - Offset(Result.Nodes.getNodeAs<DeclRefExpr>(OffsetTag)) { + Offset(Result.Nodes.getNodeAs<Expr>(OffsetTag)){ assert(Node != nullptr && "Expecting a non-null matching result"); } @@ -1058,8 +1055,7 @@ class UUCAddAssignGadget : public FixableGadget { return stmt(isInUnspecifiedUntypedContext(expr(ignoringImpCasts( binaryOperator( hasOperatorName("+="), hasLHS(declRefExpr(toSupportedVariable())), - hasRHS(expr(anyOf(ignoringImpCasts(declRefExpr().bind(OffsetTag)), - integerLiteral().bind(IntOffsetTag))))) + hasRHS(ignoringParens(expr().bind(OffsetTag)))) .bind(UUCAddAssignTag))))); } @@ -1356,6 +1352,16 @@ PointerInitGadget::getFixits(const Strategy &S) const { return std::nullopt; } +static bool isNonNegativeIntegerExpr(const Expr *Expr, const VarDecl *VD, + const ASTContext &Ctx) { + if (auto ConstVal = Expr->getIntegerConstantExpr(Ctx)) { + if (ConstVal->isNegative()) + return false; + } else if (!Expr->getType()->isUnsignedIntegerType()) + return false; + return true; +} + std::optional<FixItList> ULCArraySubscriptGadget::getFixits(const Strategy &S) const { if (const auto *DRE = @@ -1363,14 +1369,12 @@ ULCArraySubscriptGadget::getFixits(const Strategy &S) const { if (const auto *VD = dyn_cast<VarDecl>(DRE->getDecl())) { switch (S.lookup(VD)) { case Strategy::Kind::Span: { + // If the index has a negative constant value, we give up as no valid // fix-it can be generated: const ASTContext &Ctx = // FIXME: we need ASTContext to be passed in! VD->getASTContext(); - if (auto ConstVal = Node->getIdx()->getIntegerConstantExpr(Ctx)) { - if (ConstVal->isNegative()) - return std::nullopt; - } else if (!Node->getIdx()->getType()->isUnsignedIntegerType()) + if (!isNonNegativeIntegerExpr(Node->getIdx(), VD, Ctx)) return std::nullopt; // no-op is a good fix-it, otherwise return FixItList{}; @@ -1825,19 +1829,18 @@ UUCAddAssignGadget::getFixits(const Strategy &S) const { StringRef varName = VD->getName(); const ASTContext &Ctx = VD->getASTContext(); - std::string SubSpanOffset; - if (IntOffset) { - auto ConstVal = IntOffset->getIntegerConstantExpr(Ctx); - if (ConstVal->isNegative()) - return std::nullopt; + if (!isNonNegativeIntegerExpr(Offset, VD, Ctx)) + return std::nullopt; - SmallString<256> OffsetStr; - ConstVal->toString(OffsetStr); - SubSpanOffset = OffsetStr.c_str(); - SubSpanOffset = OffsetStr.c_str(); - } else { - SubSpanOffset = Offset->getDecl()->getName().str(); - } + std::string SubSpanOffset; + const SourceManager &SM = Ctx.getSourceManager(); + const LangOptions &LangOpts = Ctx.getLangOpts(); + std::optional<StringRef> ExtentString = getExprText(Offset, SM, LangOpts);; + + if (ExtentString) + SubSpanOffset = ExtentString->str(); + else + SubSpanOffset = getUserFillPlaceHolder(); // FIXME: When does this happen? // To transform UUC(p += n) to UUC(p = p.subspan(..)): SS << varName.data() << " = " << varName.data() << ".subspan(" diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp index 30c587b2110d19b..5aad4b9558e5fa4 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp @@ -3,7 +3,7 @@ // RUN: -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s void foo(int * , int *); -void add_assign_test(int n, int *a) { +void add_assign_test(unsigned int n, int *a, int y) { int *p = new int[10]; // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> p" // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{" @@ -37,4 +37,19 @@ void add_assign_test(int n, int *a) { a += -9; // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:11}:"p = p.subspan(-9)" + + a += y; + // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:11}:"p = p.subspan(y)" +} + +int expr_test(unsigned x, int *q, int y) { + char *p = new char[8]; + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::span<char> p" + // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:13}:"{" + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:24-[[@LINE-3]]:24}:", 8}" + p += (x + 1); + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:15}:"p = p.subspan(x + 1)" + + q += (y + 7); + // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:15}:"q = q.subspan(y + 1)" } >From 432a8d32f72b870f6ca1936adc12f57b0170b34c Mon Sep 17 00:00:00 2001 From: Rashmi Mudduluru <r_muddul...@apple.com> Date: Tue, 14 Nov 2023 16:07:17 -0800 Subject: [PATCH 08/14] [-Wunsafe-buffer-usage] Fixable gadget for AddAssign: Fix formatting --- clang/lib/Analysis/UnsafeBufferUsage.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index 311e47e730ee280..a041ec6cd35fbdb 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -1054,8 +1054,9 @@ class UUCAddAssignGadget : public FixableGadget { static Matcher matcher() { return stmt(isInUnspecifiedUntypedContext(expr(ignoringImpCasts( binaryOperator( - hasOperatorName("+="), hasLHS(declRefExpr(toSupportedVariable())), - hasRHS(ignoringParens(expr().bind(OffsetTag)))) + hasOperatorName("+="), + hasLHS(declRefExpr(toSupportedVariable())), + hasRHS(ignoringParens(expr().bind(OffsetTag)))) .bind(UUCAddAssignTag))))); } @@ -1835,12 +1836,13 @@ UUCAddAssignGadget::getFixits(const Strategy &S) const { std::string SubSpanOffset; const SourceManager &SM = Ctx.getSourceManager(); const LangOptions &LangOpts = Ctx.getLangOpts(); - std::optional<StringRef> ExtentString = getExprText(Offset, SM, LangOpts);; + std::optional<StringRef> ExtentString = getExprText(Offset, SM, LangOpts); if (ExtentString) SubSpanOffset = ExtentString->str(); else - SubSpanOffset = getUserFillPlaceHolder(); // FIXME: When does this happen? + SubSpanOffset = + getUserFillPlaceHolder(); // FIXME: When does this happen? // To transform UUC(p += n) to UUC(p = p.subspan(..)): SS << varName.data() << " = " << varName.data() << ".subspan(" >From 6217e3d108affdf3594283439cacd331b651a86c Mon Sep 17 00:00:00 2001 From: Rashmi Mudduluru <r_muddul...@apple.com> Date: Wed, 15 Nov 2023 09:26:07 -0800 Subject: [PATCH 09/14] [-Wunsafe-buffer-usage] Fixable gadget for AddAssign: Fix formatting --- clang/lib/Analysis/UnsafeBufferUsage.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index a041ec6cd35fbdb..16f6a999cf75ab6 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -1043,7 +1043,7 @@ class UUCAddAssignGadget : public FixableGadget { UUCAddAssignGadget(const MatchFinder::MatchResult &Result) : FixableGadget(Kind::UUCAddAssign), Node(Result.Nodes.getNodeAs<BinaryOperator>(UUCAddAssignTag)), - Offset(Result.Nodes.getNodeAs<Expr>(OffsetTag)){ + Offset(Result.Nodes.getNodeAs<Expr>(OffsetTag)) { assert(Node != nullptr && "Expecting a non-null matching result"); } @@ -1053,8 +1053,7 @@ class UUCAddAssignGadget : public FixableGadget { static Matcher matcher() { return stmt(isInUnspecifiedUntypedContext(expr(ignoringImpCasts( - binaryOperator( - hasOperatorName("+="), + binaryOperator(hasOperatorName("+="), hasLHS(declRefExpr(toSupportedVariable())), hasRHS(ignoringParens(expr().bind(OffsetTag)))) .bind(UUCAddAssignTag))))); @@ -1841,7 +1840,7 @@ UUCAddAssignGadget::getFixits(const Strategy &S) const { if (ExtentString) SubSpanOffset = ExtentString->str(); else - SubSpanOffset = + SubSpanOffset = getUserFillPlaceHolder(); // FIXME: When does this happen? // To transform UUC(p += n) to UUC(p = p.subspan(..)): >From 7236e8e3324cb733732bb4cd36a9bd85bf15ccaa Mon Sep 17 00:00:00 2001 From: Rashmi Mudduluru <r_muddul...@apple.com> Date: Wed, 15 Nov 2023 16:18:45 -0800 Subject: [PATCH 10/14] [-Wunsafe-buffer-usage][WIP] Fixable gadget for AddAssign: Address partial comments --- clang/lib/Analysis/UnsafeBufferUsage.cpp | 16 +++++++++------ ...-unsafe-buffer-usage-fixits-add-assign.cpp | 20 +++++++++++-------- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index 16f6a999cf75ab6..a2224d7f05102f2 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -1055,7 +1055,7 @@ class UUCAddAssignGadget : public FixableGadget { return stmt(isInUnspecifiedUntypedContext(expr(ignoringImpCasts( binaryOperator(hasOperatorName("+="), hasLHS(declRefExpr(toSupportedVariable())), - hasRHS(ignoringParens(expr().bind(OffsetTag)))) + hasRHS(expr().bind(OffsetTag))) .bind(UUCAddAssignTag))))); } @@ -1453,10 +1453,8 @@ static std::optional<SourceLocation> getPastLoc(const NodeTy *Node, const LangOptions &LangOpts) { SourceLocation Loc = Lexer::getLocForEndOfToken(Node->getEndLoc(), 0, SM, LangOpts); - if (Loc.isValid()) return Loc; - return std::nullopt; } @@ -1844,17 +1842,23 @@ UUCAddAssignGadget::getFixits(const Strategy &S) const { getUserFillPlaceHolder(); // FIXME: When does this happen? // To transform UUC(p += n) to UUC(p = p.subspan(..)): - SS << varName.data() << " = " << varName.data() << ".subspan(" - << SubSpanOffset << ")"; + SS << varName.data() << " = " << varName.data() << ".subspan"; + bool NotParenExpr = (Offset->IgnoreParens()->getBeginLoc() == Offset->getBeginLoc()); + if (NotParenExpr) + SS << "("; + std::optional<SourceLocation> AddAssignLocation = getEndCharLoc( AddAssignNode, Ctx.getSourceManager(), Ctx.getLangOpts()); if (!AddAssignLocation) return std::nullopt; Fixes.push_back(FixItHint::CreateReplacement( - SourceRange(AddAssignNode->getBeginLoc(), *AddAssignLocation), + SourceRange(AddAssignNode->getBeginLoc(), + Node->getOperatorLoc()), SS.str())); + if (NotParenExpr) + Fixes.push_back(FixItHint::CreateInsertion(Offset->getEndLoc().getLocWithOffset(1), ")")); return Fixes; } } diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp index 5aad4b9558e5fa4..5c03cc10025c684 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-add-assign.cpp @@ -9,7 +9,8 @@ void add_assign_test(unsigned int n, int *a, int y) { // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{" // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}" p += 2; - // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:9}:"p = p.subspan(2)" + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:7}:"p = p.subspan(" + // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:9-[[@LINE-2]]:9}:")" int *r = p; // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> r" @@ -19,27 +20,30 @@ void add_assign_test(unsigned int n, int *a, int y) { // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:11}:"" // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"[0]" r += 2; - // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:11}:"r = r.subspan(2)" + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:9}:"r = r.subspan(" + // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:11-[[@LINE-2]]:11}:")" } if (*p == 0) { // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:8}:"" // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:9-[[@LINE-2]]:9}:"[0]" p += n; - // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:11}:"p = p.subspan(n)" + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:9}:"p = p.subspan(" + // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:11-[[@LINE-2]]:11}:")" } if (*p == 1) // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:8}:"" // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:9-[[@LINE-2]]:9}:"[0]" p += 3; - // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:11}:"p = p.subspan(3)" + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:9}:"p = p.subspan(" + // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:11-[[@LINE-2]]:11}:")" a += -9; - // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:11}:"p = p.subspan(-9)" + // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:9}:"p = p.subspan(" a += y; - // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:11}:"p = p.subspan(y)" + // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:9}:"p = p.subspan(" } int expr_test(unsigned x, int *q, int y) { @@ -48,8 +52,8 @@ int expr_test(unsigned x, int *q, int y) { // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:13}:"{" // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:24-[[@LINE-3]]:24}:", 8}" p += (x + 1); - // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:15}:"p = p.subspan(x + 1)" + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:7}:"p = p.subspan" q += (y + 7); - // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:15}:"q = q.subspan(y + 1)" + // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:7}:"q = q.subspan" } >From 0cbf6f34238cb839de9d64e35f9b2ca4db26f74f Mon Sep 17 00:00:00 2001 From: Rashmi Mudduluru <r_muddul...@apple.com> Date: Wed, 15 Nov 2023 16:57:51 -0800 Subject: [PATCH 11/14] [-Wunsafe-buffer-usage][WIP] Fixable gadget for AddAssign: Clean-up --- clang/lib/Analysis/UnsafeBufferUsage.cpp | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index a2224d7f05102f2..c51e3b9c0414495 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -1830,21 +1830,11 @@ UUCAddAssignGadget::getFixits(const Strategy &S) const { if (!isNonNegativeIntegerExpr(Offset, VD, Ctx)) return std::nullopt; - std::string SubSpanOffset; - const SourceManager &SM = Ctx.getSourceManager(); - const LangOptions &LangOpts = Ctx.getLangOpts(); - std::optional<StringRef> ExtentString = getExprText(Offset, SM, LangOpts); - - if (ExtentString) - SubSpanOffset = ExtentString->str(); - else - SubSpanOffset = - getUserFillPlaceHolder(); // FIXME: When does this happen? - // To transform UUC(p += n) to UUC(p = p.subspan(..)): SS << varName.data() << " = " << varName.data() << ".subspan"; - bool NotParenExpr = (Offset->IgnoreParens()->getBeginLoc() == Offset->getBeginLoc()); + bool NotParenExpr = + (Offset->IgnoreParens()->getBeginLoc() == Offset->getBeginLoc()); if (NotParenExpr) SS << "("; @@ -1854,11 +1844,11 @@ UUCAddAssignGadget::getFixits(const Strategy &S) const { return std::nullopt; Fixes.push_back(FixItHint::CreateReplacement( - SourceRange(AddAssignNode->getBeginLoc(), - Node->getOperatorLoc()), + SourceRange(AddAssignNode->getBeginLoc(), Node->getOperatorLoc()), SS.str())); if (NotParenExpr) - Fixes.push_back(FixItHint::CreateInsertion(Offset->getEndLoc().getLocWithOffset(1), ")")); + Fixes.push_back(FixItHint::CreateInsertion( + Offset->getEndLoc().getLocWithOffset(1), ")")); return Fixes; } } >From b21e329385eb676957e426f113aa75e140078e9e Mon Sep 17 00:00:00 2001 From: Rashmi Mudduluru <r_muddul...@apple.com> Date: Thu, 16 Nov 2023 09:57:45 -0800 Subject: [PATCH 12/14] [-Wunsafe-buffer-usage][WIP] Fixable gadget for AddAssign: Clean-up --- clang/lib/Analysis/UnsafeBufferUsage.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index c51e3b9c0414495..d945a0b9cebdaf8 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -1837,7 +1837,7 @@ UUCAddAssignGadget::getFixits(const Strategy &S) const { (Offset->IgnoreParens()->getBeginLoc() == Offset->getBeginLoc()); if (NotParenExpr) SS << "("; - + std::optional<SourceLocation> AddAssignLocation = getEndCharLoc( AddAssignNode, Ctx.getSourceManager(), Ctx.getLangOpts()); if (!AddAssignLocation) >From efc57907868ca44700bd9fde3ea81a5632e2b0a1 Mon Sep 17 00:00:00 2001 From: Rashmi Mudduluru <r_muddul...@apple.com> Date: Thu, 16 Nov 2023 13:51:55 -0800 Subject: [PATCH 13/14] [-Wunsafe-buffer-usage][WIP] Fixable gadget for AddAssign: Clean-up --- clang/lib/Analysis/UnsafeBufferUsage.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index d945a0b9cebdaf8..6349e21875f9935 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -1369,7 +1369,7 @@ ULCArraySubscriptGadget::getFixits(const Strategy &S) const { if (const auto *VD = dyn_cast<VarDecl>(DRE->getDecl())) { switch (S.lookup(VD)) { case Strategy::Kind::Span: { - + // If the index has a negative constant value, we give up as no valid // fix-it can be generated: const ASTContext &Ctx = // FIXME: we need ASTContext to be passed in! >From db8cbde5bb828f896b6551779d537d37b3ba529e Mon Sep 17 00:00:00 2001 From: Rashmi Mudduluru <r_muddul...@apple.com> Date: Fri, 17 Nov 2023 10:12:27 -0800 Subject: [PATCH 14/14] [-Wunsafe-buffer-usage][WIP] Fixable gadget for AddAssign: Address comments --- clang/lib/Analysis/UnsafeBufferUsage.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index 6349e21875f9935..a1efb76be68b7d7 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -1822,7 +1822,7 @@ UUCAddAssignGadget::getFixits(const Strategy &S) const { if (const VarDecl *VD = dyn_cast<VarDecl>(DREs.front()->getDecl())) { if (S.lookup(VD) == Strategy::Kind::Span) { FixItList Fixes; - std::stringstream SS; + const Stmt *AddAssignNode = getBaseStmt(); StringRef varName = VD->getName(); const ASTContext &Ctx = VD->getASTContext(); @@ -1831,12 +1831,11 @@ UUCAddAssignGadget::getFixits(const Strategy &S) const { return std::nullopt; // To transform UUC(p += n) to UUC(p = p.subspan(..)): - SS << varName.data() << " = " << varName.data() << ".subspan"; - bool NotParenExpr = (Offset->IgnoreParens()->getBeginLoc() == Offset->getBeginLoc()); + std::string SS = varName.str() + " = " + varName.str() + ".subspan"; if (NotParenExpr) - SS << "("; + SS += "("; std::optional<SourceLocation> AddAssignLocation = getEndCharLoc( AddAssignNode, Ctx.getSourceManager(), Ctx.getLangOpts()); @@ -1845,7 +1844,7 @@ UUCAddAssignGadget::getFixits(const Strategy &S) const { Fixes.push_back(FixItHint::CreateReplacement( SourceRange(AddAssignNode->getBeginLoc(), Node->getOperatorLoc()), - SS.str())); + SS)); if (NotParenExpr) Fixes.push_back(FixItHint::CreateInsertion( Offset->getEndLoc().getLocWithOffset(1), ")")); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits