Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -5241,7 +5241,12 @@ "conversion function">; def note_delete_conversion : Note<"conversion to pointer type %0">; def warn_delete_array_type : Warning< - "'delete' applied to a pointer-to-array type %0 treated as delete[]">; + "'delete' applied to a pointer-to-array type %0 treated as 'delete[]'">; +def warn_mismatched_delete_new : Warning< + "'delete%select{|[]}0' applied to a pointer that was allocated with " + "'new%select{[]|}0'; did you mean 'delete%select{[]|}0'?">, + InGroup>; +def note_allocated_here : Note<"allocated with 'new%select{[]|}0' here">; def err_no_suitable_delete_member_function_found : Error< "no suitable member %0 in %1">; def err_ambiguous_suitable_delete_member_function_found : Error< Index: include/clang/Sema/Sema.h =================================================================== --- include/clang/Sema/Sema.h +++ include/clang/Sema/Sema.h @@ -78,10 +78,12 @@ typedef SmallVector CXXCastPath; class CXXConstructorDecl; class CXXConversionDecl; + class CXXDeleteExpr; class CXXDestructorDecl; class CXXFieldCollector; class CXXMemberCallExpr; class CXXMethodDecl; + class CXXNewExpr; class CXXScopeSpec; class CXXTemporary; class CXXTryStmt; @@ -390,6 +392,9 @@ /// \brief Set containing all declared private fields that are not used. NamedDeclSetType UnusedPrivateFields; + /// \brief All delete-expressions within the translation unit. + llvm::SmallPtrSet DeleteExprs; + typedef llvm::SmallPtrSet RecordDeclSetTy; /// PureVirtualClassDiagSet - a set of class declarations which we have @@ -8400,6 +8405,17 @@ /// \brief Check if the given expression contains 'break' or 'continue' /// statement that produces control flow different from GCC. void CheckBreakContinueBinding(Expr *E); + /// \brief Check whether given delete-expression mismatches with all new- + /// expressions used for initializing pointee. + /// + /// If pointee is initialized with array form of 'new', it should be deleted + /// with array form delete. If at least one \a matching new-expression is + /// found, the function will return false. + /// \param DE delete-expression to analyze + /// \param NewExprs new-expressions used for initialization of pointee + bool DiagnoseMismatchedNewDelete( + const CXXDeleteExpr *DE, + llvm::SmallVector &NewExprs); public: /// \brief Register a magic integral constant to be used as a type tag. Index: lib/Sema/Sema.cpp =================================================================== --- lib/Sema/Sema.cpp +++ lib/Sema/Sema.cpp @@ -841,6 +841,24 @@ } } } + if (!Diags.isIgnored(diag::warn_mismatched_delete_new, SourceLocation())) { + for (const auto &DE : DeleteExprs) { + llvm::SmallVector NewExprs; + if (!DiagnoseMismatchedNewDelete(DE, NewExprs)) + continue; + PartialDiagnostic PD = PDiag(diag::warn_mismatched_delete_new) + << DE->isArrayForm() + << DE->getArgument()->getSourceRange(); + if (!DE->isArrayForm()) + PD << FixItHint::CreateInsertion( + PP.getLocForEndOfToken(DE->getLocStart()), "[]"); + // FIXME: Find a way to suggest fix-it for removal of '[]'. + Diag(DE->getLocStart(), PD); + for (const auto *NE : NewExprs) { + Diag(NE->getExprLoc(), diag::note_allocated_here) << DE->isArrayForm(); + } + } + } // Check we've noticed that we're no longer parsing the initializer for every // variable. If we miss cases, then at best we have a performance issue and Index: lib/Sema/SemaExprCXX.cpp =================================================================== --- lib/Sema/SemaExprCXX.cpp +++ lib/Sema/SemaExprCXX.cpp @@ -2226,6 +2226,79 @@ return false; } +// Returns new-expression or null for given initializer. +static const CXXNewExpr *getNewExprInit(const Expr *E) { + assert(E != nullptr && "Expected a valid initializer expression"); + E = E->IgnoreImpCasts(); + const CXXNewExpr *NE = dyn_cast(E); + if (NE) + return NE; + if (const InitListExpr *ILE = dyn_cast(E)) + if (ILE->getNumInits() == 1) + NE = dyn_cast(ILE->getInit(0)->IgnoreParenImpCasts()); + return NE; +} + +// Returns list of new-expressions used for initializing given class member +static bool mismatchedMemberInitsNewExprs( + const MemberExpr *ME, bool IsArrayForm, + llvm::SmallVector &NewExprs) { + assert(ME != nullptr && "Expected a member expression"); + const CXXNewExpr *NE = nullptr; + const FieldDecl *FD = dyn_cast(ME->getMemberDecl()); + if (!FD) + return false; + const CXXRecordDecl *RD = cast(FD->getParent()); + for (const auto *CD : RD->ctors()) { + if (!CD->isThisDeclarationADefinition()) + continue; + for (const auto *CI : CD->inits()) { + if (FD == CI->getMember() && (NE = getNewExprInit(CI->getInit()))) { + if (NE->isArray() == IsArrayForm) + return false; + NewExprs.push_back(NE); + } + } + } + // No ctor-initializer initializes this field, check in-class initializer + if (!NE && FD->hasInClassInitializer() && + (NE = getNewExprInit(FD->getInClassInitializer()))) { + if (NE->isArray() == IsArrayForm) + return false; + NewExprs.push_back(NE); + } + return !NewExprs.empty(); +} + +// Returns list of new-expressions used for initializing given variable +static bool +mismatchedVarInitsNewExprs(const DeclRefExpr *D, bool IsArrayForm, + llvm::SmallVector &NewExprs) { + const CXXNewExpr *NE = nullptr; + if (const VarDecl *VD = dyn_cast(D->getDecl())) + if (VD->hasInit() && (NE = getNewExprInit(VD->getInit()))) { + if (NE->isArray() == IsArrayForm) + return false; + NewExprs.push_back(NE); + } + return !NewExprs.empty(); +} + +bool Sema::DiagnoseMismatchedNewDelete( + const CXXDeleteExpr *DE, + llvm::SmallVector &NewExprs) { + const Expr *E = DE->getArgument()->IgnoreCasts(); + NewExprs.clear(); + + if (const MemberExpr *ME = dyn_cast(E)) { + if (!mismatchedMemberInitsNewExprs(ME, DE->isArrayForm(), NewExprs)) + return false; + } else if (const DeclRefExpr *D = dyn_cast(E)) { + if (!mismatchedVarInitsNewExprs(D, DE->isArrayForm(), NewExprs)) + return false; + } + return !NewExprs.empty(); +} /// ActOnCXXDelete - Parsed a C++ 'delete' expression (C++ 5.3.5), as in: /// @code ::delete ptr; @endcode /// or @@ -2341,12 +2414,6 @@ } } - // C++ [expr.delete]p2: - // [Note: a pointer to a const type can be the operand of a - // delete-expression; it is not necessary to cast away the constness - // (5.2.11) of the pointer expression before it is used as the operand - // of the delete-expression. ] - if (Pointee->isArrayType() && !ArrayForm) { Diag(StartLoc, diag::warn_delete_array_type) << Type << Ex.get()->getSourceRange() @@ -2431,9 +2498,11 @@ } } - return new (Context) CXXDeleteExpr( + CXXDeleteExpr *Result = new (Context) CXXDeleteExpr( Context.VoidTy, UseGlobal, ArrayForm, ArrayFormAsWritten, UsualArrayDeleteWantsSize, OperatorDelete, Ex.get(), StartLoc); + DeleteExprs.insert(Result); + return Result; } /// \brief Check the use of the given variable as a C++ condition in an if, Index: test/Analysis/Malloc+MismatchedDeallocator+NewDelete.cpp =================================================================== --- test/Analysis/Malloc+MismatchedDeallocator+NewDelete.cpp +++ test/Analysis/Malloc+MismatchedDeallocator+NewDelete.cpp @@ -97,9 +97,11 @@ free(p); delete globalPtr; // expected-warning {{Attempt to free released memory}} } - +int *allocIntArray(unsigned c) { + return new int[c]; +} void testMismatchedChangePointeeThroughAssignment() { - int *arr = new int[4]; + int *arr = allocIntArray(4); globalPtr = arr; delete arr; // expected-warning{{Memory allocated by 'new[]' should be deallocated by 'delete[]', not 'delete'}} -} \ No newline at end of file +} Index: test/Analysis/MismatchedDeallocator-checker-test.mm =================================================================== --- test/Analysis/MismatchedDeallocator-checker-test.mm +++ test/Analysis/MismatchedDeallocator-checker-test.mm @@ -90,8 +90,11 @@ realloc(p, sizeof(long)); // expected-warning{{Memory allocated by 'new[]' should be deallocated by 'delete[]', not realloc()}} } +int *allocInt() { + return new int; +} void testNew7() { - int *p = new int; + int *p = allocInt(); delete[] p; // expected-warning{{Memory allocated by 'new' should be deallocated by 'delete', not 'delete[]'}} } @@ -100,8 +103,12 @@ delete[] p; // expected-warning{{Memory allocated by operator new should be deallocated by 'delete', not 'delete[]'}} } +int *allocIntArray(unsigned c) { + return new int[c]; +} + void testNew9() { - int *p = new int[1]; + int *p = allocIntArray(1); delete p; // expected-warning{{Memory allocated by 'new[]' should be deallocated by 'delete[]', not 'delete'}} } Index: test/Analysis/MismatchedDeallocator-path-notes.cpp =================================================================== --- test/Analysis/MismatchedDeallocator-path-notes.cpp +++ test/Analysis/MismatchedDeallocator-path-notes.cpp @@ -3,9 +3,12 @@ // RUN: FileCheck --input-file=%t.plist %s void changePointee(int *p); +int *allocIntArray(unsigned c) { + return new int[c]; // expected-note {{Memory is allocated}} +} void test() { - int *p = new int[1]; - // expected-note@-1 {{Memory is allocated}} + int *p = allocIntArray(1); // expected-note {{Calling 'allocIntArray'}} + // expected-note@-1 {{Returned allocated memory}} changePointee(p); delete p; // expected-warning {{Memory allocated by 'new[]' should be deallocated by 'delete[]', not 'delete'}} // expected-note@-1 {{Memory allocated by 'new[]' should be deallocated by 'delete[]', not 'delete'}} @@ -24,12 +27,12 @@ // CHECK-NEXT: start // CHECK-NEXT: // CHECK-NEXT: -// CHECK-NEXT: line7 +// CHECK-NEXT: line10 // CHECK-NEXT: col3 // CHECK-NEXT: file0 // CHECK-NEXT: // CHECK-NEXT: -// CHECK-NEXT: line7 +// CHECK-NEXT: line10 // CHECK-NEXT: col5 // CHECK-NEXT: file0 // CHECK-NEXT: @@ -37,13 +40,124 @@ // CHECK-NEXT: end // CHECK-NEXT: // CHECK-NEXT: -// CHECK-NEXT: line7 +// CHECK-NEXT: line10 // CHECK-NEXT: col12 // CHECK-NEXT: file0 // CHECK-NEXT: // CHECK-NEXT: +// CHECK-NEXT: line10 +// CHECK-NEXT: col24 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindevent +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line10 +// CHECK-NEXT: col12 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: ranges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line10 +// CHECK-NEXT: col12 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line10 +// CHECK-NEXT: col27 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: depth0 +// CHECK-NEXT: extended_message +// CHECK-NEXT: Calling 'allocIntArray' +// CHECK-NEXT: message +// CHECK-NEXT: Calling 'allocIntArray' +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindevent +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line6 +// CHECK-NEXT: col1 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: depth1 +// CHECK-NEXT: extended_message +// CHECK-NEXT: Entered call from 'test' +// CHECK-NEXT: message +// CHECK-NEXT: Entered call from 'test' +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindcontrol +// CHECK-NEXT: edges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: start +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line6 +// CHECK-NEXT: col1 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line6 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: end +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line7 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: // CHECK-NEXT: line7 -// CHECK-NEXT: col14 +// CHECK-NEXT: col8 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindcontrol +// CHECK-NEXT: edges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: start +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line7 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line7 +// CHECK-NEXT: col8 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: end +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line7 +// CHECK-NEXT: col10 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line7 +// CHECK-NEXT: col12 // CHECK-NEXT: file0 // CHECK-NEXT: // CHECK-NEXT: @@ -55,6 +169,35 @@ // CHECK-NEXT: location // CHECK-NEXT: // CHECK-NEXT: line7 +// CHECK-NEXT: col10 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: ranges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line7 +// CHECK-NEXT: col10 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line7 +// CHECK-NEXT: col19 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: depth1 +// CHECK-NEXT: extended_message +// CHECK-NEXT: Memory is allocated +// CHECK-NEXT: message +// CHECK-NEXT: Memory is allocated +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindevent +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line10 // CHECK-NEXT: col12 // CHECK-NEXT: file0 // CHECK-NEXT: @@ -62,22 +205,22 @@ // CHECK-NEXT: // CHECK-NEXT: // CHECK-NEXT: -// CHECK-NEXT: line7 +// CHECK-NEXT: line10 // CHECK-NEXT: col12 // CHECK-NEXT: file0 // CHECK-NEXT: // CHECK-NEXT: -// CHECK-NEXT: line7 -// CHECK-NEXT: col21 +// CHECK-NEXT: line10 +// CHECK-NEXT: col27 // CHECK-NEXT: file0 // CHECK-NEXT: // CHECK-NEXT: // CHECK-NEXT: // CHECK-NEXT: depth0 // CHECK-NEXT: extended_message -// CHECK-NEXT: Memory is allocated +// CHECK-NEXT: Returned allocated memory // CHECK-NEXT: message -// CHECK-NEXT: Memory is allocated +// CHECK-NEXT: Returned allocated memory // CHECK-NEXT: // CHECK-NEXT: // CHECK-NEXT: kindcontrol @@ -87,25 +230,25 @@ // CHECK-NEXT: start // CHECK-NEXT: // CHECK-NEXT: -// CHECK-NEXT: line7 +// CHECK-NEXT: line10 // CHECK-NEXT: col12 // CHECK-NEXT: file0 // CHECK-NEXT: // CHECK-NEXT: -// CHECK-NEXT: line7 -// CHECK-NEXT: col14 +// CHECK-NEXT: line10 +// CHECK-NEXT: col24 // CHECK-NEXT: file0 // CHECK-NEXT: // CHECK-NEXT: // CHECK-NEXT: end // CHECK-NEXT: // CHECK-NEXT: -// CHECK-NEXT: line10 +// CHECK-NEXT: line13 // CHECK-NEXT: col3 // CHECK-NEXT: file0 // CHECK-NEXT: // CHECK-NEXT: -// CHECK-NEXT: line10 +// CHECK-NEXT: line13 // CHECK-NEXT: col8 // CHECK-NEXT: file0 // CHECK-NEXT: @@ -117,7 +260,7 @@ // CHECK-NEXT: kindevent // CHECK-NEXT: location // CHECK-NEXT: -// CHECK-NEXT: line10 +// CHECK-NEXT: line13 // CHECK-NEXT: col3 // CHECK-NEXT: file0 // CHECK-NEXT: @@ -125,12 +268,12 @@ // CHECK-NEXT: // CHECK-NEXT: // CHECK-NEXT: -// CHECK-NEXT: line10 +// CHECK-NEXT: line13 // CHECK-NEXT: col10 // CHECK-NEXT: file0 // CHECK-NEXT: // CHECK-NEXT: -// CHECK-NEXT: line10 +// CHECK-NEXT: line13 // CHECK-NEXT: col10 // CHECK-NEXT: file0 // CHECK-NEXT: @@ -151,7 +294,7 @@ // CHECK-NEXT: issue_hash4 // CHECK-NEXT: location // CHECK-NEXT: -// CHECK-NEXT: line10 +// CHECK-NEXT: line13 // CHECK-NEXT: col3 // CHECK-NEXT: file0 // CHECK-NEXT: Index: test/CodeGenCXX/new.cpp =================================================================== --- test/CodeGenCXX/new.cpp +++ test/CodeGenCXX/new.cpp @@ -290,14 +290,14 @@ // CHECK-LABEL: define void @_ZN5N36641fEv void f() { // CHECK: call noalias i8* @_Znwm(i64 4) [[ATTR_BUILTIN_NEW:#[^ ]*]] - int *p = new int; + int *p = new int; // expected-note {{allocated with 'new' here}} // CHECK: call void @_ZdlPv({{.*}}) [[ATTR_BUILTIN_DELETE:#[^ ]*]] delete p; // CHECK: call noalias i8* @_Znam(i64 12) [[ATTR_BUILTIN_NEW]] int *q = new int[3]; // CHECK: call void @_ZdaPv({{.*}}) [[ATTR_BUILTIN_DELETE]] - delete [] p; + delete[] p; // expected-warning {{'delete[]' applied to a pointer that was allocated with 'new'; did you mean 'delete'?}} // CHECK: call i8* @_ZnamRKSt9nothrow_t(i64 3, {{.*}}) [[ATTR_BUILTIN_NOTHROW_NEW:#[^ ]*]] (void) new (nothrow) S[3]; Index: test/SemaCXX/delete.cpp =================================================================== --- test/SemaCXX/delete.cpp +++ test/SemaCXX/delete.cpp @@ -1,9 +1,86 @@ -// RUN: %clang_cc1 -fsyntax-only -verify %s +// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s // RUN: cp %s %t -// RUN: %clang_cc1 -fixit -x c++ %t +// RUN: %clang_cc1 -fixit -x c++ -std=c++11 %t // RUN: %clang_cc1 -E -o - %t | FileCheck %s void f(int a[10][20]) { // CHECK: delete[] a; delete a; // expected-warning {{'delete' applied to a pointer-to-array type}} } +namespace MemberCheck { +struct S { + int *a = new int[5]; // expected-note {{allocated with 'new[]' here}} + // expected-note@-1 {{allocated with 'new[]' here}} + // expected-note@-2 {{allocated with 'new[]' here}} + int *b; + int *c; + static int *d; + S() : b(new int[1]), c(new int) {} // expected-note {{allocated with 'new[]' here}} + // expected-note@-1 {{allocated with 'new[]' here}} + // expected-note@-2 {{allocated with 'new' here}} + ~S() { + // CHECK: delete[] a; + delete a; // expected-warning {{'delete' applied to a pointer that was allocated with 'new[]'; did you mean 'delete[]'?}} + // CHECK: delete[] b; + delete b; // expected-warning {{'delete' applied to a pointer that was allocated with 'new[]'; did you mean 'delete[]'?}} + delete[] c; // expected-warning {{'delete[]' applied to a pointer that was allocated with 'new'; did you mean 'delete'?}} + } +}; +struct S2 : S { + ~S2() { + delete a; // expected-warning {{'delete' applied to a pointer that was allocated with 'new[]'; did you mean 'delete[]'?}} + } +}; +int *S::d = new int[42]; // expected-note {{allocated with 'new[]' here}} +void f(S *s) { + int *a = new int[1]; // expected-note {{allocated with 'new[]' here}} + //CHECK: delete[] a; + delete a; // expected-warning {{'delete' applied to a pointer that was allocated with 'new[]'; did you mean 'delete[]'?}} + // CHECK: delete[] s->a; + delete s->a; // expected-warning {{'delete' applied to a pointer that was allocated with 'new[]'; did you mean 'delete[]'?}} + delete s->b; // expected-warning {{'delete' applied to a pointer that was allocated with 'new[]'; did you mean 'delete[]'?}} + delete s->c; + delete s->d; + delete S::d; // expected-warning {{'delete' applied to a pointer that was allocated with 'new[]'; did you mean 'delete[]'?}} +} + +struct MatchingNewIsOK { + int *p; + bool is_array_; + MatchingNewIsOK() : p{new int}, is_array_(false) {} + explicit MatchingNewIsOK(unsigned c) : p{new int[c]}, is_array_(true) {} + ~MatchingNewIsOK() { + if (is_array_) + delete[] p; + else + delete p; + } +}; + +struct base {}; +struct derived : base {}; +struct InitList { + base *p; + InitList() : p{new derived[1]} {} // expected-note {{allocated with 'new[]' here}} + explicit InitList(unsigned c) : p(new derived[c]) {} // expected-note {{allocated with 'new[]' here}} + InitList(unsigned c, unsigned) : p{new derived[c]} {} // expected-note {{allocated with 'new[]' here}} + InitList(const char *) : p{new derived[1]} {} // expected-note {{allocated with 'new[]' here}} + ~InitList() { + delete p; // expected-warning {{'delete' applied to a pointer that was allocated with 'new[]'; did you mean 'delete[]'?}} + delete[] p; + } +}; +} + +namespace NonMemberCheck { +void f() { + int *a = new int(5); // expected-note {{allocated with 'new' here}} + delete[] a; // expected-warning {{'delete[]' applied to a pointer that was allocated with 'new'; did you mean 'delete'?}} + int *b = new int; + delete b; + int *c{new int}; // expected-note {{allocated with 'new' here}} + int *d{new int[1]}; // expected-note {{allocated with 'new[]' here}} + delete[] c; // expected-warning {{'delete[]' applied to a pointer that was allocated with 'new'; did you mean 'delete'?}} + delete d; // expected-warning {{'delete' applied to a pointer that was allocated with 'new[]'; did you mean 'delete[]'?}} +} +}