diff mbox series

libstdc++: Fix stack corruption in std::bitset stream extraction [PR124370]

Message ID 20261001220454.3229149-1-jwakely@redhat.com
State New
Headers
Series libstdc++: Fix stack corruption in std::bitset stream extraction [PR124370] |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_simplebootstrap_build--master-arm-bootstrap success Build passed
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed

Commit Message

Jonathan Wakely Oct. 1, 2026, 10:03 p.m. UTC 
  Fix the __builtin_alloca argument to account for wide characters. Also
consider them when deciding whether to use alloca or the heap.

libstdc++-v3/ChangeLog:

	PR libstdc++/124370
	* include/std/bitset (operator>>): Account for sizeof(_CharT) in
	condition and buffer size.
	* testsuite/20_util/bitset/io/wchar_t.cc: New test.
---

Tested x86_64-linux.

We previously had no tests for wide character I/O with bitset, so the
new test covers the basics as well as the bug.

This is a regression so needs to be backported.

 libstdc++-v3/include/std/bitset               |  8 +++--
 .../testsuite/20_util/bitset/io/wchar_t.cc    | 36 +++++++++++++++++++
 2 files changed, 42 insertions(+), 2 deletions(-)
 create mode 100644 libstdc++-v3/testsuite/20_util/bitset/io/wchar_t.cc

Comments

Tomasz Kaminski Oct. 2, 2026, 5:06 a.m. UTC | #1 
On Fri, Oct 2, 2026 at 12:05 AM Jonathan Wakely <jwakely@redhat.com> wrote:

> Fix the __builtin_alloca argument to account for wide characters. Also
> consider them when deciding whether to use alloca or the heap.
>
> libstdc++-v3/ChangeLog:
>
>         PR libstdc++/124370
>         * include/std/bitset (operator>>): Account for sizeof(_CharT) in
>         condition and buffer size.
>         * testsuite/20_util/bitset/io/wchar_t.cc: New test.
> ---
>
> Tested x86_64-linux.
>
> We previously had no tests for wide character I/O with bitset, so the
> new test covers the basics as well as the bug.
>
> This is a regression so needs to be backported.
>
>  libstdc++-v3/include/std/bitset               |  8 +++--
>  .../testsuite/20_util/bitset/io/wchar_t.cc    | 36 +++++++++++++++++++
>  2 files changed, 42 insertions(+), 2 deletions(-)
>  create mode 100644 libstdc++-v3/testsuite/20_util/bitset/io/wchar_t.cc
>
> diff --git a/libstdc++-v3/include/std/bitset
> b/libstdc++-v3/include/std/bitset
> index d581606e7dde..c244ee80bbb8 100644
> --- a/libstdc++-v3/include/std/bitset
> +++ b/libstdc++-v3/include/std/bitset
> @@ -1709,7 +1709,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>  #pragma GCC diagnostic ignored "-Wc++17-extensions" // if constexpr
>        struct _Buffer
>        {
> -       static _GLIBCXX_CONSTEXPR bool _S_use_alloca() { return _Nb <=
> 256; }
> +       static _GLIBCXX_CONSTEXPR size_t
> +       _S_size_in_bytes() { return _Nb * sizeof(_CharT); }
> +
> +       static _GLIBCXX_CONSTEXPR bool
> +       _S_use_alloca() { return _S_size_in_bytes() <= 256; }
>
I would prefer something like:
private:
          static const size_t _S_local =  _Nb * sizeof(_CharT) <= 256 ?
_S_bytes : 1;
          _CharT _M_buf[_S_local];


>
>         explicit _Buffer(_CharT* __p) : _M_ptr(__p) { }
>
And then:
           _Buffer() : _M_ptr(&__local)
           {
               if _GLIBCXX_CONSTEXPR(_S_local < _Nb)
                  _M_ptr =  new _CharT[_Nb];
           }

        ~_Buffer()
        {
         if _GLIBCXX_CONSTEXPR(_S_local < _Nb)
            delete[] _M_ptr;
        }


> @@ -1723,7 +1727,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>        };
>        _CharT* __ptr;
>        if _GLIBCXX_CONSTEXPR (_Buffer::_S_use_alloca())
> -       __ptr = (_CharT*)__builtin_alloca(_Nb);
> +       __ptr = (_CharT*)__builtin_alloca(_Buffer::_S_size_in_bytes());
>
Is this guaranteed to always give us proper aligment for wchar_t? Does not
matter with my proposal,
as we will create array of _CharT on stack.

