]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
libstdc++: Handle exceptions in std::ostream::sentry destructor
authorJonathan Wakely <jwakely@redhat.com>
Wed, 15 Jan 2025 13:52:01 +0000 (13:52 +0000)
committerJonathan Wakely <redi@gcc.gnu.org>
Fri, 7 Feb 2025 14:53:30 +0000 (14:53 +0000)
Because basic_ostream::sentry::~sentry is implicitly noexcept, we can't
let any exceptions escape from it, or the program would terminate. If
the streambuf's sync() function throws, or if it returns an error and
setting badbit in the stream state throws, then the program would
terminate.

LWG 835 intended to prevent exceptions from being thrown by the
std::basic_ostream::sentry destructor, but failed to cover the case
where the streambuf's sync() member throws an exception. LWG 4188 is
needed to fix that part. In any case, LWG 835 was never implemented for
libstdc++ so this does that, as well as my proposed fix for 4188 (that
badbit should be set if pubsync() exits via an exception).

In order to avoid a second try-catch block to handle an exception that
might be thrown by setting badbit, this introduces an RAII helper class
that temporarily clears the stream's exceptions mask, then restores it
afterwards.

The try-catch block doesn't handle the forced_unwind exception
explicitly, because catching and rethrowing that would just terminate
when it reached the sentry's implicit noexcept(true) anyway.

libstdc++-v3/ChangeLog:

* include/bits/ostream.h (basic_ostream::_Disable_exceptions):
RAII helper type.
(basic_ostream::sentry::~sentry): Use _Disable_exceptions. Add
try-catch block around call to pubsync.
* testsuite/27_io/basic_ostream/exceptions/char/lwg4188.cc: New
test.
* testsuite/27_io/basic_ostream/exceptions/wchar_t/lwg4188.cc:
New test.

libstdc++-v3/include/bits/ostream.h
libstdc++-v3/testsuite/27_io/basic_ostream/exceptions/char/lwg4188.cc [new file with mode: 0644]
libstdc++-v3/testsuite/27_io/basic_ostream/exceptions/wchar_t/lwg4188.cc [new file with mode: 0644]

index 8ee63d2d66e54657c7c440bc3afb8147226761e1..d19a76ab2474c774fb0dc2a1ff85f5cb0b68c3d7 100644 (file)
@@ -507,6 +507,27 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
          return __d;
        }
 #pragma GCC diagnostic pop
+
+      // RAII type to clear and restore an ostream's exceptions mask.
+      struct _Disable_exceptions
+      {
+       _Disable_exceptions(basic_ostream& __os)
+       : _M_os(__os), _M_exception(_M_os._M_exception)
+       { _M_os._M_exception = ios_base::goodbit; }
+
+       ~_Disable_exceptions()
+       { _M_os._M_exception = _M_exception; }
+
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wc++11-extensions" // deleted functions
+       _Disable_exceptions(const _Disable_exceptions&) = delete;
+       _Disable_exceptions& operator=(const _Disable_exceptions&) = delete;
+#pragma GCC diagnostic pop
+
+      private:
+       basic_ostream& _M_os;
+       const ios_base::iostate _M_exception;
+      };
     };
 
   /**
@@ -543,18 +564,29 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       /**
        *  @brief  Possibly flushes the stream.
        *
-       *  If @c ios_base::unitbuf is set in @c os.flags(), and
-       *  @c std::uncaught_exception() is true, the sentry destructor calls
-       *  @c flush() on the output stream.
+       *  If `ios_base::unitbuf` is set in `os.flags()`, and
+       *  `std::uncaught_exception()` is true, the sentry destructor flushes
+       *  the output stream.
       */
       ~sentry()
       {
-       // XXX MT
-       if (bool(_M_os.flags() & ios_base::unitbuf) && !uncaught_exception())
+       // _GLIBCXX_RESOLVE_LIB_DEFECTS
+       // 397. ostream::sentry dtor throws exceptions
+       // 835. Tying two streams together (correction to DR 581)
+       // 4188. ostream::sentry destructor should handle exceptions
+       if (bool(_M_os.flags() & ios_base::unitbuf) && _M_os.good()
+             && !uncaught_exception()) // XXX MT
          {
-           // Can't call flush directly or else will get into recursive lock.
-           if (_M_os.rdbuf() && _M_os.rdbuf()->pubsync() == -1)
-             _M_os.setstate(ios_base::badbit);
+           _Disable_exceptions __noex(_M_os);
+           __try
+             {
+               // Can't call _M_os.flush() directly because that constructs
+               // another sentry.
+               if (_M_os.rdbuf() && _M_os.rdbuf()->pubsync() == -1)
+                 _M_os.setstate(ios_base::badbit);
+             }
+           __catch(...)
+             { _M_os.setstate(ios_base::badbit); }
          }
       }
 #pragma GCC diagnostic pop
