[analyzer] Fix a false memory leak reports involving placement new #144341
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Placement new does not allocate memory, so it should not be reported as a memory leak. A recent MallocChecker refactor changed inlining of placement-new calls with manual evaluation by MallocChecker. llvm@339282d This change avoids marking the value returned by placement new as allocated and hence avoids the false leak reports. --- CPP-6375
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang Author: Arseniy Zaostrovnykh (necto) ChangesPlacement new does not allocate memory, so it should not be reported as a memory leak. A recent MallocChecker refactor changed inlining of placement-new calls with manual evaluation by MallocChecker. 339282d This change avoids marking the value returned by placement new as allocated and hence avoids the false leak reports. Note that the there are two syntaxes to invoke placement new: CPP-6375 Full diff: https://github.com/llvm/llvm-project/pull/144341.diff 2 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index fef33509c0b6e..4a78f64797bba 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -1371,6 +1371,19 @@ void MallocChecker::checkIfFreeNameIndex(ProgramStateRef State,
C.addTransition(State);
}
+bool isVoidStar(QualType T) {
+ return !T.isNull() && T->isPointerType() && T->getPointeeType()->isVoidType();
+}
+
+const Expr* getPlacementNewBufferArg(const CallExpr *CE, const FunctionDecl *FD) {
+ if (CE->getNumArgs() == 1)
+ return nullptr;
+ // Second argument of placement new must be void*
+ if (!isVoidStar(FD->getParamDecl(1)->getType()))
+ return nullptr;
+ return CE->getArg(1);
+}
+
void MallocChecker::checkCXXNewOrCXXDelete(ProgramStateRef State,
const CallEvent &Call,
CheckerContext &C) const {
@@ -1386,6 +1399,14 @@ void MallocChecker::checkCXXNewOrCXXDelete(ProgramStateRef State,
// processed by the checkPostStmt callbacks for CXXNewExpr and
// CXXDeleteExpr.
const FunctionDecl *FD = C.getCalleeDecl(CE);
+ if (const auto *BufArg = getPlacementNewBufferArg(CE, FD)) {
+ // Placement new does not allocate memory
+ auto RetVal = State->getSVal(BufArg, Call.getLocationContext());
+ State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+ C.addTransition(State);
+ return;
+ }
+
switch (FD->getOverloadedOperator()) {
case OO_New:
State = MallocMemAux(C, Call, CE->getArg(0), UndefinedVal(), State,
diff --git a/clang/test/Analysis/NewDelete-checker-test.cpp b/clang/test/Analysis/NewDelete-checker-test.cpp
index 06754f669b1e6..0820600a63559 100644
--- a/clang/test/Analysis/NewDelete-checker-test.cpp
+++ b/clang/test/Analysis/NewDelete-checker-test.cpp
@@ -26,9 +26,10 @@
// RUN: -analyzer-checker=cplusplus.NewDeleteLeaks
//
// RUN: %clang_analyze_cc1 -std=c++17 -fblocks -verify %s \
-// RUN: -verify=expected,leak \
+// RUN: -verify=expected,leak,inspection \
// RUN: -analyzer-checker=core \
-// RUN: -analyzer-checker=cplusplus.NewDeleteLeaks
+// RUN: -analyzer-checker=cplusplus.NewDeleteLeaks \
+// RUN: -analyzer-checker=debug.ExprInspection
#include "Inputs/system-header-simulator-cxx.h"
@@ -63,6 +64,42 @@ void testGlobalNoThrowPlacementExprNewBeforeOverload() {
int *p = new(std::nothrow) int;
} // leak-warning{{Potential leak of memory pointed to by 'p'}}
+//----- Standard pointer placement operators
+void testGlobalPointerPlacementNew() {
+ int i;
+ void *p1 = operator new(0, &i); // no warn
+
+ void *p2 = operator new[](0, &i); // no warn
+
+ int *p3 = new(&i) int; // no warn
+
+ int *p4 = new(&i) int[0]; // no warn
+}
+
+template<typename T>
+void clang_analyzer_dump(T x);
+
+void testPlacementNewBufValue() {
+ int i = 10;
+ int *p = new(&i) int;
+ clang_analyzer_dump(p); // inspection-warning{{&i}}
+ clang_analyzer_dump(*p); // inspection-warning{{10}}
+}
+
+void testPlacementNewBufValueExplicitOp() {
+ int i = 10;
+ int *p = (int*)operator new(sizeof(int), &i);
+ clang_analyzer_dump(p); // inspection-warning{{&i}}
+ clang_analyzer_dump(*p); // inspection-warning{{10}}
+}
+
+void testPlacementArrNewBufValueExplicitArrOp() {
+ int i = 10;
+ int *p = (int*)operator new[](sizeof(int), &i);
+ clang_analyzer_dump(p); // inspection-warning{{&i}}
+ clang_analyzer_dump(*p); // inspection-warning{{10}}
+}
+
//----- Other cases
void testNewMemoryIsInHeap() {
int *p = new int;
|
Contributor
Author
Member
|
@llvm/pr-subscribers-clang-static-analyzer-1 Author: Arseniy Zaostrovnykh (necto) ChangesPlacement new does not allocate memory, so it should not be reported as a memory leak. A recent MallocChecker refactor changed inlining of placement-new calls with manual evaluation by MallocChecker. 339282d This change avoids marking the value returned by placement new as allocated and hence avoids the false leak reports. Note that the there are two syntaxes to invoke placement new: CPP-6375 Full diff: https://github.com/llvm/llvm-project/pull/144341.diff 2 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index fef33509c0b6e..4a78f64797bba 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -1371,6 +1371,19 @@ void MallocChecker::checkIfFreeNameIndex(ProgramStateRef State,
C.addTransition(State);
}
+bool isVoidStar(QualType T) {
+ return !T.isNull() && T->isPointerType() && T->getPointeeType()->isVoidType();
+}
+
+const Expr* getPlacementNewBufferArg(const CallExpr *CE, const FunctionDecl *FD) {
+ if (CE->getNumArgs() == 1)
+ return nullptr;
+ // Second argument of placement new must be void*
+ if (!isVoidStar(FD->getParamDecl(1)->getType()))
+ return nullptr;
+ return CE->getArg(1);
+}
+
void MallocChecker::checkCXXNewOrCXXDelete(ProgramStateRef State,
const CallEvent &Call,
CheckerContext &C) const {
@@ -1386,6 +1399,14 @@ void MallocChecker::checkCXXNewOrCXXDelete(ProgramStateRef State,
// processed by the checkPostStmt callbacks for CXXNewExpr and
// CXXDeleteExpr.
const FunctionDecl *FD = C.getCalleeDecl(CE);
+ if (const auto *BufArg = getPlacementNewBufferArg(CE, FD)) {
+ // Placement new does not allocate memory
+ auto RetVal = State->getSVal(BufArg, Call.getLocationContext());
+ State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+ C.addTransition(State);
+ return;
+ }
+
switch (FD->getOverloadedOperator()) {
case OO_New:
State = MallocMemAux(C, Call, CE->getArg(0), UndefinedVal(), State,
diff --git a/clang/test/Analysis/NewDelete-checker-test.cpp b/clang/test/Analysis/NewDelete-checker-test.cpp
index 06754f669b1e6..0820600a63559 100644
--- a/clang/test/Analysis/NewDelete-checker-test.cpp
+++ b/clang/test/Analysis/NewDelete-checker-test.cpp
@@ -26,9 +26,10 @@
// RUN: -analyzer-checker=cplusplus.NewDeleteLeaks
//
// RUN: %clang_analyze_cc1 -std=c++17 -fblocks -verify %s \
-// RUN: -verify=expected,leak \
+// RUN: -verify=expected,leak,inspection \
// RUN: -analyzer-checker=core \
-// RUN: -analyzer-checker=cplusplus.NewDeleteLeaks
+// RUN: -analyzer-checker=cplusplus.NewDeleteLeaks \
+// RUN: -analyzer-checker=debug.ExprInspection
#include "Inputs/system-header-simulator-cxx.h"
@@ -63,6 +64,42 @@ void testGlobalNoThrowPlacementExprNewBeforeOverload() {
int *p = new(std::nothrow) int;
} // leak-warning{{Potential leak of memory pointed to by 'p'}}
+//----- Standard pointer placement operators
+void testGlobalPointerPlacementNew() {
+ int i;
+ void *p1 = operator new(0, &i); // no warn
+
+ void *p2 = operator new[](0, &i); // no warn
+
+ int *p3 = new(&i) int; // no warn
+
+ int *p4 = new(&i) int[0]; // no warn
+}
+
+template<typename T>
+void clang_analyzer_dump(T x);
+
+void testPlacementNewBufValue() {
+ int i = 10;
+ int *p = new(&i) int;
+ clang_analyzer_dump(p); // inspection-warning{{&i}}
+ clang_analyzer_dump(*p); // inspection-warning{{10}}
+}
+
+void testPlacementNewBufValueExplicitOp() {
+ int i = 10;
+ int *p = (int*)operator new(sizeof(int), &i);
+ clang_analyzer_dump(p); // inspection-warning{{&i}}
+ clang_analyzer_dump(*p); // inspection-warning{{10}}
+}
+
+void testPlacementArrNewBufValueExplicitArrOp() {
+ int i = 10;
+ int *p = (int*)operator new[](sizeof(int), &i);
+ clang_analyzer_dump(p); // inspection-warning{{&i}}
+ clang_analyzer_dump(*p); // inspection-warning{{10}}
+}
+
//----- Other cases
void testNewMemoryIsInHeap() {
int *p = new int;
|
balazs-benics-sonarsource
requested review from
NagyDonat,
Xazax-hun and
pskrgag
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
tbaederr
reviewed
Jun 16, 2025
Co-authored-by: Balázs Benics <[email protected]>
NagyDonat
approved these changes
Jun 16, 2025
Contributor
NagyDonat
left a comment
NagyDonat
left a comment
There was a problem hiding this comment.
LGTM, nice little patch 😄
pskrgag
reviewed
Jun 16, 2025
pskrgag
approved these changes
Jun 16, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Placement new does not allocate memory, so it should not be reported as a memory leak. A recent MallocChecker refactor changed inlining of placement-new calls with manual evaluation by MallocChecker. 339282d
This change avoids marking the value returned by placement new as allocated and hence avoids the false leak reports.
Note that the there are two syntaxes to invoke placement new:
new (p) intand an explicit operator calloperator new(sizeof(int), p). The first syntax was already properly handled by the engine. This change corrects handling of the second syntax.CPP-6375