[libcxx] [test] Skip alloc counter checks for operations within the libc++ DLL

[mstorsjo]

Summary

If libc++ is built as a DLL, calls to operator new within the DLL aren't
overridden if a user provides their own operator in calling code.
Therefore, the alloc counter doesn't pick up on allocations done within
std::string, so skip that check if running on windows. (Technically,
we could keep the checks if running on windows when not built as a DLL,
but trying to keep the conditionals simple.)

[mstorsjo]

Ping

[mstorsjo]

@curdeius This one is the next one for getting rid of LIBCXX-WINDOWS-FIXME in the filesystem tests (this one drops the number of remaining fixmes from 7 to 3). What do you think of this one? I've tried to keep an explanatory comment next to each case.

[ldionne]

libcxx/test/std/input.output/filesystems/class.path/path.member/path.concat.pass.cpp

144–146

Should this be considered a bug in the Windows runtime? If so, it's not doing any good to work around it in our test suite, we should instead:

1. File a bug report to get it fixed, and
2. Mark the whole test as being broken until they fix it.

[mstorsjo]

libcxx/test/std/input.output/filesystems/class.path/path.member/path.concat.pass.cpp

144–146

It's not a bug in the Windows runtime that can be fixed reasonably, it's only between libc++ and the executable formats used on the platform.

The root cause is that PE-COFF DLLs are "sealed" two-level namespaces.

If you provide operator new in user code, this overrides libc++'s operator new within that executable, and within libc++ if it is linked statically.

If libc++ is linked as a DLL, all references to operator new within the DLL are hardwired at link time to the operator new provided there. It doesn't have the ELF/MachO feature of a GOT where the executable can override any symbol used in libraries loaded.

So here, the operator new that is overridden in the test, to count allocations, isn't invoked and doesn't count allocations that happen within the libc++ DLL. It does count the ones that are done by calls in the test executable.

[mstorsjo]

libcxx/test/std/input.output/filesystems/class.path/path.member/path.concat.pass.cpp

144–146

If you'd prefer, I could narrow it down to the windows+dll case - that'd be a bit more verbose, but should be doable.

[mstorsjo]

Narrowed down the condition to only when specifically built as DLL, included another file with the same issue.

[mstorsjo]

Folded yet another case of the same into the same patch.

[ldionne]

Thanks for the explanation. That sounds like a bug in the object format, basically, cause that means we can't implement C++ properly (which requires being able to override operator new).

[mstorsjo]

In D100219#2730479, @ldionne wrote:

Thanks for the explanation. That sounds like a bug in the object format, basically, cause that means we can't implement C++ properly (which requires being able to override operator new).

Yeah I guess so.

The same effect also means that one process can have multiple copies of the C runtime (there are, historically, multiple different ones that can be linked dynamically, and each DLL can also have a private copy of the C runtime linked in statically). Then one has to take care that the same runtime that e.g. allocated memory with malloc also frees it, and e.g. calls to functions like open/write/close are confined to the same instance of the runtime.

libcxx/test/std/input.output/filesystems/class.path/path.member/path.append.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // UNSUPPORTED: c++03
 
 // These tests require locale for non-char paths
 // UNSUPPORTED: libcpp-has-no-localization
 
-// XFAIL: LIBCXX-WINDOWS-FIXME
-
 // <filesystem>
 
 // class path
 
 // path& operator/=(path const&)
 // template <class Source>
 //      path& operator/=(Source const&);
 // template <class Source>
void doAppendSourceAllocTest(AppendOperatorTestcase const& TC)
   }
   // input iterator - For non-native char types, appends needs to copy the
   // iterator range into a contiguous block of memory before it can perform the
   // code_cvt conversions.
   // For "char" no allocations will be performed because no conversion is
   // required.
   // On Windows, the append method is more complex and uses intermediate
   // path objects, which causes extra allocations.
