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/2] 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/2] 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); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits