[libcxx] Make allocator<T>:allocate throw bad_array_new_length

[miyuki]

Summary

Currently the member functions std::allocator<T>::allocate,
std::experimental::pmr::polymorphic_allocator::allocate and
std::resource_adaptor<T>::do_allocate throw an exception of type
std::length_error when the requested size exceeds the maximum size.

According to the C++ standard ([allocator.members]/4,
[mem.poly.allocator.mem]/1), std::allocator<T>::allocate and
std::pmr::polymorphic_allocator::allocate must throw a
std::bad_array_new_length exception in this case.

The patch fixes the issue with std::allocator<T>::allocate and changes
the type the exception thrown by
std::experimental::pmr::resource_adaptor<T>::do_allocate to
std::bad_array_new_length as well for consistency.

The patch resolves LWG 3237, LWG 3038 and LWG 3190.

[• Quuxplusone]

I like this, but @ldionne needs to look.

libcxx/docs/Status/Cxx20Issues.csv

165

Here and above: s/""/"14.0"/?

libcxx/include/experimental/memory_resource

187

For the record: Personally I don't think it matters whether we change experimental::pmr::polymorphic_allocator or not. I have adopted this patch into my next update of D89057 (std::pmr::polymorphic_allocator).

[ldionne]

I'm fine with the change in itself, however if we introduce a new function in the dylib, we'll need to be clever about rolling it out. If someone tries to back-deploy to an older OS where that function isn't in the dylib, they'll get an error when their binary is loaded cause it'll rely on the non-existent symbol. This can be worked around by looking at the deployment target and only relying on the symbol if we're on a target that has the symbol, but that's more complicated. Instead, I would suggest just making this an inline function in the header, similarly to what we do for __throw_bad_any_cast and some others.

libcxx/include/new

151

For consistency?

[ldionne]