+  // In DLL builds on Windows, the overridden operator new won't pick up
+  // allocations done within the DLL, so the RequireAllocationGuard below
+  // won't necessarily see allocations in the cases where they're expected.
 #ifdef _WIN32
   bool DisableAllocations = false;
 #else
   bool DisableAllocations = std::is_same<CharT, char>::value;
 #endif
   {
     path LHS(L); PathReserve(LHS, ReserveSize);
     InputIter RHS(R);
     {
       RequireAllocationGuard  g; // requires 1 or more allocations occur by default
       if (DisableAllocations) g.requireExactly(0);
+      else TEST_ONLY_WIN32_DLL(g.requireAtLeast(0));
       LHS /= RHS;
     }
     assert(PathEq(LHS, E));
   }
   {
     path LHS(L); PathReserve(LHS, ReserveSize);
     InputIter RHS(R);
     InputIter REnd(StrEnd(R));
     {
       RequireAllocationGuard g;
       if (DisableAllocations) g.requireExactly(0);
+      else TEST_ONLY_WIN32_DLL(g.requireAtLeast(0));
       LHS.append(RHS, REnd);
     }
     assert(PathEq(LHS, E));
   }
 }
 
 template <class CharT>
 void doAppendSourceTest(AppendOperatorTestcase const& TC)

libcxx/test/std/input.output/filesystems/class.path/path.member/path.assign/move.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // UNSUPPORTED: c++03
 
-// XFAIL: LIBCXX-WINDOWS-FIXME
-
 // <filesystem>
 
 // class path
 
 // path& operator=(path&&) noexcept
 
 #include "filesystem_include.h"
 #include <type_traits>
 #include <cassert>
 
 #include "test_macros.h"
 #include "count_new.h"
 
 
 int main(int, char**) {
   using namespace fs;
   static_assert(std::is_nothrow_move_assignable<path>::value, "");
   assert(globalMemCounter.checkOutstandingNewEq(0));
   const std::string s("we really really really really really really really "
                       "really really long string so that we allocate");
-  assert(globalMemCounter.checkOutstandingNewEq(1));
+  // On windows, the operator new from count_new.h can't override the default
+  // operator for calls within the libc++ DLL.
+  TEST_NOT_WIN32_DLL(assert(globalMemCounter.checkOutstandingNewEq(1)));
   const fs::path::string_type ps(s.begin(), s.end());
   path p(s);
   {
     DisableAllocationGuard g;
     path p2;
     path& pref = (p2 = std::move(p));
     assert(p2.native() == ps);
     assert(p.native() != ps); // Testing moved from state
     assert(&pref == &p2);
   }
 
   return 0;
 }

libcxx/test/std/input.output/filesystems/class.path/path.member/path.concat.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // UNSUPPORTED: c++03
 
 // These tests require locale for non-char paths
 // UNSUPPORTED: libcpp-has-no-localization
 
-// XFAIL: LIBCXX-WINDOWS-FIXME
-
 // <filesystem>
 
 // class path
 
 // path& operator+=(const path& x);
 // path& operator+=(const string_type& x);
 // path& operator+=(string_view x);
 // path& operator+=(const value_type* x);
// CharT*
     }
     assert(LHS == E);
   }
   // input iterator - For non-native char types, appends needs to copy the
   // iterator range into a contiguous block of memory before it can perform the
   // code_cvt conversions.
   // For the path native type, no allocations will be performed because no
   // conversion is required.
+
+  // In DLL builds on Windows, the overridden operator new won't pick up
+  // allocations done within the DLL, so the RequireAllocationGuard below
+  // won't necessarily see allocations in the cases where they're expected.
   bool DisableAllocations = std::is_same<CharT, path::value_type>::value;
   {
     path LHS(L); PathReserve(LHS, ReserveSize);
     InputIter RHS(R);
     {
       RequireAllocationGuard  g; // requires 1 or more allocations occur by default
       if (DisableAllocations) g.requireExactly(0);
+      else TEST_ONLY_WIN32_DLL(g.requireAtLeast(0));
       LHS += RHS;
     }
     assert(LHS == E);
   }
   {
     path LHS(L); PathReserve(LHS, ReserveSize);
     InputIter RHS(R);
     InputIter REnd(StrEnd(R));
     {
       RequireAllocationGuard g;
       if (DisableAllocations) g.requireExactly(0);
+      else TEST_ONLY_WIN32_DLL(g.requireAtLeast(0));
       LHS.concat(RHS, REnd);
     }
     assert(LHS == E);
   }
 }
 
 template <class CharT>
 void doConcatSourceTest(ConcatOperatorTestcase const& TC)

libcxx/test/std/input.output/filesystems/class.path/path.member/path.construct/move.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // UNSUPPORTED: c++03
 
