diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp --- a/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp @@ -111,32 +111,57 @@ if (!SizeOfPlaceCI) return true; - if ((SizeOfPlaceCI->getValue() < SizeOfTargetCI->getValue()) || - (IsArrayTypeAllocated && - SizeOfPlaceCI->getValue() >= SizeOfTargetCI->getValue())) { + const llvm::APSInt &PlaceSize = SizeOfPlaceCI->getValue(); + const llvm::APSInt &TargetSize = SizeOfTargetCI->getValue(); + + bool ArrayOverheadCondition = + // C++17 and earlier standards state that: + // new(2, f) T[5] results in a call of operator new[](sizeof(T) * 5 + + // y, 2, f). Here, ... and y are non-negative unspecified values + // representing array allocation overhead; the result of the + // new-expression will be offset by this amount from the value returned + // by operator new[]. This overhead may be applied in all array + // new-expressions, including those referencing the library function + // operator new[](std::size_t, void*) and other placement allocation + // functions. The amount of overhead may vary from one invocation of + // new to another. + // Since the array overhead is an *unspecified* value, it makes sense to + // warn only if the size of the Place is equal to the size of the Target. + // Otherwise, e.g if we assumed that the overhead is sizeof(size_t), + // then we might report a false positive. + IsArrayTypeAllocated && (PlaceSize == TargetSize) && + // Checking for array cookie does not makes sense if the standard is + // C++20 or later. C++20 states that array overhead(cookie) is not + // created if the new expression calls the non-allocating (placement) + // form of the allocation function. + // C++20, section 7.6.2.7 [expr.new], paragraph 15: + // That argument shall be no less than the size of the object being + // created; it may be greater than the size of the object being created + // only if the object is an array and the allocation function is not a + // non-allocating form (17.6.2.3). + // C++20, section 7.6.2.7 [expr.new], paragraph 19: + // This overhead may be applied in all array new-expressions, including + // those referencing a placement allocation function, except when + // referencing the library function operator new[](std::size_t, void*). + // Related Defect Report: + // 2382. Array allocation overhead for non-allocating placement new + // https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#2382 + !C.getASTContext().getLangOpts().CPlusPlus20; + + if ((PlaceSize < TargetSize) || ArrayOverheadCondition) { 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()) + if (ArrayOverheadCondition) 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(), SizeOfTargetCI->getValue())); + "whereas the allocated array type requires a " + "non-negative unspecified amount for array allocation overhead", + PlaceSize, TargetSize)); 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())); + PlaceSize, TargetSize)); auto R = std::make_unique(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 --- a/clang/test/Analysis/placement-new.cpp +++ b/clang/test/Analysis/placement-new.cpp @@ -2,7 +2,14 @@ // RUN: -analyzer-checker=core \ // RUN: -analyzer-checker=cplusplus.NewDelete \ // RUN: -analyzer-checker=cplusplus.PlacementNew \ -// RUN: -analyzer-output=text -verify \ +// RUN: -analyzer-output=text -verify=expected,cpp11 \ +// RUN: -triple x86_64-unknown-linux-gnu + +// RUN: %clang_analyze_cc1 -std=c++20 %s \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=cplusplus.NewDelete \ +// RUN: -analyzer-checker=cplusplus.PlacementNew \ +// RUN: -analyzer-output=text -verify=expected \ // RUN: -triple x86_64-unknown-linux-gnu #include "Inputs/system-header-simulator-cxx.h" @@ -157,27 +164,51 @@ } // namespace testHierarchy namespace testArrayTypesAllocation { -void f1() { +void test_less() { struct S { short a; }; - - // 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/2]; // expected-note {{'buffer1' initialized here}} + ::new (buffer1) S[N]; // expected-warning{{Storage provided to placement new is only 32 bytes, whereas the allocated type requires 64 bytes}} expected-note 1 {{}} } -void f2() { +void test_equal() { struct S { short a; }; + // Bad (not enough space). + // This should not appear if the standard is >= C++20. + // C++20 states that array overhead(cookie) is not created if the new + // expression calls the non-allocating (placement) form of the allocation + // function. + // C++20, section 7.6.2.7 [expr.new], paragraph 15: + // That argument shall be no less than the size of the object being + // created; it may be greater than the size of the object being created + // only if the object is an array and the allocation function is not a + // non-allocating form (17.6.2.3). + // C++20, section 7.6.2.7 [expr.new], paragraph 19: + // This overhead may be applied in all array new-expressions, including + // those referencing a placement allocation function, except when + // referencing the library function operator new[](std::size_t, void*). + // Related Defect Report: + // 2382. Array allocation overhead for non-allocating placement new + // https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#2382 - // 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 buffer1[sizeof(S) * N]; // cpp11-note {{'buffer1' initialized here}} + ::new (buffer1) S[N]; // cpp11-warning{{Storage provided to placement new is only 64 bytes, whereas the allocated array type requires a non-negative unspecified amount for array allocation overhead}} cpp11-note 1 {{}} +} + +int test_greater() { + struct s { + int x; + }; + s arr[4]; + new (arr + 1) s[1]; // no-warning + return 0; } + } // namespace testArrayTypesAllocation namespace testStructAlign {