diff mbox

PR libstdc++/53984 handle exceptions in basic_istream::sentry

Message ID 20170725204326.GA28500@redhat.com
State New
Headers show

Commit Message

Jonathan Wakely July 25, 2017, 8:43 p.m. UTC 
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.
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.

Comments

Andreas Schwab July 26, 2017, 2:21 p.m. UTC | #1 
On Jul 25 2017, Jonathan Wakely <jwakely@redhat.com> wrote:

> 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 "" }

ERROR: 27_io/basic_fstream/53984.cc: unknown dg option: dg-require-file-io 18 {} for " dg-require-file-io 18 "" "

Andreas.
Paolo Carlini July 26, 2017, 2:27 p.m. UTC | #2 
Hi,

On 26/07/2017 16:21, Andreas Schwab wrote:
> ERROR: 27_io/basic_fstream/53984.cc: unknown dg option: 
> dg-require-file-io 18 {} for " dg-require-file-io 18 "" " 
Should be already fixed, a trivial typo.

Thanks,
Paolo.
Paolo Carlini July 26, 2017, 6:14 p.m. UTC | #3 
Hi again,

On 26/07/2017 16:27, Paolo Carlini wrote:
> Hi,
>
> On 26/07/2017 16:21, Andreas Schwab wrote:
>> ERROR: 27_io/basic_fstream/53984.cc: unknown dg option: 
>> dg-require-file-io 18 {} for " dg-require-file-io 18 "" " 
> Should be already fixed, a trivial typo.
... but now the new test simply fails for me. If I don't spot something 
else trivial over the next few hours I guess better waiting for Jon to 
look into that.

Paolo.
Ville Voutilainen July 26, 2017, 8:43 p.m. UTC | #4 
On 26 July 2017 at 21:14, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Hi again,
>
> On 26/07/2017 16:27, Paolo Carlini wrote:
>>
>> Hi,
>>
>> On 26/07/2017 16:21, Andreas Schwab wrote:
>>>
>>> ERROR: 27_io/basic_fstream/53984.cc: unknown dg option:
>>> dg-require-file-io 18 {} for " dg-require-file-io 18 "" "
>>
>> Should be already fixed, a trivial typo.
>
> ... but now the new test simply fails for me. If I don't spot something else
> trivial over the next few hours I guess better waiting for Jon to look into
> that.


Fails how?
diff mbox

Patch

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();
+}