martong created this revision.
martong added reviewers: isuckatcs, NoQ, bruntib, steakhal.
Herald added subscribers: manas, ASDenysPetrov, gamesh411, dkrupp, donat.nagy,
Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware,
xazax.hun.
Herald added a reviewer: Szelethus.
Herald added a project: All.
martong requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Currently, we warn if the given place is not enough to hold an extra
data for the array allocation overhead (cookie). I.e. if the size of the
place is equal to the size of the target. Also, we warn even if the size
of the place is greater than the size of the target. The current logic
does not know the size of the overhead(cookie), thus we just simply
warn and this is clearly wrong as it leads to false reports. This patch
eliminates the false reports and handles C++20 with special care.
Fixes #56264
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.
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
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D129280
Files:
clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
clang/test/Analysis/placement-new.cpp
Index: clang/test/Analysis/placement-new.cpp
===================================================================
--- clang/test/Analysis/placement-new.cpp
+++ 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 might require more space for 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 {
Index: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
+++ 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 might require more space for "
+ "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<PathSensitiveBugReport>(SBT, Msg, N);
bugreporter::trackExpressionValue(N, NE->getPlacementArg(0), *R);
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
