llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: None (shreya-jain) <details> <summary>Changes</summary> Add operator[] accesses of unique_ptr<T[]> to -Wunsafe-buffer-usage as a subcategory under -Wunsafe-buffer-usage-unique-ptr-array-access. --- Full diff: https://github.com/llvm/llvm-project/pull/156773.diff 7 Files Affected: - (modified) clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h (+7) - (modified) clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def (+1) - (modified) clang/include/clang/Basic/DiagnosticGroups.td (+2-1) - (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+2) - (modified) clang/lib/Analysis/UnsafeBufferUsage.cpp (+105-2) - (modified) clang/lib/Sema/AnalysisBasedWarnings.cpp (+13) - (modified) clang/test/SemaCXX/warn-unsafe-buffer-usage-debug-unclaimed/warn-unsafe-buffer-usage-debug-unclaimed.cpp (+14) ``````````diff diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h index 9b53f1dc1d759..ea41eb3becfcf 100644 --- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h +++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h @@ -14,6 +14,7 @@ #ifndef LLVM_CLANG_ANALYSIS_ANALYSES_UNSAFEBUFFERUSAGE_H #define LLVM_CLANG_ANALYSIS_ANALYSES_UNSAFEBUFFERUSAGE_H +#include "clang/AST/ASTTypeTraits.h" #include "clang/AST/Decl.h" #include "clang/AST/Expr.h" #include "clang/AST/Stmt.h" @@ -139,6 +140,12 @@ class UnsafeBufferUsageHandler { FixItList &&Fixes, const Decl *D, const FixitStrategy &VarTargetTypes) = 0; + // Invoked when an array subscript operator[] is used on a + // std::unique_ptr<T[]>. + virtual void handleUnsafeUniquePtrArrayAccess(const DynTypedNode &Node, + bool IsRelatedToDecl, + ASTContext &Ctx) = 0; + #ifndef NDEBUG public: bool areDebugNotesRequested() { diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def index 09fa510bc0472..fae5f8b8aa8e3 100644 --- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def +++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def @@ -38,6 +38,7 @@ WARNING_GADGET(PointerArithmetic) WARNING_GADGET(UnsafeBufferUsageAttr) WARNING_GADGET(UnsafeBufferUsageCtorAttr) WARNING_GADGET(DataInvocation) +WARNING_GADGET(UniquePtrArrayAccess) WARNING_OPTIONAL_GADGET(UnsafeLibcFunctionCall) WARNING_OPTIONAL_GADGET(SpanTwoParamConstructor) // Uses of `std::span(arg0, arg1)` FIXABLE_GADGET(ULCArraySubscript) // `DRE[any]` in an Unspecified Lvalue Context diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 0c994e0b5ca4d..4b27a42c6dd81 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -1750,7 +1750,8 @@ def ReadOnlyPlacementChecks : DiagGroup<"read-only-types">; // Warnings and fixes to support the "safe buffers" programming model. def UnsafeBufferUsageInContainer : DiagGroup<"unsafe-buffer-usage-in-container">; def UnsafeBufferUsageInLibcCall : DiagGroup<"unsafe-buffer-usage-in-libc-call">; -def UnsafeBufferUsage : DiagGroup<"unsafe-buffer-usage", [UnsafeBufferUsageInContainer, UnsafeBufferUsageInLibcCall]>; +def UnsafeBufferUsageInUniquePtrArrayAccess : DiagGroup<"unsafe-buffer-usage-in-unique-ptr-array-access">; +def UnsafeBufferUsage : DiagGroup<"unsafe-buffer-usage", [UnsafeBufferUsageInContainer, UnsafeBufferUsageInLibcCall, UnsafeBufferUsageInUniquePtrArrayAccess]>; // Warnings and notes InstallAPI verification. def InstallAPIViolation : DiagGroup<"installapi-violation">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index c76385ad73a91..6bdebde1ec19e 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -13240,6 +13240,8 @@ def note_safe_buffer_usage_suggestions_disabled : Note< def warn_unsafe_buffer_usage_in_container : Warning< "the two-parameter std::span construction is unsafe as it can introduce mismatch between buffer size and the bound information">, InGroup<UnsafeBufferUsageInContainer>, DefaultIgnore; +def warn_unsafe_buffer_usage_unique_ptr_array_access : Warning<"direct access using operator[] on std::unique_ptr<T[]> is unsafe due to lack of bounds checking">, + InGroup<UnsafeBufferUsageInUniquePtrArrayAccess>, DefaultIgnore; #ifndef NDEBUG // Not a user-facing diagnostic. Useful for debugging false negatives in // -fsafe-buffer-usage-suggestions (i.e. lack of -Wunsafe-buffer-usage fixits). diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index 1d7b8722103aa..7a0a0ee7fb694 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -13,6 +13,7 @@ #include "clang/AST/Attr.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" #include "clang/AST/DynamicRecursiveASTVisitor.h" #include "clang/AST/Expr.h" #include "clang/AST/FormatString.h" @@ -1318,6 +1319,105 @@ static bool isSupportedVariable(const DeclRefExpr &Node) { return D != nullptr && isa<VarDecl>(D); } +static bool isUniquePtrArray(const CXXRecordDecl *RecordDecl) { + if (!RecordDecl || !RecordDecl->isInStdNamespace() || + RecordDecl->getNameAsString() != "unique_ptr") { + return false; + } + + const ClassTemplateSpecializationDecl *class_template_specialization_decl = + dyn_cast<ClassTemplateSpecializationDecl>(RecordDecl); + if (!class_template_specialization_decl) { + return false; + } + + const TemplateArgumentList &template_args = + class_template_specialization_decl->getTemplateArgs(); + + if (template_args.size() == 0) { + return false; + } + + const TemplateArgument &first_arg = template_args[0]; + + if (first_arg.getKind() != TemplateArgument::Type) { + return false; + } + + QualType referred_type = first_arg.getAsType(); + + if (referred_type->isArrayType()) { + return true; + } + + return false; +} + +class UniquePtrArrayAccessGadget : public WarningGadget { + static constexpr const char *const AccessorTag = "unique_ptr_array_access"; + const CXXOperatorCallExpr *TheAccessorExpr; + +public: + UniquePtrArrayAccessGadget(const MatchResult &Result) + : WarningGadget(Kind::UniquePtrArrayAccess), + TheAccessorExpr(Result.getNodeAs<CXXOperatorCallExpr>(AccessorTag)) { + assert(TheAccessorExpr && + "UniquePtrArrayAccessGadget requires a matched CXXOperatorCallExpr"); + } + + static bool classof(const Gadget *G) { + return G->getKind() == Kind::UniquePtrArrayAccess; + } + + static bool matches(const Stmt *S, const ASTContext &Ctx, + MatchResult &Result) { + + const CXXOperatorCallExpr *OpCall = dyn_cast<CXXOperatorCallExpr>(S); + if (!OpCall || OpCall->getOperator() != OO_Subscript) { + return false; + } + + const Expr *Callee = OpCall->getCallee()->IgnoreParenImpCasts(); + if (!Callee) { + return false; + } + + const CXXMethodDecl *Method = + dyn_cast_or_null<CXXMethodDecl>(OpCall->getDirectCallee()); + if (!Method) { + return false; + } + + if (Method->getNameAsString() != "operator[]") { + return false; + } + + const CXXRecordDecl *RecordDecl = Method->getParent(); + if (!isUniquePtrArray(RecordDecl)) { + return false; + } + + Result.addNode(AccessorTag, DynTypedNode::create(*OpCall)); + return true; + } + void handleUnsafeOperation(UnsafeBufferUsageHandler &Handler, + bool IsRelatedToDecl, + ASTContext &Ctx) const override { + Handler.handleUnsafeUniquePtrArrayAccess( + DynTypedNode::create(*TheAccessorExpr), IsRelatedToDecl, Ctx); + } + + SourceLocation getSourceLoc() const override { + if (TheAccessorExpr) { + return TheAccessorExpr->getOperatorLoc(); + } + return SourceLocation(); + } + + DeclUseList getClaimedVarUseSites() const override { return {}; } + SmallVector<const Expr *, 1> getUnsafePtrs() const override { return {}; } +}; + using FixableGadgetList = std::vector<std::unique_ptr<FixableGadget>>; using WarningGadgetList = std::vector<std::unique_ptr<WarningGadget>>; @@ -2632,10 +2732,13 @@ std::set<const Expr *> clang::findUnsafePointers(const FunctionDecl *FD) { const VariableGroupsManager &, FixItList &&, const Decl *, const FixitStrategy &) override {} - bool isSafeBufferOptOut(const SourceLocation &) const override { + void handleUnsafeUniquePtrArrayAccess(const DynTypedNode &Node, + bool IsRelatedToDecl, + ASTContext &Ctx) override {} + bool ignoreUnsafeBufferInContainer(const SourceLocation &) const override { return false; } - bool ignoreUnsafeBufferInContainer(const SourceLocation &) const override { + bool isSafeBufferOptOut(const SourceLocation &) const override { return false; } bool ignoreUnsafeBufferInLibcCall(const SourceLocation &) const override { diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 1b66d83df5171..5f51d38c6cfb9 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2606,6 +2606,19 @@ class UnsafeBufferUsageReporter : public UnsafeBufferUsageHandler { #endif } + void handleUnsafeUniquePtrArrayAccess(const DynTypedNode &Node, + bool IsRelatedToDecl, + ASTContext &Ctx) override { + SourceLocation Loc; + std::string Message; + + Loc = Node.get<Stmt>()->getBeginLoc(); + Message = "Direct operator[] access on std::unique_ptr<T[]> is unsafe " + "(no bounds check)."; + S.Diag(Loc, diag::warn_unsafe_buffer_usage_unique_ptr_array_access) + << Message << Node.getSourceRange(); + } + bool isSafeBufferOptOut(const SourceLocation &Loc) const override { return S.PP.isSafeBufferOptOut(S.getSourceManager(), Loc); } diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-debug-unclaimed/warn-unsafe-buffer-usage-debug-unclaimed.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-debug-unclaimed/warn-unsafe-buffer-usage-debug-unclaimed.cpp index ab3d925753d47..e504d1f97ada3 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-debug-unclaimed/warn-unsafe-buffer-usage-debug-unclaimed.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-debug-unclaimed/warn-unsafe-buffer-usage-debug-unclaimed.cpp @@ -32,6 +32,20 @@ void test_unclaimed_use(int *p) { // expected-warning{{'p' is an unsafe pointer p[5] = 5; // expected-note{{used in buffer access here}} } +namespace std { +inline namespace __1 { +template <class T> class unique_ptr { +public: + T &operator[](long long i) const; +}; +} // namespace __1 +} // namespace std + +void basic_unique_ptr() { + std::unique_ptr<int[]> p1; + p1[0]; // expected-warning{{direct access using operator[] on std::unique_ptr<T[]> is unsafe due to lack of bounds checking}} +} + // CHECK: Root # 1 // CHECK: |- DeclRefExpr # 4 // CHECK: |-- UnaryOperator(++) # 1 `````````` </details> https://github.com/llvm/llvm-project/pull/156773 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
