[PATCH v2] libstdc++: Remove UB in _Arg_value union alternative assignment

[Tomasz Kaminski]

On Thu, Mar 5, 2026 at 7:35 AM Tomasz Kamiński <tkaminsk@redhat.com> wrote:

> The _Arg_value::_M_set method, initialized the union member, by
> assigning to reference to that member produced by _M_get(*this).
> However, per language rules, such assignment has undefined behavior,
> if alternative was not already active, same as for any object not
> within it's lifetime.
>
> To address above, we modify _M_set to use placement new for the class
> types, and invoke _S_access with two arguments for all other types.
> The _S_access (rename of _S_get) is modified to assign the value of
> the second parameter (if provided) to the union member. Such direct
> assignments are treated specially in the language (see N5032
> [class.union.general] p5), and will start lifetime of trivially default
> constructible alternative.
>
> libstdc++-v3/ChangeLog:
>
>         * include/std/format (_Arg_value::_M_get): Rename to...
>         (_Arg_value::_M_access): Modified to accept optional
>         second parameter that is assigned to value.
>         (_Arg_value::_M_get): Handle rename.
>         (_Arg_value::_M_set): Use construct_at for basic_string_view,
>         handle, and two-argument _S_access for other types.
>
> Signed-off-by: Tomasz Kamiński <tkaminsk@redhat.com>
> Signed-off-by: Ivan Lazaric <ivan.lazaric1@gmail.com>
> Co-authored-by: Ivan Lazaric <ivan.lazaric1@gmail.com>
> ---
> v2:
> - fixes typos and add mention to [class.union.general] p5 in commit
>   description
> - add comments to _M_set function
>
> Testing on x86_64-linux. All *format* test passed.
> OK for trunk when all test passes?
>
>  libstdc++-v3/include/std/format | 62 +++++++++++++++++++--------------
>  1 file changed, 35 insertions(+), 27 deletions(-)
>
> diff --git a/libstdc++-v3/include/std/format
> b/libstdc++-v3/include/std/format
> index b014936a21e..9b5d5d499f5 100644
> --- a/libstdc++-v3/include/std/format
> +++ b/libstdc++-v3/include/std/format
> @@ -4212,67 +4212,70 @@ namespace __format
>         { _S_get<_Tp>() = __val; }
>  #endif
>
> -      template<typename _Tp, typename _Self>
> +      // Returns reference to the _Arg_value member with the type _Tp.
> +      // Value of second argument (if provided), is assigned to that
> member.
> +      template<typename _Tp, typename _Self, typename... _Value>
>         [[__gnu__::__always_inline__]]
>         static auto&
> -       _S_get(_Self& __u) noexcept
> +       _S_access(_Self& __u, _Value... __value) noexcept
>         {
> +         static_assert(sizeof...(_Value) <= 1);
>           if constexpr (is_same_v<_Tp, bool>)
> -           return __u._M_bool;
> +           return (__u._M_bool = ... = __value);
>           else if constexpr (is_same_v<_Tp, _CharT>)
> -           return __u._M_c;
> +           return (__u._M_c = ... = __value);
>           else if constexpr (is_same_v<_Tp, int>)
> -           return __u._M_i;
> +           return (__u._M_i = ... = __value);
>           else if constexpr (is_same_v<_Tp, unsigned>)
> -           return __u._M_u;
> +           return (__u._M_u = ... = __value);
>           else if constexpr (is_same_v<_Tp, long long>)
> -           return __u._M_ll;
> +           return (__u._M_ll = ... = __value);
>           else if constexpr (is_same_v<_Tp, unsigned long long>)
> -           return __u._M_ull;
> +           return (__u._M_ull = ... = __value);
>           else if constexpr (is_same_v<_Tp, float>)
> -           return __u._M_flt;
> +           return (__u._M_flt = ... = __value);
>           else if constexpr (is_same_v<_Tp, double>)
> -           return __u._M_dbl;
> +           return (__u._M_dbl = ... = __value);
>  #ifndef _GLIBCXX_LONG_DOUBLE_ALT128_COMPAT
>           else if constexpr (is_same_v<_Tp, long double>)
> -           return __u._M_ldbl;
> +           return (__u._M_ldbl = ... = __value);
>  #else
>           else if constexpr (is_same_v<_Tp, __ibm128>)
> -           return __u._M_ibm128;
> +           return (__u._M_ibm128 = ... = __value);
>           else if constexpr (is_same_v<_Tp, __ieee128>)
> -           return __u._M_ieee128;
> +           return (__u._M_ieee128 = ... = __value);
>  #endif
>  #ifdef __SIZEOF_FLOAT128__
>           else if constexpr (is_same_v<_Tp, __float128>)
> -           return __u._M_float128;
> +           return (__u._M_float128 = ... = __value);
>  #endif
>           else if constexpr (is_same_v<_Tp, const _CharT*>)
> -           return __u._M_str;
> +           return (__u._M_str = ... = __value);
>           else if constexpr (is_same_v<_Tp, basic_string_view<_CharT>>)
> -           return __u._M_sv;
> +           return (__u._M_sv = ... = __value);
>           else if constexpr (is_same_v<_Tp, const void*>)
> -           return __u._M_ptr;
> +           return (__u._M_ptr = ... = __value);
>  #ifdef __SIZEOF_INT128__
>           else if constexpr (is_same_v<_Tp, __int128>)
> -           return __u._M_i128;
> +           return (__u._M_i128 = ... = __value);
>           else if constexpr (is_same_v<_Tp, unsigned __int128>)
> -           return __u._M_u128;
> +           return (__u._M_u128 = ... = __value);
>  #endif
>  #ifdef __BFLT16_DIG__
>           else if constexpr (is_same_v<_Tp, __bflt16_t>)
> -           return __u._M_bf16;
> +           return (__u._M_bf16 = ... = __value);
>  #endif
>  #ifdef __FLT16_DIG__
>           else if constexpr (is_same_v<_Tp, _Float16>)
> -           return __u._M_f16;
> +           return (__u._M_f16 = ... = __value);
>  #endif
>  #ifdef __FLT32_DIG__
>           else if constexpr (is_same_v<_Tp, _Float32>)
> -           return __u._M_f32;
> +           return (__u._M_f32 = ... = __value);
>  #endif
>  #ifdef __FLT64_DIG__
>           else if constexpr (is_same_v<_Tp, _Float64>)
> -           return __u._M_f64;
> +           return (__u._M_f64 = ... = __value);
>  #endif
>           else if constexpr (is_same_v<_Tp, handle>)
>             return __u._M_handle;
> @@ -4283,23 +4286,28 @@ namespace __format
>         [[__gnu__::__always_inline__]]
>         auto&
>         _M_get() noexcept
> -       { return _S_get<_Tp>(*this); }
> +       { return _S_access<_Tp>(*this); }
>
>        template<typename _Tp>
>         [[__gnu__::__always_inline__]]
>         const auto&
>         _M_get() const noexcept
> -       { return _S_get<_Tp>(*this); }
> +       { return _S_access<_Tp>(*this); }
>
>        template<typename _Tp>
>         [[__gnu__::__always_inline__]]
>         void
>         _M_set(_Tp __v) noexcept
>         {
> -         if constexpr (is_same_v<_Tp, handle>)
> +         // Explicitly construct types without trivial default
> constructor.
> +         if constexpr (is_same_v<_Tp, basic_string_view<_CharT>>)
> +           std::construct_at(&_M_sv, __v);
> +         else if constexpr (is_same_v<_Tp, handle>)
>             std::construct_at(&_M_handle, __v);
>           else
> -           _S_get<_Tp>(*this) = __v;
> +           // Builtin types, has trivial default constructor and
> assignment
>
Will change that to: "Builtin types are trivially default constructible,
and assignment..."
They lack a constructor, so that was technically incorrect.

> +           // changes active member per N5032 [class.union.general] p5.
> +           _S_access<_Tp>(*this, __v);
>         }
>        };
>
> --
> 2.53.0
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://gcc.gnu.org/pipermail/libstdc++/attachments/20260305/2dc526fa/attachment.htm>