The reason why I'm fine with this is that I expect few people will get broken by this. It's thrown on failure to allocate, which is a corner case (I mean, it's more of a corner case than something like out_of_range), and also I don't think a lot of people are expecting that you'd catch a failure to allocate with std::length_error.

[• Quuxplusone]

libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/allocate.pass.cpp

68

The reason why I'm fine with this is that I expect few people will get broken by this. It's thrown on failure to allocate, which is a corner case (I mean, it's more of a corner case than something like out_of_range), and also I don't think a lot of people are expecting that you'd catch a failure to allocate with std::length_error.

Btw, notice that bad_array_new_length inherits from (and thus can-be-caught-as) bad_alloc, which is great. The fact that length_error cannot be caught as bad_alloc explains why, on the left side of this diff, we had to catch the least-common-ancestor class exception.

Also btw, @miyuki, how about fixing the comment on line 64 while you're here?

[miyuki]

libcxx/include/new

151

The function __throw_bad_array_new_length is implemented in libstdc++, so it doesn't get defined when using libsupc++ as the ABI library. There is no __throw_bad_array_new_length function in libstdc++, but I thought that this might change in the future, so I named the function differently to avoid possible name clash.

But now I noticed that there is a __cxa_throw_bad_array_new_length function which is part of the Itanium ABI and is implemented in both libc++abi and libsupc++ (introduced in CXXABI_1.3.8, GCC 4.9). Should I use it instead?

[miyuki]

Addressed Arthur's comments.

[miyuki]

libcxx/include/new

151

I meant to say "The function __throw_bad_alloc is implemented in libstdc++"

[ldionne]

libcxx/include/new

151

The issue I see with using __cxa_throw_bad_array_new_length is the same as with defining __libcpp_throw_bad_array_new_length in the dylib -- a program deployed against an OS where the libc++.dylib (well, libc++abi.dylib really) doesn't ship that symbol would fail to run.

[• Quuxplusone]

libcxx/include/new

151

__cxa_throw_bad_array_new_length is part of the Itanium C++ ABI, so it should be available on most systems... but yeah, perhaps it wouldn't be there on Windows. And also, looking at git grep __cxa_ libcxx/, we can see that libc++ generally doesn't contain calls to __cxa_ functions, so there's no reason to start now.
I think an inline __throw_bad_array_new_length [no _libcpp_] defined in the header sounds like the most conventional, and thus best, answer.

[ldionne]

Also, can we please add a release note for this?

Requesting changes so it shows up correctly in the queue.

[miyuki]

Addressed Louis' comments.

[miyuki]

Rebased

[miyuki]

Updated Linux ABI list

[DavidSpickett]

The Arm pretty printer failures are my fault, fixed in https://reviews.llvm.org/rG7ee133d3fce730d39dba68b81d8ab2285ba8b349.

[• Quuxplusone]

This LGTM; @ldionne ping?
Also, @miyuki, please re-upload the diff so that we get a clean buildkite run. Let's make sure this passes CI.

libcxx/docs/ReleaseNotes.rst

54–55

[ldionne]

Thanks!

[miyuki]

1. Rebased
2. Fixed grammar in ReleaseNotes

[miyuki]

Rebased

libcxx/docs/ReleaseNotes.rst

- There's initial support for the C++20 header ``<format>``. The implementation
   considered ABI stable.
 - There's a new CMake option ``LIBCXX_ENABLE_UNICODE`` to disable Unicode
   support in the ``<format>`` header. This only affects the estimation of the
   output width of the format functions.
 
 API Changes
 -----------
 
-- ...
+- The functions ``std::allocator<T>::allocate`` and
+  ``std::experimental::pmr::polymorphic_allocator<T>::allocate`` now throw
+  an exception of the type ``std::bad_array_new_length`` when the requested size
+  exceeds the maximum supported size as required by the C++ standard. Previously
+  the type ``std::length_error`` was used.
 
 Build System Changes
 --------------------
 
 - Building the libc++ shared or static library requires a C++ 20 capable compiler.
   Use ``-DLLVM_ENABLE_PROJECTS='clang;compiler-rt' -DLLVM_ENABLE_RUNTIMES='libcxx;libcxxabi'``
   to build libc++ using a fresh build of Clang.

libcxx/docs/Status/Cxx20Issues.csv

 "`2960 <https://wg21.link/LWG2960>`__","[fund.ts.v3] ``nonesuch``\  is insufficiently useless","San Diego","|Complete|",""
 "`2995 <https://wg21.link/LWG2995>`__","``basic_stringbuf``\  default constructor forbids it from using SSO capacity","San Diego","",""
 "`2996 <https://wg21.link/LWG2996>`__","Missing rvalue overloads for ``shared_ptr``\  operations","San Diego","",""
 "`3008 <https://wg21.link/LWG3008>`__","``make_shared``\  (sub)object destruction semantics are not specified","San Diego","",""
 "`3022 <https://wg21.link/LWG3022>`__","``is_convertible<derived*, base*>``\  may lead to ODR","San Diego","Resolved by 1285R0",""
 "`3025 <https://wg21.link/LWG3025>`__","Map-like container deduction guides should use ``pair<Key, T>``\ , not ``pair<const Key, T>``\ ","San Diego","|Complete|",""
 "`3031 <https://wg21.link/LWG3031>`__","Algorithms and predicates with non-const reference arguments","San Diego","",""
 "`3037 <https://wg21.link/LWG3037>`__","``polymorphic_allocator``\  and incomplete types","San Diego","",""
-"`3038 <https://wg21.link/LWG3038>`__","``polymorphic_allocator::allocate``\  should not allow integer overflow to create vulnerabilities","San Diego","",""
+"`3038 <https://wg21.link/LWG3038>`__","``polymorphic_allocator::allocate``\  should not allow integer overflow to create vulnerabilities","San Diego","|Complete|","14.0"
 "`3054 <https://wg21.link/LWG3054>`__","``uninitialized_copy``\  appears to not be able to meet its exception-safety guarantee","San Diego","",""
 "`3065 <https://wg21.link/LWG3065>`__","LWG 2989 missed that all ``path``\ 's other operators should be hidden friends as well","San Diego","|Complete|",""
 "`3096 <https://wg21.link/LWG3096>`__","``path::lexically_relative``\  is confused by trailing slashes","San Diego","|Complete|",""
 "`3116 <https://wg21.link/LWG3116>`__","``*OUTERMOST_ALLOC_TRAITS*``\  needs ``remove_reference_t``\ ","San Diego","",""
 "`3122 <https://wg21.link/LWG3122>`__","``__cpp_lib_chrono_udls``\  was accidentally dropped","San Diego","|Complete|",""
 "`3127 <https://wg21.link/LWG3127>`__","``basic_osyncstream::rdbuf``\  needs a ````const_cast````\ ","San Diego","",""
 "`3128 <https://wg21.link/LWG3128>`__","``strstream::rdbuf``\  needs a ````const_cast````\ ","San Diego","|Nothing To Do|",""
 "`3129 <https://wg21.link/LWG3129>`__","``regex_token_iterator``\  constructor uses wrong pointer arithmetic","San Diego","",""

 "`3199 <https://wg21.link/LWG3199>`__","``istream >> bitset<0>``\  fails","Cologne","",""
 "`3202 <https://wg21.link/LWG3202>`__","P0318R1 was supposed to be revised","Cologne","|Complete|",""
 "`3206 <https://wg21.link/LWG3206>`__","``year_month_day``\  conversion to ``sys_days``\  uses not-existing member function","Cologne","|Complete|",""
 "`3208 <https://wg21.link/LWG3208>`__","``Boolean``\ 's expression requirements are ordered inconsistently","Cologne","|Nothing To Do|",""
 "`3209 <https://wg21.link/LWG3209>`__","Expression in ``year::ok()``\  returns clause is ill-formed","Cologne","|Complete|",""
 "","","","",""
 "`3231 <https://wg21.link/LWG3231>`__","``year_month_day_last::day``\  specification does not cover ``!ok()``\  values","Belfast","|Nothing To Do|",""
 "`3225 <https://wg21.link/LWG3225>`__","``zoned_time``\  converting constructor shall not be ``noexcept``\ ","Belfast","",""
-"`3190 <https://wg21.link/LWG3190>`__","``std::allocator::allocate``\  sometimes returns too little storage","Belfast","",""
+"`3190 <https://wg21.link/LWG3190>`__","``std::allocator::allocate``\  sometimes returns too little storage","Belfast","|Complete|","14.0"
 "`3218 <https://wg21.link/LWG3218>`__","Modifier for ``%d``\  parse flag does not match POSIX and ``format``\  specification","Belfast","",""
 "`3224 <https://wg21.link/LWG3224>`__","``zoned_time``\  constructor from ``TimeZonePtr``\  does not specify initialization of ``tp_``\ ","Belfast","",""
 "`3230 <https://wg21.link/LWG3230>`__","Format specifier ``%y/%Y``\  is missing locale alternative versions","Belfast","",""
 "`3232 <https://wg21.link/LWG3232>`__","Inconsistency in ``zoned_time``\  deduction guides","Belfast","",""
 "`3222 <https://wg21.link/LWG3222>`__","P0574R1 introduced preconditions on non-existent parameters","Belfast","",""
 "`3221 <https://wg21.link/LWG3221>`__","Result of ``year_month``\  arithmetic with ``months``\  is ambiguous","Belfast","|Complete|","8.0"
 "`3235 <https://wg21.link/LWG3235>`__","``parse``\  manipulator without abbreviation is not callable","Belfast","",""
 "`3246 <https://wg21.link/LWG3246>`__","What are the constraints on the template parameter of ``basic_format_arg``\ ?","Belfast","",""

 "`3141 <https://wg21.link/LWG3141>`__","``CopyConstructible``\  doesn't preserve source values","Prague","|Nothing to do|",""
 "`3150 <https://wg21.link/LWG3150>`__","``UniformRandomBitGenerator``\  should validate ``min``\  and ``max``\ ","Prague","|Complete|","13.0"
 "`3175 <https://wg21.link/LWG3175>`__","The ``CommonReference``\  requirement of concept ``SwappableWith``\  is not satisfied in the example","Prague","|Complete|","13.0"
 "`3194 <https://wg21.link/LWG3194>`__","``ConvertibleTo``\  prose does not match code","Prague","|Complete|","13.0"
 "`3200 <https://wg21.link/LWG3200>`__","``midpoint``\  should not constrain ``T``\  is complete","Prague","|Nothing To Do|",""
 "`3201 <https://wg21.link/LWG3201>`__","``lerp``\  should be marked as ``noexcept``\ ","Prague","|Complete|",""
 "`3226 <https://wg21.link/LWG3226>`__","``zoned_time``\  constructor from ``string_view``\  should accept ``zoned_time<Duration2, TimeZonePtr2>``\ ","Prague","",""
 "`3233 <https://wg21.link/LWG3233>`__","Broken requirements for ``shared_ptr``\  converting constructors","Prague","",""
-"`3237 <https://wg21.link/LWG3237>`__","LWG 3038 and 3190 have inconsistent PRs","Prague","",""
+"`3237 <https://wg21.link/LWG3237>`__","LWG 3038 and 3190 have inconsistent PRs","Prague","|Complete|","14.0"
 "`3238 <https://wg21.link/LWG3238>`__","Insufficiently-defined behavior of ``std::function``\  deduction guides","Prague","",""
 "`3242 <https://wg21.link/LWG3242>`__","``std::format``\ : missing rules for ``arg-id``\  in ``width``\  and ``precision``\ ","Prague","|Complete|","Clang 14"
 "`3243 <https://wg21.link/LWG3243>`__","``std::format``\  and negative zeroes","Prague","",""
 "`3247 <https://wg21.link/LWG3247>`__","``ranges::iter_move``\  should perform ADL-only lookup of ``iter_move``\ ","Prague","",""
 "`3248 <https://wg21.link/LWG3248>`__","``std::format``\  ``#b``\ , ``#B``\ , ``#o``\ , ``#x``\ , and ``#X``\   presentation types misformat negative numbers","Prague","",""
 "`3250 <https://wg21.link/LWG3250>`__","``std::format``\ : ``#``\  (alternate form) for NaN and inf","Prague","",""
 "`3251 <https://wg21.link/LWG3251>`__","Are ``std::format``\  alignment specifiers applied to string arguments?","Prague","",""
 "`3252 <https://wg21.link/LWG3252>`__","Parse locale's aware modifiers for commands are not consistent with POSIX spec","Prague","",""

libcxx/include/__memory/allocator.h

public:
 
     template <class _Up>
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
     allocator(const allocator<_Up>&) _NOEXCEPT { }
 
     _LIBCPP_NODISCARD_AFTER_CXX17 _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
     _Tp* allocate(size_t __n) {
         if (__n > allocator_traits<allocator>::max_size(*this))
-            __throw_length_error("allocator<T>::allocate(size_t n)"
-                                 " 'n' exceeds maximum supported size");
+            __throw_bad_array_new_length();
         if (__libcpp_is_constant_evaluated()) {
             return static_cast<_Tp*>(::operator new(__n * sizeof(_Tp)));
         } else {
             return static_cast<_Tp*>(_VSTD::__libcpp_allocate(__n * sizeof(_Tp), _LIBCPP_ALIGNOF(_Tp)));
         }
     }
 
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
public:
 
     template <class _Up>
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
     allocator(const allocator<_Up>&) _NOEXCEPT { }
 
     _LIBCPP_NODISCARD_AFTER_CXX17 _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17
     const _Tp* allocate(size_t __n) {
         if (__n > allocator_traits<allocator>::max_size(*this))
-            __throw_length_error("allocator<const T>::allocate(size_t n)"
-                                 " 'n' exceeds maximum supported size");
+            __throw_bad_array_new_length();
         if (__libcpp_is_constant_evaluated()) {
             return static_cast<const _Tp*>(::operator new(__n * sizeof(_Tp)));
         } else {
             return static_cast<const _Tp*>(_VSTD::__libcpp_allocate(__n * sizeof(_Tp), _LIBCPP_ALIGNOF(_Tp)));
         }
     }
 
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17

libcxx/include/experimental/memory_resource

public:
     {}
 
     polymorphic_allocator &
     operator=(polymorphic_allocator const &) = delete;
 
     // 8.6.3, memory.polymorphic.allocator.mem
     _LIBCPP_INLINE_VISIBILITY
     _ValueType* allocate(size_t __n) {
-        if (__n > __max_size()) {
-            __throw_length_error(
-                "std::experimental::pmr::polymorphic_allocator<T>::allocate(size_t n)"
-                " 'n' exceeds maximum supported size");
-        }
+        if (__n > __max_size())
+            __throw_bad_array_new_length();
         return static_cast<_ValueType*>(
             __res_->allocate(__n * sizeof(_ValueType), _LIBCPP_ALIGNOF(_ValueType))
         );
     }
 
     _LIBCPP_INLINE_VISIBILITY
     void deallocate(_ValueType * __p, size_t __n) _NOEXCEPT {
         _LIBCPP_ASSERT(__n <= __max_size(),
public:
     _LIBCPP_INLINE_VISIBILITY
     allocator_type get_allocator() const
     { return __alloc_; }
 
 // 8.7.3, memory.resource.adaptor.mem
 private:
     virtual void * do_allocate(size_t __bytes, size_t)
     {
-        if (__bytes > __max_size()) {
-            __throw_length_error(
-                "std::experimental::pmr::resource_adaptor<T>::do_allocate(size_t bytes, size_t align)"
-                " 'bytes' exceeds maximum supported size");
-        }
+        if (__bytes > __max_size())
+            __throw_bad_array_new_length();
         size_t __s = __aligned_allocation_size(__bytes, _MaxAlign) / _MaxAlign;
         return __alloc_.allocate(__s);
     }
 
     virtual void do_deallocate(void * __p, size_t __bytes, size_t)
     {
         _LIBCPP_ASSERT(__bytes <= __max_size(),
             "do_deallocate called for size which exceeds the maximum allocation size");

libcxx/include/new

 
 typedef void (*new_handler)();
 _LIBCPP_FUNC_VIS new_handler set_new_handler(new_handler) _NOEXCEPT;
 _LIBCPP_FUNC_VIS new_handler get_new_handler() _NOEXCEPT;
 
 #endif // !_LIBCPP_ABI_VCRUNTIME
 
 _LIBCPP_NORETURN _LIBCPP_FUNC_VIS void __throw_bad_alloc();  // not in C++ spec
 
+_LIBCPP_NORETURN inline _LIBCPP_INLINE_VISIBILITY
+void __throw_bad_array_new_length()
+{
+#ifndef _LIBCPP_NO_EXCEPTIONS
+    throw bad_array_new_length();
+#else
+    _VSTD::abort();
+#endif
+}
+
 #if !defined(_LIBCPP_HAS_NO_LIBRARY_ALIGNED_ALLOCATION) && \
     !defined(_LIBCPP_ABI_VCRUNTIME)
 #ifndef _LIBCPP_CXX03_LANG
 enum class _LIBCPP_ENUM_VIS align_val_t : size_t { };
 #else
 enum align_val_t { __zero = 0, __max = (size_t)-1 };
 #endif
 #endif

libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/allocate.pass.cpp

try {
         a.allocate(maxSize);
     } catch (...) {
         assert(false);
     }
 
     size_t sizeTypeMax = std::numeric_limits<std::size_t>::max();
     if (maxSize != sizeTypeMax)
     {
-        // Test that allocating size_t(~0) throws bad alloc.
+        // Test that allocating size_t(~0) throws bad_array_new_length.
         try {
             a.allocate(sizeTypeMax);
             assert(false);
-        } catch (std::exception const&) {
+        } catch (std::bad_array_new_length const&) {
         }
 
         // Test that allocating even one more than the max size does throw.
         size_t overSize = maxSize + 1;
         try {
             a.allocate(overSize);
             assert(false);
-        } catch (std::exception const&) {
+        } catch (std::bad_array_new_length const&) {
         }
     }
 }
 #endif // TEST_HAS_NO_EXCEPTIONS
 
 int main(int, char**)
 {
     {

libcxx/test/std/experimental/memory/memory.resource.adaptor/memory.resource.adaptor.mem/do_allocate_and_deallocate.pass.cpp

#ifndef TEST_HAS_NO_EXCEPTIONS
     for (std::size_t size = sizeTypeMax; size > testSizeEnd; --size) {
         AllocController P;
         R1 r{Alloc(P)};
         ex::memory_resource & m1 = r;
 
         try {
             m1.allocate(size);
             assert(false);
-        } catch (std::exception const&) {
+        } catch (std::bad_array_new_length const&) {
         }
     }
 #endif
 }
 
 int main(int, char**)
 {
     check_allocate_deallocate<CountingAllocator<char>>();
     check_allocate_deallocate<MinAlignedAllocator<char>>();
     check_alloc_max_size();
 
   return 0;
 }

libcxx/test/std/utilities/memory/default.allocator/allocator.members/allocate.size.pass.cpp

 
 template <typename T>
 void test_max(size_t count)
 {
     std::allocator<T> a;
     try {
         TEST_IGNORE_NODISCARD a.allocate(count);
         assert(false);
-    } catch (const std::exception &) {
+    } catch (const std::bad_array_new_length &) {
     }
 }
 
 template <typename T>
 void test()
 {
     // Bug 26812 -- allocating too large
     typedef std::allocator<T> A;