On 05/08/20 22:25 +0100, Jonathan Wakely wrote:
P0487R1 resolved LWG 2499 for C++20 by removing the operator>> overloads
that have high risk of buffer overflows. They were replaced by
equivalents that only accept a reference to an array, and so can
guarantee not to write past the end of the array.
In order to support both the old and new functionality, this patch
introduces a new overloaded __istream_extract function which takes a
maximum length. The new operator>> overloads use the array size as the
maximum length. The old overloads now use __builtin_object_size to
determine the available buffer size if available (which requires -O2) or
use numeric_limits<streamsize>::max()/sizeof(char_type) otherwise. This
is a change in behaviour, as the old overloads previously always used
numeric_limits<streamsize>::max(), without considering sizeof(char_type)
and without attempting to prevent overflows.
Because they now do little more than call __istream_extract, the old
operator>> overloads are very small inline functions. This means there
is no advantage to explicitly instantiating them in the library (in fact
that would prevent the __builtin_object_size checks from ever working).
As a result, the explicit instantiation declarations can be removed from
the header. The explicit instantiation definitions are still needed, for
backwards compatibility with existing code that expects to link to the
definitions in the library.
While working on this change I noticed that src/c++11/istream-inst.cc
has the following explicit instantiation definition:
template istream& operator>>(istream&, char*);
This had no effect (and so should not have been present in that file),
because there was an explicit specialization declared in <istream> and
defined in src/++98/istream.cc. However, this change removes the
explicit specialization, and now the explicit instantiation definition
is necessary to ensure the symbol gets defined in the library.
Martin, Jakub, could you please double-check the usage of
__builtin_object_size? (line 805 in libstdc++-v3/include/std/istream)
Do you see any problems with using it here? If it can't tell the size
then we just assume it's larger than the string to be extracted, which
is what the old code did anyway. I do have an idea for stronger
checking in debug mode, which I'll post in a minute.
The attached (uncommitted and not fully tested) patch would make the
new __istream_extract functions return a bool indicating whether
extraction stopped because the maximum number of characters was
reached (as opposed to reaching EOF or whitespace).
This would allow us to abort the program with a debug mode assertion
if limiting the size to the __builtin_object_size value prevented a
buffer overflow from occurring.
diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver
index b6ce76c1f20..a7d8ee83ce3 100644
--- a/libstdc++-v3/config/abi/pre/gnu.ver
+++ b/libstdc++-v3/config/abi/pre/gnu.ver
@@ -2307,7 +2307,7 @@ GLIBCXX_3.4.29 {
# std::__istream_extract(istream&, char*, streamsize)
_ZSt17__istream_extractRSiPc[ilx];
# std::__istream_extract(wistream&, wchar_t*, streamsize)
- _ZSt17__istream_extractIwSt11char_traitsIwEEvRSt13basic_istreamIT_T0_EPS3_[ilx];
+ _ZSt17__istream_extractIwSt11char_traitsIwEEbRSt13basic_istreamIT_T0_EPS3_[ilx];
} GLIBCXX_3.4.28;
diff --git a/libstdc++-v3/include/bits/istream.tcc b/libstdc++-v3/include/bits/istream.tcc
index b8f530f6ef5..cb780999ff7 100644
--- a/libstdc++-v3/include/bits/istream.tcc
+++ b/libstdc++-v3/include/bits/istream.tcc
@@ -986,7 +986,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
template<typename _CharT, typename _Traits>
- void
+ bool
__istream_extract(basic_istream<_CharT, _Traits>& __in, _CharT* __s,
streamsize __num)
{
@@ -1043,6 +1043,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__err |= ios_base::failbit;
if (__err)
__in.setstate(__err);
+ return __extracted == __num - 1;
}
// 27.6.1.4 Standard basic_istream manipulators
@@ -1098,7 +1099,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
extern template class basic_istream<wchar_t>;
extern template wistream& ws(wistream&);
extern template wistream& operator>>(wistream&, wchar_t&);
- extern template void __istream_extract(wistream&, wchar_t*, streamsize);
+ extern template bool __istream_extract(wistream&, wchar_t*, streamsize);
extern template wistream& wistream::_M_extract(unsigned short&);
extern template wistream& wistream::_M_extract(unsigned int&);
diff --git a/libstdc++-v3/include/std/istream b/libstdc++-v3/include/std/istream
index cb8e9f87c90..16562e74706 100644
--- a/libstdc++-v3/include/std/istream
+++ b/libstdc++-v3/include/std/istream
@@ -764,10 +764,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template<typename _CharT, typename _Traits>
- void
+ bool
__istream_extract(basic_istream<_CharT, _Traits>&, _CharT*, streamsize);
- void __istream_extract(istream&, char*, streamsize);
+ bool __istream_extract(istream&, char*, streamsize);
//@{
/**
@@ -805,6 +805,21 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
streamsize __n = __builtin_object_size(__s, 2) / sizeof(_CharT);
if (__n == 0)
__n = __gnu_cxx::__numeric_traits<streamsize>::__max / sizeof(_CharT);
+#if _GLIBCXX_DEBUG
+ else
+ {
+ streamsize __w = __in.width();
+ if (0 <= __w || __w > __n)
+ {
+ if (std::__istream_extract(__in, __s, __n) && !__in.eof())
+ {
+ _CharT __c = _Traits::to_char_type(__in.peek());
+ _GLIBCXX_DEBUG_ASSERT(std::isspace(__c, __in.getloc()));
+ }
+ return __in;
+ }
+ }
+#endif
std::__istream_extract(__in, __s, __n);
return __in;
}
diff --git a/libstdc++-v3/include/std/streambuf b/libstdc++-v3/include/std/streambuf
index 3e512364b86..07dff01344e 100644
--- a/libstdc++-v3/include/std/streambuf
+++ b/libstdc++-v3/include/std/streambuf
@@ -166,7 +166,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
void>::__type
advance(istreambuf_iterator<_CharT2>&, _Distance);
- friend void __istream_extract(istream&, char*, streamsize);
+ friend bool __istream_extract(istream&, char*, streamsize);
template<typename _CharT2, typename _Traits2, typename _Alloc>
friend basic_istream<_CharT2, _Traits2>&
diff --git a/libstdc++-v3/src/c++11/istream-inst.cc b/libstdc++-v3/src/c++11/istream-inst.cc
index 20d9158e770..a6bc79dd39c 100644
--- a/libstdc++-v3/src/c++11/istream-inst.cc
+++ b/libstdc++-v3/src/c++11/istream-inst.cc
@@ -71,7 +71,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template class basic_istream<wchar_t>;
template wistream& ws(wistream&);
template wistream& operator>>(wistream&, wchar_t&);
- template void __istream_extract(wistream&, wchar_t*, streamsize);
+ template bool __istream_extract(wistream&, wchar_t*, streamsize);
#ifndef _GLIBCXX_INLINE_VERSION
// XXX GLIBCXX_ABI Deprecated
diff --git a/libstdc++-v3/src/c++98/istream.cc b/libstdc++-v3/src/c++98/istream.cc
index 7a48779d337..5e1119107bc 100644
--- a/libstdc++-v3/src/c++98/istream.cc
+++ b/libstdc++-v3/src/c++98/istream.cc
@@ -204,7 +204,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
return *this;
}
- void
+ bool
__istream_extract(istream& __in, char* __s, streamsize __num)
{
typedef basic_istream<char> __istream_type;
@@ -281,6 +281,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__err |= ios_base::failbit;
if (__err)
__in.setstate(__err);
+ return __extracted == __num - 1;
}
#ifdef _GLIBCXX_USE_WCHAR_T