>        else
>         __ptr = new _CharT[_Nb];
>        const _Buffer __buf(__ptr);
> diff --git a/libstdc++-v3/testsuite/20_util/bitset/io/wchar_t.cc
> b/libstdc++-v3/testsuite/20_util/bitset/io/wchar_t.cc
> new file mode 100644
> index 000000000000..893ebd37352a
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/20_util/bitset/io/wchar_t.cc
> @@ -0,0 +1,36 @@
> +// { dg-do run }
> +
> +// C++98 23.3.5.3 bitset operators
> +
> +#include <bitset>
> +#include <sstream>
> +#include <testsuite_hooks.h>
> +
> +// Bug 124370 - Out-of-bounds write for wistream >> bitset
> +void
> +test_pr124370()
> +{
> +  std::wistringstream input(L"10011011001101");
> +  std::bitset<10> b;
> +  input >> b;
> +  VERIFY( b == std::bitset<10>(0b1001101100) );
> +}
> +
> +// Round-trip through a wide stream.
> +template<int N>
> +void
> +test_round_trip()
> +{
> +  std::bitset<N> a(0b100110110011010101101010), b;
> +  std::wstringstream ss;
> +  ss << a;
> +  ss >> b;
> +  VERIFY( b == a );
> +}
> +
> +int main()
> +{
> +  test_pr124370();
> +  test_round_trip<24>();
> +  test_round_trip<1024>();
> +}
> --
> 2.55.0
>
>
Tomasz Kaminski Oct. 2, 2026, 5:14 a.m. UTC | #2 
On Fri, Oct 2, 2026 at 7:06 AM Tomasz Kaminski <tkaminsk@redhat.com> wrote:

>
>
> On Fri, Oct 2, 2026 at 12:05 AM Jonathan Wakely <jwakely@redhat.com>
> wrote:
>
>> Fix the __builtin_alloca argument to account for wide characters. Also
>> consider them when deciding whether to use alloca or the heap.
>>
>> libstdc++-v3/ChangeLog:
>>
>>         PR libstdc++/124370
>>         * include/std/bitset (operator>>): Account for sizeof(_CharT) in
>>         condition and buffer size.
>>         * testsuite/20_util/bitset/io/wchar_t.cc: New test.
>> ---
>>
>> Tested x86_64-linux.
>>
>> We previously had no tests for wide character I/O with bitset, so the
>> new test covers the basics as well as the bug.
>>
>> This is a regression so needs to be backported.
>>
>>  libstdc++-v3/include/std/bitset               |  8 +++--
>>  .../testsuite/20_util/bitset/io/wchar_t.cc    | 36 +++++++++++++++++++
>>  2 files changed, 42 insertions(+), 2 deletions(-)
>>  create mode 100644 libstdc++-v3/testsuite/20_util/bitset/io/wchar_t.cc
>>
>> diff --git a/libstdc++-v3/include/std/bitset
>> b/libstdc++-v3/include/std/bitset
>> index d581606e7dde..c244ee80bbb8 100644
>> --- a/libstdc++-v3/include/std/bitset
>> +++ b/libstdc++-v3/include/std/bitset
>> @@ -1709,7 +1709,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>>  #pragma GCC diagnostic ignored "-Wc++17-extensions" // if constexpr
>>        struct _Buffer
>>        {
>> -       static _GLIBCXX_CONSTEXPR bool _S_use_alloca() { return _Nb <=
>> 256; }
>> +       static _GLIBCXX_CONSTEXPR size_t
>> +       _S_size_in_bytes() { return _Nb * sizeof(_CharT); }
>> +
>> +       static _GLIBCXX_CONSTEXPR bool
>> +       _S_use_alloca() { return _S_size_in_bytes() <= 256; }
>>
> I would prefer something like:
> private:
>           static const size_t _S_local =  _Nb * sizeof(_CharT) <= 256 ?
> _S_bytes : 1;
>
I meant:
              static const size_t _S_local =  _Nb * sizeof(_CharT) <= 256 ?
_Nb : 1;

>           _CharT _M_buf[_S_local];
>
>
>>
>>         explicit _Buffer(_CharT* __p) : _M_ptr(__p) { }
>>
> And then:
>            _Buffer() : _M_ptr(&__local)
>            {
>                if _GLIBCXX_CONSTEXPR(_S_local < _Nb)
>                   _M_ptr =  new _CharT[_Nb];
>            }
>
>         ~_Buffer()
>         {
>          if _GLIBCXX_CONSTEXPR(_S_local < _Nb)
>             delete[] _M_ptr;
>         }
>
>
>> @@ -1723,7 +1727,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>>        };
>>        _CharT* __ptr;
>>        if _GLIBCXX_CONSTEXPR (_Buffer::_S_use_alloca())
>> -       __ptr = (_CharT*)__builtin_alloca(_Nb);
>> +       __ptr = (_CharT*)__builtin_alloca(_Buffer::_S_size_in_bytes());
>>
> Is this guaranteed to always give us proper aligment for wchar_t? Does not
> matter with my proposal,
> as we will create array of _CharT on stack.
>
>>        else
>>         __ptr = new _CharT[_Nb];
>>        const _Buffer __buf(__ptr);
>> diff --git a/libstdc++-v3/testsuite/20_util/bitset/io/wchar_t.cc
>> b/libstdc++-v3/testsuite/20_util/bitset/io/wchar_t.cc
>> new file mode 100644
>> index 000000000000..893ebd37352a
>> --- /dev/null
>> +++ b/libstdc++-v3/testsuite/20_util/bitset/io/wchar_t.cc
>> @@ -0,0 +1,36 @@
>> +// { dg-do run }
>> +
>> +// C++98 23.3.5.3 bitset operators
>> +
>> +#include <bitset>
>> +#include <sstream>
>> +#include <testsuite_hooks.h>
>> +
>> +// Bug 124370 - Out-of-bounds write for wistream >> bitset
>> +void
>> +test_pr124370()
>> +{
>> +  std::wistringstream input(L"10011011001101");
>> +  std::bitset<10> b;
>> +  input >> b;
>> +  VERIFY( b == std::bitset<10>(0b1001101100) );
>> +}
>> +
>> +// Round-trip through a wide stream.
>> +template<int N>
>> +void
>> +test_round_trip()
>> +{
>> +  std::bitset<N> a(0b100110110011010101101010), b;
>> +  std::wstringstream ss;
>> +  ss << a;
>> +  ss >> b;
>> +  VERIFY( b == a );
>> +}
>> +
>> +int main()
>> +{
>> +  test_pr124370();
>> +  test_round_trip<24>();
>> +  test_round_trip<1024>();
>> +}
>> --
>> 2.55.0
>>
>>
Jonathan Wakely Oct. 2, 2026, 8:12 a.m. UTC | #3 
On Fri, 2 Oct 2026 at 06:06, Tomasz Kaminski <tkaminsk@redhat.com> wrote:
>
>
>
> On Fri, Oct 2, 2026 at 12:05 AM Jonathan Wakely <jwakely@redhat.com> wrote:
>>
>> Fix the __builtin_alloca argument to account for wide characters. Also
>> consider them when deciding whether to use alloca or the heap.
>>
>> libstdc++-v3/ChangeLog:
>>
>>         PR libstdc++/124370
>>         * include/std/bitset (operator>>): Account for sizeof(_CharT) in
>>         condition and buffer size.
>>         * testsuite/20_util/bitset/io/wchar_t.cc: New test.
>> ---
>>
>> Tested x86_64-linux.
>>
>> We previously had no tests for wide character I/O with bitset, so the
>> new test covers the basics as well as the bug.
>>
>> This is a regression so needs to be backported.
>>
>>  libstdc++-v3/include/std/bitset               |  8 +++--
>>  .../testsuite/20_util/bitset/io/wchar_t.cc    | 36 +++++++++++++++++++
>>  2 files changed, 42 insertions(+), 2 deletions(-)
>>  create mode 100644 libstdc++-v3/testsuite/20_util/bitset/io/wchar_t.cc
>>
>> diff --git a/libstdc++-v3/include/std/bitset b/libstdc++-v3/include/std/bitset
>> index d581606e7dde..c244ee80bbb8 100644
>> --- a/libstdc++-v3/include/std/bitset
>> +++ b/libstdc++-v3/include/std/bitset
>> @@ -1709,7 +1709,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>>  #pragma GCC diagnostic ignored "-Wc++17-extensions" // if constexpr
>>        struct _Buffer
>>        {
>> -       static _GLIBCXX_CONSTEXPR bool _S_use_alloca() { return _Nb <= 256; }
>> +       static _GLIBCXX_CONSTEXPR size_t
>> +       _S_size_in_bytes() { return _Nb * sizeof(_CharT); }
>> +
>> +       static _GLIBCXX_CONSTEXPR bool
>> +       _S_use_alloca() { return _S_size_in_bytes() <= 256; }
>
> I would prefer something like:
> private:
>           static const size_t _S_local =  _Nb * sizeof(_CharT) <= 256 ? _S_bytes : 1;
>           _CharT _M_buf[_S_local];
>
>>
>>
>>         explicit _Buffer(_CharT* __p) : _M_ptr(__p) { }
>
> And then:
>            _Buffer() : _M_ptr(&__local)
>            {
>                if _GLIBCXX_CONSTEXPR(_S_local < _Nb)
>                   _M_ptr =  new _CharT[_Nb];
>            }
>
>         ~_Buffer()
>         {
>          if _GLIBCXX_CONSTEXPR(_S_local < _Nb)
>             delete[] _M_ptr;
>         }
>
>>
>> @@ -1723,7 +1727,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
>>        };
>>        _CharT* __ptr;
>>        if _GLIBCXX_CONSTEXPR (_Buffer::_S_use_alloca())
>> -       __ptr = (_CharT*)__builtin_alloca(_Nb);
>> +       __ptr = (_CharT*)__builtin_alloca(_Buffer::_S_size_in_bytes());
>
> Is this guaranteed to always give us proper aligment for wchar_t?

