https://gcc.gnu.org/g:1b18a9e53960f34f40ed6e2f0f9320269bbafba6
commit r16-4422-g1b18a9e53960f34f40ed6e2f0f9320269bbafba6 Author: Jonathan Wakely <[email protected]> Date: Sat Oct 11 11:22:38 2025 +0100 libstdc++: Ensure filesystem::path internals are sufficiently aligned [PR122255] We need the memory allocated for a path::_List::_Impl to be at least 4-byte aligned, so that we can use the two least significant bits to store a _Type value. Use __STDCPP_DEFAULT_NEW_ALIGNMENT__ to check that in a static_assert. Also add a static_assert to check the memory will be aligned suitably for the _Impl object itself. In practice both assertions should pass as long as operator new guarantees to return memory with at least 4-byte alignment, which seems to be true for malloc on GCC's supported targets. Allocation of path::_List::_Impl objects is refactored into a new _Impl::create function so that the memory allocation is done in one place, rather than being repeated in path::_List::_Impl::copy and path::_List::reserve. If we late decide to use aligned-new to support targets that fail the new static assertions we won't need to do that in two different places. Calling operator delete already only happens in one place, the _Impl_deleter. The create function is actually implemented in terms of another new function, create_unchecked. The overflow checks in create aren't needed when copying an existing object, because we already checked its size doesn't overflow. Destroying the components is now done by a destructor, which the _Impl_deleter invokes. libstdc++-v3/ChangeLog: PR libstdc++/122255 * src/c++17/fs_path.cc (path::_List::_Impl::~_Impl): Define destructor. (path::_List::_Impl::copy): Use create_unchecked. (path::_List::_Impl): Add static assertions. (path::_List::_Impl::create): New static member function. (path::_List::_Impl::create_unchecked): Likewise. (path::_List::_Impl_deleter::operator()): Use destructor. (path::_List::reserve): Use create. Reviewed-by: Tomasz Kamiński <[email protected]> Diff: --- libstdc++-v3/src/c++17/fs_path.cc | 74 ++++++++++++++++++++++++++------------- 1 file changed, 50 insertions(+), 24 deletions(-) diff --git a/libstdc++-v3/src/c++17/fs_path.cc b/libstdc++-v3/src/c++17/fs_path.cc index 215afa08ad25..03bb5ecb7be1 100644 --- a/libstdc++-v3/src/c++17/fs_path.cc +++ b/libstdc++-v3/src/c++17/fs_path.cc @@ -34,6 +34,7 @@ #include <filesystem> #include <algorithm> #include <array> +#include <new> #include <bits/stl_uninitialized.h> #include <ext/numeric_traits.h> // __gnu_cxx::__int_traits @@ -207,6 +208,10 @@ struct path::_List::_Impl _Impl(int cap) : _M_size(0), _M_capacity(cap) { } + ~_Impl() { clear(); } + + // Align the first member like the value_type so that we can store one or + // more objects of that type immediately after the memory occupied by *this. alignas(value_type) int _M_size; int _M_capacity; @@ -246,29 +251,67 @@ struct path::_List::_Impl unique_ptr<_Impl, _Impl_deleter> copy() const { const auto n = size(); - void* p = ::operator new(sizeof(_Impl) + n * sizeof(value_type)); - unique_ptr<_Impl, _Impl_deleter> newptr(::new (p) _Impl{n}); + // *this already has n elements so don't need to check if n overflows: + auto newptr = create_unchecked(n); std::uninitialized_copy_n(begin(), n, newptr->begin()); newptr->_M_size = n; return newptr; } + // We use the two least significant bits to store a _Type value so + // require memory aligned to at least 4 bytes: + static_assert(__STDCPP_DEFAULT_NEW_ALIGNMENT__ >= 4); + // Require memory suitably aligned for an _Impl and its value types: + static_assert(__STDCPP_DEFAULT_NEW_ALIGNMENT__ >= alignof(value_type)); + // Clear the lowest two bits from the pointer (i.e. remove the _Type value) static _Impl* notype(_Impl* p) { constexpr uintptr_t mask = ~(uintptr_t)0x3; return reinterpret_cast<_Impl*>(reinterpret_cast<uintptr_t>(p) & mask); } + + // Create a new _Impl with capacity for n components. + static unique_ptr<_Impl, _Impl_deleter> + create(int n) + { + using __gnu_cxx::__int_traits; + // Nobody should need paths with this many components. + if (n >= __int_traits<int>::__max / 4) + std::__throw_bad_alloc(); + + if constexpr (__int_traits<int>::__max >= __int_traits<size_t>::__max) + { + // Check that the calculation in create_unchecked(n) won't overflow. + size_t bytes; + if (__builtin_mul_overflow(n, sizeof(value_type), &bytes) + || __builtin_add_overflow(sizeof(_Impl), bytes, &bytes)) + std::__throw_bad_alloc(); + } + // Otherwise, it can't overflow, even for 20-bit size_t on msp430. + + return create_unchecked(n); + } + + // pre: no overflow in Si + n * Sv + static unique_ptr<_Impl, _Impl_deleter> + create_unchecked(int n) + { + void* p = ::operator new(sizeof(_Impl) + n * sizeof(value_type)); + return std::unique_ptr<_Impl, _Impl_deleter>(::new(p) _Impl{n}); + } }; -void path::_List::_Impl_deleter::operator()(_Impl* p) const noexcept +// Destroy and deallocate an _Impl. +void +path::_List::_Impl_deleter::operator()(_Impl* p) const noexcept { p = _Impl::notype(p); if (p) { - __glibcxx_assert(p->_M_size <= p->_M_capacity); - p->clear(); - ::operator delete(p, sizeof(*p) + p->_M_capacity * sizeof(value_type)); + const auto n = p->_M_capacity; + p->~_Impl(); + ::operator delete(p, sizeof(_Impl) + n * sizeof(_Impl::value_type)); } } @@ -455,24 +498,7 @@ path::_List::reserve(int newcap, bool exact = false) newcap = nextcap; } - using __gnu_cxx::__int_traits; - // Nobody should need paths with this many components. - if (newcap >= __int_traits<int>::__max / 4) - std::__throw_bad_alloc(); - - size_t bytes; - if constexpr (__int_traits<int>::__max >= __int_traits<size_t>::__max) - { - size_t components; - if (__builtin_mul_overflow(newcap, sizeof(value_type), &components) - || __builtin_add_overflow(sizeof(_Impl), components, &bytes)) - std::__throw_bad_alloc(); - } - else // This won't overflow, even for 20-bit size_t on msp430. - bytes = sizeof(_Impl) + newcap * sizeof(value_type); - - void* p = ::operator new(bytes); - std::unique_ptr<_Impl, _Impl_deleter> newptr(::new(p) _Impl{newcap}); + auto newptr = _Impl::create(newcap); const int cursize = curptr ? curptr->size() : 0; if (cursize) {
