[libcxx] [test] Fix mismatches between aligned operator new and std::free

[mstorsjo]

Summary

The XFAIL comments about VCRuntime not providing aligned operator new
are outdated; these days VCRuntime does provide them.

However, the tests used to fail on Windows, as the pointers allocated
with an aligned operator new (which is implemented with _aligned_malloc
on Windows) can't be freed using std::free() on Windows (but they need
to be freed with the corresponding function _aligned_free instead).

Instead override the aligned operator new to return a dummy suitably
aligned pointer instead, like other tests that override aligned operator
new.

Also override operator delete[] instead of plain operator delete
in the array testcase; the fallback from operator delete[] to
user defined operator delete doesn't work in all DLL build
configurations on Windows.

By providing the aligned operator new within the tests, this also makes
these test cases pass when testing back deployment on macOS 10.9.

[• Quuxplusone]

libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/delete_align_val_t_replace.pass.cpp

61

Your commit summary mentions that the old test relied on

the fallback from operator delete[] to user defined operator delete

Are you sure that this was intentional, and not just a typo in the test? (Personally I didn't know that any such fallback rule existed! And wouldn't that be more of a compiler issue than a libc++ issue, anyway?)
I'd feel better if we did the delete->delete[] renaming in one commit/PR and then whatever Windows fixup is still needed, in a separate PR.

70

This DoNotOptimize seems unnecessary. I think you can just un-name the parameter.

libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/delete_align_val_t_replace.pass.cpp

33

Any intuition for whether this use of __STDCPP_DEFAULT_NEW_ALIGNMENT__ is correct, or if it should just be alignof(std::max_align_t)? (Related to my request-for-investigation on D105905.)

51

Here and line 76, the DoNotOptimize seems unnecessary.

[ldionne]

LGTM in essence, with a few comments. Thanks for looking into this.

libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/delete_align_val_t_replace.pass.cpp

47

I don't think we need TEST_THROW_SPEC at all, since this isn't tested in C++03 mode.

Same goes for TEST_NOEXCEPT -- it can just be turned into noexcept.

[mstorsjo]

libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/delete_align_val_t_replace.pass.cpp

47

Ok, I'll simplify those while touching the code.

61

I presume it was just a typo in the test, yes. (There's such a fallback, but we shouldn't be testing that implicitly everywhere but only in a test set out to specifically test that, yeah, as not all systems might support it.)

@ldionne WDYT, keep the delete -> delete[] change here or split to a separate patch?

libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/delete_align_val_t_replace.pass.cpp

33

I'm not entirely sure about the finer details about which one matters where, but I think the most correct one here might be something like std::max(__STDCPP_DEFAULT_NEW_ALIGNMENT__, alignof(std::max_align_t)) * 2, to make sure it really is more aligned than what otherwise is provided. (But this is unrelated to the patch at hand here anyway.)

51

I guess so yeah. FWIW I copied inspiration for these from new_align_val_t_replace.pass.cpp in the same dir, where you might be able to remove some such extra calls.

[mstorsjo]

Removed DoNotOptimize, simplified TEST_THROW_SPEC and TEST_NOEXCEPT assuming the test only runs on C++17.

[ldionne]

libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/delete_align_val_t_replace.pass.cpp

61

I'm fine with it being done here, since you called it out in the commit message. Pedantically, a separate patch would be better, but I'm trying to balance pedantic correctness and productivity :-).

libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/delete_align_val_t_replace.pass.cpp

 // None of the current GCC compilers support this.
 // UNSUPPORTED: gcc-5, gcc-6
 
 // Aligned allocation was not provided before macosx10.14 and as a result we
 // get availability errors when the deployment target is older than macosx10.14.
 // However, support for that was broken prior to Clang 8 and AppleClang 11.
 // UNSUPPORTED: apple-clang-9, apple-clang-10
 // UNSUPPORTED: clang-5, clang-6, clang-7
