The initial fix for PR 118099 handled the case where writing the pending
output sequence to the external file failed and _M_convert_to_external
returned false. But _M_convert_to_external can also fail by throwing an
exception in which case the same bug exists: the put area's next pointer
(i.e. pptr) has already been advanced and is not rewound when
basic_filebuf::overflow exits via an exception. As in the PR 118099
testcase, the caller is left with pptr set past epptr, and successive
calls to overflow will write further past the end of the managed buffer
with each failure.
This commit fixes it by not advancing the put area's next pointer at
all, instead we just increment the size passed to _M_convert_to_external
to account for the extra character in the put area. This relies on the
previous commit which changed _M_convert_to_external to no longer use
pptr() internally (when retrying after a partial codecvt conversion).
If _M_convert_to_external was still looking at pptr() directly and
expecting it to have been bumped then we could not change overflow to no
longer bump it.
Because this change depends on that fix in _M_convert_to_external there
is a potential ABI issue that needs care. If the new bump-less version
of basic_filebuf::overflow gets inlined into a translation unit but the
linker resolves its call to _M_convert_to_external to an old definition
that still calls pptr(), we would not transfer the overflow character to
the external file when retrying after a partial conversion. Although
this scenario seems unlikely because std::filebuf and std::wfilebuf are
both explicitly instantiated in libstdc++.so (and the explicit
instantiations use the new definition of _M_convert_to_external) it
would be safer to ensure the problem cannot occur.
This commit adds an _M_convert_to_external(const _CharT*, streamsize)
overload which takes a const pointer, and changes overflow to use that
instead. This ensures that the new bump-less overflow uses a new symbol
with the new behaviour, and will never bind to an old definition that
inspects pptr(). The old overload taking a non-const pointer needs to be
preserved for ABI compatibility, but can be defined to simply forward
its arguments (after a const_cast) to the new overload. With this
change, instantiations of the old version of overflow can safely bind to
either the old definition of _M_convert_to_external (which calls pptr())
or the new definition (which doesn't) because both definitions are
compatible with the old overflow. Instantiations of the new version of
overflow will only ever bind to the new overload because it has a
different mangled name.
The failing_codecvt in the new test was generated by an LLM because I
was too lazy to remember how to write codecvt facets.
Assisted-by: LLM
libstdc++-v3/ChangeLog:
PR libstdc++/118099
* config/abi/pre/gnu.ver: Tighten old patterns for basic_filebuf
and export new symbols.
* include/bits/fstream.tcc (basic_filebuf::overflow): Do not
bump the next pointer for the overflow character. Pass const
pointers to _M_convert_to_external.
(_M_convert_to_external(_CharT*, streamsize)): Forward to new
overload.
(_M_convert_to_external(const _CharT*, streamsize)): Define new
overload.
* include/std/fstream (_M_convert_to_external): Define new
overload and suppress old overload for _GLIBCXX_INLINE_VERSION.
* testsuite/27_io/basic_filebuf/overflow/char/118099-codecvt.cc:
New test.
* testsuite/27_io/basic_filebuf/overflow/wchar_t/118099-codecvt.cc:
New test.
---
Tested x86_64-linux.
libstdc++-v3/config/abi/pre/gnu.ver | 7 +-
libstdc++-v3/include/bits/fstream.tcc | 120 +++++++++---------
libstdc++-v3/include/std/fstream | 7 +-
.../overflow/char/118099-codecvt.cc | 76 +++++++++++
.../overflow/wchar_t/118099-codecvt.cc | 76 +++++++++++
5 files changed, 224 insertions(+), 62 deletions(-)
create mode 100644
libstdc++-v3/testsuite/27_io/basic_filebuf/overflow/char/118099-codecvt.cc
create mode 100644
libstdc++-v3/testsuite/27_io/basic_filebuf/overflow/wchar_t/118099-codecvt.cc
diff --git a/libstdc++-v3/config/abi/pre/gnu.ver
b/libstdc++-v3/config/abi/pre/gnu.ver
index 4fbb87acee3f..d846ce6d3e36 100644
--- a/libstdc++-v3/config/abi/pre/gnu.ver
+++ b/libstdc++-v3/config/abi/pre/gnu.ver
@@ -434,7 +434,9 @@ GLIBCXX_3.4 {
_ZNSt13basic_filebufI[cw]St11char_traitsI[cw]EE15*;
_ZNSt13basic_filebufI[cw]St11char_traitsI[cw]EE16*;
_ZNSt13basic_filebufI[cw]St11char_traitsI[cw]EE19*;
- _ZNSt13basic_filebufI[cw]St11char_traitsI[cw]EE2*;
+
_ZNSt13basic_filebufI[cw]St11char_traitsI[cw]EE22_M_convert_to_externalEP[cw][ilx];
+
_ZNSt13basic_filebufI[cw]St11char_traitsI[cw]EE26_M_destroy_internal_buffer*;
+
_ZNSt13basic_filebufI[cw]St11char_traitsI[cw]EE27_M_allocate_internal_buffer*;
_ZNSt13basic_filebufI[cw]St11char_traitsI[cw]EE3*;
_ZNSt13basic_filebufI[cw]St11char_traitsI[cw]EE4openEPKc*;
_ZNSt13basic_filebufI[cw]St11char_traitsI[cw]EE4sync*;
@@ -2641,6 +2643,9 @@ GLIBCXX_3.4.37 {
_ZNSt7__cxx1112basic_stringI[cw]St11char_traitsI[cw]ESaI[cw]EE*_S_allocate_*;
_ZNSt7__cxx1112basic_stringI[cw]St11char_traitsI[cw]ESaI[cw]EE*_M_create_*;
+ # basic_filebuf::_M_convert_to_external(const C*, streamsize)
+
_ZNSt13basic_filebufI[cw]St11char_traitsI[cw]EE22_M_convert_to_externalEPK[cw][ilx];
+
} GLIBCXX_3.4.36;
# Symbols in the support library (libsupc++) have their own tag.
diff --git a/libstdc++-v3/include/bits/fstream.tcc
b/libstdc++-v3/include/bits/fstream.tcc
index 5d13633b6841..427e7b3a0c43 100644
--- a/libstdc++-v3/include/bits/fstream.tcc
+++ b/libstdc++-v3/include/bits/fstream.tcc
@@ -561,23 +561,25 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
if (this->pbase() < this->pptr())
{
+ streamsize __n = this->pptr() - this->pbase();
+
// If appropriate, append the overflow char.
if (!__testeof)
{
*this->pptr() = traits_type::to_char_type(__c);
- this->pbump(1);
+ ++__n;
}
+ // This must be const _CharT* to call the right overload.
+ const _CharT* __p = this->pbase();
+
// Convert pending sequence to external representation,
- // and output.
- if (_M_convert_to_external(this->pbase(),
- this->pptr() - this->pbase()))
+ // and output. Include the overflow char if appropriate.
+ if (_M_convert_to_external(__p, __n))
{
_M_set_buffer(0);
__ret = traits_type::not_eof(__c);
}
- else if (!__testeof)
- this->pbump(-1);
}
else if (_M_buf_size > 1)
{
@@ -596,7 +598,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
else
{
// Unbuffered.
- char_type __conv = traits_type::to_char_type(__c);
+ const char_type __conv = traits_type::to_char_type(__c);
if (__testeof || _M_convert_to_external(&__conv, 1))
{
_M_writing = true;
@@ -607,65 +609,63 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
return __ret;
}
+ template<typename _CharT, typename _Traits>
+ bool
+ basic_filebuf<_CharT, _Traits>::
+ _M_convert_to_external(const _CharT* __ibuf, streamsize __ilen)
+ {
+ if (__check_facet(_M_codecvt).always_noconv())
+ return _M_file.xsputn(reinterpret_cast<const char*>(__ibuf), __ilen)
+ == __ilen;
+
+ // Worst-case number of external bytes needed.
+ // XXX Not done encoding() == -1.
+ const streamsize __maxlen = __ilen * _M_codecvt->max_length();
+ char* const __buf = static_cast<char*>(__builtin_alloca(__maxlen));
+ streamsize __blen = __maxlen;
+
+ char* __bend;
+ const char_type* __iend;
+ codecvt_base::result __r;
+ __r = _M_codecvt->out(_M_state_cur, __ibuf, __ibuf + __ilen,
+ __iend, __buf, __buf + __blen, __bend);
+ __blen = __bend - __buf;
+
+ if (__r == codecvt_base::ok)
+ return _M_file.xsputn(__buf, __blen) == __blen;
+ else if (__r == codecvt_base::noconv)
+ return _M_file.xsputn(reinterpret_cast<const char*>(__ibuf), __ilen)
+ == __ilen;
+ else if (__r == codecvt_base::error)
+ __throw_ios_failure(__N("basic_filebuf::_M_convert_to_external "
+ "conversion error"));
+
+ // Conversion was partial, some output is in __buf.
+ if (_M_file.xsputn(__buf, __blen) != __blen)
+ return false;
+
+ // Try once more for partial conversions.
+ __r = _M_codecvt->out(_M_state_cur, __iend, __ibuf + __ilen,
+ __iend, __buf, __buf + __maxlen, __bend);
+
+ if (__r == codecvt_base::error)
+ __throw_ios_failure(__N("basic_filebuf::_M_convert_to_external "
+ "conversion error"));
+
+ __blen = __bend - __buf;
+ return _M_file.xsputn(__buf, __blen) == __blen;
+ }
+
+#if ! _GLIBCXX_INLINE_VERSION
+ // Prior to GCC 17 basic_filebuf::overflow called this overload.
template<typename _CharT, typename _Traits>
bool
basic_filebuf<_CharT, _Traits>::
_M_convert_to_external(_CharT* __ibuf, streamsize __ilen)
{
- // Sizes of external and pending output.
- streamsize __elen;
- streamsize __plen;
- if (__check_facet(_M_codecvt).always_noconv())
- {
- __elen = _M_file.xsputn(reinterpret_cast<char*>(__ibuf), __ilen);
- __plen = __ilen;
- }
- else
- {
- // Worst-case number of external bytes needed.
- // XXX Not done encoding() == -1.
- streamsize __blen = __ilen * _M_codecvt->max_length();
- char* __buf = static_cast<char*>(__builtin_alloca(__blen));
-
- char* __bend;
- const char_type* __iend;
- codecvt_base::result __r;
- __r = _M_codecvt->out(_M_state_cur, __ibuf, __ibuf + __ilen,
- __iend, __buf, __buf + __blen, __bend);
-
- if (__r == codecvt_base::ok || __r == codecvt_base::partial)
- __blen = __bend - __buf;
- else if (__r == codecvt_base::noconv)
- {
- // Same as the always_noconv case above.
- __buf = reinterpret_cast<char*>(__ibuf);
- __blen = __ilen;
- }
- else
- __throw_ios_failure(__N("basic_filebuf::_M_convert_to_external "
- "conversion error"));
-
- __elen = _M_file.xsputn(__buf, __blen);
- __plen = __blen;
-
- // Try once more for partial conversions.
- if (__r == codecvt_base::partial && __elen == __plen)
- {
- __r = _M_codecvt->out(_M_state_cur, __iend, __ibuf + __ilen,
- __iend, __buf, __buf + __blen, __bend);
- if (__r != codecvt_base::error)
- {
- const streamsize __rlen = __bend - __buf;
- __elen = _M_file.xsputn(__buf, __rlen);
- __plen = __rlen;
- }
- else
- __throw_ios_failure(__N("basic_filebuf::_M_convert_to_external "
- "conversion error"));
- }
- }
- return __elen == __plen;
+ return _M_convert_to_external(const_cast<const _CharT*>(__ibuf), __ilen);
}
+#endif
template<typename _CharT, typename _Traits>
streamsize
diff --git a/libstdc++-v3/include/std/fstream b/libstdc++-v3/include/std/fstream
index 653d417c6288..9549638205ad 100644
--- a/libstdc++-v3/include/std/fstream
+++ b/libstdc++-v3/include/std/fstream
@@ -436,10 +436,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
virtual int_type
overflow(int_type __c = _Traits::eof());
- // Convert internal byte sequence to external, char-based
+ // Convert internal _CharT sequence to external, char-based
// sequence via codecvt.
+ bool
+ _M_convert_to_external(const char_type*, streamsize);
+
+#if ! _GLIBCXX_INLINE_VERSION // ABI compat
bool
_M_convert_to_external(char_type*, streamsize);
+#endif
/**
* @brief Manipulates the buffer.
diff --git
a/libstdc++-v3/testsuite/27_io/basic_filebuf/overflow/char/118099-codecvt.cc
b/libstdc++-v3/testsuite/27_io/basic_filebuf/overflow/char/118099-codecvt.cc
new file mode 100644
index 000000000000..99645e810975
--- /dev/null
+++ b/libstdc++-v3/testsuite/27_io/basic_filebuf/overflow/char/118099-codecvt.cc
@@ -0,0 +1,76 @@
+// { dg-do run }
+// { dg-require-fileio "" }
+
+// PR libstdc++/118099
+// basic_filebuf::overflow leaves streambuf in inconsistent state on I/O error
+//
+// Companion test of 118099.cc testing the throwing failure path in overflow:
+// a codecvt whose do_out always returns error makes _M_convert_to_external
+// throw after the overflow character has been written to the end of put area.
+// If the put pointer is advanced before the throw (and only rewound on the
+// non-throwing error path), a caller that catches the exception and keeps
+// writing will cause overflow(c) to write past the end of the managed buffer,
+// just like the original PR libstdc++/118099 testcase.
+
+#include <fstream>
+#include <locale>
+#include <testsuite_hooks.h>
+
+typedef char C;
+
+struct failing_codecvt : std::codecvt<C, char, std::mbstate_t>
+{
+protected:
+ result
+ do_out(state_type&, const intern_type* from, const intern_type*,
+ const intern_type*& from_next,
+ extern_type* to, extern_type*, extern_type*& to_next) const
+ {
+ from_next = from;
+ to_next = to;
+ return error;
+ }
+
+ bool do_always_noconv() const throw() { return false; }
+};
+
+static void
+test_pr118099_codecvt()
+{
+ const C val = C('x');
+ const C canary = ~val;
+ const int N = 16;
+ C buf[N];
+ for (int i = 0; i < N; ++i)
+ buf[i] = canary;
+
+ std::basic_filebuf<C> fb;
+ fb.pubimbue(std::locale(std::locale::classic(), new failing_codecvt));
+ fb.pubsetbuf(buf, N/2); // filebuf can use buf[0..7]
+ fb.open("tmp_118099_codecvt", std::ios_base::out | std::ios_base::trunc);
+ VERIFY( fb.is_open() );
+
+ // Libstdc++ filebuf throws ios::failure for the codecvt error,
+ // but the standard allows overflow to return eof instead.
+ // To make this test portable, we check for either kind of error.
+ bool got_error = false;
+ for (int i = 0; i < N; ++i)
+ try {
+ std::char_traits<C>::int_type r = fb.sputc(val);
+ if (std::char_traits<C>::eq_int_type(r, std::char_traits<C>::eof()))
+ got_error = true;
+ } catch (const std::exception&) {
+ got_error = true;
+ }
+ VERIFY( got_error ); // a codecvt error actually happened
+
+ for (int i = 0; i < N/2; ++i)
+ VERIFY( buf[i] == val );
+ for (int i = N/2; i < N; ++i)
+ VERIFY( buf[i] == canary ); // nothing was written past buf[0..7]
+}
+
+int main()
+{
+ test_pr118099_codecvt();
+}
diff --git
a/libstdc++-v3/testsuite/27_io/basic_filebuf/overflow/wchar_t/118099-codecvt.cc
b/libstdc++-v3/testsuite/27_io/basic_filebuf/overflow/wchar_t/118099-codecvt.cc
new file mode 100644
index 000000000000..27420006fada
--- /dev/null
+++
b/libstdc++-v3/testsuite/27_io/basic_filebuf/overflow/wchar_t/118099-codecvt.cc
@@ -0,0 +1,76 @@
+// { dg-do run }
+// { dg-require-fileio "" }
+
+// PR libstdc++/118099
+// basic_filebuf::overflow leaves streambuf in inconsistent state on I/O error
+//
+// Companion test of 118099.cc testing the throwing failure path in overflow:
+// a codecvt whose do_out always returns error makes _M_convert_to_external
+// throw after the overflow character has been written to the end of put area.
+// If the put pointer is advanced before the throw (and only rewound on the
+// non-throwing error path), a caller that catches the exception and keeps
+// writing will cause overflow(c) to write past the end of the managed buffer,
+// just like the original PR libstdc++/118099 testcase.
+
+#include <fstream>
+#include <locale>
+#include <testsuite_hooks.h>
+
+typedef wchar_t C;
+
+struct failing_codecvt : std::codecvt<C, char, std::mbstate_t>
+{
+protected:
+ result
+ do_out(state_type&, const intern_type* from, const intern_type*,
+ const intern_type*& from_next,
+ extern_type* to, extern_type*, extern_type*& to_next) const
+ {
+ from_next = from;
+ to_next = to;
+ return error;
+ }
+
+ bool do_always_noconv() const throw() { return false; }
+};
+
+static void
+test_pr118099_codecvt()
+{
+ const C val = C('x');
+ const C canary = ~val;
+ const int N = 16;
+ C buf[N];
+ for (int i = 0; i < N; ++i)
+ buf[i] = canary;
+
+ std::basic_filebuf<C> fb;
+ fb.pubimbue(std::locale(std::locale::classic(), new failing_codecvt));
+ fb.pubsetbuf(buf, N/2); // filebuf can use buf[0..7]
+ fb.open("tmp_118099_codecvt", std::ios_base::out | std::ios_base::trunc);
+ VERIFY( fb.is_open() );
+
+ // Libstdc++ filebuf throws ios::failure for the codecvt error,
+ // but the standard allows overflow to return eof instead.
+ // To make this test portable, we check for either kind of error.
+ bool got_error = false;
+ for (int i = 0; i < N; ++i)
+ try {
+ std::char_traits<C>::int_type r = fb.sputc(val);
+ if (std::char_traits<C>::eq_int_type(r, std::char_traits<C>::eof()))
+ got_error = true;
+ } catch (const std::exception&) {
+ got_error = true;
+ }
+ VERIFY( got_error ); // a codecvt error actually happened
+
+ for (int i = 0; i < N/2; ++i)
+ VERIFY( buf[i] == val );
+ for (int i = N/2; i < N; ++i)
+ VERIFY( buf[i] == canary ); // nothing was written past buf[0..7]
+}
+
+int main()
+{
+ test_pr118099_codecvt();
+}
--
2.56.0
