[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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.