r345802 - [analyzer] pr39348: MallocChecker: Realize that sized delete isn't custom delete.
Artem Dergachev via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 31 17:43:35 PDT 2018
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 at -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 at -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 at -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 at -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 at -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
More information about the cfe-commits
mailing list