"The object is aligned on the default stack alignment boundary for the
target determined by the __BIGGEST_ALIGNMENT__ macro."
https://gcc.gnu.org/onlinedocs/gcc/Stack-Allocation.html#index-_005f_005fbuiltin_005falloca

> Does not matter with my proposal,
> as we will create array of _CharT on stack.

Indeed
Jonathan Wakely Oct. 2, 2026, 8:25 a.m. UTC | #4 
On Fri, 2 Oct 2026 at 09:12, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> On Fri, 2 Oct 2026 at 06:06, Tomasz Kaminski <tkaminsk@redhat.com> wrote:
> >
> >
> >
> > On Fri, Oct 2, 2026 at 12:05 AM Jonathan Wakely <jwakely@redhat.com> wrote:
> >>
> >> Fix the __builtin_alloca argument to account for wide characters. Also
> >> consider them when deciding whether to use alloca or the heap.
> >>
> >> libstdc++-v3/ChangeLog:
> >>
> >>         PR libstdc++/124370
> >>         * include/std/bitset (operator>>): Account for sizeof(_CharT) in
> >>         condition and buffer size.
> >>         * testsuite/20_util/bitset/io/wchar_t.cc: New test.
> >> ---
> >>
> >> Tested x86_64-linux.
> >>
> >> We previously had no tests for wide character I/O with bitset, so the
> >> new test covers the basics as well as the bug.
> >>
> >> This is a regression so needs to be backported.
> >>
> >>  libstdc++-v3/include/std/bitset               |  8 +++--
> >>  .../testsuite/20_util/bitset/io/wchar_t.cc    | 36 +++++++++++++++++++
> >>  2 files changed, 42 insertions(+), 2 deletions(-)
> >>  create mode 100644 libstdc++-v3/testsuite/20_util/bitset/io/wchar_t.cc
> >>
> >> diff --git a/libstdc++-v3/include/std/bitset b/libstdc++-v3/include/std/bitset
> >> index d581606e7dde..c244ee80bbb8 100644
> >> --- a/libstdc++-v3/include/std/bitset
> >> +++ b/libstdc++-v3/include/std/bitset
> >> @@ -1709,7 +1709,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
> >>  #pragma GCC diagnostic ignored "-Wc++17-extensions" // if constexpr
> >>        struct _Buffer
> >>        {
> >> -       static _GLIBCXX_CONSTEXPR bool _S_use_alloca() { return _Nb <= 256; }
> >> +       static _GLIBCXX_CONSTEXPR size_t
> >> +       _S_size_in_bytes() { return _Nb * sizeof(_CharT); }
> >> +
> >> +       static _GLIBCXX_CONSTEXPR bool
> >> +       _S_use_alloca() { return _S_size_in_bytes() <= 256; }
> >
> > I would prefer something like:
> > private:
> >           static const size_t _S_local =  _Nb * sizeof(_CharT) <= 256 ? _S_bytes : 1;
> >           _CharT _M_buf[_S_local];
> >
> >>
> >>
> >>         explicit _Buffer(_CharT* __p) : _M_ptr(__p) { }
> >
> > And then:
> >            _Buffer() : _M_ptr(&__local)
> >            {
> >                if _GLIBCXX_CONSTEXPR(_S_local < _Nb)
> >                   _M_ptr =  new _CharT[_Nb];
> >            }
> >
> >         ~_Buffer()
> >         {
> >          if _GLIBCXX_CONSTEXPR(_S_local < _Nb)
> >             delete[] _M_ptr;
> >         }
> >
> >>
> >> @@ -1723,7 +1727,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
> >>        };
> >>        _CharT* __ptr;
> >>        if _GLIBCXX_CONSTEXPR (_Buffer::_S_use_alloca())
> >> -       __ptr = (_CharT*)__builtin_alloca(_Nb);
> >> +       __ptr = (_CharT*)__builtin_alloca(_Buffer::_S_size_in_bytes());
> >
> > Is this guaranteed to always give us proper aligment for wchar_t?
>
> "The object is aligned on the default stack alignment boundary for the
> target determined by the __BIGGEST_ALIGNMENT__ macro."
> https://gcc.gnu.org/onlinedocs/gcc/Stack-Allocation.html#index-_005f_005fbuiltin_005falloca
>
> > Does not matter with my proposal,
> > as we will create array of _CharT on stack.
>
> Indeed