-// XFAIL: use_system_cxx_lib && target={{.+}}-apple-macosx10.{{9|10|11|12|13}}
-
-// On Windows libc++ doesn't provide its own definitions for new/delete
-// but instead depends on the ones in VCRuntime. However VCRuntime does not
-// yet provide aligned new/delete definitions so this test fails to compile/link.
-// XFAIL: LIBCXX-WINDOWS-FIXME
 
 // Libcxx when built for z/OS doesn't contain the aligned allocation functions,
 // nor does the dynamic library shipped with z/OS.
 // UNSUPPORTED: target={{.+}}-zos{{.*}}
 
 #include <new>
 #include <cstddef>
 #include <cstdlib>
 #include <cassert>
 
 #include "test_macros.h"
 
 constexpr auto OverAligned = __STDCPP_DEFAULT_NEW_ALIGNMENT__ * 2;
 
 int unsized_delete_called = 0;
 int unsized_delete_nothrow_called = 0;
 int aligned_delete_called = 0;
 
 void reset() {
     unsized_delete_called = 0;
     unsized_delete_nothrow_called = 0;
     aligned_delete_called = 0;
 }
 
-void operator delete(void* p) TEST_NOEXCEPT
+alignas(OverAligned) char DummyData[OverAligned * 4];
+
+void* operator new [] (std::size_t s, std::align_val_t) TEST_THROW_SPEC(std::bad_alloc)
+{
+    assert(s <= sizeof(DummyData));
+    void *Ret = DummyData;
+    DoNotOptimize(Ret);
+    return Ret;
+}
+
+void operator delete [] (void* p) TEST_NOEXCEPT
 {
     ++unsized_delete_called;
     std::free(p);
 }
 
-void operator delete(void* p, const std::nothrow_t&) TEST_NOEXCEPT
+void operator delete [] (void* p, const std::nothrow_t&) TEST_NOEXCEPT
 {
     ++unsized_delete_nothrow_called;
     std::free(p);
 }
 
 void operator delete [] (void* p, std::align_val_t) TEST_NOEXCEPT
 {
     ++aligned_delete_called;
-    std::free(p);
+    DoNotOptimize(p);
 }
 
 struct alignas(OverAligned) A {};
 struct alignas(std::max_align_t) B {};
 
 int main(int, char**)
 {
     reset();

libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/delete_align_val_t_replace.pass.cpp

 // None of the current GCC compilers support this.
 // UNSUPPORTED: gcc-5, gcc-6
 
 // Aligned allocation was not provided before macosx10.14 and as a result we
 // get availability errors when the deployment target is older than macosx10.14.
 // However, support for that was broken prior to Clang 8 and AppleClang 11.
 // UNSUPPORTED: apple-clang-9, apple-clang-10
 // UNSUPPORTED: clang-5, clang-6, clang-7
-// XFAIL: use_system_cxx_lib && target={{.+}}-apple-macosx10.{{9|10|11|12|13}}
-
-// On Windows libc++ doesn't provide its own definitions for new/delete
-// but instead depends on the ones in VCRuntime. However VCRuntime does not
-// yet provide aligned new/delete definitions so this test fails to compile/link.
-// XFAIL: LIBCXX-WINDOWS-FIXME
 
 // Libcxx when built for z/OS doesn't contain the aligned allocation functions,
 // nor does the dynamic library shipped with z/OS.
 // UNSUPPORTED: target={{.+}}-zos{{.*}}
 
 #include <new>
 #include <cstddef>
 #include <cstdlib>
 #include <cassert>
 
 #include "test_macros.h"
 
 constexpr auto OverAligned = __STDCPP_DEFAULT_NEW_ALIGNMENT__ * 2;
 
 int unsized_delete_called = 0;
 int unsized_delete_nothrow_called = 0;
 int aligned_delete_called = 0;
 
 void reset() {
     unsized_delete_called = 0;
     unsized_delete_nothrow_called = 0;
     aligned_delete_called = 0;
 }
 
+alignas(OverAligned) char DummyData[OverAligned * 4];
+
+void* operator new (std::size_t s, std::align_val_t) TEST_THROW_SPEC(std::bad_alloc)
+{
+    assert(s <= sizeof(DummyData));
+    void *Ret = DummyData;
+    DoNotOptimize(Ret);
+    return Ret;
+}
+
 void operator delete(void* p) TEST_NOEXCEPT
 {
     ++unsized_delete_called;
     std::free(p);
 }
 
 void operator delete(void* p, const std::nothrow_t&) TEST_NOEXCEPT
 {
     ++unsized_delete_nothrow_called;
     std::free(p);
 }
 
 void operator delete(void* p, std::align_val_t) TEST_NOEXCEPT
 {
     ++aligned_delete_called;
-    std::free(p);
+    DoNotOptimize(p);
 }
 
 struct alignas(OverAligned) A {};
 struct alignas(std::max_align_t) B {};
 
 int main(int, char**)
 {
     reset();