public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jonathan Wakely <jwakely@redhat.com>
To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org
Subject: [PATCH] PR libstdc++/53984 handle exceptions in basic_istream::sentry
Date: Tue, 25 Jul 2017 20:43:00 -0000	[thread overview]
Message-ID: <20170725204326.GA28500@redhat.com> (raw)

[-- Attachment #1: Type: text/plain, Size: 970 bytes --]

The standard says that formatted/unformatted input operations should
catch any exceptions, set iostate bits in the istream, and rethrow if
the exception mask says to do so.

We're failing to do that when the exception happens in the
istream::sentry constructor, while it extracts characters to skip
whitespace. This means that input operations exit with an exception
even when the mask says not to. The fix is simply to add a try/catch
to the sentry constructor.

I've also clarified some comments related to these semantics.

	PR libstdc++/53984
	* include/bits/basic_ios.h (basic_ios::_M_setstate): Adjust comment.
	* include/bits/istream.tcc (basic_istream::sentry): Handle exceptions
	during construction.
	* include/std/istream: Adjust comments for formatted input functions
	and unformatted input functions.
	* testsuite/27_io/basic_fstream/53984.cc: New.
	* testsuite/27_io/basic_istream/sentry/char/53984.cc: New.

Tested powerpc64le-linux, committed to trunk.


[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 7284 bytes --]

commit a9a626df1531d0f06b147140b0f405f0847a0a42
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Jul 25 19:12:22 2017 +0100

    PR libstdc++/53984 handle exceptions in basic_istream::sentry
    
    	PR libstdc++/53984
    	* include/bits/basic_ios.h (basic_ios::_M_setstate): Adjust comment.
    	* include/bits/istream.tcc (basic_istream::sentry): Handle exceptions
    	during construction.
    	* include/std/istream: Adjust comments for formatted input functions
    	and unformatted input functions.
    	* testsuite/27_io/basic_fstream/53984.cc: New.
    	* testsuite/27_io/basic_istream/sentry/char/53984.cc: New.

diff --git a/libstdc++-v3/include/bits/basic_ios.h b/libstdc++-v3/include/bits/basic_ios.h
index 318e41b..f0b8682 100644
--- a/libstdc++-v3/include/bits/basic_ios.h
+++ b/libstdc++-v3/include/bits/basic_ios.h
@@ -157,8 +157,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       setstate(iostate __state)
       { this->clear(this->rdstate() | __state); }
 
-      // Flip the internal state on for the proper state bits, then re
-      // throws the propagated exception if bit also set in
+      // Flip the internal state on for the proper state bits, then
+      // rethrows the propagated exception if bit also set in
       // exceptions().
       void
       _M_setstate(iostate __state)
diff --git a/libstdc++-v3/include/bits/istream.tcc b/libstdc++-v3/include/bits/istream.tcc
index b390f74..92df9d1 100644
--- a/libstdc++-v3/include/bits/istream.tcc
+++ b/libstdc++-v3/include/bits/istream.tcc
@@ -48,28 +48,36 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     {
       ios_base::iostate __err = ios_base::goodbit;
       if (__in.good())
-	{
-	  if (__in.tie())
-	    __in.tie()->flush();
-	  if (!__noskip && bool(__in.flags() & ios_base::skipws))
-	    {
-	      const __int_type __eof = traits_type::eof();
-	      __streambuf_type* __sb = __in.rdbuf();
-	      __int_type __c = __sb->sgetc();
+	__try
+	  {
+	    if (__in.tie())
+	      __in.tie()->flush();
+	    if (!__noskip && bool(__in.flags() & ios_base::skipws))
+	      {
+		const __int_type __eof = traits_type::eof();
+		__streambuf_type* __sb = __in.rdbuf();
+		__int_type __c = __sb->sgetc();
 
-	      const __ctype_type& __ct = __check_facet(__in._M_ctype);
-	      while (!traits_type::eq_int_type(__c, __eof)
-		     && __ct.is(ctype_base::space, 
-				traits_type::to_char_type(__c)))
-		__c = __sb->snextc();
+		const __ctype_type& __ct = __check_facet(__in._M_ctype);
+		while (!traits_type::eq_int_type(__c, __eof)
+		       && __ct.is(ctype_base::space,
+				  traits_type::to_char_type(__c)))
+		  __c = __sb->snextc();
 
-	      // _GLIBCXX_RESOLVE_LIB_DEFECTS
-	      // 195. Should basic_istream::sentry's constructor ever
-	      // set eofbit?
-	      if (traits_type::eq_int_type(__c, __eof))
-		__err |= ios_base::eofbit;
-	    }
-	}
+		// _GLIBCXX_RESOLVE_LIB_DEFECTS
+		// 195. Should basic_istream::sentry's constructor ever
+		// set eofbit?
+		if (traits_type::eq_int_type(__c, __eof))
+		  __err |= ios_base::eofbit;
+	      }
+	  }
+	__catch(__cxxabiv1::__forced_unwind&)
+	  {
+	    __in._M_setstate(ios_base::badbit);
+	    __throw_exception_again;
+	  }
+	__catch(...)
+	  { __in._M_setstate(ios_base::badbit); }
 
       if (__in.good() && __err == ios_base::goodbit)
 	_M_ok = true;
diff --git a/libstdc++-v3/include/std/istream b/libstdc++-v3/include/std/istream
index 1fa2555..233cc6b 100644
--- a/libstdc++-v3/include/std/istream
+++ b/libstdc++-v3/include/std/istream
@@ -150,9 +150,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        *  whatever data is appropriate for the type of the argument.
        *
        *  If an exception is thrown during extraction, ios_base::badbit
-       *  will be turned on in the stream's error state without causing an
-       *  ios_base::failure to be thrown.  The original exception will then
-       *  be rethrown.
+       *  will be turned on in the stream's error state (without causing an
+       *  ios_base::failure to be thrown) and the original exception will
+       *  be rethrown if badbit is set in the exceptions mask.
       */
 
       //@{
@@ -286,9 +286,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        *  by gcount().
        *
        *  If an exception is thrown during extraction, ios_base::badbit
-       *  will be turned on in the stream's error state without causing an
-       *  ios_base::failure to be thrown.  The original exception will then
-       *  be rethrown.
+       *  will be turned on in the stream's error state (without causing an
+       *  ios_base::failure to be thrown) and the original exception will
+       *  be rethrown if badbit is set in the exceptions mask.
       */
 
       /**
diff --git a/libstdc++-v3/testsuite/27_io/basic_fstream/53984.cc b/libstdc++-v3/testsuite/27_io/basic_fstream/53984.cc
new file mode 100644
index 0000000..e84072e
--- /dev/null
+++ b/libstdc++-v3/testsuite/27_io/basic_fstream/53984.cc
@@ -0,0 +1,36 @@
+// Copyright (C) 2017 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-require-file-io "" }
+
+#include <fstream>
+#include <testsuite_hooks.h>
+
+void
+test01()
+{
+  std::fstream in(".");
+  int x;
+  in >> x;
+  VERIFY( in.bad() );
+}
+
+int
+main()
+{
+  test01();
+}
diff --git a/libstdc++-v3/testsuite/27_io/basic_istream/sentry/char/53984.cc b/libstdc++-v3/testsuite/27_io/basic_istream/sentry/char/53984.cc
new file mode 100644
index 0000000..9c08c72
--- /dev/null
+++ b/libstdc++-v3/testsuite/27_io/basic_istream/sentry/char/53984.cc
@@ -0,0 +1,41 @@
+// Copyright (C) 2017 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+#include <streambuf>
+#include <istream>
+#include <testsuite_hooks.h>
+
+struct SB : std::streambuf
+{
+  virtual int_type underflow() { throw 1; }
+};
+
+void
+test01()
+{
+  SB sb;
+  std::istream is(&sb);
+  int i;
+  is >> i;
+  VERIFY( is.bad() );
+}
+
+int
+main()
+{
+  test01();
+}

             reply	other threads:[~2017-07-25 20:43 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-25 20:43 Jonathan Wakely [this message]
2017-07-26 14:21 ` Andreas Schwab
2017-07-26 14:27   ` Paolo Carlini
2017-07-26 18:14     ` Paolo Carlini
2017-07-26 20:43       ` Ville Voutilainen
2017-07-26 22:06       ` Jonathan Wakely
2017-07-27  8:50         ` Bin.Cheng
2017-08-11  3:04         ` Jonathan Wakely

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170725204326.GA28500@redhat.com \
    --to=jwakely@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=libstdc++@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).