[llvm-branch-commits] [libcxx] 5de1bcb - [🍒] Unconditionally lower std::string's alignment requirement from 16 to 8 (#68925) (#79480)

[via llvm-branch-commits]

Author: Eric
Date: 2024-02-06T17:01:38-08:00
New Revision: 5de1bcb70db21cfd2c28ef70c67fd1833ce1e6da

URL: https://github.com/llvm/llvm-project/commit/5de1bcb70db21cfd2c28ef70c67fd1833ce1e6da
DIFF: https://github.com/llvm/llvm-project/commit/5de1bcb70db21cfd2c28ef70c67fd1833ce1e6da.diff

LOG: [🍒] Unconditionally lower std::string's alignment requirement from 16 to 8 (#68925) (#79480)

This change saves memory by providing the allocator more freedom to
allocate the most
efficient size class by dropping the alignment requirements for
std::string's
pointer from 16 to 8. This changes the output of std::string::max_size,
which makes it ABI breaking.

That said, the discussion concluded that we don't care about this ABI
break. and would like this change enabled universally.

The ABI break isn't one of layout or "class size", but rather the value
of "max_size()" changes, which in turn changes whether `std::bad_alloc`
or `std::length_error` is thrown for large allocations.

This change is the child of PR #68807, which enabled the change behind
an ABI flag.

Added: 
    

Modified: 
    libcxx/docs/ReleaseNotes/18.rst
    libcxx/include/__config
    libcxx/include/string
    libcxx/test/libcxx/strings/basic.string/string.capacity/allocation_size.pass.cpp
    libcxx/test/libcxx/strings/basic.string/string.capacity/max_size.pass.cpp
    libcxx/test/std/strings/basic.string/string.capacity/max_size.pass.cpp

Removed: 
    


################################################################################
diff  --git a/libcxx/docs/ReleaseNotes/18.rst b/libcxx/docs/ReleaseNotes/18.rst
index 450a6f25d1ea5e..391e0ebc52a428 100644
--- a/libcxx/docs/ReleaseNotes/18.rst
+++ b/libcxx/docs/ReleaseNotes/18.rst
@@ -275,11 +275,10 @@ ABI Affecting Changes
   results in an ABI break, however in practice we expect uses of ``std::projected`` in ABI-sensitive places to be
   extremely rare. Any error resulting from this change should result in a link-time error.
 
-- Under the unstable ABI, the internal alignment requirements for heap allocations
-  inside ``std::string`` has decreased from 16 to 8. This saves memory since string requests fewer additional
-  bytes than it did previously. However, this also changes the return value of ``std::string::max_size``
-  and can cause code compiled against older libc++ versions but linked at runtime to a new version
-  to throw a 
diff erent exception when attempting allocations that are too large
+- The internal alignment requirements for heap allocations inside ``std::string`` has decreased from 16 to 8. This
+  saves memory since string requests fewer additional bytes than it did previously. However, this also changes the
+  return value of ``std::string::max_size`` and can cause code compiled against older libc++ versions but linked at
+  runtime to a new version to throw a 
diff erent exception when attempting allocations that are too large
   (``std::bad_alloc`` vs ``std::length_error``).
 
 - The layout of some range adaptors that use the ``movable-box`` exposition-only type as an implementation

diff  --git a/libcxx/include/__config b/libcxx/include/__config
index 1b8fc4f7ba15cd..8b2eaf69d17042 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -174,11 +174,6 @@
 // The implementation moved to the header, but we still export the symbols from
 // the dylib for backwards compatibility.
 #    define _LIBCPP_ABI_DO_NOT_EXPORT_TO_CHARS_BASE_10
-// Save memory by providing the allocator more freedom to allocate the most
-// efficient size class by dropping the alignment requirements for std::string's
-// pointer from 16 to 8. This changes the output of std::string::max_size,
-// which makes it ABI breaking
-#    define _LIBCPP_ABI_STRING_8_BYTE_ALIGNMENT
 #  elif _LIBCPP_ABI_VERSION == 1
 #    if !(defined(_LIBCPP_OBJECT_FORMAT_COFF) || defined(_LIBCPP_OBJECT_FORMAT_XCOFF))
 // Enable compiling copies of now inline methods into the dylib to support