We can do this to save a few more bytes on the stack, but maybe the
union is overkill:

      static const bool __use_local = _Nb * sizeof(_CharT) <= 256;

      struct _Buffer
      {
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wc++17-extensions" // if constexpr
        _Buffer()
        {
          if _GLIBCXX_CONSTEXPR (!__use_local)
            _M_ptr = new _CharT[_Nb];
        }

        ~_Buffer()
        {
          if _GLIBCXX_CONSTEXPR (!__use_local)
            delete[] _M_ptr;
        }

        _CharT* get()
        {
          if _GLIBCXX_CONSTEXPR (!__use_local)
            return _M_ptr;
          else
            return _M_local;
        }
#pragma GCC diagnostic pop

        union {
          _CharT* _M_ptr;
          _CharT _M_local[__use_local ? _Nb : 1];
        };
      } __buf;

      _CharT* __ptr = __buf.get();
Tomasz Kaminski Oct. 2, 2026, 8:29 a.m. UTC | #5 
On Fri, Oct 2, 2026 at 10:26 AM Jonathan Wakely <jwakely@redhat.com> wrote:

> On Fri, 2 Oct 2026 at 09:12, Jonathan Wakely <jwakely@redhat.com> wrote:
> >
> > On Fri, 2 Oct 2026 at 06:06, Tomasz Kaminski <tkaminsk@redhat.com>
> wrote:
> > >
> > >
> > >
> > > On Fri, Oct 2, 2026 at 12:05 AM Jonathan Wakely <jwakely@redhat.com>
> wrote:
> > >>
> > >> Fix the __builtin_alloca argument to account for wide characters. Also
> > >> consider them when deciding whether to use alloca or the heap.
> > >>
> > >> libstdc++-v3/ChangeLog:
> > >>
> > >>         PR libstdc++/124370
> > >>         * include/std/bitset (operator>>): Account for sizeof(_CharT)
> in
> > >>         condition and buffer size.
> > >>         * testsuite/20_util/bitset/io/wchar_t.cc: New test.
> > >> ---
> > >>
> > >> Tested x86_64-linux.
> > >>
> > >> We previously had no tests for wide character I/O with bitset, so the
> > >> new test covers the basics as well as the bug.
> > >>
> > >> This is a regression so needs to be backported.
> > >>
> > >>  libstdc++-v3/include/std/bitset               |  8 +++--
> > >>  .../testsuite/20_util/bitset/io/wchar_t.cc    | 36
> +++++++++++++++++++
> > >>  2 files changed, 42 insertions(+), 2 deletions(-)
> > >>  create mode 100644
> libstdc++-v3/testsuite/20_util/bitset/io/wchar_t.cc
> > >>
> > >> diff --git a/libstdc++-v3/include/std/bitset
> b/libstdc++-v3/include/std/bitset
> > >> index d581606e7dde..c244ee80bbb8 100644
> > >> --- a/libstdc++-v3/include/std/bitset
> > >> +++ b/libstdc++-v3/include/std/bitset
> > >> @@ -1709,7 +1709,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
> > >>  #pragma GCC diagnostic ignored "-Wc++17-extensions" // if constexpr
> > >>        struct _Buffer
> > >>        {
> > >> -       static _GLIBCXX_CONSTEXPR bool _S_use_alloca() { return _Nb
> <= 256; }
> > >> +       static _GLIBCXX_CONSTEXPR size_t
> > >> +       _S_size_in_bytes() { return _Nb * sizeof(_CharT); }
> > >> +
> > >> +       static _GLIBCXX_CONSTEXPR bool
> > >> +       _S_use_alloca() { return _S_size_in_bytes() <= 256; }
> > >
> > > I would prefer something like:
> > > private:
> > >           static const size_t _S_local =  _Nb * sizeof(_CharT) <= 256
> ? _S_bytes : 1;
> > >           _CharT _M_buf[_S_local];
> > >
> > >>
> > >>
> > >>         explicit _Buffer(_CharT* __p) : _M_ptr(__p) { }
> > >
> > > And then:
> > >            _Buffer() : _M_ptr(&__local)
> > >            {
> > >                if _GLIBCXX_CONSTEXPR(_S_local < _Nb)
> > >                   _M_ptr =  new _CharT[_Nb];
> > >            }
> > >
> > >         ~_Buffer()
> > >         {
> > >          if _GLIBCXX_CONSTEXPR(_S_local < _Nb)
> > >             delete[] _M_ptr;
> > >         }
> > >
> > >>
> > >> @@ -1723,7 +1727,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
> > >>        };
> > >>        _CharT* __ptr;
> > >>        if _GLIBCXX_CONSTEXPR (_Buffer::_S_use_alloca())
> > >> -       __ptr = (_CharT*)__builtin_alloca(_Nb);
> > >> +       __ptr =
> (_CharT*)__builtin_alloca(_Buffer::_S_size_in_bytes());
> > >
> > > Is this guaranteed to always give us proper aligment for wchar_t?
> >
> > "The object is aligned on the default stack alignment boundary for the
> > target determined by the __BIGGEST_ALIGNMENT__ macro."
> >
> https://gcc.gnu.org/onlinedocs/gcc/Stack-Allocation.html#index-_005f_005fbuiltin_005falloca
> >
> > > Does not matter with my proposal,
> > > as we will create array of _CharT on stack.
> >
> > Indeed
>
> We can do this to save a few more bytes on the stack, but maybe the
> union is overkill:
>
I think it is overkill, 4B on stack is one temporaryr float.

