[analyzer] Fix FP for cplusplus.placement new #149240 by Aethezz · Pull Request #150161 · llvm/llvm-project (original) (raw)

Hmm. The test case presented in that https://stackoverflow.com/questions/15254/can-placement-new-for-arrays-be-used-in-a-portable-way/75418614#75418614 link does show MSVC misbehaving as late as VS 2019 16.5 (for some reason MSVC 16.6 is missing on godbolt). https://godbolt.org/z/E891E31da

And indeed, knowing what to look for, https://learn.microsoft.com/en-us/cpp/overview/visual-cpp-language-conformance?view=msvc-170 shows

P1969R0 CWG 2382: Array allocation overhead for non-allocating placement new VS 2019 16.7 [20]

[20]: In versions through Visual Studio 2019 version 16.10, these features are enabled by the /std:c++latest compiler option. Visual Studio 2019 version 16.11 added the /std:c++20 compiler option to enable these features.

And they called it out in the VS2019 16.7 blog: https://devblogs.microsoft.com/cppblog/c20-features-in-visual-studio-2019-versions-16-7-and-16-8/

Implemented CWG 2382 Array allocation overhead for non-allocating placement new

So yeah, it wasn't just a textual bug in the standard, MSVC actually implemented it wrong. Now, since this was a defect report (rather than a C++17 -> 20 change), this fix is not gated behind /std:c++latest (or /std:c++20). But compilers MSVC 16.6 and older seemingly do have a defect here (at least for types with non-trivial destructors), which was fixed shortly after the CWG voted to accept it (wording changed in CD5 in November 2019, fix released with 16.7 in May 2020).

I also find one other similar citation in github over at https://github.com/ska-sa/spead2/blob/f711c6bd44d73ffdaed594aa163707d8d33ce5bd/src/recv_chunk_stream.cpp#L140-L144 is one other place mentioning this, and saying it's only for "polymorphic classes" and not "scalar types". As best I can determine, the distinguishing factor is having a non-trivial destructor (which would make sense, given what the count is used for in a non-placement array new).

// no extra space //omitted ~A(); ~A() = default; // 8 bytes of extra space ~A() {}; virtual ~A() = default; virtual ~A() final = default;

So I guess the question is whether clang analyzer should care about an MSVC defect fixed 5 years ago, applying only to out-of-support versions (the only Visual Studio 2019 16.x still in support is the 16.11 servicing baseline, which is fixed). And clang itself never had the bug. I suppose in a very broad view it's a "portability" issue; but the code is standards-conforming.

One could gate it behind -fms-compatibility/-fms-compatibility-version (so clangd et al would give the warning only to projects actually choosing to emulate a defective version of MSVC), like IDE integration. Or just decide it's not clang's job to give warnings about MSVC bugs, even when they may result in it miscompiling conforming code.