diff  --git a/libcxx/include/string b/libcxx/include/string
index e97139206d4fa7..ba169c3dbfc9e6 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -1937,12 +1937,7 @@ private:
     return (__s + (__a - 1)) & ~(__a - 1);
   }
   enum {
-    __alignment =
-#ifdef _LIBCPP_ABI_STRING_8_BYTE_ALIGNMENT
-        8
-#else
-        16
-#endif
+    __alignment = 8
   };
   static _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 size_type __recommend(size_type __s) _NOEXCEPT {
     if (__s < __min_cap) {

diff  --git a/libcxx/test/libcxx/strings/basic.string/string.capacity/allocation_size.pass.cpp b/libcxx/test/libcxx/strings/basic.string/string.capacity/allocation_size.pass.cpp
index c7df56c815a805..1110e3d3ec568a 100644
--- a/libcxx/test/libcxx/strings/basic.string/string.capacity/allocation_size.pass.cpp
+++ b/libcxx/test/libcxx/strings/basic.string/string.capacity/allocation_size.pass.cpp
@@ -6,6 +6,8 @@
 //
 //===----------------------------------------------------------------------===//
 
+// XFAIL: stdlib=apple-libc++ && target={{.+}}-apple-macosx{{10.13|10.15|11.0}}
+
 // <string>
 
 // This test demonstrates the smaller allocation sizes when the alignment
@@ -17,14 +19,8 @@
 
 #include "test_macros.h"
 
-// alignment of the string heap buffer is hardcoded to either 16 or 8
-
-const std::size_t alignment =
-#ifdef _LIBCPP_ABI_STRING_8_BYTE_ALIGNMENT
-    8;
-#else
-    16;
-#endif
+// alignment of the string heap buffer is hardcoded to 8
+const std::size_t alignment = 8;
 
 int main(int, char**) {
   std::string input_string;

diff  --git a/libcxx/test/libcxx/strings/basic.string/string.capacity/max_size.pass.cpp b/libcxx/test/libcxx/strings/basic.string/string.capacity/max_size.pass.cpp
index a3cb79522f2e1e..726570beb6d1ae 100644
--- a/libcxx/test/libcxx/strings/basic.string/string.capacity/max_size.pass.cpp
+++ b/libcxx/test/libcxx/strings/basic.string/string.capacity/max_size.pass.cpp
@@ -17,14 +17,8 @@
 
 #include "test_macros.h"
 
-// alignment of the string heap buffer is hardcoded to 16
-
-static const std::size_t alignment =
-#ifdef _LIBCPP_ABI_STRING_8_BYTE_ALIGNMENT
-    8;
-#else
-    16;
-#endif
+// alignment of the string heap buffer is hardcoded to 8
+static const std::size_t alignment = 8;
 
 template <class = int>
 TEST_CONSTEXPR_CXX20 void full_size() {

diff  --git a/libcxx/test/std/strings/basic.string/string.capacity/max_size.pass.cpp b/libcxx/test/std/strings/basic.string/string.capacity/max_size.pass.cpp
index 52dbde45dbb265..32ce1c8bf617dc 100644
--- a/libcxx/test/std/strings/basic.string/string.capacity/max_size.pass.cpp
+++ b/libcxx/test/std/strings/basic.string/string.capacity/max_size.pass.cpp
@@ -7,6 +7,12 @@
 //===----------------------------------------------------------------------===//
 
 // UNSUPPORTED: no-exceptions
+
+// After changing the alignment of the allocated pointer from 16 to 8, the exception thrown is no longer `bad_alloc`
+// but instead length_error on systems using new headers but older dylibs.
+//
+// XFAIL: stdlib=apple-libc++ && target={{.+}}-apple-macosx{{10.13|10.15|11.0}}
+
 // <string>
 
 // size_type max_size() const; // constexpr since C++20