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.
---
v2: Just assert that __STDCPP_DEFAULT_NEW_ALIGNMENT__ is sufficient,
instead of adding code that probably won't be needed. Split the new
create function into create and create_unchecked. Add a destructor.
Tested x86_64-linux.
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)
{
--
2.51.0
