]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
libstdc++: Ensure filesystem::path internals are sufficiently aligned [PR122255]
authorJonathan Wakely <jwakely@redhat.com>
Sat, 11 Oct 2025 10:22:38 +0000 (11:22 +0100)
committerJonathan Wakely <redi@gcc.gnu.org>
Tue, 14 Oct 2025 14:42:31 +0000 (15:42 +0100)
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 <tkaminsk@redhat.com>
libstdc++-v3/src/c++17/fs_path.cc

index 215afa08ad25f036e0ea7f778f5e9efb3b7d4763..03bb5ecb7be1a254b782e21cea348f89691d0454 100644 (file)
@@ -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)
        {