[PATCH v3] libstdc++: Fix SIGSEGV in std::print with a setvbuf'd FILE
Tomasz Kaminski
tkaminsk@redhat.com
Thu Aug 20 16:33:22 GMT 2026
On Thu, Aug 20, 2026 at 5:28 PM Jonathan Wakely <jwakely@redhat.com> wrote:
> On Thu, 20 Aug 2026 at 15:21, Tomasz Kaminski <tkaminsk@redhat.com> wrote:
> >
> >
> >
> > On Wed, Aug 19, 2026 at 4:40 PM Jonathan Wakely <jwakely@redhat.com>
> wrote:
> >>
> >> On Wed, 19 Aug 2026 at 15:24, Tomasz Kaminski <tkaminsk@redhat.com>
> wrote:
> >> >
> >> >
> >> >
> >> > On Wed, Aug 19, 2026 at 4:17 PM Jonathan Wakely <jwakely@redhat.com>
> wrote:
> >> >>
> >> >> On Mon, 17 Aug 2026 at 09:58 +0200, Tomasz Kamiński wrote:
> >> >> >From: Anlai Lu <agicy@qq.com>
> >> >> >
> >> >> >_File_sink::_M_write_buf() returns a span with a null data pointer
> when
> >> >> >the FILE buffer was pre-allocated by setvbuf but never written
> (glibc
> >> >> >keeps _IO_write_ptr as nullptr until the first write). The empty()
> check
> >> >> >in the constructor then fails to force buffer initialization and
> the format
> >> >> >engine memcpys into nullptr.
> >> >>
> >> >> Please replace the paragraph above with the first two paragraphs
> >> >> from the original [PATCH 0/1] email, and a shorter form of the third
> >> >> paragraph:
> >> >>
> >> >> This is a regression introduced by the P3107R5 implementation in
> >> >> r16-4350-g8bd872f1ea7414. Before that formatting to a FILE* wrote
> to
> >> >> _Str_sink and used fwrite, which handles this legal stream state.
> >> >>
> >> >>
> >> >> >Calls __overflow when _IO_write_ptr is null in _File_sink
> constructor.
> >> >>
> >> >> Let's say:
> >> >>
> >> >> This fixes it by calling __overflow from the _File_sink
> constructor
> >> >> when _IO_write_ptr is null.
> >> >>
> >> >> >
> >> >> >libstdc++-v3/ChangeLog:
> >> >> >
> >> >> > * include/bits/print.h (_File_sink::_File::_File)
> >> >> > [_GLIBCXX_USE_STDIO_LOCKING && _GLIBCXX_USE_GLIBC_STDIO_EXT]:
> >> >> > Move __overflow call to separte member function.
> >> >> > (_File_sink::_File::_M_init_write_buf)
> >> >> > [_GLIBCXX_USE_STDIO_LOCKING && _GLIBCXX_USE_GLIBC_STDIO_EXT]:
> >> >> > Extracted from constructor, call __overflow for null
> >> >> > _M_file->_IO_write_ptr.
> >> >> > (_File_sink::_File_sink)
> >> >> > [_GLIBCXX_USE_STDIO_LOCKING && _GLIBCXX_USE_GLIBC_STDIO_EXT]:
> >> >> > Call _M_init_write_buf instead of _M_writebuf.
> >> >> > * testsuite/27_io/print/1.cc: Add test_print_setvbuf.
> >> >> >
> >> >> >Co-authored-by: Tomasz Kamiński <tkaminsk@redhat.com>
> >> >> >Signed-off-by: Anlai Lu <agicy@qq.com>
> >> >> >Signed-off-by: Tomasz Kamiński <tkaminsk@redhat.com>
> >> >> >---
> >> >> >v3 introduced _M_init_write_buf that I suggested on review.
> >> >> >
> >> >> >Tested on x86_64-linux. OK for trunk and 16?
> >> >> >
> >> >> > libstdc++-v3/include/bits/print.h | 28
> ++++++++++++++++---------
> >> >> > libstdc++-v3/testsuite/27_io/print/1.cc | 17 +++++++++++++++
> >> >> > 2 files changed, 35 insertions(+), 10 deletions(-)
> >> >> >
> >> >> >diff --git a/libstdc++-v3/include/bits/print.h
> b/libstdc++-v3/include/bits/print.h
> >> >> >index 20724844726..d63267c8ecf 100644
> >> >> >--- a/libstdc++-v3/include/bits/print.h
> >> >> >+++ b/libstdc++-v3/include/bits/print.h
> >> >> >@@ -80,20 +80,28 @@ namespace __format
> >> >> > ::funlockfile(__f);
> >> >> > __throw_system_error(EACCES);
> >> >> > }
> >> >> >- // Allocate buffer if needed:
> >> >> >- if (_M_write_buf().empty())
> >> >> >- if (::__overflow(__f, EOF) == EOF)
> >> >> >- {
> >> >> >- const int __err = errno;
> >> >> >- ::funlockfile(__f);
> >> >> >- __throw_system_error(__err);
> >> >> >- }
> >> >> > }
> >> >> >
> >> >> > ~_File() { ::funlockfile(_M_file); }
> >> >> >
> >> >> > _File(_File&&) = delete;
> >> >> >
> >> >> >+ // Allocate FILE's output buffer if needed, and returns span
> >> >>
> >> >> s/returns span/return a span/
> >> >>
> >> >> >+ // viewing unused portion of it.
> >> >> >+ std::span<char>
> >> >> >+ _M_init_write_buf()
> >> >> >+ {
> >> >> >+ // After setvbuf glibc pre-allocates the buffer but
> _IO_write_ptr
> >> >> >+ // remains null until the first write.
> >> >>
> >> >> The two comment lines above are indented inconsistently, the first
> one
> >> >> uses spaces and the second uses a tab.
> >> >>
> >> >> >+ if (!_M_file->_IO_write_ptr || _M_write_buf().empty())
> >> >> >+ if (::__overflow(_M_file, EOF) == EOF)
> >> >> >+ {
> >> >> >+ const int __err = errno;
> >> >> >+ __throw_system_error(__err);
> >> >>
> >> >> There's no need to store errno before throwing (because we're not
> >> >> calling funlockfile here) so it can be just:
> >> >>
> >> >> __throw_system_error(errno);
> >> >>
> >> >> OK with those changes, thanks.
> >> >
> >> > OK also to backport after some time to 16?
> >>
> >> Yes, OK for gcc-16 too.
> >> No need to wait, I think this is obviously more correct and safer than
> >> the current code.
> >
> > I was thinking about test, i.e. that there is some architecture on which
> > setvbuf does not work, and I will need to add some dg-requires to it.
>
> I'd still just push to gcc-16 now. If it causes a new FAIL, we can fix
> it on both branches.
>
Backported it (with follow up squashed) to GCC-16 just now.
>
> setvbuf is in C89 and the testcase has dg-require-fileio, which
> requires POSIX open, lseek and read. I would expect any system
> providing those POSIX APIs to also provide C89 stdio.
>
> >>
> >>
> >> >>
> >> >>
> >> >>
> >> >> >+ }
> >> >> >+ return _M_write_buf();
> >> >> >+ }
> >> >> >+
> >> >> > // A span viewing the unused portion of the stream's output
> buffer.
> >> >> > std::span<char>
> >> >> > _M_write_buf() noexcept
> >> >> >@@ -158,8 +166,8 @@ namespace __format
> >> >> > : _M_file(__f), _M_add_newline(__add_newline)
> >> >> > {
> >> >> > if (!_M_file._M_unbuffered())
> >> >> >- // Write directly to the FILE's output buffer.
> >> >> >- this->_M_reset(_M_file._M_write_buf());
> >> >> >+ // Allocate FILE's output buffer if needed, and write
> directly to it.
> >> >> >+ this->_M_reset(_M_file._M_init_write_buf());
> >> >> > }
> >> >> >
> >> >> > // This calls I/O functions which are cancellation points, so
> they
> >> >> >diff --git a/libstdc++-v3/testsuite/27_io/print/1.cc
> b/libstdc++-v3/testsuite/27_io/print/1.cc
> >> >> >index 4fd7f5dc925..641a97c1f9c 100644
> >> >> >--- a/libstdc++-v3/testsuite/27_io/print/1.cc
> >> >> >+++ b/libstdc++-v3/testsuite/27_io/print/1.cc
> >> >> >@@ -67,6 +67,22 @@ test_print_raw()
> >> >> > VERIFY( txt == "\xa3" );
> >> >> > }
> >> >> >
> >> >> >+void
> >> >> >+test_print_setvbuf()
> >> >> >+{
> >> >> >+ __gnu_test::scoped_file f;
> >> >> >+ FILE* strm = std::fopen(f.path.string().c_str(), "w");
> >> >> >+ VERIFY( strm );
> >> >> >+ VERIFY( std::setvbuf(strm, nullptr, _IOFBF, 4096) == 0 );
> >> >> >+ std::string str{"Hello, World!"};
> >> >> >+ std::print(strm, "{}", str);
> >> >> >+ std::fclose(strm);
> >> >> >+
> >> >> >+ std::ifstream in(f.path);
> >> >> >+ std::string txt(std::istreambuf_iterator<char>(in), {});
> >> >> >+ VERIFY( txt == "Hello, World!" );
> >> >> >+}
> >> >> >+
> >> >> > void
> >> >> > test_vprint_nonunicode()
> >> >> > {
> >> >> >@@ -142,6 +158,7 @@ int main()
> >> >> > test_print_file();
> >> >> > test_println_file();
> >> >> > test_print_raw();
> >> >> >+ test_print_setvbuf();
> >> >> > test_vprint_nonunicode();
> >> >> > #ifdef __cpp_exceptions
> >> >> > test_errors();
> >> >> >--
> >> >> >2.55.0
> >> >> >
> >> >> >
> >> >>
> >>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://gcc.gnu.org/pipermail/libstdc++/attachments/20260820/874cb163/attachment.htm>
More information about the Libstdc++
mailing list