martong updated this revision to Diff 443174.
martong added a comment.
- Replace "might" with "unspecified value"
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D129280/new/
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 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 {
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 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<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