>
>       static const bool __use_local = _Nb * sizeof(_CharT) <= 256;
>
>       struct _Buffer
>       {
> #pragma GCC diagnostic push
> #pragma GCC diagnostic ignored "-Wc++17-extensions" // if constexpr
>         _Buffer()
>         {
>           if _GLIBCXX_CONSTEXPR (!__use_local)
>             _M_ptr = new _CharT[_Nb];
>         }
>
>         ~_Buffer()
>         {
>           if _GLIBCXX_CONSTEXPR (!__use_local)
>             delete[] _M_ptr;
>         }
>
>         _CharT* get()
>         {
>           if _GLIBCXX_CONSTEXPR (!__use_local)
>             return _M_ptr;
>           else
>             return _M_local;
>         }
> #pragma GCC diagnostic pop
>
>         union {
>           _CharT* _M_ptr;
>           _CharT _M_local[__use_local ? _Nb : 1];
>         };
>       } __buf;
>
>       _CharT* __ptr = __buf.get();
>
>
Jonathan Wakely Oct. 2, 2026, 8:30 a.m. UTC | #6 
On Fri, 2 Oct 2026 at 09:12, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> On Fri, 2 Oct 2026 at 06:06, Tomasz Kaminski <tkaminsk@redhat.com> wrote:
> >
> >
> >
> > On Fri, Oct 2, 2026 at 12:05 AM Jonathan Wakely <jwakely@redhat.com> wrote:
> >>
> >> Fix the __builtin_alloca argument to account for wide characters. Also
> >> consider them when deciding whether to use alloca or the heap.
> >>
> >> libstdc++-v3/ChangeLog:
> >>
> >>         PR libstdc++/124370
> >>         * include/std/bitset (operator>>): Account for sizeof(_CharT) in
> >>         condition and buffer size.
> >>         * testsuite/20_util/bitset/io/wchar_t.cc: New test.
> >> ---
> >>
> >> Tested x86_64-linux.
> >>
> >> We previously had no tests for wide character I/O with bitset, so the
> >> new test covers the basics as well as the bug.
> >>
> >> This is a regression so needs to be backported.
> >>
> >>  libstdc++-v3/include/std/bitset               |  8 +++--
> >>  .../testsuite/20_util/bitset/io/wchar_t.cc    | 36 +++++++++++++++++++
> >>  2 files changed, 42 insertions(+), 2 deletions(-)
> >>  create mode 100644 libstdc++-v3/testsuite/20_util/bitset/io/wchar_t.cc
> >>
> >> diff --git a/libstdc++-v3/include/std/bitset b/libstdc++-v3/include/std/bitset
> >> index d581606e7dde..c244ee80bbb8 100644
> >> --- a/libstdc++-v3/include/std/bitset
> >> +++ b/libstdc++-v3/include/std/bitset
> >> @@ -1709,7 +1709,11 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
> >>  #pragma GCC diagnostic ignored "-Wc++17-extensions" // if constexpr
> >>        struct _Buffer
> >>        {
> >> -       static _GLIBCXX_CONSTEXPR bool _S_use_alloca() { return _Nb <= 256; }
> >> +       static _GLIBCXX_CONSTEXPR size_t
> >> +       _S_size_in_bytes() { return _Nb * sizeof(_CharT); }
> >> +
> >> +       static _GLIBCXX_CONSTEXPR bool
> >> +       _S_use_alloca() { return _S_size_in_bytes() <= 256; }
> >
> > I would prefer something like:
> > private:
> >           static const size_t _S_local =  _Nb * sizeof(_CharT) <= 256 ? _S_bytes : 1;
> >           _CharT _M_buf[_S_local];
> >
> >>
> >>
> >>         explicit _Buffer(_CharT* __p) : _M_ptr(__p) { }
> >
> > And then:
> >            _Buffer() : _M_ptr(&__local)
> >            {
> >                if _GLIBCXX_CONSTEXPR(_S_local < _Nb)
> >                   _M_ptr =  new _CharT[_Nb];
> >            }
> >
> >         ~_Buffer()
> >         {
> >          if _GLIBCXX_CONSTEXPR(_S_local < _Nb)
> >             delete[] _M_ptr;
> >         }
> >
> >>
> >> @@ -1723,7 +1727,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
> >>        };
> >>        _CharT* __ptr;
> >>        if _GLIBCXX_CONSTEXPR (_Buffer::_S_use_alloca())
> >> -       __ptr = (_CharT*)__builtin_alloca(_Nb);
> >> +       __ptr = (_CharT*)__builtin_alloca(_Buffer::_S_size_in_bytes());
> >
> > Is this guaranteed to always give us proper aligment for wchar_t?
>
> "The object is aligned on the default stack alignment boundary for the
> target determined by the __BIGGEST_ALIGNMENT__ macro."
> https://gcc.gnu.org/onlinedocs/gcc/Stack-Allocation.html#index-_005f_005fbuiltin_005falloca

