Author: Aethezz Date: 2025-07-30T15:14:06+02:00 New Revision: 635e6d76530328b8412fbf985708dad26e3f8ea5
URL: https://github.com/llvm/llvm-project/commit/635e6d76530328b8412fbf985708dad26e3f8ea5 DIFF: https://github.com/llvm/llvm-project/commit/635e6d76530328b8412fbf985708dad26e3f8ea5.diff LOG: [analyzer] Fix FP for cplusplus.placement new #149240 (#150161) Fix false positive where warnings were asserted for placement new even when no additional space is requested The PlacementNewChecker incorrectly triggered warnings when the storage provided matched or exceeded the allocated type size, causing false positives. Now the warning triggers only when the provided storage is strictly less than the required size. Add test cases covering exact size, undersize, and oversize scenarios to validate the fix. Fixes #149240 Added: Modified: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp clang/test/Analysis/placement-new.cpp Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp index 839c8bcd90210..a227ca0cb70bb 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp @@ -111,32 +111,12 @@ 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())); + 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()); 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..50bbde29cfd59 100644 --- a/clang/test/Analysis/placement-new.cpp +++ b/clang/test/Analysis/placement-new.cpp @@ -166,10 +166,29 @@ void f1() { short a; }; - // bad (not enough space). + // On some systems, (notably before MSVC 16.7), a non-allocating placement + // array new could allocate more memory than the nominal size of the array. + + // Since CWG 2382 (implemented in MSVC 16.7), overhead was disallowed for non-allocating placement new. + // See: + // https://learn.microsoft.com/en-us/cpp/overview/visual-cpp-language-conformance?view=msvc-170 + // https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1969r0.html#2382 + + // However, as of 17.1, there is a regression when the type comes from a template + // parameter where MSVC reintroduces overhead. + // See: + // https://developercommunity.visualstudio.com/t/10777485 + // https://godbolt.org/z/E1z1Tsfvj + + // The checker doesn't warn here because this behavior only affects older + // MSVC versions (<16.7) or certain specific versions (17.1). + // Suppressing warnings avoids false positives on standard-compliant compilers + // and modern MSVC versions, but users of affected MSVC versions should be + // aware of potential buffer size issues. + 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]; // no-warning: See comments above } void f2() { @@ -177,10 +196,11 @@ void f2() { short a; }; - // maybe ok but we need to warn. + // On some systems, placement array new could allocate more memory than the nominal size of the array. + // See the comment at f1() above for more details. 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]; // no-warning: See comments above } } // namespace testArrayTypesAllocation _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