-// XFAIL: LIBCXX-WINDOWS-FIXME
-
 // <filesystem>
 
 // class path
 
 // path(path&&) noexcept
 
 #include "filesystem_include.h"
 #include <type_traits>
 #include <cassert>
 
 #include "test_macros.h"
 #include "count_new.h"
 
 
 int main(int, char**) {
   using namespace fs;
   static_assert(std::is_nothrow_move_constructible<path>::value, "");
   assert(globalMemCounter.checkOutstandingNewEq(0));
   const std::string s("we really really really really really really really "
                       "really really long string so that we allocate");
-  assert(globalMemCounter.checkOutstandingNewEq(1));
+  // On windows, the operator new from count_new.h can't override the default
+  // operator for calls within the libc++ DLL.
+  TEST_NOT_WIN32_DLL(assert(globalMemCounter.checkOutstandingNewEq(1)));
   const fs::path::string_type ps(s.begin(), s.end());
   path p(s);
   {
     DisableAllocationGuard g;
     path p2(std::move(p));
     assert(p2.native() == ps);
     assert(p.native() != ps); // Testing moved from state
   }
 
   return 0;
 }

libcxx/test/std/localization/locale.categories/category.ctype/facet.ctype.special/facet.ctype.char.dtor/dtor.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // <locale>
 
 // template <> class ctype<char>
 
 // ~ctype();
 
-// XFAIL: LIBCXX-WINDOWS-FIXME
-
 #include <locale>
 #include <cassert>
 
 #include "count_new.h"
 
 #include "test_macros.h"
 
 int main(int, char**)
assert(globalMemCounter.checkDeleteArrayCalledEq(0));
         assert(globalMemCounter.checkDeleteArrayCalledEq(0));
     }
     assert(globalMemCounter.checkDeleteArrayCalledEq(0));
     {
         std::locale l(std::locale::classic(),
             new std::ctype<char>(new std::ctype<char>::mask[256], true));
         assert(globalMemCounter.checkDeleteArrayCalledEq(0));
     }
-    assert(globalMemCounter.checkDeleteArrayCalledEq(1));
+    // On windows, the operator new from count_new.h can't override the default
+    // operator for calls within the libc++ DLL.
+    TEST_NOT_WIN32_DLL(assert(globalMemCounter.checkDeleteArrayCalledEq(1)));
 
   return 0;
 }

libcxx/test/std/thread/thread.threads/thread.thread.class/thread.thread.constr/F.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // UNSUPPORTED: libcpp-has-no-threads
 
-// XFAIL: LIBCXX-WINDOWS-FIXME
-
 // <thread>
 
 // class thread
 
 // template <class F, class ...Args> thread(F&& f, Args&&... args);
 
 // UNSUPPORTED: sanitizer-new-delete
 
throw_one = 0xFFF;
         std::thread t(f);
         t.join();
     }
     numAllocs = 0xFFF - throw_one;
     // i <= numAllocs means the last iteration is expected not to throw.
     for (int i=0; i <= numAllocs; ++i) {
         throw_one = i;
         f_run = false;
-        unsigned old_outstanding = outstanding_new;
+        TEST_NOT_WIN32_DLL(unsigned old_outstanding = outstanding_new);
         try {
             std::thread t(f);
             assert(i == numAllocs); // Only final iteration will not throw.
             t.join();
             assert(f_run);
         } catch (std::bad_alloc const&) {
             assert(i < numAllocs);
             assert(!f_run); // (2.2)
         }
-        assert(old_outstanding == outstanding_new); // (2.3)
+        // In DLL builds on Windows, the overridden operators new/delete won't
+        // override calls from within the DLL, so this won't match.
+        TEST_NOT_WIN32_DLL(assert(old_outstanding == outstanding_new)); // (2.3)
     }
     f_run = false;
     throw_one = 0xFFF;
 #endif
 }
 
 int main(int, char**)
 {

libcxx/test/support/test_macros.h

 #endif
 
 #ifdef _WIN32
 #define TEST_NOT_WIN32(...) ((void)0)
 #else
 #define TEST_NOT_WIN32(...) __VA_ARGS__
 #endif
 
+#if defined(_WIN32) && !defined(_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS)
+#define TEST_NOT_WIN32_DLL(...) ((void)0)
+#define TEST_ONLY_WIN32_DLL(...) __VA_ARGS__
+#else
+#define TEST_NOT_WIN32_DLL(...) __VA_ARGS__
+#define TEST_ONLY_WIN32_DLL(...) ((void)0)
+#endif
+
 #ifdef _WIN32
 #define TEST_WIN_NO_FILESYSTEM_PERMS_NONE
 #endif
 
 #if defined(__GNUC__)
 #pragma GCC diagnostic pop
 #endif
 
 #endif // SUPPORT_TEST_MACROS_HPP