[clang] e7596a9 - [-Wunsafe-buffer-usage] Add Fixable for simple pointer dereference
via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 22 15:38:46 PDT 2023
Author: MalavikaSamak
Date: 2023-03-22T15:32:51-07:00
New Revision: e7596a99fca6d1df14275f5293e447a4d87af06a
URL: https://github.com/llvm/llvm-project/commit/e7596a99fca6d1df14275f5293e447a4d87af06a
DIFF: https://github.com/llvm/llvm-project/commit/e7596a99fca6d1df14275f5293e447a4d87af06a.diff
LOG: [-Wunsafe-buffer-usage] Add Fixable for simple pointer dereference
This patch introduces PointerDereferenceGadget, a FixableGadget that emits
fixits to handle cases where a pointer that is identified as unsafe is
dereferenced. The current implementation only handles cases where the strategy
is to change the type of the raw pointer to std::span. The fixit for this
strategy is to fetch the first element from the corresponding span instance.
For example for the code below, the PointerDereferenceGadget emits a fixit for
S3 (S1, S2 are to be handled by other gadgets):
S1: int *ptr = new int[10];
S2: int val1 = ptr[k]; // Unsafe operation
S3: int val2 = *ptr; => Fixit: int val2 = ptr[0];
Differential revision: https://reviews.llvm.org/D143206
Added:
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-deref.cpp
Modified:
clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
clang/lib/Analysis/UnsafeBufferUsage.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
index 89f7c1ed2ba24..a8485682c1d1f 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
@@ -32,6 +32,7 @@ WARNING_GADGET(PointerArithmetic)
WARNING_GADGET(UnsafeBufferUsageAttr)
FIXABLE_GADGET(ULCArraySubscript)
FIXABLE_GADGET(DerefSimplePtrArithFixable)
+FIXABLE_GADGET(PointerDereference)
#undef FIXABLE_GADGET
#undef WARNING_GADGET
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 04e11d0471a7d..95e4c8388bc44 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -463,6 +463,45 @@ class ULCArraySubscriptGadget : public FixableGadget {
return {};
}
};
+
+class PointerDereferenceGadget : public FixableGadget {
+ static constexpr const char *const BaseDeclRefExprTag = "BaseDRE";
+ static constexpr const char *const OperatorTag = "op";
+
+ const DeclRefExpr *BaseDeclRefExpr = nullptr;
+ const UnaryOperator *Op = nullptr;
+
+public:
+ PointerDereferenceGadget(const MatchFinder::MatchResult &Result)
+ : FixableGadget(Kind::PointerDereference),
+ BaseDeclRefExpr(
+ Result.Nodes.getNodeAs<DeclRefExpr>(BaseDeclRefExprTag)),
+ Op(Result.Nodes.getNodeAs<UnaryOperator>(OperatorTag)) {}
+
+ static bool classof(const Gadget *G) {
+ return G->getKind() == Kind::PointerDereference;
+ }
+
+ static Matcher matcher() {
+ auto Target =
+ unaryOperator(
+ hasOperatorName("*"),
+ has(expr(ignoringParenImpCasts(
+ declRefExpr(to(varDecl())).bind(BaseDeclRefExprTag)))))
+ .bind(OperatorTag);
+
+ return expr(isInUnspecifiedLvalueContext(Target));
+ }
+
+ DeclUseList getClaimedVarUseSites() const override {
+ return {BaseDeclRefExpr};
+ }
+
+ virtual const Stmt *getBaseStmt() const final { return Op; }
+
+ virtual std::optional<FixItList> getFixits(const Strategy &S) const override;
+};
+
} // namespace
namespace {
@@ -914,6 +953,37 @@ DerefSimplePtrArithFixableGadget::getFixits(const Strategy &s) const {
return std::nullopt; // something wrong or unsupported, give up
}
+std::optional<FixItList>
+PointerDereferenceGadget::getFixits(const Strategy &S) const {
+ const VarDecl *VD = cast<VarDecl>(BaseDeclRefExpr->getDecl());
+ switch (S.lookup(VD)) {
+ case Strategy::Kind::Span: {
+ ASTContext &Ctx = VD->getASTContext();
+ SourceManager &SM = Ctx.getSourceManager();
+ // Required changes: *(ptr); => (ptr[0]); and *ptr; => ptr[0]
+ // Deletes the *operand
+ CharSourceRange derefRange = clang::CharSourceRange::getCharRange(
+ Op->getBeginLoc(), Op->getBeginLoc().getLocWithOffset(1));
+ // Inserts the [0]
+ std::optional<SourceLocation> endOfOperand =
+ getEndCharLoc(BaseDeclRefExpr, SM, Ctx.getLangOpts());
+ if (endOfOperand) {
+ return FixItList{{FixItHint::CreateRemoval(derefRange),
+ FixItHint::CreateInsertion(
+ endOfOperand.value().getLocWithOffset(1), "[0]")}};
+ }
+ }
+ case Strategy::Kind::Iterator:
+ case Strategy::Kind::Array:
+ case Strategy::Kind::Vector:
+ llvm_unreachable("Strategy not implemented yet!");
+ case Strategy::Kind::Wontfix:
+ llvm_unreachable("Invalid strategy!");
+ }
+
+ return std::nullopt;
+}
+
// 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.
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-deref.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-deref.cpp
new file mode 100644
index 0000000000000..4a02bbdf71182
--- /dev/null
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-deref.cpp
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+void basic_dereference() {
+ int tmp;
+ auto p = new int[10];
+ // 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]]:23-[[@LINE-3]]:23}:", 10}"
+ tmp = p[5];
+ int val = *p;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:13-[[@LINE-1]]:14}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:15-[[@LINE-2]]:15}:"[0]"
+}
+
+int return_method() {
+ auto p = new int[10];
+ // 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]]:23-[[@LINE-3]]:23}:", 10}"
+ int tmp = p[5];
+ return *p;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:11}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"[0]"
+}
+
+void foo(int v) {
+}
+
+void method_invocation() {
+ auto p = new int[10];
+ // 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]]:23-[[@LINE-3]]:23}:", 10}"
+
+ int tmp = p[5];
+
+ foo(*p);
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:8}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:9-[[@LINE-2]]:9}:"[0]"
+}
+
+void binary_operation() {
+ auto p = new int[10];
+ // 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]]:23-[[@LINE-3]]:23}:", 10}"
+
+ int tmp = p[5];
+
+ int k = *p + 20;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:12}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:13}:"[0]"
+
+}
+
More information about the cfe-commits
mailing list