Author: dergachev Date: Wed Oct 31 17:43:35 2018 New Revision: 345802 URL: http://llvm.org/viewvc/llvm-project?rev=345802&view=rev Log: [analyzer] pr39348: MallocChecker: Realize that sized delete isn't custom delete.
MallocChecker no longer thinks that operator delete() that accepts the size of the object to delete (available since C++14 or under -fsized-deallocation) is some weird user-defined operator. Instead, it handles it like normal delete. Additionally, it exposes a regression in NewDelete-intersections.mm's testStandardPlacementNewAfterDelete() test, where the diagnostic is delayed from before the call of placement new into the code of placement new in the header. This happens because the check for pass-into-function-after-free for placement arguments is located in checkNewAllocator(), which happens after the allocator is inlined, which is too late. Move this use-after-free check into checkPreCall instead, where it works automagically because the guard that prevents it from working is useless and can be removed as well. This commit causes regressions under -analyzer-config c++-allocator-inlining=false but this option is essentially unsupported because the respective feature has been enabled by default quite a while ago. Differential Revision: https://reviews.llvm.org/D53543 Added: cfe/trunk/test/Analysis/NewDelete-sized-deallocation.cpp Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp cfe/trunk/test/Analysis/NewDelete-custom.cpp Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp?rev=345802&r1=345801&r2=345802&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Wed Oct 31 17:43:35 2018 @@ -712,10 +712,8 @@ bool MallocChecker::isCMemFunction(const return false; } -// Tells if the callee is one of the following: -// 1) A global non-placement new/delete operator function. -// 2) A global placement operator function with the single placement argument -// of type std::nothrow_t. +// Tells if the callee is one of the builtin new/delete operators, including +// placement operators and other standard overloads. bool MallocChecker::isStandardNewDelete(const FunctionDecl *FD, ASTContext &C) const { if (!FD) @@ -726,23 +724,11 @@ bool MallocChecker::isStandardNewDelete( Kind != OO_Delete && Kind != OO_Array_Delete) return false; - // Skip all operator new/delete methods. - if (isa<CXXMethodDecl>(FD)) - return false; - - // Return true if tested operator is a standard placement nothrow operator. - if (FD->getNumParams() == 2) { - QualType T = FD->getParamDecl(1)->getType(); - if (const IdentifierInfo *II = T.getBaseTypeIdentifier()) - return II->getName().equals("nothrow_t"); - } - - // Skip placement operators. - if (FD->getNumParams() != 1 || FD->isVariadic()) - return false; - - // One of the standard new/new[]/delete/delete[] non-placement operators. - return true; + // This is standard if and only if it's not defined in a user file. + SourceLocation L = FD->getLocation(); + // If the header for operator delete is not included, it's still defined + // in an invalid source location. Check to make sure we don't crash. + return !L.isValid() || C.getSourceManager().isInSystemHeader(L); } llvm::Optional<ProgramStateRef> MallocChecker::performKernelMalloc( @@ -1087,12 +1073,6 @@ static bool treatUnusedNewEscaped(const void MallocChecker::processNewAllocation(const CXXNewExpr *NE, CheckerContext &C, SVal Target) const { - if (NE->getNumPlacementArgs()) - for (CXXNewExpr::const_arg_iterator I = NE->placement_arg_begin(), - E = NE->placement_arg_end(); I != E; ++I) - if (SymbolRef Sym = C.getSVal(*I).getAsSymbol()) - checkUseAfterFree(Sym, C, *I); - if (!isStandardNewDelete(NE->getOperatorNew(), C.getASTContext())) return; @@ -2438,10 +2418,6 @@ void MallocChecker::checkPreCall(const C isCMemFunction(FD, Ctx, AF_IfNameIndex, MemoryOperationKind::MOK_Free))) return; - - if (ChecksEnabled[CK_NewDeleteChecker] && - isStandardNewDelete(FD, Ctx)) - return; } // Check if the callee of a method is deleted. Modified: cfe/trunk/test/Analysis/NewDelete-custom.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/NewDelete-custom.cpp?rev=345802&r1=345801&r2=345802&view=diff ============================================================================== --- cfe/trunk/test/Analysis/NewDelete-custom.cpp (original) +++ cfe/trunk/test/Analysis/NewDelete-custom.cpp Wed Oct 31 17:43:35 2018 @@ -1,12 +1,8 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,unix.Malloc -std=c++11 -analyzer-config c++-allocator-inlining=false -fblocks -verify %s -// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks,unix.Malloc -std=c++11 -analyzer-config c++-allocator-inlining=false -DLEAKS=1 -fblocks -verify %s -// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,unix.Malloc -std=c++11 -DALLOCATOR_INLINING=1 -fblocks -verify %s -// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks,unix.Malloc -std=c++11 -DLEAKS=1 -DALLOCATOR_INLINING=1 -fblocks -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks,unix.Malloc -std=c++11 -fblocks -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks,unix.Malloc -std=c++11 -fblocks -verify %s -analyzer-config c++-allocator-inlining=false #include "Inputs/system-header-simulator-cxx.h" -#if !(LEAKS && !ALLOCATOR_INLINING) // expected-no-diagnostics -#endif void *allocator(std::size_t size); @@ -24,24 +20,18 @@ public: void testNewMethod() { void *p1 = C::operator new(0); // no warn - C *p2 = new C; // no warn + C *p2 = new C; // no-warning - C *c3 = ::new C; + C *c3 = ::new C; // no-warning } -#if LEAKS && !ALLOCATOR_INLINING -// expected-warning@-2{{Potential leak of memory pointed to by 'c3'}} -#endif void testOpNewArray() { void *p = operator new[](0); // call is inlined, no warn } void testNewExprArray() { - int *p = new int[0]; + int *p = new int[0]; // no-warning } -#if LEAKS && !ALLOCATOR_INLINING -// expected-warning@-2{{Potential leak of memory pointed to by 'p'}} -#endif //----- Custom non-placement operators @@ -50,12 +40,8 @@ void testOpNew() { } void testNewExpr() { - int *p = new int; + int *p = new int; // no-warning } -#if LEAKS && !ALLOCATOR_INLINING -// expected-warning@-2{{Potential leak of memory pointed to by 'p'}} -#endif - //----- Custom NoThrow placement operators void testOpNewNoThrow() { @@ -63,11 +49,8 @@ void testOpNewNoThrow() { } void testNewExprNoThrow() { - int *p = new(std::nothrow) int; + int *p = new(std::nothrow) int; // no-warning } -#if LEAKS && !ALLOCATOR_INLINING -// expected-warning@-2{{Potential leak of memory pointed to by 'p'}} -#endif //----- Custom placement operators void testOpNewPlacement() { Added: cfe/trunk/test/Analysis/NewDelete-sized-deallocation.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/NewDelete-sized-deallocation.cpp?rev=345802&view=auto ============================================================================== --- cfe/trunk/test/Analysis/NewDelete-sized-deallocation.cpp (added) +++ cfe/trunk/test/Analysis/NewDelete-sized-deallocation.cpp Wed Oct 31 17:43:35 2018 @@ -0,0 +1,39 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus -verify -analyzer-output=text %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus -verify -analyzer-output=text %s -fsized-deallocation +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus -verify -analyzer-output=text %s -DINCLUDE_INCLUDES +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus -verify -analyzer-output=text %s -DINCLUDE_INCLUDES -fsized-deallocation +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus -verify -analyzer-output=text %s -DINCLUDE_INCLUDES -DTEST_INLINABLE_ALLOCATORS +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus -verify -analyzer-output=text %s -DINCLUDE_INCLUDES -DTEST_INLINABLE_ALLOCATORS -fsized-deallocation + +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus -verify -analyzer-output=text %s -std=c++14 +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus -verify -analyzer-output=text %s -std=c++14 -fsized-deallocation +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus -verify -analyzer-output=text %s -std=c++14 -DINCLUDE_INCLUDES +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus -verify -analyzer-output=text %s -std=c++14 -DINCLUDE_INCLUDES -fsized-deallocation +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus -verify -analyzer-output=text %s -std=c++14 -DINCLUDE_INCLUDES -DTEST_INLINABLE_ALLOCATORS +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus -verify -analyzer-output=text %s -std=c++14 -DINCLUDE_INCLUDES -DTEST_INLINABLE_ALLOCATORS -fsized-deallocation + +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus -verify -analyzer-output=text %s -std=c++17 +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus -verify -analyzer-output=text %s -std=c++17 -fsized-deallocation +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus -verify -analyzer-output=text %s -std=c++17 -DINCLUDE_INCLUDES +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus -verify -analyzer-output=text %s -std=c++17 -DINCLUDE_INCLUDES -fsized-deallocation +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus -verify -analyzer-output=text %s -std=c++17 -DINCLUDE_INCLUDES -DTEST_INLINABLE_ALLOCATORS +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus -verify -analyzer-output=text %s -std=c++17 -DINCLUDE_INCLUDES -DTEST_INLINABLE_ALLOCATORS -fsized-deallocation + +// Test all three: undeclared operator delete, operator delete forward-declared +// in the system header, operator delete defined in system header. +#ifdef INCLUDE_INCLUDES +// TEST_INLINABLE_ALLOCATORS is used within this include. +#include "Inputs/system-header-simulator-cxx.h" +#endif + +void leak() { + int *x = new int; // expected-note{{Memory is allocated}} +} // expected-warning{{Potential leak of memory pointed to by 'x'}} + // expected-note@-1{{Potential leak of memory pointed to by 'x'}} + +// This function was incorrectly diagnosed as leak under -fsized-deallocation +// because the sized operator delete was mistaken for a custom delete. +void no_leak() { + int *x = new int; // no-note + delete x; +} // no-warning _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
