[clang] 6a0f2e5 - [-Wunsafe-buffer-usage] Add Fixable for dereference of simple ptr arithmetic
via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 20 17:07:18 PDT 2023
Author: ziqingluo-90
Date: 2023-03-20T17:07:03-07:00
New Revision: 6a0f2e539b8ef1f510f62aceb36430b95e40f0d3
URL: https://github.com/llvm/llvm-project/commit/6a0f2e539b8ef1f510f62aceb36430b95e40f0d3
DIFF: https://github.com/llvm/llvm-project/commit/6a0f2e539b8ef1f510f62aceb36430b95e40f0d3.diff
LOG: [-Wunsafe-buffer-usage] Add Fixable for dereference of simple ptr arithmetic
For each expression `e` of the form `*(DRE + n)` (or `*(n + DRE)`), where
`DRE` has a pointer type and `n` is an integer literal, `e` will be
transformed to `DRE[n]` (or `n[DRE]` respectively), if
- `e` is at the left-hand side of an assignment or is an lvalue being casted to an rvalue; and
- the variable referred by `DRE` is going to be transformed to be of `std::span` type.
Reviewed by: jkorous, NoQ
Differential revision: https://reviews.llvm.org/D142795
Added:
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-deref-simple-ptr-arith.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 75657d8d9a584..89f7c1ed2ba24 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
@@ -31,6 +31,7 @@ WARNING_GADGET(ArraySubscript)
WARNING_GADGET(PointerArithmetic)
WARNING_GADGET(UnsafeBufferUsageAttr)
FIXABLE_GADGET(ULCArraySubscript)
+FIXABLE_GADGET(DerefSimplePtrArithFixable)
#undef FIXABLE_GADGET
#undef WARNING_GADGET
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 116bb4954b168..04e11d0471a7d 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "clang/Analysis/Analyses/UnsafeBufferUsage.h"
+#include "clang/AST/Decl.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Lex/Lexer.h"
@@ -558,6 +559,56 @@ class Strategy {
};
} // namespace
+// Representing a fixable expression of the form `*(ptr + 123)` or `*(123 +
+// ptr)`:
+class DerefSimplePtrArithFixableGadget : public FixableGadget {
+ static constexpr const char *const BaseDeclRefExprTag = "BaseDRE";
+ static constexpr const char *const DerefOpTag = "DerefOp";
+ static constexpr const char *const AddOpTag = "AddOp";
+ static constexpr const char *const OffsetTag = "Offset";
+
+ const DeclRefExpr *BaseDeclRefExpr = nullptr;
+ const UnaryOperator *DerefOp = nullptr;
+ const BinaryOperator *AddOp = nullptr;
+ const IntegerLiteral *Offset = nullptr;
+
+public:
+ DerefSimplePtrArithFixableGadget(const MatchFinder::MatchResult &Result)
+ : FixableGadget(Kind::DerefSimplePtrArithFixable),
+ BaseDeclRefExpr(
+ Result.Nodes.getNodeAs<DeclRefExpr>(BaseDeclRefExprTag)),
+ DerefOp(Result.Nodes.getNodeAs<UnaryOperator>(DerefOpTag)),
+ AddOp(Result.Nodes.getNodeAs<BinaryOperator>(AddOpTag)),
+ Offset(Result.Nodes.getNodeAs<IntegerLiteral>(OffsetTag)) {}
+
+ static Matcher matcher() {
+ // clang-format off
+ auto ThePtr = expr(hasPointerType(),
+ ignoringImpCasts(declRefExpr(to(varDecl())).bind(BaseDeclRefExprTag)));
+ auto PlusOverPtrAndInteger = expr(anyOf(
+ binaryOperator(hasOperatorName("+"), hasLHS(ThePtr),
+ hasRHS(integerLiteral().bind(OffsetTag)))
+ .bind(AddOpTag),
+ binaryOperator(hasOperatorName("+"), hasRHS(ThePtr),
+ hasLHS(integerLiteral().bind(OffsetTag)))
+ .bind(AddOpTag)));
+ return isInUnspecifiedLvalueContext(unaryOperator(
+ hasOperatorName("*"),
+ hasUnaryOperand(ignoringParens(PlusOverPtrAndInteger)))
+ .bind(DerefOpTag));
+ // clang-format on
+ }
+
+ virtual std::optional<FixItList> getFixits(const Strategy &s) const final;
+
+ // TODO remove this method from FixableGadget interface
+ virtual const Stmt *getBaseStmt() const final { return nullptr; }
+
+ virtual DeclUseList getClaimedVarUseSites() const final {
+ return {BaseDeclRefExpr};
+ }
+};
+
/// Scan the function and return a list of gadgets found with provided kits.
static std::tuple<FixableGadgetList, WarningGadgetList, DeclUseTracker>
findGadgets(const Decl *D, const UnsafeBufferUsageHandler &Handler) {
@@ -812,6 +863,57 @@ static StringRef getExprText(const Expr *E, const SourceManager &SM,
LangOpts);
}
+std::optional<FixItList>
+DerefSimplePtrArithFixableGadget::getFixits(const Strategy &s) const {
+ const VarDecl *VD = dyn_cast<VarDecl>(BaseDeclRefExpr->getDecl());
+
+ if (VD && s.lookup(VD) == Strategy::Kind::Span) {
+ ASTContext &Ctx = VD->getASTContext();
+ // std::span can't represent elements before its begin()
+ if (auto ConstVal = Offset->getIntegerConstantExpr(Ctx))
+ if (ConstVal->isNegative())
+ return std::nullopt;
+
+ // note that the expr may (oddly) has multiple layers of parens
+ // example:
+ // *((..(pointer + 123)..))
+ // goal:
+ // pointer[123]
+ // Fix-It:
+ // remove '*('
+ // replace ' + ' with '['
+ // replace ')' with ']'
+
+ // example:
+ // *((..(123 + pointer)..))
+ // goal:
+ // 123[pointer]
+ // Fix-It:
+ // remove '*('
+ // replace ' + ' with '['
+ // replace ')' with ']'
+
+ const Expr *LHS = AddOp->getLHS(), *RHS = AddOp->getRHS();
+ const SourceManager &SM = Ctx.getSourceManager();
+ const LangOptions &LangOpts = Ctx.getLangOpts();
+ CharSourceRange StarWithTrailWhitespace =
+ clang::CharSourceRange::getCharRange(DerefOp->getOperatorLoc(),
+ LHS->getBeginLoc());
+ CharSourceRange PlusWithSurroundingWhitespace =
+ clang::CharSourceRange::getCharRange(getPastLoc(LHS, SM, LangOpts),
+ RHS->getBeginLoc());
+ CharSourceRange ClosingParenWithPrecWhitespace =
+ clang::CharSourceRange::getCharRange(getPastLoc(AddOp, SM, LangOpts),
+ getPastLoc(DerefOp, SM, LangOpts));
+
+ return FixItList{
+ {FixItHint::CreateRemoval(StarWithTrailWhitespace),
+ FixItHint::CreateReplacement(PlusWithSurroundingWhitespace, "["),
+ FixItHint::CreateReplacement(ClosingParenWithPrecWhitespace, "]")}};
+ }
+ return std::nullopt; // something wrong or unsupported, 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.
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-deref-simple-ptr-arith.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-deref-simple-ptr-arith.cpp
new file mode 100644
index 0000000000000..c533a9bd3d38d
--- /dev/null
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-deref-simple-ptr-arith.cpp
@@ -0,0 +1,199 @@
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits -fsyntax-only %s 2>&1 | FileCheck %s
+
+// TODO test we don't mess up vertical whitespace
+// TODO test
diff erent whitespaces
+// TODO test
diff erent contexts
+ // when it's on the right side
+
+void basic() {
+ int *ptr;
+// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> ptr"
+ *(ptr+5)=1;
+// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:5}:""
+// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:8-[[@LINE-2]]:9}:"["
+// CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:11}:"]"
+}
+
+// The weird preceding semicolon ensures that we preserve that range intact.
+void char_ranges() {
+ int *p;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:9}:"std::span<int> p"
+
+ ;* ( p + 5 ) = 1;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:4-[[@LINE-1]]:8}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:9-[[@LINE-2]]:12}:"["
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:13-[[@LINE-3]]:15}:"]"
+
+ ;* (p+5)= 1;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:4-[[@LINE-1]]:9}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:10-[[@LINE-2]]:11}:"["
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:13}:"]"
+
+ ;*( p+5)= 1;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:4-[[@LINE-1]]:9}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:10-[[@LINE-2]]:11}:"["
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:13}:"]"
+
+ ;*( p+5)= 1;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:4-[[@LINE-1]]:9}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:10-[[@LINE-2]]:11}:"["
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:13}:"]"
+
+ ;*( p +5)= 1;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:4-[[@LINE-1]]:7}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:8-[[@LINE-2]]:12}:"["
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:13-[[@LINE-3]]:14}:"]"
+
+ ;*(p+ 5)= 1;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:4-[[@LINE-1]]:6}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:7-[[@LINE-2]]:11}:"["
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:13}:"]"
+
+ ;*(p+ 5 )= 1;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:4-[[@LINE-1]]:6}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:7-[[@LINE-2]]:9}:"["
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:14}:"]"
+
+ ;*(p+ 5) = 1;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:4-[[@LINE-1]]:6}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:7-[[@LINE-2]]:9}:"["
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:11}:"]"
+
+ ; *(p+5)= 1;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:9}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:10-[[@LINE-2]]:11}:"["
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:13}:"]"
+
+ ;*(p+123456)= 1;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:4-[[@LINE-1]]:6}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:7-[[@LINE-2]]:8}:"["
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:14-[[@LINE-3]]:15}:"]"
+
+ ;* (p+123456)= 1;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:4-[[@LINE-1]]:9}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:10-[[@LINE-2]]:11}:"["
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:17-[[@LINE-3]]:18}:"]"
+
+ ;*( p+123456)= 1;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:4-[[@LINE-1]]:9}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:10-[[@LINE-2]]:11}:"["
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:17-[[@LINE-3]]:18}:"]"
+
+ ;*( p+123456)= 1;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:4-[[@LINE-1]]:9}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:10-[[@LINE-2]]:11}:"["
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:17-[[@LINE-3]]:18}:"]"
+
+ ;*(p +123456)= 1;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:4-[[@LINE-1]]:6}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:7-[[@LINE-2]]:11}:"["
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:17-[[@LINE-3]]:18}:"]"
+
+ ;*(p+ 123456)= 1;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:4-[[@LINE-1]]:6}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:7-[[@LINE-2]]:11}:"["
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:17-[[@LINE-3]]:18}:"]"
+
+ ;*(p+123456 )= 1;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:4-[[@LINE-1]]:6}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:7-[[@LINE-2]]:8}:"["
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:14-[[@LINE-3]]:18}:"]"
+
+ ;*(p+123456) = 1;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:4-[[@LINE-1]]:6}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:7-[[@LINE-2]]:8}:"["
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:14-[[@LINE-3]]:15}:"]"
+
+ int *ptrrrrrr;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:16}:"std::span<int> ptrrrrrr"
+
+ ;* ( ptrrrrrr + 123456 )= 1;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:4-[[@LINE-1]]:8}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:19}:"["
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:25-[[@LINE-3]]:27}:"]"
+
+ ;* (ptrrrrrr+123456)= 1;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:4-[[@LINE-1]]:9}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:17-[[@LINE-2]]:18}:"["
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:24-[[@LINE-3]]:25}:"]"
+
+ ;*( ptrrrrrr+123456)= 1;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:4-[[@LINE-1]]:9}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:17-[[@LINE-2]]:18}:"["
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:24-[[@LINE-3]]:25}:"]"
+
+ ;*( ptrrrrrr+123456)= 1;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:4-[[@LINE-1]]:9}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:17-[[@LINE-2]]:18}:"["
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:24-[[@LINE-3]]:25}:"]"
+
+ ;*(ptrrrrrr +123456)= 1;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:4-[[@LINE-1]]:6}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:14-[[@LINE-2]]:18}:"["
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:24-[[@LINE-3]]:25}:"]"
+
+ ;*(ptrrrrrr+ 123456)= 1;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:4-[[@LINE-1]]:6}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:14-[[@LINE-2]]:18}:"["
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:24-[[@LINE-3]]:25}:"]"
+
+ ;*(ptrrrrrr+123456 )= 1;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:4-[[@LINE-1]]:6}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:14-[[@LINE-2]]:15}:"["
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:21-[[@LINE-3]]:25}:"]"
+
+ ;*(ptrrrrrr+123456) = 1;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:4-[[@LINE-1]]:6}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:14-[[@LINE-2]]:15}:"["
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:21-[[@LINE-3]]:22}:"]"
+}
+
+void base_on_rhs() {
+ int* ptr;
+ *(10 + ptr) = 1;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:5}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:7-[[@LINE-2]]:10}:"["
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:13-[[@LINE-3]]:14}:"]"
+}
+
+void many_parens() {
+ int* ptr;
+ *(( (10 + ptr)) ) = 1;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:8}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:10-[[@LINE-2]]:13}:"["
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:16-[[@LINE-3]]:20}:"]"
+}
+
+void lvaue_to_rvalue() {
+ int * ptr;
+ int tmp = *(ptr + 10);
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:13-[[@LINE-1]]:15}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:18-[[@LINE-2]]:21}:"["
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:24}:"]"
+}
+
+// Fixits emitted for the cases below would be incorrect.
+// CHECK-NOT: fix-it:
+// Array subsctipt opertor of std::span accepts unsigned integer.
+void negative() {
+ int* ptr;
+ *(ptr + -5) = 1; // skip
+}
+
+void subtraction() {
+ int* ptr;
+ *(ptr - 5) = 1; // skip
+}
+
+void subtraction_of_negative() {
+ int* ptr;
+ *(ptr - -5) = 1; // FIXME: implement fixit (uncommon case - low priority)
+}
+
+
+void bindingDecl(int *p, int *q) {
+ int * a[2] = {p, q};
+ auto [x, y] = a;
+
+ *(x + 1) = 1; // FIXME: deal with `BindingDecl`s
+}
More information about the cfe-commits
mailing list