But __BIGGEST_ALIGNMENT__ == 2 on msp430 and wchar_t is 4 bytes, so
that wouldn't be OK.

There's __builtin_alloca_with_align which Clang also supports. But we
don't need it if we just use an array of _CharT.

>
> > Does not matter with my proposal,
> > as we will create array of _CharT on stack.
>
> Indeed
diff mbox series

Patch

diff --git a/libstdc++-v3/include/std/bitset b/libstdc++-v3/include/std/bitset
index d581606e7dde..c244ee80bbb8 100644
--- a/libstdc++-v3/include/std/bitset
+++ b/libstdc++-v3/include/std/bitset
@@ -1709,7 +1709,11 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 #pragma GCC diagnostic ignored "-Wc++17-extensions" // if constexpr
       struct _Buffer
       {
-	static _GLIBCXX_CONSTEXPR bool _S_use_alloca() { return _Nb <= 256; }
+	static _GLIBCXX_CONSTEXPR size_t
+	_S_size_in_bytes() { return _Nb * sizeof(_CharT); }
+
+	static _GLIBCXX_CONSTEXPR bool
+	_S_use_alloca() { return _S_size_in_bytes() <= 256; }
 
 	explicit _Buffer(_CharT* __p) : _M_ptr(__p) { }
 
@@ -1723,7 +1727,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       };
       _CharT* __ptr;
       if _GLIBCXX_CONSTEXPR (_Buffer::_S_use_alloca())
