Index: clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def =================================================================== --- clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def +++ clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def @@ -35,6 +35,7 @@ FIXABLE_GADGET(PointerDereference) FIXABLE_GADGET(UPCAddressofArraySubscript) // '&DRE[any]' in an Unspecified Pointer Context FIXABLE_GADGET(DerefSimplePtrArithFixable) +FIXABLE_GADGET(UPCStandalonePointer) #undef FIXABLE_GADGET #undef WARNING_GADGET Index: clang/lib/Analysis/UnsafeBufferUsage.cpp =================================================================== --- clang/lib/Analysis/UnsafeBufferUsage.cpp +++ clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -169,15 +169,20 @@ // 2. the operand of a cast operation; or // ... auto CallArgMatcher = - callExpr(hasAnyArgument(allOf( - hasPointerType() /* array also decays to pointer type*/, - InnerMatcher)), - unless(callee(functionDecl(hasAttr(attr::UnsafeBufferUsage))))); + callExpr(forEachArgumentWithParam(InnerMatcher, + hasPointerType() /* array also decays to pointer type*/), + unless(callee(functionDecl(hasAttr(attr::UnsafeBufferUsage))))); + auto CastOperandMatcher = explicitCastExpr(hasCastKind(CastKind::CK_PointerToIntegral), castSubExpr(allOf(hasPointerType(), InnerMatcher))); - return stmt(anyOf(CallArgMatcher, CastOperandMatcher)); + auto CompOperandMatcher = + binaryOperator(hasAnyOperatorName("!=", "==", "<", "<=", ">", ">="), + eachOf(hasLHS(allOf(hasPointerType(), InnerMatcher)), + hasRHS(allOf(hasPointerType(), InnerMatcher)))); + + return stmt(anyOf(CallArgMatcher, CastOperandMatcher, CompOperandMatcher)); // FIXME: any more cases? (UPC excludes the RHS of an assignment. For now we // don't have to check that.) } @@ -493,6 +498,40 @@ } }; +// Fixable gadget to handle the expressions DRE in the unspecified pointer +// context. +class UPCStandalonePointerGadget : public FixableGadget { +private: + static constexpr const char *const DeclRefExprTag = "PtrAccess"; + const DeclRefExpr *Node; + +public: + UPCStandalonePointerGadget(const MatchFinder::MatchResult &Result) + : FixableGadget(Kind::UPCStandalonePointer), + Node(Result.Nodes.getNodeAs(DeclRefExprTag)) { + assert(Node != nullptr && "Expecting a non-null matching result"); + } + + static bool classof(const Gadget *G) { + return G->getKind() == Kind::UPCStandalonePointer; + } + + static Matcher matcher() { + auto ArrayOrPtr = anyOf(hasPointerType(), hasArrayType()); + auto target = expr( + ignoringParenImpCasts(declRefExpr(allOf(ArrayOrPtr, to(varDecl()))).bind(DeclRefExprTag))); + return stmt(isInUnspecifiedPointerContext(target)); + } + + virtual std::optional getFixits(const Strategy &S) const override; + + virtual const Stmt *getBaseStmt() const override { return Node; } + + virtual DeclUseList getClaimedVarUseSites() const override { + return {Node}; + } +}; + class PointerDereferenceGadget: public FixableGadget { static constexpr const char *const BaseDeclRefExprTag = "BaseDRE"; static constexpr const char *const OperatorTag = "op"; @@ -1095,6 +1134,33 @@ return std::nullopt; // something went wrong. no fix-it } + +// Generates fix-its replacing an expression of the form UPC(DRE) with +// `DRE.data()` +std::optional UPCStandalonePointerGadget::getFixits(const Strategy &S) + const { + const VarDecl *VD = cast(Node->getDecl()); + switch (S.lookup(VD)) { + case Strategy::Kind::Span: { + ASTContext &Ctx = VD->getASTContext(); + SourceManager &SM = Ctx.getSourceManager(); + // Inserts the .data() after the DRE + std::optional endOfOperand = + getEndCharLoc(Node, SM, Ctx.getLangOpts()); + + return FixItList{{FixItHint::CreateInsertion( + endOfOperand.value().getLocWithOffset(1), ".data()")}}; + } + case Strategy::Kind::Wontfix: + case Strategy::Kind::Iterator: + case Strategy::Kind::Array: + case Strategy::Kind::Vector: + llvm_unreachable("unsupported strategies for FixableGadgets"); + } + + 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. Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-access.cpp =================================================================== --- /dev/null +++ clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-access.cpp @@ -0,0 +1,110 @@ +// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s + +void foo(int* v) { +} + +void m1(int* v1, int* v2, int) { + +} + +void condition_check_nullptr() { + auto p = new int[10]; + // 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]]:23-[[@LINE-3]]:23}:", 10}" + + int tmp = p[5]; + if(p != nullptr) {} + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:".data()" +} + +void condition_check() { + auto p = new int[10]; + // 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]]:23-[[@LINE-3]]:23}:", 10}" + + auto q = new int[10]; + + int tmp = p[5]; + if(p == q) {} + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:".data()" + + if(q != p){} + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:12-[[@LINE-1]]:12}:".data()" + + if(p < q) {} + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:".data()" + + if(p <= q) {} + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:".data()" + + if(p > q) {} + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:".data()" + + if(p >= q) {} + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:".data()" + + if( p == q && p != nullptr) {} + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:8-[[@LINE-1]]:8}:".data()" + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:18-[[@LINE-2]]:18}:".data()" +} + +void condition_check_two_usafe_buffers() { + auto p = new int[10]; + // 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]]:23-[[@LINE-3]]:23}:", 10}" + + auto q = new int[10]; + // 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}:", 10}" + + int tmp = p[5]; + tmp = q[5]; + + if(p == q) {} + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:".data()" + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:".data()" +} + +void unsafe_method_invocation_single_param() { + auto p = new int[10]; + // 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]]:23-[[@LINE-3]]:23}:", 10}" + + int tmp = p[5]; + foo(p); + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:8-[[@LINE-1]]:8}:".data()" + +} + +void safe_method_invocation_single_param() { + auto p = new int[10]; + foo(p); +} + +void unsafe_method_invocation_double_param() { + auto p = new int[10]; + // 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]]:23-[[@LINE-3]]:23}:", 10}" + + int tmp = p[5]; + m1(p, p, 10); + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:".data()" + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:10-[[@LINE-2]]:10}:".data()" + + auto q = new int[10]; + + m1(p, q, 4); + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:".data()" + + m1(q, p, 9); + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:10}:".data()" + + m1(q, q, 8); +} +