diff --git a/libstdc++-v3/testsuite/27_io/basic_ostream/exceptions/char/lwg4188.cc b/libstdc++-v3/testsuite/27_io/basic_ostream/exceptions/char/lwg4188.cc
new file mode 100644 (file)
index 0000000..4a73266
--- /dev/null
@@ -0,0 +1,50 @@
+// { dg-do run }
+
+// 4188. ostream::sentry destructor should handle exceptions
+
+#include <ostream>
+#include <streambuf>
+#include <testsuite_hooks.h>
+
+struct bad_streambuf : std::streambuf
+{
+  int sync() { return -1; }
+};
+
+void
+test_returns_error()
+{
+  bad_streambuf buf;
+  std::ostream out(&buf);
+  out.setf(std::ios::unitbuf);
+  out.exceptions(std::ios::badbit);
+  out.write("", 0); // constructs sentry
+  VERIFY( out.bad() );
+}
+
+struct exceptionally_bad_streambuf : std::streambuf
+{
+  int sync() { throw std::ios::failure("unsyncable"); }
+};
+
+void
+test_throws()
+{
+  exceptionally_bad_streambuf buf;
+  std::ostream out(&buf);
+  out.setf(std::ios::unitbuf);
+  out.write("", 0); // constructs sentry
+  VERIFY( out.bad() );
+
+  // Repeat with badbit in exceptions mask
+  out.clear();
+  out.exceptions(std::ios::badbit);
+  out.write("", 0); // constructs sentry
+  VERIFY( out.bad() );
+}
+
+int main()
+{
+  test_returns_error();
+  test_throws();
+}
diff --git a/libstdc++-v3/testsuite/27_io/basic_ostream/exceptions/wchar_t/lwg4188.cc b/libstdc++-v3/testsuite/27_io/basic_ostream/exceptions/wchar_t/lwg4188.cc
new file mode 100644 (file)
index 0000000..a3354e1
--- /dev/null
@@ -0,0 +1,50 @@
+// { dg-do run }
+
+// 4188. ostream::sentry destructor should handle exceptions
+
+#include <ostream>
+#include <streambuf>
+#include <testsuite_hooks.h>
+
+struct bad_streambuf : std::wstreambuf
+{
+  int sync() { return -1; }
+};
+
+void
+test_returns_error()
+{
+  bad_streambuf buf;
+  std::wostream out(&buf);
+  out.setf(std::wios::unitbuf);
+  out.exceptions(std::wios::badbit);
+  out.write(L"", 0); // constructs sentry
+  VERIFY( out.bad() );
+}
+
+struct exceptionally_bad_streambuf : std::wstreambuf
+{
+  int sync() { throw std::wios::failure("unsyncable"); }
+};
+
+void
+test_throws()
+{
+  exceptionally_bad_streambuf buf;
+  std::wostream out(&buf);
+  out.setf(std::wios::unitbuf);
+  out.write(L"", 0); // constructs sentry
+  VERIFY( out.bad() );
+
+  // Repeat with badbit in exceptions mask
+  out.clear();
+  out.exceptions(std::wios::badbit);
+  out.write(L"", 0); // constructs sentry
+  VERIFY( out.bad() );
+}
+
+int main()
+{
+  test_returns_error();
+  test_throws();
+}