-	__ptr = (_CharT*)__builtin_alloca(_Nb);
+	__ptr = (_CharT*)__builtin_alloca(_Buffer::_S_size_in_bytes());
       else
 	__ptr = new _CharT[_Nb];
       const _Buffer __buf(__ptr);
diff --git a/libstdc++-v3/testsuite/20_util/bitset/io/wchar_t.cc b/libstdc++-v3/testsuite/20_util/bitset/io/wchar_t.cc
new file mode 100644
index 000000000000..893ebd37352a
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/bitset/io/wchar_t.cc
@@ -0,0 +1,36 @@ 
+// { dg-do run }
+
+// C++98 23.3.5.3 bitset operators
+
+#include <bitset>
+#include <sstream>
+#include <testsuite_hooks.h>
+
+// Bug 124370 - Out-of-bounds write for wistream >> bitset
+void
+test_pr124370()
+{
+  std::wistringstream input(L"10011011001101");
+  std::bitset<10> b;
+  input >> b;
+  VERIFY( b == std::bitset<10>(0b1001101100) );
+}
+
+// Round-trip through a wide stream.
+template<int N>
+void
+test_round_trip()
+{
+  std::bitset<N> a(0b100110110011010101101010), b;
+  std::wstringstream ss;
+  ss << a;
+  ss >> b;
+  VERIFY( b == a );
+}
+
+int main()
+{
+  test_pr124370();
+  test_round_trip<24>();
+  test_round_trip<1024>();
+}