public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug libstdc++/93672] std::basic_istream::ignore hangs if delim MSB is set
[not found] <bug-93672-4@http.gcc.gnu.org/bugzilla/>
@ 2024-04-03 17:18 ` redi at gcc dot gnu.org
2024-04-03 17:41 ` redi at gcc dot gnu.org
` (6 subsequent siblings)
7 siblings, 0 replies; 8+ messages in thread
From: redi at gcc dot gnu.org @ 2024-04-03 17:18 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93672
Jonathan Wakely <redi at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |ASSIGNED
Assignee|unassigned at gcc dot gnu.org |redi at gcc dot gnu.org
Last reconfirmed| |2024-04-03
Ever confirmed|0 |1
--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Looks like I missed this one when it was filed.
The fix is:
--- a/libstdc++-v3/src/c++98/istream.cc
+++ b/libstdc++-v3/src/c++98/istream.cc
@@ -126,6 +126,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
const int_type __eof = traits_type::eof();
__streambuf_type* __sb = this->rdbuf();
int_type __c = __sb->sgetc();
+ __delim = traits_type::to_int_type(__cdelim);
bool __large_ignore = false;
while (true)
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug libstdc++/93672] std::basic_istream::ignore hangs if delim MSB is set
[not found] <bug-93672-4@http.gcc.gnu.org/bugzilla/>
2024-04-03 17:18 ` [Bug libstdc++/93672] std::basic_istream::ignore hangs if delim MSB is set redi at gcc dot gnu.org
@ 2024-04-03 17:41 ` redi at gcc dot gnu.org
2024-04-03 19:40 ` redi at gcc dot gnu.org
` (5 subsequent siblings)
7 siblings, 0 replies; 8+ messages in thread
From: redi at gcc dot gnu.org @ 2024-04-03 17:41 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93672
--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
On second thoughts, I don't think that fix is right.
istream::ignore takes an int_type for the delimiter, so passing it a char_type
with a negative value will confuse it. For example, str.ignore(n, '\xff`) will
treat the delimiter as EOF and ignore up to n chars, ignoring any '\xff`
characters that might be in the stream buffer's get area. That means it's wrong
to call ignore(n, '\xff') on a platform where char is signed, because it won't
do what you expect (unless you're really intending to treat \xff as EOF).
This case is similar, it ignores up to n characters, or until sgetc() returns a
character equal to '\x80' ... but that can never happen because sgetc() never
returns a negative value unless it reaches EOF.
So there is a gcc bug here, because we should not loop forever. But the problem
is that we use inconsistent conditions for deciding whether we've found the
delimiter.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug libstdc++/93672] std::basic_istream::ignore hangs if delim MSB is set
[not found] <bug-93672-4@http.gcc.gnu.org/bugzilla/>
2024-04-03 17:18 ` [Bug libstdc++/93672] std::basic_istream::ignore hangs if delim MSB is set redi at gcc dot gnu.org
2024-04-03 17:41 ` redi at gcc dot gnu.org
@ 2024-04-03 19:40 ` redi at gcc dot gnu.org
2024-04-04 9:53 ` [Bug libstdc++/93672] [11/12/13/14 Regression] " redi at gcc dot gnu.org
` (4 subsequent siblings)
7 siblings, 0 replies; 8+ messages in thread
From: redi at gcc dot gnu.org @ 2024-04-03 19:40 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93672
--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
So maybe:
--- a/libstdc++-v3/src/c++98/istream.cc
+++ b/libstdc++-v3/src/c++98/istream.cc
@@ -112,7 +112,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
basic_istream<char>::
ignore(streamsize __n, int_type __delim)
{
- if (traits_type::eq_int_type(__delim, traits_type::eof()))
+ // If __delim is eof() we ignore up to __n chars, and for any other
+ // negative value using eq_int_type(sgetc(), __delim) will never be
true,
+ // so just treat all negative __delim values as eof().
+ if (__delim < 0)
return ignore(__n);
_M_gcount = 0;
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug libstdc++/93672] [11/12/13/14 Regression] std::basic_istream::ignore hangs if delim MSB is set
[not found] <bug-93672-4@http.gcc.gnu.org/bugzilla/>
` (2 preceding siblings ...)
2024-04-03 19:40 ` redi at gcc dot gnu.org
@ 2024-04-04 9:53 ` redi at gcc dot gnu.org
2024-04-15 18:29 ` cvs-commit at gcc dot gnu.org
` (3 subsequent siblings)
7 siblings, 0 replies; 8+ messages in thread
From: redi at gcc dot gnu.org @ 2024-04-04 9:53 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93672
Jonathan Wakely <redi at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|std::basic_istream::ignore |[11/12/13/14 Regression]
|hangs if delim MSB is set |std::basic_istream::ignore
| |hangs if delim MSB is set
Known to fail| |11.4.0, 12.3.0, 13.2.0,
| |14.0, 4.0.0, 9.1.0
Known to work| |3.4.6
Target Milestone|--- |11.5
--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
The incorrect behaviour was introduced with r0-59139-g80dddedcaf316e
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug libstdc++/93672] [11/12/13/14 Regression] std::basic_istream::ignore hangs if delim MSB is set
[not found] <bug-93672-4@http.gcc.gnu.org/bugzilla/>
` (3 preceding siblings ...)
2024-04-04 9:53 ` [Bug libstdc++/93672] [11/12/13/14 Regression] " redi at gcc dot gnu.org
@ 2024-04-15 18:29 ` cvs-commit at gcc dot gnu.org
2024-04-15 18:32 ` [Bug libstdc++/93672] [11/12/13 " redi at gcc dot gnu.org
` (2 subsequent siblings)
7 siblings, 0 replies; 8+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2024-04-15 18:29 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93672
--- Comment #5 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:
https://gcc.gnu.org/g:2d694414ada8e3b58f504c1b175d31088529632e
commit r14-9978-g2d694414ada8e3b58f504c1b175d31088529632e
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Thu Apr 4 10:33:33 2024 +0100
libstdc++: Fix infinite loop in std::istream::ignore(n, delim) [PR93672]
A negative delim value passed to std::istream::ignore can never match
any character in the stream, because the comparison is done using
traits_type::eq_int_type(sb->sgetc(), delim) and sgetc() never returns
negative values (except at EOF). The optimized version of ignore for the
std::istream specialization uses traits_type::find to locate the delim
character in the streambuf, which _can_ match a negative delim on
platforms where char is signed, but then we do another comparison using
eq_int_type which fails. The code then keeps looping forever, with
traits_type::find locating the character and traits_type::eq_int_type
saying it's not a match, so traits_type::find is used again and finds
the same character again.
A possible fix would be to check with eq_int_type after a successful
find, to see whether we really have a match. However, that would be
suboptimal since we know that a negative delimiter will never match
using eq_int_type. So a better fix is to adjust the check at the top of
the function that handles delim==eof(), so that we treat all negative
delim values as equivalent to EOF. That way we don't bother using find
to search for something that will never match with eq_int_type.
The version of ignore in the primary template doesn't need a change,
because it doesn't use traits_type::find, instead characters are
extracted one-by-one and always matched using eq_int_type. That avoids
the inconsistency between find and eq_int_type. The specialization for
std::wistream does use traits_type::find, but traits_type::to_int_type
is equivalent to an implicit conversion from wchar_t to wint_t, so
passing a wchar_t directly to ignore without using to_int_type works.
libstdc++-v3/ChangeLog:
PR libstdc++/93672
* src/c++98/istream.cc (istream::ignore(streamsize, int_type)):
Treat all negative delimiter values as eof().
* testsuite/27_io/basic_istream/ignore/char/93672.cc: New test.
* testsuite/27_io/basic_istream/ignore/wchar_t/93672.cc: New
test.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug libstdc++/93672] [11/12/13 Regression] std::basic_istream::ignore hangs if delim MSB is set
[not found] <bug-93672-4@http.gcc.gnu.org/bugzilla/>
` (4 preceding siblings ...)
2024-04-15 18:29 ` cvs-commit at gcc dot gnu.org
@ 2024-04-15 18:32 ` redi at gcc dot gnu.org
2024-05-02 14:16 ` cvs-commit at gcc dot gnu.org
2024-05-02 14:17 ` [Bug libstdc++/93672] [11/12 " redi at gcc dot gnu.org
7 siblings, 0 replies; 8+ messages in thread
From: redi at gcc dot gnu.org @ 2024-04-15 18:32 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93672
Jonathan Wakely <redi at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|[11/12/13/14 Regression] |[11/12/13 Regression]
|std::basic_istream::ignore |std::basic_istream::ignore
|hangs if delim MSB is set |hangs if delim MSB is set
--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Fixed on trunk so far.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug libstdc++/93672] [11/12/13 Regression] std::basic_istream::ignore hangs if delim MSB is set
[not found] <bug-93672-4@http.gcc.gnu.org/bugzilla/>
` (5 preceding siblings ...)
2024-04-15 18:32 ` [Bug libstdc++/93672] [11/12/13 " redi at gcc dot gnu.org
@ 2024-05-02 14:16 ` cvs-commit at gcc dot gnu.org
2024-05-02 14:17 ` [Bug libstdc++/93672] [11/12 " redi at gcc dot gnu.org
7 siblings, 0 replies; 8+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2024-05-02 14:16 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93672
--- Comment #7 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Jonathan Wakely
<redi@gcc.gnu.org>:
https://gcc.gnu.org/g:fcf60d0baafa1245f768ac375dc60a07e92e9673
commit r13-8675-gfcf60d0baafa1245f768ac375dc60a07e92e9673
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Thu Apr 4 10:33:33 2024 +0100
libstdc++: Fix infinite loop in std::istream::ignore(n, delim) [PR93672]
A negative delim value passed to std::istream::ignore can never match
any character in the stream, because the comparison is done using
traits_type::eq_int_type(sb->sgetc(), delim) and sgetc() never returns
negative values (except at EOF). The optimized version of ignore for the
std::istream specialization uses traits_type::find to locate the delim
character in the streambuf, which _can_ match a negative delim on
platforms where char is signed, but then we do another comparison using
eq_int_type which fails. The code then keeps looping forever, with
traits_type::find locating the character and traits_type::eq_int_type
saying it's not a match, so traits_type::find is used again and finds
the same character again.
A possible fix would be to check with eq_int_type after a successful
find, to see whether we really have a match. However, that would be
suboptimal since we know that a negative delimiter will never match
using eq_int_type. So a better fix is to adjust the check at the top of
the function that handles delim==eof(), so that we treat all negative
delim values as equivalent to EOF. That way we don't bother using find
to search for something that will never match with eq_int_type.
The version of ignore in the primary template doesn't need a change,
because it doesn't use traits_type::find, instead characters are
extracted one-by-one and always matched using eq_int_type. That avoids
the inconsistency between find and eq_int_type. The specialization for
std::wistream does use traits_type::find, but traits_type::to_int_type
is equivalent to an implicit conversion from wchar_t to wint_t, so
passing a wchar_t directly to ignore without using to_int_type works.
libstdc++-v3/ChangeLog:
PR libstdc++/93672
* src/c++98/istream.cc (istream::ignore(streamsize, int_type)):
Treat all negative delimiter values as eof().
* testsuite/27_io/basic_istream/ignore/char/93672.cc: New test.
* testsuite/27_io/basic_istream/ignore/wchar_t/93672.cc: New
test.
(cherry picked from commit 2d694414ada8e3b58f504c1b175d31088529632e)
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug libstdc++/93672] [11/12 Regression] std::basic_istream::ignore hangs if delim MSB is set
[not found] <bug-93672-4@http.gcc.gnu.org/bugzilla/>
` (6 preceding siblings ...)
2024-05-02 14:16 ` cvs-commit at gcc dot gnu.org
@ 2024-05-02 14:17 ` redi at gcc dot gnu.org
7 siblings, 0 replies; 8+ messages in thread
From: redi at gcc dot gnu.org @ 2024-05-02 14:17 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93672
Jonathan Wakely <redi at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Known to work| |13.2.1, 14.0
Summary|[11/12/13 Regression] |[11/12 Regression]
|std::basic_istream::ignore |std::basic_istream::ignore
|hangs if delim MSB is set |hangs if delim MSB is set
--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Fixed for 13.3 and 14.1 so far.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-05-02 14:17 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <bug-93672-4@http.gcc.gnu.org/bugzilla/>
2024-04-03 17:18 ` [Bug libstdc++/93672] std::basic_istream::ignore hangs if delim MSB is set redi at gcc dot gnu.org
2024-04-03 17:41 ` redi at gcc dot gnu.org
2024-04-03 19:40 ` redi at gcc dot gnu.org
2024-04-04 9:53 ` [Bug libstdc++/93672] [11/12/13/14 Regression] " redi at gcc dot gnu.org
2024-04-15 18:29 ` cvs-commit at gcc dot gnu.org
2024-04-15 18:32 ` [Bug libstdc++/93672] [11/12/13 " redi at gcc dot gnu.org
2024-05-02 14:16 ` cvs-commit at gcc dot gnu.org
2024-05-02 14:17 ` [Bug libstdc++/93672] [11/12 " redi at gcc dot gnu.org
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).