Fix PR40230 - std::pair may have padding on FreeBSD.

[EricWF]

Summary

FreeBSD ships a very old and deprecated ABI for std::pair where the copy and move constructors are not allowed to be trivial. D25389 change how this was implemented by introducing a non-trivial base class. This patch, introduced in October 2016, introduced an ABI bug that caused nested std::pair instantiations to have padding. For example:

using PairT = std::pair< std::pair<char, char>, char >;
static_assert(offsetof(PairT, first) == 0, "First member should exist at offset zero"); // Fails on FreeBSD!

The bug occurs because the base class for the first element (the nested pair) cannot be put at offset zero because the top-level pair already has the same base class laid out there.

This patch fixes that ABI bug by templating the dummy base class on the same parameters as the pair.

Technically this fix is an ABI break for users who depend on the "broken" ABI introduced in 2016. I'm putting this up for review so that the FreeBSD maintainers can sign off on fixing the ABI by breaking the ABI.
Another option, since we have to "break" the ABI to fix it, would be to move FreeBSD off the deprecated non-trivial pair ABI instead.

Also see:

• https://llvm.org/PR40230
• https://reviews.llvm.org/D21329

[rsmith]

LGTM (assuming the FreeBSD folks are OK with it).

[mclow.lists]

I'm fine with this as well - assuming the FreeBSD people are happy.

include/utility

 struct _LIBCPP_TEMPLATE_VIS piecewise_construct_t { };
 #if defined(_LIBCPP_CXX03_LANG) || defined(_LIBCPP_BUILDING_LIBRARY)
 extern _LIBCPP_EXPORTED_FROM_ABI const piecewise_construct_t piecewise_construct;// = piecewise_construct_t();
 #else
 /* _LIBCPP_INLINE_VAR */ constexpr piecewise_construct_t piecewise_construct = piecewise_construct_t();
 #endif
 
 #if defined(_LIBCPP_DEPRECATED_ABI_DISABLE_PAIR_TRIVIAL_COPY_CTOR)
+template <class, class>
 struct __non_trivially_copyable_base {
   _LIBCPP_CONSTEXPR _LIBCPP_INLINE_VISIBILITY
   __non_trivially_copyable_base() _NOEXCEPT {}
   _LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_INLINE_VISIBILITY
   __non_trivially_copyable_base(__non_trivially_copyable_base const&) _NOEXCEPT {}
 };
 #endif
 
 template <class _T1, class _T2>
 struct _LIBCPP_TEMPLATE_VIS pair
 #if defined(_LIBCPP_DEPRECATED_ABI_DISABLE_PAIR_TRIVIAL_COPY_CTOR)
-: private __non_trivially_copyable_base
+: private __non_trivially_copyable_base<_T1, _T2>
 #endif
 {
     typedef _T1 first_type;
     typedef _T2 second_type;
 
     _T1 first;
     _T2 second;
 

test/libcxx/utilities/utility/pairs/pairs.pair/non_trivial_copy_move_ABI.pass.cpp

 #undef _LIBCPP_ABI_UNSTABLE
 #undef _LIBCPP_ABI_VERSION
 #define _LIBCPP_ABI_VERSION 1
 #define _LIBCPP_DEPRECATED_ABI_DISABLE_PAIR_TRIVIAL_COPY_CTOR
 
 #include <utility>
 #include <type_traits>
 #include <cstdlib>
+#include <cstddef>
 #include <cassert>
 
 #include "test_macros.h"
 
 #if !defined(_LIBCPP_DEPRECATED_ABI_DISABLE_PAIR_TRIVIAL_COPY_CTOR)
 #error trivial ctor ABI macro defined
 #endif
 

 
 struct Trivial {
     Trivial(Trivial const&) = default;
 };
 static_assert(!HasNonTrivialABI<Trivial>::value, "");
 #endif
 
 
-int main()
+void test_trivial()
 {
     {
         typedef std::pair<int, short> P;
         static_assert(std::is_copy_constructible<P>::value, "");
         static_assert(HasNonTrivialABI<P>::value, "");
     }
 #if TEST_STD_VER >= 11
     {
#if TEST_STD_VER >= 11
         static_assert(!std::is_copy_constructible<P>::value, "");
         static_assert(!std::is_trivially_copy_constructible<P>::value, "");
         static_assert(std::is_move_constructible<P>::value, "");
         static_assert(!std::is_trivially_move_constructible<P>::value, "");
         static_assert(HasNonTrivialABI<P>::value, "");
     }
 #endif
 }
+
+void test_layout() {
+    typedef std::pair<std::pair<char, char>, char> PairT;
+    static_assert(sizeof(PairT) == 3, "");
+    static_assert(TEST_ALIGNOF(PairT) == TEST_ALIGNOF(char), "");
+    static_assert(offsetof(PairT, first) == 0, "");
+}
+
+int main() {
+    test_trivial();
+    test_layout();
+}

test/libcxx/utilities/utility/pairs/pairs.pair/trivial_copy_move_ABI.pass.cpp

 // to manually turn it on.
 #if defined(__FreeBSD__)
 #define _LIBCPP_ABI_UNSTABLE
 #endif
 
 #include <utility>
 #include <type_traits>
 #include <cstdlib>
+#include <cstddef>
 #include <cassert>
 
 #include "test_macros.h"
 
 #if defined(_LIBCPP_DEPRECATED_ABI_DISABLE_PAIR_TRIVIAL_COPY_CTOR)
 #error Non-trivial ctor ABI macro defined
 #endif
 

 
 struct Trivial {
     Trivial(Trivial const&) = default;
 };
 static_assert(HasTrivialABI<Trivial>::value, "");
 #endif
 
 
-int main()
+void test_trivial()
 {
     {
         typedef std::pair<int, short> P;
         static_assert(std::is_copy_constructible<P>::value, "");
         static_assert(HasTrivialABI<P>::value, "");
     }
 #if TEST_STD_VER >= 11
     {
#if TEST_STD_VER >= 11
         static_assert(!std::is_copy_constructible<P>::value, "");
         static_assert(!std::is_trivially_copy_constructible<P>::value, "");
         static_assert(std::is_move_constructible<P>::value, "");
         static_assert(std::is_trivially_move_constructible<P>::value, "");
         static_assert(HasTrivialABI<P>::value, "");
     }
 #endif
 }
+
+void test_layout() {
+    typedef std::pair<std::pair<char, char>, char> PairT;
+    static_assert(sizeof(PairT) == 3, "");
+    static_assert(TEST_ALIGNOF(PairT) == TEST_ALIGNOF(char), "");
+    static_assert(offsetof(PairT, first) == 0, "");
+}
+
+int main() {
+    test_trivial();
+    test_layout();
+}