[libcxx-commits] [PATCH] D105962: [libcxx] [test] Fix mismatches between aligned operator new and std::free

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo created this revision.
mstorsjo requested review of this revision.
Herald added a project: libc++.
Herald added a reviewer: libc++.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105962

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


Index: libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/delete_align_val_t_replace.pass.cpp
===================================================================
--- libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/delete_align_val_t_replace.pass.cpp
+++ libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/delete_align_val_t_replace.pass.cpp
@@ -18,12 +18,6 @@
 // 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.
@@ -48,6 +42,16 @@
     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;
@@ -63,7 +67,7 @@
 void operator delete(void* p, std::align_val_t) TEST_NOEXCEPT
 {
     ++aligned_delete_called;
-    std::free(p);
+    DoNotOptimize(p);
 }
 
 struct alignas(OverAligned) A {};
Index: libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/delete_align_val_t_replace.pass.cpp
===================================================================
--- libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/delete_align_val_t_replace.pass.cpp
+++ libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/delete_align_val_t_replace.pass.cpp
@@ -18,12 +18,6 @@
 // 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.
@@ -48,13 +42,23 @@
     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);
@@ -63,7 +67,7 @@
 void operator delete [] (void* p, std::align_val_t) TEST_NOEXCEPT
 {
     ++aligned_delete_called;
-    std::free(p);
+    DoNotOptimize(p);
 }
 
 struct alignas(OverAligned) A {};


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D105962.358527.patch
Type: text/x-patch
Size: 3640 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20210714/1cc3d618/attachment-0001.bin>