[PATCH v3] libstdc++: Fix SIGSEGV in std::print with a setvbuf'd FILE
Tomasz Kamiński <[email protected]>
| Newsgroups | gmane.comp.gcc.patches,gmane.comp.gcc.libstdc++.devel |
|---|---|
| Message-ID | <[email protected]> |
From: Anlai Lu <[email protected]> _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. Calls __overflow when _IO_write_ptr is null in _File_sink constructor. 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 <[email protected]> Signed-off-by: Anlai Lu <[email protected]> Signed-off-by: Tomasz Kamiński <[email protected]> --- 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 + // 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. + if (!_M_file->_IO_write_ptr || _M_write_buf().empty()) + if (::__overflow(_M_file, EOF) == EOF) + { + const int __err = errno; + __throw_system_error(__err); + } + 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