Skip to content

Commit 635e6d7

Browse files
authored
[analyzer] Fix FP for cplusplus.placement new llvm#149240 (llvm#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 llvm#149240
1 parent 807a82d commit 635e6d7

File tree

2 files changed

+31
-31
lines changed

2 files changed

+31
-31
lines changed

clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp

Lines changed: 5 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -111,32 +111,12 @@ bool PlacementNewChecker::checkPlaceCapacityIsSufficient(
111111
if (!SizeOfPlaceCI)
112112
return true;
113113

114-
if ((SizeOfPlaceCI->getValue() < SizeOfTargetCI->getValue()) ||
115-
(IsArrayTypeAllocated &&
116-
SizeOfPlaceCI->getValue() >= SizeOfTargetCI->getValue())) {
114+
if ((SizeOfPlaceCI->getValue() < SizeOfTargetCI->getValue())) {
117115
if (ExplodedNode *N = C.generateErrorNode(C.getState())) {
118-
std::string Msg;
119-
// TODO: use clang constant
120-
if (IsArrayTypeAllocated &&
121-
SizeOfPlaceCI->getValue() > SizeOfTargetCI->getValue())
122-
Msg = std::string(llvm::formatv(
123-
"{0} bytes is possibly not enough for array allocation which "
124-
"requires {1} bytes. Current overhead requires the size of {2} "
125-
"bytes",
126-
SizeOfPlaceCI->getValue(), SizeOfTargetCI->getValue(),
127-
*SizeOfPlaceCI->getValue() - SizeOfTargetCI->getValue()));
128-
else if (IsArrayTypeAllocated &&
129-
SizeOfPlaceCI->getValue() == SizeOfTargetCI->getValue())
130-
Msg = std::string(llvm::formatv(
131-
"Storage provided to placement new is only {0} bytes, "
132-
"whereas the allocated array type requires more space for "
133-
"internal needs",
134-
SizeOfPlaceCI->getValue()));
135-
else
136-
Msg = std::string(llvm::formatv(
137-
"Storage provided to placement new is only {0} bytes, "
138-
"whereas the allocated type requires {1} bytes",
139-
SizeOfPlaceCI->getValue(), SizeOfTargetCI->getValue()));
116+
std::string Msg =
117+
llvm::formatv("Storage provided to placement new is only {0} bytes, "
118+
"whereas the allocated type requires {1} bytes",
119+
SizeOfPlaceCI->getValue(), SizeOfTargetCI->getValue());
140120

141121
auto R = std::make_unique<PathSensitiveBugReport>(SBT, Msg, N);
142122
bugreporter::trackExpressionValue(N, NE->getPlacementArg(0), *R);

clang/test/Analysis/placement-new.cpp

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -166,21 +166,41 @@ void f1() {
166166
short a;
167167
};
168168

169-
// bad (not enough space).
169+
// On some systems, (notably before MSVC 16.7), a non-allocating placement
170+
// array new could allocate more memory than the nominal size of the array.
171+
172+
// Since CWG 2382 (implemented in MSVC 16.7), overhead was disallowed for non-allocating placement new.
173+
// See:
174+
// https://learn.microsoft.com/en-us/cpp/overview/visual-cpp-language-conformance?view=msvc-170
175+
// https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1969r0.html#2382
176+
177+
// However, as of 17.1, there is a regression when the type comes from a template
178+
// parameter where MSVC reintroduces overhead.
179+
// See:
180+
// https://developercommunity.visualstudio.com/t/10777485
181+
// https://godbolt.org/z/E1z1Tsfvj
182+
183+
// The checker doesn't warn here because this behavior only affects older
184+
// MSVC versions (<16.7) or certain specific versions (17.1).
185+
// Suppressing warnings avoids false positives on standard-compliant compilers
186+
// and modern MSVC versions, but users of affected MSVC versions should be
187+
// aware of potential buffer size issues.
188+
170189
const unsigned N = 32;
171-
alignas(S) unsigned char buffer1[sizeof(S) * N]; // expected-note {{'buffer1' initialized here}}
172-
::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 {{}}
190+
alignas(S) unsigned char buffer1[sizeof(S) * N];
191+
::new (buffer1) S[N]; // no-warning: See comments above
173192
}
174193

175194
void f2() {
176195
struct S {
177196
short a;
178197
};
179198

180-
// maybe ok but we need to warn.
199+
// On some systems, placement array new could allocate more memory than the nominal size of the array.
200+
// See the comment at f1() above for more details.
181201
const unsigned N = 32;
182-
alignas(S) unsigned char buffer2[sizeof(S) * N + sizeof(int)]; // expected-note {{'buffer2' initialized here}}
183-
::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 {{}}
202+
alignas(S) unsigned char buffer2[sizeof(S) * N + sizeof(int)];
203+
::new (buffer2) S[N]; // no-warning: See comments above
184204
}
185205
} // namespace testArrayTypesAllocation
186206

0 commit comments

Comments
 (0)