[libc++] Use bit field for checking if string is in long or short mode

[philnik]

Summary

This makes the code a bit simpler and (I think) removes the undefined behaviour from the normal string layout.

[philnik]

• Fix pretty printers

[Mordante]

In general I like this direction, but I'm somewhat concerned regarding ABI breaks.

libcxx/include/string

681

Do we have a test that validates the size of this struct?

On some platforms (IIRC AIX) the members __cap_ and __is_long_ won't be stored in one size_type since they have different types.

681

Did you verify this isn't an ABI break when libc++14 and libc++15 compiled code is mixed?
We should make sure that doesn't break an big and little endian.

[Mordante]

libcxx/include/string

696

Since you now understand how this works, it would be nice to add some documentation.

717

Is this a copy paste?

1500

Mainly curious, but why not a _LIBCPP_ASSERT?

libcxx/utils/gdb/libcxx/printers.py

192

Does this also break the LLDB pretty printer?

[philnik]

libcxx/include/string

681

https://godbolt.org/z/axTMxqaj3 is the code I used to verify that everything is OK. I think this should be good enough to say that they are equivalent. Do you know if there is any documentation about the bit field layout on different platforms?

717

No, depending on the string layout the factors have to be reversed. It's essentially: If you use the LSB for is_long you have to multiply by two, otherwise keep the same number.

1500

This ensures optimal code generation. GCC doesn't have __builtin_assume. This is a workaround.

libcxx/utils/gdb/libcxx/printers.py

192

Probably. Would be nice to have a test runner for that.

[Mordante]

libcxx/include/string

681

Good to know.
BTW it's not AIX but win32 where the different types are an issue. (AIX had something else with bitfields)
https://godbolt.org/z/n3zr5v4sr The different types make the struct 32 bytes on Windows.

I don't know where the information regarding the bit field layout can be found.
(I guess Google or the LLVM sources can help out.)

717

I see, I missed the #else on line 701.

1500

From a maintainability PoV I like _LIBCPP_ASSERT better. Would it be an option to adjust _LIBCPP_ASSERT? the "generate" this code when using GCC?

libcxx/utils/gdb/libcxx/printers.py

192

I already planned to look into that, D97044#3440904 ;-)

[ldionne]

libcxx/include/string

696

Indeed -- it would be awesome if you could add some documentation explaining how these representations work, and that may also help in catching potential issues with this patch.

[philnik]

libcxx/include/string

1500

I agree that _LIBCPP_ASSERT would be nicer to read. The problem is that using this trick for _LIBCPP_ASSERT could lead to extra code generation in more complex scenarios. We would have to introduce something else like _LIBCPP_ASSUME, which should only be used for trivial cases. But for two instances I don't think this is justified.

libcxx/utils/gdb/libcxx/printers.py

192

Do you know where I would have to look to know what the LLDB pretty printers do?

[philnik]

• Address comments; Try to fix CI

[ldionne]

I really like this cleanup, I think it makes it much easier to understand what's going on. I do have some comments, but this is looking pretty good.

libcxx/include/string

695–700

IMO this adds a bit of clarity.

1499

I think I understand that you based the bound on the number representable by the __size_ bitfield. We could also establish it based on the maximum capacity of the small string buffer by using __s >= __min_cap instead. (>= because of the null terminator).

1500

Can you try again with _LIBCPP_ASSERT by making __libcpp_assertion_handler noreturn? This is how GCC implements their own assert(...), so presumably there might be some optimization opportunities for them there. I think this would be much better than using unreachable().

Edit: Yuck, this indeed won't help when assertions are disabled on GCC. However, looking at the codegen, it appears that the only difference is that the compiler adds a single instruction to ensure that __s <= 127. Unless you can show us a case where there is a significant difference, I think the way to go is to use _LIBCPP_ASSERT and for GCC to be able to pick it up.

1506–1507

Same comment about using _LIBCPP_ASSERT -- unless we have a very convincing reason not to use it, let's use it.

3227–3232

After discussing it, I think we agree this is a bit easier to digest.

3236

