ziqingluo-90 updated this revision to Diff 503928.
ziqingluo-90 added a comment.
Rebased and addressed comments.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144304/new/
https://reviews.llvm.org/D144304
Files:
clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
clang/lib/Analysis/UnsafeBufferUsage.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-addressof-arraysubscript.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pre-increment.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
Index: clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
===================================================================
--- clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
@@ -34,7 +34,7 @@
void * voidPtrCall(void);
char * charPtrCall(void);
-void testArraySubscripts(int *p, int **pp) {
+void testArraySubscripts(int *p, int **pp) { // expected-note{{change type of 'pp' to 'std::span' to preserve bounds information}}
// expected-warning@-1{{'p' is an unsafe pointer used for buffer access}}
// expected-warning@-2{{'pp' is an unsafe pointer used for buffer access}}
foo(p[1], // expected-note{{used in buffer access here}}
@@ -108,7 +108,7 @@
sizeof(decltype(p[1]))); // expected-note{{used in buffer access here}}
}
-void testQualifiedParameters(const int * p, const int * const q, const int a[10], const int b[10][10]) {
+void testQualifiedParameters(const int * p, const int * const q, const int a[10], const int b[10][10]) { // expected-note{{change type of 'a' to 'std::span' to preserve bounds information}}
// expected-warning@-1{{'p' is an unsafe pointer used for buffer access}}
// expected-warning@-2{{'q' is an unsafe pointer used for buffer access}}
// expected-warning@-3{{'a' is an unsafe pointer used for buffer access}}
@@ -323,7 +323,7 @@
template void fArr<int>(int t[]); // expected-note {{in instantiation of}}
-int testReturn(int t[]) {
+int testReturn(int t[]) { // expected-note{{change type of 't' to 'std::span' to preserve bounds information}}
// expected-warning@-1{{'t' is an unsafe pointer used for buffer access}}
return t[1]; // expected-note{{used in buffer access here}}
}
Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pre-increment.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pre-increment.cpp
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+void foo(int * , int *);
+
+void simple() {
+ int * p = new int[10];
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::span<int> p"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:13}:"{"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:24-[[@LINE-3]]:24}:", 10}"
+ bool b = ++p;
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:12-[[@LINE-1]]:15}:"(p = p.subspan(1)).data()"
+ unsigned long n = (unsigned long) ++p;
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:37-[[@LINE-1]]:40}:"(p = p.subspan(1)).data()"
+ if (++p) {
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:10}:"(p = p.subspan(1)).data()"
+ }
+ if (++p - ++p) {
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:10}:"(p = p.subspan(1)).data()"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:16}:"(p = p.subspan(1)).data()"
+ }
+ foo(++p, p);
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:10}:"(p = p.subspan(1)).data()"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:13}:".data()"
+
+ // FIXME: Don't know how to fix the following cases:
+ // CHECK-NOT: fix-it:"{{.*}}":{
+ int * g = new int[10];
+ int * a[10];
+ a[0] = ++g;
+ foo(g++, g);
+ if (++(a[0])) {
+ }
+}
Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-addressof-arraysubscript.cpp
===================================================================
--- clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-addressof-arraysubscript.cpp
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-addressof-arraysubscript.cpp
@@ -16,17 +16,17 @@
void address_to_bool(int x) {
int * p = new int[10];
bool a = (bool) &p[5];
- // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:19-[[@LINE-1]]:24}:"(p.data() + 5)"
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:19-[[@LINE-1]]:24}:"&p.data()[5]"
bool b = (bool) &p[x];
- // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:19-[[@LINE-1]]:24}:"(p.data() + x)"
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:19-[[@LINE-1]]:24}:"&p.data()[x]"
bool a1 = &p[5];
- // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:13-[[@LINE-1]]:18}:"(p.data() + 5)"
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:13-[[@LINE-1]]:18}:"&p.data()[5]"
bool b1 = &p[x];
- // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:13-[[@LINE-1]]:18}:"(p.data() + x)"
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:13-[[@LINE-1]]:18}:"&p.data()[x]"
if (&p[5]) {
- // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:12}:"(p.data() + 5)"
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:12}:"&p.data()[5]"
return;
}
}
@@ -73,8 +73,6 @@
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:5-[[@LINE-1]]:10}:"p.data()"
}
-// CHECK-NOT: fix-it
-
void pointer_subtraction(int x) {
int * p = new int[10];
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::span<int> p"
@@ -82,11 +80,11 @@
// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:24-[[@LINE-3]]:24}:", 10}"
int n = &p[9] - &p[4];
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:16}:"(p.data() + 9)"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:19-[[@LINE-2]]:24}:"(p.data() + 4)"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:16}:"&p.data()[9]"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:19-[[@LINE-2]]:24}:"&p.data()[4]"
if (&p[9] - &p[x]) {
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:12}:"(p.data() + 9)"
- // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:15-[[@LINE-2]]:20}:"(p.data() + x)"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:12}:"&p.data()[9]"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:15-[[@LINE-2]]:20}:"&p.data()[x]"
return;
}
}
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===================================================================
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -146,6 +146,11 @@
return innerMatcher.matches(*Node.getSubExpr(), Finder, Builder);
}
+// Matches a `UnaryOperator` whose operator is pre-increment:
+AST_MATCHER(UnaryOperator, isPreInc) {
+ return Node.getOpcode() == UnaryOperator::Opcode::UO_PreInc;
+}
+
// Returns a matcher that matches any expression 'e' such that `innerMatcher`
// matches 'e' and 'e' is in an Unspecified Lvalue Context.
static auto isInUnspecifiedLvalueContext(internal::Matcher<Expr> innerMatcher) {
@@ -629,6 +634,45 @@
return {};
}
};
+
+// Representing a pointer type expression of the form `++Ptr` in an Unspecified
+// Pointer Context (UPC):
+class UPCPreIncrementGadget : public FixableGadget {
+private:
+ static constexpr const char *const UPCPreIncrementTag =
+ "PointerPreIncrementUnderUPC";
+ const UnaryOperator *Node; // the `++Ptr` node
+
+public:
+ UPCPreIncrementGadget(const MatchFinder::MatchResult &Result)
+ : FixableGadget(Kind::UPCPreIncrement),
+ Node(Result.Nodes.getNodeAs<UnaryOperator>(UPCPreIncrementTag)) {
+ assert(Node != nullptr && "Expecting a non-null matching result");
+ }
+
+ static bool classof(const Gadget *G) {
+ return G->getKind() == Kind::UPCPreIncrement;
+ }
+
+ static Matcher matcher() {
+ // Note here we match `++Ptr` for any expression `Ptr` of pointer type.
+ // Although currently we can only provide fix-its when `Ptr` is a DRE, we
+ // can have the matcher be general, so long as `getClaimedVarUseSites` does
+ // things right.
+ return stmt(isInUnspecifiedPointerContext(expr(ignoringImpCasts(
+ unaryOperator(isPreInc(),
+ hasUnaryOperand(declRefExpr())
+ ).bind(UPCPreIncrementTag)))));
+ }
+
+ 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->getSubExpr())};
+ }
+};
} // namespace
namespace {
@@ -1206,6 +1250,34 @@
return std::nullopt;
}
+std::optional<FixItList> UPCPreIncrementGadget::getFixits(const Strategy &S) const {
+ DeclUseList DREs = getClaimedVarUseSites();
+
+ if (DREs.size() != 1)
+ return std::nullopt; // In cases of `++Ptr` 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 *PreIncNode = getBaseStmt();
+ StringRef varName = VD->getName();
+ const ASTContext &Ctx = VD->getASTContext();
+
+ // To transform UPC(++p) to UPC((p = p.subspan(1)).data()):
+ SS << "(" << varName.data() << " = " << varName.data()
+ << ".subspan(1)).data()";
+ Fixes.push_back(FixItHint::CreateReplacement(
+ SourceRange(PreIncNode->getBeginLoc(),
+ getEndCharLoc(PreIncNode, Ctx.getSourceManager(),
+ Ctx.getLangOpts())),
+ SS.str()));
+ return Fixes;
+ }
+ }
+ return std::nullopt; // Not in the cases that we can handle for now, give up.
+}
+
// For a non-null initializer `Init` of `T *` type, this function returns
// `FixItHint`s producing a list initializer `{Init, S}` as a part of a fix-it
// to output stream.
Index: clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
===================================================================
--- clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
+++ clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
@@ -34,6 +34,7 @@
FIXABLE_GADGET(ULCArraySubscript) // `DRE[any]` in an Unspecified Lvalue Context
FIXABLE_GADGET(PointerDereference)
FIXABLE_GADGET(UPCAddressofArraySubscript) // '&DRE[any]' in an Unspecified Pointer Context
+FIXABLE_GADGET(UPCPreIncrement) // '++Ptr' in an Unspecified Pointer Context
FIXABLE_GADGET(DerefSimplePtrArithFixable)
FIXABLE_GADGET(PointerCtxAccess)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
