Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -5316,7 +5316,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/ExternalSemaSource.h =================================================================== --- include/clang/Sema/ExternalSemaSource.h +++ include/clang/Sema/ExternalSemaSource.h @@ -28,6 +28,7 @@ class CXXConstructorDecl; class CXXRecordDecl; +class CXXDeleteExpr; class DeclaratorDecl; class LookupResult; struct ObjCMethodList; @@ -79,6 +80,9 @@ virtual void ReadUndefinedButUsed( llvm::DenseMap &Undefined); + virtual void ReadMismatchingDeleteExpressions(llvm::DenseMap< + FieldDecl *, llvm::SmallVector, 4>> &); + /// \brief Do last resort, unqualified lookup on a LookupResult that /// Sema cannot find. /// Index: include/clang/Sema/MultiplexExternalSemaSource.h =================================================================== --- include/clang/Sema/MultiplexExternalSemaSource.h +++ include/clang/Sema/MultiplexExternalSemaSource.h @@ -226,6 +226,10 @@ void ReadUndefinedButUsed( llvm::DenseMap &Undefined) override; + void ReadMismatchingDeleteExpressions(llvm::DenseMap< + FieldDecl *, llvm::SmallVector, 4>> & + Exprs) override; + /// \brief Do last resort, unqualified lookup on a LookupResult that /// Sema cannot find. /// Index: include/clang/Sema/Sema.h =================================================================== --- include/clang/Sema/Sema.h +++ include/clang/Sema/Sema.h @@ -78,6 +78,7 @@ typedef SmallVector CXXCastPath; class CXXConstructorDecl; class CXXConversionDecl; + class CXXDeleteExpr; class CXXDestructorDecl; class CXXFieldCollector; class CXXMemberCallExpr; @@ -393,6 +394,15 @@ llvm::SmallSetVector UnusedLocalTypedefNameCandidates; + /// \brief Delete-expressions to be analyzed at the end of translation unit + /// + /// This list contains class members, and locations of delete-expressions + /// that could not be proven as to whether they mismatch with new-expression + /// used in initializer of the field. + typedef std::pair DeleteExprLoc; + typedef llvm::SmallVector DeleteLocs; + llvm::DenseMap DeleteExprs; + typedef llvm::SmallPtrSet RecordDeclSetTy; /// PureVirtualClassDiagSet - a set of class declarations which we have @@ -892,6 +902,11 @@ void getUndefinedButUsed( SmallVectorImpl > &Undefined); + /// Retrieves list of suspicious delete-expressions that will be checked at + /// the end of translation unit. + void getMismatchingDeleteExpressions( + llvm::DenseMap &Exprs); + typedef std::pair GlobalMethods; typedef llvm::DenseMap GlobalMethodPool; @@ -8518,6 +8533,20 @@ /// 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 + void DiagnoseMismatchedNewDelete(FieldDecl *FD, SourceLocation DeleteLoc, + bool IsArrayForm); + void AnalyzeDeleteExprMismatch(const CXXDeleteExpr *DE); + void AnalyzeDeleteExprMismatch(FieldDecl *Field, SourceLocation DeleteLoc, + bool DeleteWasArrayForm); + public: /// \brief Register a magic integral constant to be used as a type tag. void RegisterTypeTagForDatatype(const IdentifierInfo *ArgumentKind, Index: include/clang/Serialization/ASTBitCodes.h =================================================================== --- include/clang/Serialization/ASTBitCodes.h +++ include/clang/Serialization/ASTBitCodes.h @@ -549,6 +549,9 @@ /// \brief Record code for potentially unused local typedef names. UNUSED_LOCAL_TYPEDEF_NAME_CANDIDATES = 52, + + /// \brief Delete expressions that will be analyzed later. + DELETE_EXPRS_TO_ANALYZE = 53 }; /// \brief Record types used within a source manager block. Index: include/clang/Serialization/ASTReader.h =================================================================== --- include/clang/Serialization/ASTReader.h +++ include/clang/Serialization/ASTReader.h @@ -796,6 +796,9 @@ /// SourceLocation of a matching ODR-use. SmallVector UndefinedButUsed; + /// \brief Delete expressions to analyze at the end of translation unit. + SmallVector DelayedDeleteExprs; + // \brief A list of late parsed template function data. SmallVector LateParsedTemplates; @@ -1781,6 +1784,10 @@ void ReadUndefinedButUsed( llvm::DenseMap &Undefined) override; + void ReadMismatchingDeleteExpressions(llvm::DenseMap< + FieldDecl *, llvm::SmallVector, 4>> & + Exprs); + void ReadTentativeDefinitions( SmallVectorImpl &TentativeDefs) override; Index: lib/Sema/MultiplexExternalSemaSource.cpp =================================================================== --- lib/Sema/MultiplexExternalSemaSource.cpp +++ lib/Sema/MultiplexExternalSemaSource.cpp @@ -204,7 +204,15 @@ for(size_t i = 0; i < Sources.size(); ++i) Sources[i]->ReadUndefinedButUsed(Undefined); } - + +void MultiplexExternalSemaSource::ReadMismatchingDeleteExpressions( + llvm::DenseMap, 4>> & + Exprs) { + for (auto &Source : Sources) + Source->ReadMismatchingDeleteExpressions(Exprs); +} + bool MultiplexExternalSemaSource::LookupUnqualified(LookupResult &R, Scope *S){ for(size_t i = 0; i < Sources.size(); ++i) Sources[i]->LookupUnqualified(R, S); Index: lib/Sema/Sema.cpp =================================================================== --- lib/Sema/Sema.cpp +++ lib/Sema/Sema.cpp @@ -864,6 +864,17 @@ } } + if (!Diags.isIgnored(diag::warn_mismatched_delete_new, SourceLocation())) { + if (ExternalSource) + ExternalSource->ReadMismatchingDeleteExpressions(DeleteExprs); + for (const auto &DeletedFieldInfo : DeleteExprs) { + for (const auto &DeleteExprLoc : DeletedFieldInfo.second) { + AnalyzeDeleteExprMismatch(DeletedFieldInfo.first, DeleteExprLoc.first, + DeleteExprLoc.second); + } + } + } + // 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 // at worst a rejects-valid bug. @@ -1223,6 +1234,9 @@ llvm::DenseMap &Undefined) { } +void ExternalSemaSource::ReadMismatchingDeleteExpressions(llvm::DenseMap< + FieldDecl *, llvm::SmallVector, 4>> &) {} + void PrettyDeclStackTraceEntry::print(raw_ostream &OS) const { SourceLocation Loc = this->Loc; if (!Loc.isValid() && TheDecl) Loc = TheDecl->getLocation(); @@ -1471,3 +1485,8 @@ return dyn_cast(FunctionScopes.back()); } + +void Sema::getMismatchingDeleteExpressions( + llvm::DenseMap &Exprs) { + Exprs = DeleteExprs; +} Index: lib/Sema/SemaExprCXX.cpp =================================================================== --- lib/Sema/SemaExprCXX.cpp +++ lib/Sema/SemaExprCXX.cpp @@ -2228,6 +2228,277 @@ return false; } +namespace { +class MismatchingNewDeleteDetector { +public: + enum MismatchResult { + /// Indicates that there is no mismatch or a mismatch cannot be proven. + NoMismatch, + /// Indicates that variable is initialized with mismatching form of \a new. + VarInitMismatches, + /// Indicates that member is initialized with mismatching form of \a new. + MemberInitMismatches, + /// Indicates that at 1 or more constructors' definitions could not been + /// analyzed, and they will be checked again at the end of translation unit. + AnalyzeLater + }; + + /// \brief Checks whether pointee of \p DE is initialized with mismatching + /// \brief form of \a new. + /// \param DE delete-expression to check. + /// + /// This constructor is intended to be called from \a ActOnCXXDelete. + explicit MismatchingNewDeleteDetector(const CXXDeleteExpr *DE) + : Field(nullptr), IsArrayForm(DE->isArrayForm()), DE(DE), EndOfTU(false), + HasUndefinedConstructors(false) {} + + /// \brief Checks whether \p Field is initialized with mismatching form of + /// \brief \a new. + /// \param Field Suspected field. + /// \param IsArrayForm true, if delete-expression was in array form. + /// + /// This constructor is intended to be called from + /// \a ActOnEndOfTranslationUnit. + MismatchingNewDeleteDetector(FieldDecl *Field, bool IsArrayForm) + : Field(Field), IsArrayForm(IsArrayForm), DE(nullptr), EndOfTU(true), + HasUndefinedConstructors(false) {} + + /// \brief Checks whether pointee of a delete-expression is initialized with + /// \brief matching form of new-expression. + /// + /// If return value is \c VarInitMismatches or \c MemberInitMismatches at the + /// point where delete-expression is encountered, then a warning will be + /// issued immediately. If eturn value is \c AnalyzeLater at the point where + /// delete-expression is seen, then member will be analyzed at the end of + /// translation unit. \c AnalyzeLater is returned iff at least one constructor + /// couldn't be analyzed. If at least one constructor initializes the member + /// with matching type of new, the return value is \c NoMismatch. + MismatchResult analyzeDeleteExpr(); + /// List of mismatching new-expressions used for initialization of the pointee + llvm::SmallVector NewExprs; + /// Field to analyze at the end of translation unit. + FieldDecl *Field; + /// Indicates whether delete-expression was in array form. + const bool IsArrayForm; + +private: + const CXXDeleteExpr *DE; + const bool EndOfTU; + /// \brief Indicates that there is at least one constructor without body. + bool HasUndefinedConstructors; + /// \brief Returns \c CXXNewExpr from given initialization expression. + /// \param E Expression used for initializing pointee in delete-expression. + /// \param E can be a single-element \c InitListExpr consisting of + /// \param E new-expression. + const CXXNewExpr *getNewExprFromInitListOrExpr(const Expr *E); + /// \brief Returns whether member is initialized with mismatching form of + /// \c new either by the member initializer or in-class initialization. + /// + /// If bodies of all constructors are not visible at the end of translation + /// unit or at least one constructor initializes member with the matching + /// form of \c new, mismatch cannot be proven, and this function will return + /// \c NoMismatch. + MismatchResult mismatchedMemberInitsNewExprs(const MemberExpr *ME); + /// \brief Returns whether variable is initialized with mismatching form of + /// \c new. + /// + /// If variable is initialized with mismatching form of \c new, returns true. + /// If variable is initialized with matching form of \c new or variable is not + /// initialized with a \c new expression, this function will return false. + /// \param D Variable to analyze. + bool mismatchedVarInitNewExpr(const DeclRefExpr *D); + /// \brief Analyzes class member. + MismatchResult analyzeField(); + /// \brief Checks whether the constructor initializes pointee with mismatching + /// \brief form of \c new. + /// + /// Returns true, if member is initialized with mismatching form of \c new in + /// member initializer list, or given constructor isn't defined at the point + /// where delete-expression is seen, or member isn't initialized by the + /// constructor. Returns false, if member is initialized with the matching + /// form of \c new in this constructor's initializer. + bool analyzeConstructor(const CXXConstructorDecl *CD); + /// \brief Checks whether member is initialized with mismatching form of + /// \brief \c new in member initializer list. + bool analyzeCtorInitializer(const CXXCtorInitializer *CI); + /// Checks whether member is initialized with mismatching form of \c new by + /// in-class initializer. + MismatchResult analyzeInClassInitializer(); +}; +} + +MismatchingNewDeleteDetector::MismatchResult +MismatchingNewDeleteDetector::analyzeDeleteExpr() { + NewExprs.clear(); + if (!DE) + return analyzeField(); + const Expr *E = DE->getArgument()->IgnoreParenImpCasts(); + if (const MemberExpr *ME = dyn_cast(E)) { + return mismatchedMemberInitsNewExprs(ME); + } else if (const DeclRefExpr *D = dyn_cast(E)) { + if (mismatchedVarInitNewExpr(D)) + return VarInitMismatches; + } + return NoMismatch; +} + +const CXXNewExpr * +MismatchingNewDeleteDetector::getNewExprFromInitListOrExpr(const Expr *E) { + assert(E != nullptr && "Expected a valid initializer expression"); + E = E->IgnoreImpCasts(); + const CXXNewExpr *NE = dyn_cast(E); + if (!NE) { + if (const InitListExpr *ILE = dyn_cast(E)) + if (ILE->getNumInits() == 1) + NE = dyn_cast(ILE->getInit(0)->IgnoreParenImpCasts()); + } + return NE; +} + +bool MismatchingNewDeleteDetector::analyzeCtorInitializer( + const CXXCtorInitializer *CI) { + const CXXNewExpr *NE = nullptr; + if (Field == CI->getMember() && + (NE = getNewExprFromInitListOrExpr(CI->getInit()))) { + if (NE->isArray() == IsArrayForm) + return false; + else + NewExprs.push_back(NE); + } + return true; +} + +bool +MismatchingNewDeleteDetector::analyzeConstructor(const CXXConstructorDecl *CD) { + if (CD->isImplicit()) + return true; + const FunctionDecl *Definition = CD; + if (!CD->isThisDeclarationADefinition() && !CD->isDefined(Definition)) { + HasUndefinedConstructors = true; + return !EndOfTU; + } + for (const auto *CI : cast(Definition)->inits()) { + if (!analyzeCtorInitializer(CI)) + return false; + } + return true; +} + +MismatchingNewDeleteDetector::MismatchResult +MismatchingNewDeleteDetector::analyzeInClassInitializer() { + assert(Field != nullptr && "This should be called only for members"); + if (HasUndefinedConstructors) + return EndOfTU ? NoMismatch : AnalyzeLater; + if (!NewExprs.empty()) + return MemberInitMismatches; + if (!Field->hasInClassInitializer()) + return NoMismatch; + if (const CXXNewExpr *NE = + getNewExprFromInitListOrExpr(Field->getInClassInitializer())) { + if (NE->isArray() != IsArrayForm) { + NewExprs.push_back(NE); + return MemberInitMismatches; + } + } + return NoMismatch; +} + +MismatchingNewDeleteDetector::MismatchResult +MismatchingNewDeleteDetector::analyzeField() { + assert(Field != nullptr && + "Analysis must be run from Sema::ActOnCXXDelete, with pointer to " + "CXXDeleteExpr or from Sema::ActOnEndOfTranslationUnit, where 'Field' " + "is non-null"); + const CXXRecordDecl *RD = cast(Field->getParent()); + for (const auto *CD : RD->ctors()) { + if (!analyzeConstructor(CD)) + return NoMismatch; + } + return analyzeInClassInitializer(); +} + +MismatchingNewDeleteDetector::MismatchResult +MismatchingNewDeleteDetector::mismatchedMemberInitsNewExprs( + const MemberExpr *ME) { + assert(ME != nullptr && "Expected a member expression"); + Field = dyn_cast(ME->getMemberDecl()); + if (!Field) + return NoMismatch; + return analyzeField(); +} + +bool +MismatchingNewDeleteDetector::mismatchedVarInitNewExpr(const DeclRefExpr *D) { + const CXXNewExpr *NE = nullptr; + if (const VarDecl *VD = dyn_cast(D->getDecl())) { + if (VD->hasInit() && (NE = getNewExprFromInitListOrExpr(VD->getInit())) && + NE->isArray() != IsArrayForm) { + NewExprs.push_back(NE); + } + } + return !NewExprs.empty(); +} + +void Sema::DiagnoseMismatchedNewDelete(FieldDecl *FD, SourceLocation DeleteLoc, + bool IsArrayForm) { + SemaDiagnosticBuilder D = Diag(DeleteLoc, diag::warn_mismatched_delete_new) + << IsArrayForm; + SourceLocation EndOfDelete = getLocForEndOfToken(DeleteLoc); + if (!IsArrayForm) + D << FixItHint::CreateInsertion(EndOfDelete, "[]"); + else { + // FIXME: This seems... inefficient. + SourceLocation RSquare = Lexer::findLocationAfterToken( + DeleteLoc, tok::l_square, SourceMgr, getLangOpts(), true); + if (RSquare.isInvalid()) + return; + + D << FixItHint::CreateRemoval(SourceRange(EndOfDelete, RSquare)); + } +} + +void Sema::AnalyzeDeleteExprMismatch(const CXXDeleteExpr *DE) { + if (Diags.isIgnored(diag::warn_mismatched_delete_new, SourceLocation())) + return; + MismatchingNewDeleteDetector Detector(DE); + switch (Detector.analyzeDeleteExpr()) { + case MismatchingNewDeleteDetector::VarInitMismatches: + case MismatchingNewDeleteDetector::MemberInitMismatches: { + DiagnoseMismatchedNewDelete(Detector.Field, DE->getLocStart(), + DE->isArrayForm()); + for (const auto *NE : Detector.NewExprs) + Diag(NE->getExprLoc(), diag::note_allocated_here) << DE->isArrayForm(); + break; + } + case MismatchingNewDeleteDetector::AnalyzeLater: { + DeleteExprs[Detector.Field].push_back( + std::make_pair(DE->getLocStart(), DE->isArrayForm())); + break; + } + case MismatchingNewDeleteDetector::NoMismatch: + break; + } +} + +void Sema::AnalyzeDeleteExprMismatch(FieldDecl *Field, SourceLocation DeleteLoc, + bool DeleteWasArrayForm) { + MismatchingNewDeleteDetector Detector(Field, DeleteWasArrayForm); + switch (Detector.analyzeDeleteExpr()) { + case MismatchingNewDeleteDetector::VarInitMismatches: + llvm_unreachable("This analysis should have been done for class members."); + case MismatchingNewDeleteDetector::AnalyzeLater: + llvm_unreachable("Analysis cannot be postponed any point beyond end of " + "translation unit."); + case MismatchingNewDeleteDetector::MemberInitMismatches: + DiagnoseMismatchedNewDelete(Detector.Field, DeleteLoc, DeleteWasArrayForm); + for (const auto *NE : Detector.NewExprs) + Diag(NE->getExprLoc(), diag::note_allocated_here) << DeleteWasArrayForm; + break; + case MismatchingNewDeleteDetector::NoMismatch: + break; + } +} + /// ActOnCXXDelete - Parsed a C++ 'delete' expression (C++ 5.3.5), as in: /// @code ::delete ptr; @endcode /// or @@ -2343,12 +2614,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() @@ -2423,7 +2688,7 @@ DeleteName); MarkFunctionReferenced(StartLoc, OperatorDelete); - + // Check access and ambiguity of operator delete and destructor. if (PointeeRD) { if (CXXDestructorDecl *Dtor = LookupDestructor(PointeeRD)) { @@ -2433,9 +2698,11 @@ } } - return new (Context) CXXDeleteExpr( + CXXDeleteExpr *Result = new (Context) CXXDeleteExpr( Context.VoidTy, UseGlobal, ArrayForm, ArrayFormAsWritten, UsualArrayDeleteWantsSize, OperatorDelete, Ex.get(), StartLoc); + AnalyzeDeleteExprMismatch(Result); + return Result; } /// \brief Check the use of the given variable as a C++ condition in an if, Index: lib/Serialization/ASTReader.cpp =================================================================== --- lib/Serialization/ASTReader.cpp +++ lib/Serialization/ASTReader.cpp @@ -3202,6 +3202,18 @@ ReadSourceLocation(F, Record, I).getRawEncoding()); } break; + case DELETE_EXPRS_TO_ANALYZE: + for (unsigned I = 0, N = Record.size(); I != N;) { + DelayedDeleteExprs.push_back(getGlobalDeclID(F, Record[I++])); + const uint64_t Count = ReadAPInt(Record, I).getZExtValue(); + DelayedDeleteExprs.push_back(Count); + for (uint64_t C = 0; C < Count; ++C) { + DelayedDeleteExprs.push_back(ReadSourceLocation(F, Record, I).getRawEncoding()); + bool IsArrayForm = Record[I++] == 1; + DelayedDeleteExprs.push_back(IsArrayForm); + } + } + break; case IMPORTED_MODULES: { if (F.Kind != MK_Module) { @@ -7150,6 +7162,21 @@ } } +void ASTReader::ReadMismatchingDeleteExpressions(llvm::DenseMap< + FieldDecl *, llvm::SmallVector, 4>> & + Exprs) { + for (unsigned Idx = 0, N = DelayedDeleteExprs.size(); Idx != N;) { + FieldDecl *FD = cast(GetDecl(DelayedDeleteExprs[Idx++])); + uint64_t Count = DelayedDeleteExprs[Idx++]; + for (uint64_t C = 0; C < Count; ++C) { + SourceLocation DeleteLoc = + SourceLocation::getFromRawEncoding(DelayedDeleteExprs[Idx++]); + const bool IsArrayForm = DelayedDeleteExprs[Idx++]; + Exprs[FD].push_back(std::make_pair(DeleteLoc, IsArrayForm)); + } + } +} + void ASTReader::ReadTentativeDefinitions( SmallVectorImpl &TentativeDefs) { for (unsigned I = 0, N = TentativeDefinitions.size(); I != N; ++I) { Index: lib/Serialization/ASTWriter.cpp =================================================================== --- lib/Serialization/ASTWriter.cpp +++ lib/Serialization/ASTWriter.cpp @@ -4337,6 +4337,22 @@ AddSourceLocation(I->second, UndefinedButUsed); } + // Build a record containing all delete-expressions that we would like to + // analyze later in AST. + RecordData DeleteExprsToAnalyze; + + llvm::DenseMap, + 4>> DeleteExprs; + SemaRef.getMismatchingDeleteExpressions(DeleteExprs); + for (const auto &DeleteExprsInfo : DeleteExprs) { + AddDeclRef(DeleteExprsInfo.first, DeleteExprsToAnalyze); + AddAPInt(APInt(8, DeleteExprsInfo.second.size()), DeleteExprsToAnalyze); + for (const auto &DeleteLoc : DeleteExprsInfo.second) { + AddSourceLocation(DeleteLoc.first, DeleteExprsToAnalyze); + DeleteExprsToAnalyze.push_back(DeleteLoc.second); + } + } + // Write the control block WriteControlBlock(PP, Context, isysroot, OutputFile); @@ -4594,7 +4610,10 @@ // Write the undefined internal functions and variables, and inline functions. if (!UndefinedButUsed.empty()) Stream.EmitRecord(UNDEFINED_BUT_USED, UndefinedButUsed); - + + if (!DeleteExprsToAnalyze.empty()) + Stream.EmitRecord(DELETE_EXPRS_TO_ANALYZE, DeleteExprsToAnalyze); + // Write the visible updates to DeclContexts. for (auto *DC : UpdatedDeclContexts) WriteDeclContextVisibleUpdate(DC); 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-mismatch.h =================================================================== --- /dev/null +++ test/SemaCXX/delete-mismatch.h @@ -0,0 +1,15 @@ +// Header for PCH test delete.cpp +namespace pch_test { +struct X { + int *a; + X(); + X(int); + X(bool) + : a(new int[1]) { } // expected-note{{allocated with 'new[]' here}} + ~X() + { + delete a; // expected-warning{{'delete' applied to a pointer that was allocated with 'new[]'; did you mean 'delete[]'?}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:9}:"[]" + } +}; +} Index: test/SemaCXX/delete.cpp =================================================================== --- test/SemaCXX/delete.cpp +++ test/SemaCXX/delete.cpp @@ -1,9 +1,129 @@ -// RUN: %clang_cc1 -fsyntax-only -verify %s -// RUN: cp %s %t -// RUN: %clang_cc1 -fixit -x c++ %t -// RUN: %clang_cc1 -E -o - %t | FileCheck %s +// Test without PCH +// RUN: %clang_cc1 -fsyntax-only -include %S/delete-mismatch.h -fdiagnostics-parseable-fixits -std=c++11 %s 2>&1 | FileCheck %s + +// Test with PCH +// RUN: %clang_cc1 -x c++-header -std=c++11 -emit-pch -o %t %S/delete-mismatch.h +// RUN: %clang_cc1 -std=c++11 -include-pch %t -DWITH_PCH -fsyntax-only -verify %s -ast-dump void f(int a[10][20]) { - // CHECK: delete[] a; delete a; // expected-warning {{'delete' applied to a pointer-to-array type}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:9}:"[]" } +namespace MemberCheck { +struct S { + int *a = new int[5]; // expected-note4 {{allocated with 'new[]' here}} + int *b; + int *c; + static int *d; + S(); + S(int); + ~S() { + delete a; // expected-warning {{'delete' applied to a pointer that was allocated with 'new[]'; did you mean 'delete[]'?}} + 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'?}} + } + void f(); +}; + +void S::f() +{ + delete a; // expected-warning {{'delete' applied to a pointer that was allocated with 'new[]'; did you mean 'delete[]'?}} + delete b; // expected-warning {{'delete' applied to a pointer that was allocated with 'new[]'; did you mean 'delete[]'?}} +} + +S::S() +: b(new int[1]), c(new int) {} // expected-note3 {{allocated with 'new[]' here}} +// expected-note@-1 {{allocated with 'new' here}} + +S::S(int i) +: b(new int[i]), c(new int) {} // expected-note3 {{allocated with 'new[]' here}} +// expected-note@-1 {{allocated with 'new' here}} + +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}} + delete a; // expected-warning {{'delete' applied to a pointer that was allocated with 'new[]'; did you mean 'delete[]'?}} + 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[]'?}} +} + +// At least one constructor initializes field with matching form of 'new'. +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; + } +}; + +// At least one constructor's body is missing; no proof of mismatch. +struct CantProve_MissingCtorDefinition { + int *p; + CantProve_MissingCtorDefinition(); + CantProve_MissingCtorDefinition(int); + ~CantProve_MissingCtorDefinition(); +}; + +CantProve_MissingCtorDefinition::CantProve_MissingCtorDefinition() + : p(new int) +{ } + +CantProve_MissingCtorDefinition::~CantProve_MissingCtorDefinition() +{ + 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 { +#define DELETE_ARRAY(x) delete[] (x) +#define DELETE(x) delete (x) +void f() { + int *a = new int(5); // expected-note2 {{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-note2 {{allocated with 'new[]' here}} + delete [ ] c; // expected-warning {{'delete[]' applied to a pointer that was allocated with 'new'; did you mean 'delete'?}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:17}:"" + delete d; // expected-warning {{'delete' applied to a pointer that was allocated with 'new[]'; did you mean 'delete[]'?}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:9}:"[]" + DELETE_ARRAY(a); // 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[]'?}} +} +} +#ifndef WITH_PCH +pch_test::X::X() + : a(new int[1]) // expected-note{{allocated with 'new[]' here}} +{ } +pch_test::X::X(int i) + : a(new int[i]) // expected-note{{allocated with 'new[]' here}} +{ } +#endif