I see the CI is failing on backdeployment macOS arm64. You just explained to me that this was caused by a bug where we used to return an incorrect max_size() in Big Endian -- the test would expect that we throw length_error, but we don't and instead we try allocating, which leads to bad_alloc instead. This patch fixes that, but since max_size() has been inlined into the dylib, the test will fail in backdeployment.

Since it seems that we are fixing a bug as part of this patch, let's add a regression test for it. We can then mark that test (and the failing over_max_size.pass.cpp) as // XFAIL: use_system_cxx_lib && target=arm64{{.+}}-apple-macosx{{11.0|12.0}}, with an appropriate comment.

libcxx/utils/gdb/libcxx/printers.py

1

@saugustine The pretty-printer tests have been disabled on recent Clang versions since D118067. It would be nice to fix them.

[philnik]

libcxx/include/string

3236

I don't think we have to mark the new test as XFAIL because it doesn't use the dylib version.

[philnik]

• Address comments

[philnik]

• Use the correct max_size

[ldionne]

LGTM with fixed test and passing CI.

Note that this is really tricky. The code looks correct upon inspection, and I think we should catch pretty much any issue via CI. However, if weird stuff starts happening with std::string in the upcoming few weeks (when I'm OOO), this should be the number one suspect, and it should be considered fine to revert this in case we suspect std::string is broken.

libcxx/test/libcxx/strings/basic.string/string.capacity/max_size.pass.cpp

10

Please add a short comment explaining what you're testing in this test. We already have a max_size.pass.cpp file in test/std, so how is this one different? The comment should address that.

25–27

Instead of those #ifdefs, I think we should instead be checking for specific platforms/architectures. Otherwise, this test informs us less about the behavior that we expect on specific platforms, and more about what the library itself does. So for example, if we changed the library in a way that broke behavior on a given platform, this test could keep passing as long as we were consistent w.r.t. _LIBCPP_BIG_ENDIAN and _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT.

So instead, I suggest going for something like

#if defined(__APPLE__) && defined(__aarch64__)
// test behavior for alternate string layout
#elif ...
// test behavior for normal string layout
#endif

I don't think it should be *too* hard to come up with those #ifdefs to satisfy the CI.

libcxx/test/std/strings/basic.string/string.capacity/over_max_size.pass.cpp

11

Please add a comment explaining why you're adding this one. Something like

// Prior to http://llvm.org/D123580, there was a bug with how the max_size()
// was calculated. That was inlined into some functions in the dylib, which leads
// to failures when running this test against an older system dylib.
// XFAIL: use_system_cxx_lib && target=arm64{{.+}}-apple-macosx{{11.0|12.0}}

I don't know why the above XFAIL line doesn't have a comment, but one should have been added when we XFAILed it. Of course you don't need to add one, but that's the spirit.

[Mordante]

LGTM, with some minor remarks. I think it would be good to wait for feedback from the LLDB devs to validate whether this breaks the data formatters.

libcxx/include/string

707

Thanks for the description!

1499

libcxx/test/libcxx/strings/basic.string/string.capacity/max_size.pass.cpp

20

For completeness please add tests for std::wstring and std::u8string, both properly guarded.
Note that the sizeof(wchar_t) can be 16 or 32-bit.

libcxx/utils/gdb/libcxx/printers.py

192

Unfortunately no. @jingham seems to be the Data formatter code owner.

[jgorbe]

libcxx/utils/gdb/libcxx/printers.py

192

There was a recent lldb change fixing prettyprinters after a similar change to string: https://github.com/llvm/llvm-project/commit/45428412fd7c9900d3d6ac9803aa7dcf6adfa6fe

If the gdb prettyprinter needed fixing for this change, chances are that lldb will need a similar update too.

[philnik]

• Address comments

[philnik]

libcxx/utils/gdb/libcxx/printers.py

192

Could someone from #lldb help me figure out what to change in the pretty printers? I looked at the file, but I don't really understand how it works and TBH I don't really feel like spending a lot of time figuring it out. If nobody says anything I'll land this in a week.

As a side note: it would be really nice if there were a few more comments inside LibCxx.cpp to explain what happens there. That would make fixing the pretty printer a lot easier. The code is probably not very hard (at least it doesn't look like it), but I am looking for a few things that I can't find and I have no idea what some of the things mean.

[philnik]

• Fix CI

[philnik]

• Add Windows

[dblaikie]

libcxx/utils/gdb/libcxx/printers.py

192

Looks like something around https://github.com/llvm/llvm-project/blob/2e6ac54cf48aa04f7b05c382c33135b16d3f01ea/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp#L597 (& the similar masking in the else block a few lines down) - I guess a specific lookup for the new field would be needed, rather than the bitmasking.

[philnik]

libcxx/utils/gdb/libcxx/printers.py

192

Yes, but what do the numbers in size_mode_locations mean? Why is there no checking if it's big or little endian? Is it unsupported maybe? Does it work because of something else? Is there a reason that g_data_name exists instead of comparing directly? Should I add another layout style or should I just update the code for the new layout?
I don't know anything about the LLDB codebase, so I don't understand the code and I don't know how I should change it.

[philnik]

• Try to fix Windows

[labath]

libcxx/utils/gdb/libcxx/printers.py

192

I don't think there's been any official policy decision either way, but historically we haven't been asking libc++ authors to update lldb pretty printers -- we would just fix them up on the lldb side when we noticed the change. The thing that has changed recently is that google started relying (and testing) more on lldb, which considerably shortened the time it takes to notice this change, and also makes it difficult for some people to make progress while we are in this state. But I don't think that means that updating the pretty printer is suddenly your responsibility.

As for your questions, I'll try to answer them as best as I can:

what do the numbers in size_mode_locations mean?

These are the indexes of fields in the string object. For some reason (unknown to me), the pretty printer uses indexes rather than field names for its work. Prompted by the previous patch, I've been trying to change that, but I haven't done it yet, as I was trying to improve the testing story (more on that later).

Why is there no checking if it's big or little endian? Is it unsupported maybe?

Most likely yes. Although most parts of lldb support big endian, I am not aware of anyone testing it on a regular basis, so it's quite likely that a lot of things are in fact broken.

Is there a reason that g_data_name exists instead of comparing directly?

LLDB uses a global string pool, so this is an attempt to reduce the number of string pool queries. The pattern is not consistently used everywhere, and overall, I wouldn't be too worried about it.

Should I add another layout style or should I just update the code for the new layout?

As the pretty printers ship with lldb, they are expected to support not just the current format, but also the past ones (within reason). This is what makes adding a new format (or just refactoring the existing code) difficult, and it's why I was trying to come up with better tests for this (it remains to be seen if I am successful).

Anyway, I think I should be able to make that pretty printer work with this patch. I should have something today or tomorrow, if you're ok with waiting that long.

[philnik]

libcxx/utils/gdb/libcxx/printers.py

192

Thanks for the answers! I think that it wouldn't be that hard for us to update the pretty printers if we have some test coverage and documentation for it. For now, is there any person/group we should ping if we suspect that we break the pretty printers? I'll wait a few days. It's not that important to land this patch soon.

[ldionne]

libcxx/utils/gdb/libcxx/printers.py

192

The situation with pretty printers has been a source of frustration for the 4 years I've worked on libc++. I have been reaching out to various LLDB folks to get help setting up pre-commit CI for the LLDB pretty-printers in libc++'s own pipeline so that we can detect breakages in advance, but this has not been conclusive so far.

@labath @jgorbe Would you be willing to help us set up a CI job that runs the LLDB pretty printers (and only that) in our pre-commit CI infrastructure? We have the machines and all the infrastructure in place. We just need the right CMake + lit invocations. Also CC @Mordante , since he had been investigating that IIRC.

If we could notice breakages in advance, we could fix the pretty printers in the same patch where we make a change to libc++. We could call out for help from LLDB folks when needed. This would be a much smoother experience for everyone -- we would not need to revert our patches ever, and the LLDB folks would not be broken by changes that come out of the blue (as far as they are concerned).

[Mordante]

libcxx/utils/gdb/libcxx/printers.py

192

I've indeed been working on that, but not managed yet. I can build lldb, but the dataformatter tests return UNSUPPORTED. I haven't had time to investigate this further. I hope to find some time soon. But if @labath or @jgorbe have hints how to do this I would be interested to know. Alternatively what's the best way to contact you Discourse or Discord?

[labath]

libcxx/utils/gdb/libcxx/printers.py

192

I'm sorry about the delay. The lldb-libc++ integration is broken in several ways (different ways on different platforms), so I'm not all that surprised that it's not working for you. I don't really consider the data formatters my responsibility so I only go near them when I really have to. Still, I agree that this is not a good situation to be in, and the offer of offloading the data formatter work to the libc++ team is definitely appealing. So, I'll try to find some time to make that happen. I'm sorry I can't promise anything specific.

[Mordante]

libcxx/utils/gdb/libcxx/printers.py

192

Thanks for the information. The libc++ CI uses a Ubuntu 20.04 Docker image on amd64.

libcxx/include/string

 #include <__debug>
 #include <__format/enable_insertable.h>
 #include <__ios/fpos.h>
 #include <__iterator/wrap_iter.h>
 #include <__memory/allocate_at_least.h>
 #include <__utility/auto_cast.h>
 #include <__utility/move.h>
 #include <__utility/swap.h>
+#include <__utility/unreachable.h>
+#include <climits>
 #include <compare>
 #include <cstdio>  // EOF
 #include <cstdlib>
 #include <cstring>
 #include <initializer_list>
 #include <iosfwd>
 #include <iterator>
+#include <limits>
 #include <memory>
 #include <stdexcept>
 #include <string_view>
 #include <type_traits>
 #include <version>
 
 // TODO: remove these headers
 #include <__functional/binary_function.h>

 private:
 
 #ifdef _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
 
     struct __long
     {
         pointer   __data_;
         size_type __size_;
-        size_type __cap_;
+        size_type __cap_ : sizeof(size_type) * CHAR_BIT - 1;
+        size_type __is_long_ : 1;
     };
 
-#ifdef _LIBCPP_BIG_ENDIAN
-    static const size_type __short_mask = 0x01;
-    static const size_type __long_mask  = 0x1ul;
-#else  // _LIBCPP_BIG_ENDIAN
-    static const size_type __short_mask = 0x80;
-    static const size_type __long_mask  = ~(size_type(~0) >> 1);
-#endif // _LIBCPP_BIG_ENDIAN
-
     enum {__min_cap = (sizeof(__long) - 1)/sizeof(value_type) > 2 ?
                       (sizeof(__long) - 1)/sizeof(value_type) : 2};
 
     struct __short
     {
         value_type __data_[__min_cap];
         unsigned char __padding[sizeof(value_type) - 1];
-        unsigned char __size_;
+        unsigned char __size_ : 7;
+        unsigned char __is_long_ : 1;
     };
 
+// The __endian_factor is required because the field we use to store the size
+// (either size_type or unsigned char depending on long/short) has one fewer
+// bit than it would if it were not a bitfield.
+//
+// If the LSB is used to store the short-flag in the short string representation,
+// we have to multiply the size by two when it is stored and divide it by two when
+// it is loaded to make sure that we always store an even number. In the long string
+// representation, we can ignore this because we can assume that we always allocate
+// an even amount of value_types.
+//
+// If the MSB is used for the short-flag, the max_size() is numeric_limits<size_type>::max() / 2.
+// This does not impact the short string representation, since we never need the MSB
+// for representing the size of a short string anyway.
+
+#ifdef _LIBCPP_BIG_ENDIAN
+    static const size_type __endian_factor = 2;
 #else
+    static const size_type __endian_factor = 1;
+#endif
+
+#else // _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
+
+#ifdef _LIBCPP_BIG_ENDIAN
+    static const size_type __endian_factor = 1;
+#else
+    static const size_type __endian_factor = 2;
+#endif
 
     struct __long
     {
-        size_type __cap_;
+        size_type __is_long_ : 1;
+        size_type __cap_ : sizeof(size_type) * CHAR_BIT - 1;
         size_type __size_;
         pointer   __data_;
     };
 
-#ifdef _LIBCPP_BIG_ENDIAN
-    static const size_type __short_mask = 0x80;
-    static const size_type __long_mask  = ~(size_type(~0) >> 1);
-#else  // _LIBCPP_BIG_ENDIAN
-    static const size_type __short_mask = 0x01;
-    static const size_type __long_mask  = 0x1ul;
-#endif // _LIBCPP_BIG_ENDIAN
-
     enum {__min_cap = (sizeof(__long) - 1)/sizeof(value_type) > 2 ?
                       (sizeof(__long) - 1)/sizeof(value_type) : 2};
 
     struct __short
     {
         union
         {
-            unsigned char __size_;
+            struct {
+                unsigned char __is_long_ : 1;
+                unsigned char __size_ : 7;
+            };
             value_type __lx;
         };
         value_type __data_[__min_cap];
     };
 
 #endif // _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
 
     union __ulx{__long __lx; __short __lxx;};
#endif
 
     _LIBCPP_INLINE_VISIBILITY bool __invariants() const;
 
     _LIBCPP_INLINE_VISIBILITY void __clear_and_shrink() _NOEXCEPT;
 
     _LIBCPP_INLINE_VISIBILITY void __shrink_or_extend(size_type __target_capacity);
 
     _LIBCPP_INLINE_VISIBILITY
-    bool __is_long() const _NOEXCEPT
-        {return bool(__r_.first().__s.__size_ & __short_mask);}
+    bool __is_long() const _NOEXCEPT {
+        return __r_.first().__s.__is_long_;
+    }
 
 #if _LIBCPP_DEBUG_LEVEL == 2
 
     bool __dereferenceable(const const_iterator* __i) const;
     bool __decrementable(const const_iterator* __i) const;
     bool __addable(const const_iterator* __i, ptrdiff_t __n) const;
     bool __subscriptable(const const_iterator* __i, ptrdiff_t __n) const;
 
iterator __insert_from_safe_copy(size_type __n, size_type __ip, _ForwardIterator __first, _ForwardIterator __last) {
             traits_type::assign(*__p, *__first);
 
         return begin() + __ip;
     }
 
     _LIBCPP_HIDE_FROM_ABI allocator_type& __alloc() _NOEXCEPT { return __r_.second(); }
     _LIBCPP_HIDE_FROM_ABI const allocator_type& __alloc() const _NOEXCEPT { return __r_.second(); }
 
-#ifdef _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
-
     _LIBCPP_INLINE_VISIBILITY
-    void __set_short_size(size_type __s) _NOEXCEPT
-#   ifdef _LIBCPP_BIG_ENDIAN
-        {__r_.first().__s.__size_ = (unsigned char)(__s << 1);}
-#   else
-        {__r_.first().__s.__size_ = (unsigned char)(__s);}
-#   endif
-
-    _LIBCPP_INLINE_VISIBILITY
-    size_type __get_short_size() const _NOEXCEPT
-#   ifdef _LIBCPP_BIG_ENDIAN
-        {return __r_.first().__s.__size_ >> 1;}
-#   else
-        {return __r_.first().__s.__size_;}
-#   endif
-
-#else  // _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
-
-    _LIBCPP_INLINE_VISIBILITY
-    void __set_short_size(size_type __s) _NOEXCEPT
-#   ifdef _LIBCPP_BIG_ENDIAN
-        {__r_.first().__s.__size_ = (unsigned char)(__s);}
-#   else
-        {__r_.first().__s.__size_ = (unsigned char)(__s << 1);}
-#   endif
+    void __set_short_size(size_type __s) _NOEXCEPT {
+        _LIBCPP_ASSERT(__s < __min_cap, "__s should never be greater or equal to the short string capacity");
+        __r_.first().__s.__size_ = __s;
+        __r_.first().__s.__is_long_ = false;
+    }
 
     _LIBCPP_INLINE_VISIBILITY
-    size_type __get_short_size() const _NOEXCEPT
-#   ifdef _LIBCPP_BIG_ENDIAN
-        {return __r_.first().__s.__size_;}
-#   else
-        {return __r_.first().__s.__size_ >> 1;}
-#   endif
-
-#endif // _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
+    size_type __get_short_size() const _NOEXCEPT {
+        _LIBCPP_ASSERT(!__r_.first().__s.__is_long_, "String has to be short when trying to get the short size");
+        return __r_.first().__s.__size_;
+    }
 
     _LIBCPP_INLINE_VISIBILITY
     void __set_long_size(size_type __s) _NOEXCEPT
         {__r_.first().__l.__size_ = __s;}
     _LIBCPP_INLINE_VISIBILITY
     size_type __get_long_size() const _NOEXCEPT
         {return __r_.first().__l.__size_;}
     _LIBCPP_INLINE_VISIBILITY
     void __set_size(size_type __s) _NOEXCEPT
         {if (__is_long()) __set_long_size(__s); else __set_short_size(__s);}
 
     _LIBCPP_INLINE_VISIBILITY
-    void __set_long_cap(size_type __s) _NOEXCEPT
-        {__r_.first().__l.__cap_  = __long_mask | __s;}
+    void __set_long_cap(size_type __s) _NOEXCEPT {
+        __r_.first().__l.__cap_ = __s / __endian_factor;
+        __r_.first().__l.__is_long_ = true;
+    }
+
     _LIBCPP_INLINE_VISIBILITY
-    size_type __get_long_cap() const _NOEXCEPT
-        {return __r_.first().__l.__cap_ & size_type(~__long_mask);}
+    size_type __get_long_cap() const _NOEXCEPT {
+        return __r_.first().__l.__cap_ * __endian_factor;
+    }
 
     _LIBCPP_INLINE_VISIBILITY
     void __set_long_pointer(pointer __p) _NOEXCEPT
         {__r_.first().__l.__data_ = __p;}
     _LIBCPP_INLINE_VISIBILITY
     pointer __get_long_pointer() _NOEXCEPT
         {return __r_.first().__l.__data_;}
     _LIBCPP_INLINE_VISIBILITY

 }
 
 template <class _CharT, class _Traits, class _Allocator>
 inline
 typename basic_string<_CharT, _Traits, _Allocator>::size_type
 basic_string<_CharT, _Traits, _Allocator>::max_size() const _NOEXCEPT
 {
     size_type __m = __alloc_traits::max_size(__alloc());
-#ifdef _LIBCPP_BIG_ENDIAN
-    return (__m <= ~__long_mask ? __m : __m/2) - __alignment;
-#else
+    if (__m <= std::numeric_limits<size_type>::max() / 2) {
         return __m - __alignment;
-#endif
+    } else {
+        bool __uses_lsb = __endian_factor == 2;
+        return __uses_lsb ? __m - __alignment : (__m / 2) - __alignment;
+    }
 }
 
 template <class _CharT, class _Traits, class _Allocator>
 void
 basic_string<_CharT, _Traits, _Allocator>::reserve(size_type __requested_capacity)
 {
     if (__requested_capacity > max_size())
         __throw_length_error();
 
     // Make sure reserve(n) never shrinks. This is technically only required in C++20
     // and later (since P0966R1), however we provide consistent behavior in all Standard
     // modes because this function is instantiated in the shared library.

libcxx/test/libcxx/strings/basic.string/string.capacity/max_size.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
+//
+//===----------------------------------------------------------------------===//
+
+// <string>
+
+#include <cassert>
+#include <string>
+
+#include "test_macros.h"
+
+bool test() {
+  // alignment of the string heap buffer is hardcoded to 16
+  const size_t alignment = 16;
+
+  std::string str;
+#ifndef TEST_HAS_NO_UNICODE_CHARS
+  std::u16string u16str;
+  std::u32string u32str;
+#endif
+#ifdef _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
+
+#  ifdef _LIBCPP_BIG_ENDIAN
+
+  assert(str.max_size() == std::numeric_limits<size_t>::max() - alignment);
+#    ifndef TEST_HAS_NO_UNICODE_CHARS
+  assert(u16str.max_size() == std::numeric_limits<size_t>::max() / 2 - alignment);
+  assert(u32str.max_size() == std::numeric_limits<size_t>::max() / 4 - alignment);
+#    endif // TEST_HAS_NO_UNICODE_CHARS
+
+#  else // _LIBCPP_BIG_ENDIAN
+
+  assert(str.max_size() == std::numeric_limits<size_t>::max() / 2 - alignment);
+#    ifndef TEST_HAS_NO_UNICODE_CHARS
+  assert(u16str.max_size() == std::numeric_limits<size_t>::max() / 2 - alignment);
+  assert(u32str.max_size() == std::numeric_limits<size_t>::max() / 4 - alignment);
+#    endif // TEST_HAS_NO_UNICODE_CHARS
+
+#  endif // _LIBCPP_BIG_ENDIAN
+
+#else // _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
+
+#  ifdef _LIBCPP_BIG_ENDIAN
+
+  assert(str.max_size() == std::numeric_limits<size_t>::max() / 2 - alignment);
+#    ifndef TEST_HAS_NO_UNICODE_CHARS
+  assert(u16str.max_size() == std::numeric_limits<size_t>::max() / 2 - alignment);
+  assert(u32str.max_size() == std::numeric_limits<size_t>::max() / 4 - alignment);
+#    endif // TEST_HAS_NO_UNICODE_CHARS
+
+#  else
+
+  assert(str.max_size() == std::numeric_limits<size_t>::max() - alignment);
+#    ifndef TEST_HAS_NO_UNICODE_CHARS
+  assert(u16str.max_size() == std::numeric_limits<size_t>::max() / 2 - alignment);
+  assert(u32str.max_size() == std::numeric_limits<size_t>::max() / 4 - alignment);
+#    endif // TEST_HAS_NO_UNICODE_CHARS
+
+#  endif // _LIBCPP_BIG_ENDIAN
+
+#endif // _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
+
+  return true;
+}
+
+int main(int, char**) {
+  test();
+#if TEST_STD_VER > 17
+  // static_assert(test());
+#endif
+
+  return 0;
+}

libcxx/test/std/strings/basic.string/string.capacity/over_max_size.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: no-exceptions
 // XFAIL: use_system_cxx_lib && target={{.+}}-apple-macosx10.{{9|10|11}}
+// XFAIL: use_system_cxx_lib && target=arm64{{.+}}-apple-macosx{{11.0|12.0}}
 
 // <string>
 
 // size_type max_size() const;
 
 #include <string>
 #include <cassert>
 #include <stdexcept>

libcxx/utils/gdb/libcxx/printers.py

 #===----------------------------------------------------------------------===##
 #
 # 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
 #
 #===----------------------------------------------------------------------===##
 """GDB pretty-printers for libc++.
 
def _get_base_subobject(child_class_value, index=0):
     return child_class_value[field]
 
 
 def _value_of_pair_first(value):
     """Convenience for _get_base_subobject, for the common case."""
     return _get_base_subobject(value, 0)["__value_"]
 
 
 class StdStringPrinter(object):
     """Print a std::string."""
 
-    def _get_short_size(self, short_field, short_size):
-        """Short size depends on both endianness and a compile-time define."""
-
-        # If the padding field is present after all this indirection, then string
-        # was compiled with _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT defined.
-        field = short_field.type.fields()[1].type.fields()[0]
-        libcpp_abi_alternate_string_layout = field.name and "__padding" in field.name
-
-        # This logical structure closely follows the original code (which is clearer
-        # in C++).  Keep them parallel to make them easier to compare.
-        if libcpp_abi_alternate_string_layout:
-            if _libcpp_big_endian:
-                return short_size >> 1
-            else:
-                return short_size
-        elif _libcpp_big_endian:
-            return short_size
-        else:
-            return short_size >> 1
-
     def __init__(self, val):
         self.val = val
 
     def to_string(self):
         """Build a python string from the data whether stored inline or separately."""
 
         value_field = _value_of_pair_first(self.val["__r_"])
         short_field = value_field["__s"]
         short_size = short_field["__size_"]
         if short_size == 0:
             return ""
-        short_mask = self.val["__short_mask"]
-        # Counter intuitive to compare the size and short_mask to see if the string
-        # is long, but that's the way the implementation does it. Note that
-        # __is_long() doesn't use get_short_size in C++.
-        is_long = short_size & short_mask
-        if is_long:
+        if short_field["__is_long_"]:
             long_field = value_field["__l"]
             data = long_field["__data_"]
             size = long_field["__size_"]
         else:
             data = short_field["__data_"]
-            size = self._get_short_size(short_field, short_size)
+            size = short_field["__size_"]
         return data.lazy_string(length=size)
 
     def display_hint(self):
         return "string"
 
 
 class StdStringViewPrinter(object):
     """Print a std::string_view."""