https://github.com/Aethezz updated
https://github.com/llvm/llvm-project/pull/150161
>From a018c87c6356326cb04bd0a98ff5842e07403f9c Mon Sep 17 00:00:00 2001
From: Aethezz <64500703+aeth...@users.noreply.github.com>
Date: Tue, 22 Jul 2025 23:19:11 -0400
Subject: [PATCH 1/3] fix false positive, warning is now only triggered when
the place size is strictly less than target size. Add test cases for when
place size == or > or < than target size.
---
.../Checkers/CheckPlacementNew.cpp | 28 ++++---------------
clang/test/SemaCXX/placement-new-bounds.cpp | 25 +++++++++++++++++
2 files changed, 30 insertions(+), 23 deletions(-)
create mode 100644 clang/test/SemaCXX/placement-new-bounds.cpp
diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
b/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
index 839c8bcd90210..4c4ca15efa4c9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
@@ -111,32 +111,14 @@ bool PlacementNewChecker::checkPlaceCapacityIsSufficient(
if (!SizeOfPlaceCI)
return true;
- if ((SizeOfPlaceCI->getValue() < SizeOfTargetCI->getValue()) ||
- (IsArrayTypeAllocated &&
- SizeOfPlaceCI->getValue() >= SizeOfTargetCI->getValue())) {
+ if ((SizeOfPlaceCI->getValue() < SizeOfTargetCI->getValue())) {
if (ExplodedNode *N = C.generateErrorNode(C.getState())) {
std::string Msg;
// TODO: use clang constant
- if (IsArrayTypeAllocated &&
- SizeOfPlaceCI->getValue() > SizeOfTargetCI->getValue())
- Msg = std::string(llvm::formatv(
- "{0} bytes is possibly not enough for array allocation which "
- "requires {1} bytes. Current overhead requires the size of {2} "
- "bytes",
- SizeOfPlaceCI->getValue(), SizeOfTargetCI->getValue(),
- *SizeOfPlaceCI->getValue() - SizeOfTargetCI->getValue()));
- else if (IsArrayTypeAllocated &&
- SizeOfPlaceCI->getValue() == SizeOfTargetCI->getValue())
- Msg = std::string(llvm::formatv(
- "Storage provided to placement new is only {0} bytes, "
- "whereas the allocated array type requires more space for "
- "internal needs",
- SizeOfPlaceCI->getValue()));
- else
- Msg = std::string(llvm::formatv(
- "Storage provided to placement new is only {0} bytes, "
- "whereas the allocated type requires {1} bytes",
- SizeOfPlaceCI->getValue(), SizeOfTargetCI->getValue()));
+ Msg = std::string(llvm::formatv(
+ "Storage provided to placement new is only {0} bytes, "
+ "whereas the allocated type requires {1} bytes",
+ SizeOfPlaceCI->getValue(), SizeOfTargetCI->getValue()));
auto R = std::make_unique<PathSensitiveBugReport>(SBT, Msg, N);
bugreporter::trackExpressionValue(N, NE->getPlacementArg(0), *R);
diff --git a/clang/test/SemaCXX/placement-new-bounds.cpp
b/clang/test/SemaCXX/placement-new-bounds.cpp
new file mode 100644
index 0000000000000..49a7352922140
--- /dev/null
+++ b/clang/test/SemaCXX/placement-new-bounds.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang -fsyntax-only -Xclang -analyze -Xclang
-analyzer-checker=cplusplus.PlacementNew -Xclang -verify -std=c++17 %s
+
+#include <new>
+
+void test_exact_size() {
+ void *buf = ::operator new(sizeof(int)*2);
+ int *placement_int = new (buf) int[2]; // no-warning
+ placement_int[0] = 42;
+ ::operator delete(buf);
+}
+
+void test_undersize() {
+ void *buf = ::operator new(sizeof(int)*1);
+ // Remember the exact text including the checker name, or use regex for
robustness.
+ int *placement_int = new (buf) int[2]; // expected-warning {{Storage
provided to placement new is only 4 bytes, whereas the allocated type requires
8 bytes [cplusplus.PlacementNew]}}
+ placement_int[0] = 42;
+ ::operator delete(buf);
+}
+
+void test_oversize() {
+ void *buf = ::operator new(sizeof(int)*4);
+ int *placement_int = new (buf) int[2]; // no-warning
+ placement_int[0] = 42;
+ ::operator delete(buf);
+}
\ No newline at end of file
>From d3e79946cf09a8c87e152b836b0b4f324c6cdd42 Mon Sep 17 00:00:00 2001
From: Aethezz <ellisonlao...@gmail.com>
Date: Wed, 23 Jul 2025 12:27:27 -0400
Subject: [PATCH 2/3] Fixed formatting issues and removed expected warnings
from other test cases
---
clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp | 8 ++++----
clang/test/Analysis/placement-new.cpp | 8 ++++----
clang/test/SemaCXX/placement-new-bounds.cpp | 1 -
3 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
b/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
index 4c4ca15efa4c9..2a57fa2e8ff4a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
@@ -115,10 +115,10 @@ bool PlacementNewChecker::checkPlaceCapacityIsSufficient(
if (ExplodedNode *N = C.generateErrorNode(C.getState())) {
std::string Msg;
// TODO: use clang constant
- Msg = std::string(llvm::formatv(
- "Storage provided to placement new is only {0} bytes, "
- "whereas the allocated type requires {1} bytes",
- SizeOfPlaceCI->getValue(), SizeOfTargetCI->getValue()));
+ Msg = std::string(
+ llvm::formatv("Storage provided to placement new is only {0} bytes, "
+ "whereas the allocated type requires {1} bytes",
+ SizeOfPlaceCI->getValue(),
SizeOfTargetCI->getValue()));
auto R = std::make_unique<PathSensitiveBugReport>(SBT, Msg, N);
bugreporter::trackExpressionValue(N, NE->getPlacementArg(0), *R);
diff --git a/clang/test/Analysis/placement-new.cpp
b/clang/test/Analysis/placement-new.cpp
index 766b11cf84c5f..82d1d87c59792 100644
--- a/clang/test/Analysis/placement-new.cpp
+++ b/clang/test/Analysis/placement-new.cpp
@@ -168,8 +168,8 @@ void f1() {
// bad (not enough space).
const unsigned N = 32;
- alignas(S) unsigned char buffer1[sizeof(S) * N]; // expected-note
{{'buffer1' initialized here}}
- ::new (buffer1) S[N]; //
expected-warning{{Storage provided to placement new is only 64 bytes, whereas
the allocated array type requires more space for internal needs}} expected-note
1 {{}}
+ alignas(S) unsigned char buffer1[sizeof(S) * N];
+ ::new (buffer1) S[N];
}
void f2() {
@@ -179,8 +179,8 @@ void f2() {
// maybe ok but we need to warn.
const unsigned N = 32;
- alignas(S) unsigned char buffer2[sizeof(S) * N + sizeof(int)]; //
expected-note {{'buffer2' initialized here}}
- ::new (buffer2) S[N]; //
expected-warning{{68 bytes is possibly not enough for array allocation which
requires 64 bytes. Current overhead requires the size of 4 bytes}}
expected-note 1 {{}}
+ alignas(S) unsigned char buffer2[sizeof(S) * N + sizeof(int)];
+ ::new (buffer2) S[N];
}
} // namespace testArrayTypesAllocation
diff --git a/clang/test/SemaCXX/placement-new-bounds.cpp
b/clang/test/SemaCXX/placement-new-bounds.cpp
index 49a7352922140..89bfb1292fc3e 100644
--- a/clang/test/SemaCXX/placement-new-bounds.cpp
+++ b/clang/test/SemaCXX/placement-new-bounds.cpp
@@ -11,7 +11,6 @@ void test_exact_size() {
void test_undersize() {
void *buf = ::operator new(sizeof(int)*1);
- // Remember the exact text including the checker name, or use regex for
robustness.
int *placement_int = new (buf) int[2]; // expected-warning {{Storage
provided to placement new is only 4 bytes, whereas the allocated type requires
8 bytes [cplusplus.PlacementNew]}}
placement_int[0] = 42;
::operator delete(buf);
>From b93bd2cfdf3b8299c952e60a64e0c378e6f81b47 Mon Sep 17 00:00:00 2001
From: Aethezz <ellisonlao...@gmail.com>
Date: Fri, 25 Jul 2025 14:32:48 -0400
Subject: [PATCH 3/3] removed the redundant test file, update comments in
placement-new.cpp test file to explain the removal of checker warning the
rare case of allocating extra memory, and removed std::string explicit
conversion operator
---
.../Checkers/CheckPlacementNew.cpp | 7 +++---
clang/test/Analysis/placement-new.cpp | 6 +++--
clang/test/SemaCXX/placement-new-bounds.cpp | 24 -------------------
3 files changed, 7 insertions(+), 30 deletions(-)
delete mode 100644 clang/test/SemaCXX/placement-new-bounds.cpp
diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
b/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
index 2a57fa2e8ff4a..a4ea359758f14 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
@@ -113,12 +113,11 @@ bool PlacementNewChecker::checkPlaceCapacityIsSufficient(
if ((SizeOfPlaceCI->getValue() < SizeOfTargetCI->getValue())) {
if (ExplodedNode *N = C.generateErrorNode(C.getState())) {
- std::string Msg;
- // TODO: use clang constant
- Msg = std::string(
+ std::string Msg =
llvm::formatv("Storage provided to placement new is only {0} bytes, "
"whereas the allocated type requires {1} bytes",
- SizeOfPlaceCI->getValue(),
SizeOfTargetCI->getValue()));
+ SizeOfPlaceCI->getValue(), SizeOfTargetCI->getValue());
+ // TODO: use clang constants
auto R = std::make_unique<PathSensitiveBugReport>(SBT, Msg, N);
bugreporter::trackExpressionValue(N, NE->getPlacementArg(0), *R);
diff --git a/clang/test/Analysis/placement-new.cpp
b/clang/test/Analysis/placement-new.cpp
index 82d1d87c59792..db3717286bf8f 100644
--- a/clang/test/Analysis/placement-new.cpp
+++ b/clang/test/Analysis/placement-new.cpp
@@ -166,7 +166,8 @@ void f1() {
short a;
};
- // bad (not enough space).
+ // on some systems, placement array new may allocate more memory than the
nominal size of the array
+ // in such cases, test code could be problematic, but the checker doesn't
warn here because this behavior is expected to be rare
const unsigned N = 32;
alignas(S) unsigned char buffer1[sizeof(S) * N];
::new (buffer1) S[N];
@@ -177,7 +178,8 @@ void f2() {
short a;
};
- // maybe ok but we need to warn.
+ // on some systems, placement array new may allocate more memory than the
nominal size of the array
+ // in such cases, test code could be problematic, but the checker doesn't
warn here because this behavior is expected to be rare
const unsigned N = 32;
alignas(S) unsigned char buffer2[sizeof(S) * N + sizeof(int)];
::new (buffer2) S[N];
diff --git a/clang/test/SemaCXX/placement-new-bounds.cpp
b/clang/test/SemaCXX/placement-new-bounds.cpp
deleted file mode 100644
index 89bfb1292fc3e..0000000000000
--- a/clang/test/SemaCXX/placement-new-bounds.cpp
+++ /dev/null
@@ -1,24 +0,0 @@
-// RUN: %clang -fsyntax-only -Xclang -analyze -Xclang
-analyzer-checker=cplusplus.PlacementNew -Xclang -verify -std=c++17 %s
-
-#include <new>
-
-void test_exact_size() {
- void *buf = ::operator new(sizeof(int)*2);
- int *placement_int = new (buf) int[2]; // no-warning
- placement_int[0] = 42;
- ::operator delete(buf);
-}
-
-void test_undersize() {
- void *buf = ::operator new(sizeof(int)*1);
- int *placement_int = new (buf) int[2]; // expected-warning {{Storage
provided to placement new is only 4 bytes, whereas the allocated type requires
8 bytes [cplusplus.PlacementNew]}}
- placement_int[0] = 42;
- ::operator delete(buf);
-}
-
-void test_oversize() {
- void *buf = ::operator new(sizeof(int)*4);
- int *placement_int = new (buf) int[2]; // no-warning
- placement_int[0] = 42;
- ::operator delete(buf);
-}
\ No newline at end of file
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
