The unbuffered path in basic_filebuf::overflow calls
_M_convert_to_external(&__conv, 1) to output a single character. But if
a partial codecvt conversion happens in _M_convert_to_external then it
calculates the number of unconverted characters in the filebuf's put
area by calling pptr(). In the unbuffered case pptr() returns a null
pointer and so the arithmetic on it is undefined.
Although GCC doesn't diagnose this, Clang's UBSan does:
.../include/c++/16/bits/fstream.tcc:655:19: runtime error: applying non-zero
offset to non-null pointer 0x7ffd0d20eca7 produced null pointer
_M_convert_to_external should only read from the buffer passed to it,
not assume that the streambuf's put area exists or is equivalent to the
buffer that was passed.
This probably isn't a problem in real code, because the null pptr() in
_M_convert_to_external was only reachable for a non-conforming codecvt
facet that lies about max_length() or for some reason performs a partial
conversion on a single character even when sufficient output space is
available (as in the new testcases). But it is more robust to not
access pptr() directly when we are called with a pointer to the sequence
that should be converted. This also enables a further correctness fix to
basic_filebuf::overflow in a subsequent commit.
The retry code can be further simplified because not only is using
pptr() unsafe, but we can just use [__iend, __ibuf + __ilen) as the
input range, instead of [__iend, __iend + (pptr() - __iend)). That was
just unnecessarily complicated.
The retry_forcing_codecvt in the new test was generated by an LLM,
although greatly simplified by me so that the bug is detected with a
simple VERIFY rather than examining the characters output by the facet.
Assisted-by: LLM
libstdc++-v3/ChangeLog:
PR libstdc++/118099
* include/bits/fstream.tcc
(basic_filebuf::_M_convert_to_external): Use __ibuf + __ilen
instead of this->pptr().
* testsuite/27_io/basic_filebuf/overflow/char/118099-unbuffered.cc:
New test.
* testsuite/27_io/basic_filebuf/overflow/wchar_t/118099-unbuffered.cc:
New test.
---
Tested x86_64-linux.
libstdc++-v3/include/bits/fstream.tcc | 9 +--
.../overflow/char/118099-unbuffered.cc | 72 +++++++++++++++++++
.../overflow/wchar_t/118099-unbuffered.cc | 72 +++++++++++++++++++
3 files changed, 147 insertions(+), 6 deletions(-)
create mode 100644
libstdc++-v3/testsuite/27_io/basic_filebuf/overflow/char/118099-unbuffered.cc
create mode 100644
libstdc++-v3/testsuite/27_io/basic_filebuf/overflow/wchar_t/118099-unbuffered.cc
diff --git a/libstdc++-v3/include/bits/fstream.tcc
b/libstdc++-v3/include/bits/fstream.tcc
index 2a1257c3e341..5d13633b6841 100644
--- a/libstdc++-v3/include/bits/fstream.tcc
+++ b/libstdc++-v3/include/bits/fstream.tcc
@@ -651,14 +651,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// Try once more for partial conversions.
if (__r == codecvt_base::partial && __elen == __plen)
{
- const char_type* __iresume = __iend;
- streamsize __rlen = this->pptr() - __iend;
- __r = _M_codecvt->out(_M_state_cur, __iresume,
- __iresume + __rlen, __iend, __buf,
- __buf + __blen, __bend);
+ __r = _M_codecvt->out(_M_state_cur, __iend, __ibuf + __ilen,
+ __iend, __buf, __buf + __blen, __bend);
if (__r != codecvt_base::error)
{
- __rlen = __bend - __buf;
+ const streamsize __rlen = __bend - __buf;
__elen = _M_file.xsputn(__buf, __rlen);
__plen = __rlen;
}
diff --git
a/libstdc++-v3/testsuite/27_io/basic_filebuf/overflow/char/118099-unbuffered.cc
b/libstdc++-v3/testsuite/27_io/basic_filebuf/overflow/char/118099-unbuffered.cc
new file mode 100644
index 000000000000..a77a635feb73
--- /dev/null
+++
b/libstdc++-v3/testsuite/27_io/basic_filebuf/overflow/char/118099-unbuffered.cc
@@ -0,0 +1,72 @@
+// { 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 unbuffered put path, which calls
+// _M_convert_to_external(&__conv, 1). When a codecvt forces the
+// "partial conversion" retry, that retry must compute the amount of input
+// left to convert from the buffer it was handed (__ibuf + __ilen), not from
+// pptr(). In unbuffered mode pptr() is null, so the old code called
+// codecvt::out with from_end == __iresume + (pptr() - __iend) == pptr(),
+// i.e. a null `from_end` for a non-null `from`.
+
+#include <fstream>
+#include <locale>
+#include <testsuite_hooks.h>
+
+typedef char C;
+
+// Emits one external char per do_out() call without consuming input, and
+// under-reports max_length(), so that _M_convert_to_external is forced to
+// take its partial-conversion retry branch. The retry is where the bogus
+// (null) from_end is passed.
+// This is probably not a conforming codecvt facet and is intended only to
+// test the libstdc++ basic_filebuf::_M_convert_to_external behaviour.
+struct retry_forcing_codecvt : std::codecvt<C, char, std::mbstate_t>
+{
+protected:
+ result
+ do_out(state_type&, const intern_type* from, const intern_type* from_end,
+ const intern_type*& from_next, extern_type* to, extern_type* to_end,
+ extern_type*& to_next) const
+ {
+ // The _M_convert_to_external bug passed non-null from and null from_end.
+ if (from != 0)
+ VERIFY( from_end != 0 );
+
+ from_next = from;
+ to_next = to;
+ if (from < from_end && to < to_end)
+ {
+ *to_next++ = *from; // emit a character; do not advance from_next
+ return partial;
+ }
+ return from == from_end ? ok : partial;
+ }
+
+ bool do_always_noconv() const throw() { return false; }
+ int do_max_length() const throw() { return 1; }
+};
+
+static void
+test_pr118099_unbuffered()
+{
+ std::basic_filebuf<C> fb;
+ fb.pubimbue(std::locale(std::locale::classic(), new retry_forcing_codecvt));
+ fb.pubsetbuf(0, 0); // unbuffered: pptr() == epptr() == null
+ fb.open("tmp_118099_unbuffered", std::ios_base::out | std::ios_base::trunc);
+ VERIFY( fb.is_open() );
+
+ // Each put goes straight to _M_convert_to_external(&__conv, 1); the codecvt
+ // above asserts it is never called with the bogus null from_end.
+ fb.sputc('a');
+ fb.sputc('b');
+ fb.sputc('c');
+}
+
+int main()
+{
+ test_pr118099_unbuffered();
+}
diff --git
a/libstdc++-v3/testsuite/27_io/basic_filebuf/overflow/wchar_t/118099-unbuffered.cc
b/libstdc++-v3/testsuite/27_io/basic_filebuf/overflow/wchar_t/118099-unbuffered.cc
new file mode 100644
index 000000000000..b2c8ae3ec8c0
--- /dev/null
+++
b/libstdc++-v3/testsuite/27_io/basic_filebuf/overflow/wchar_t/118099-unbuffered.cc
@@ -0,0 +1,72 @@
+// { 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 unbuffered put path, which calls
+// _M_convert_to_external(&__conv, 1). When a codecvt forces the
+// "partial conversion" retry, that retry must compute the amount of input
+// left to convert from the buffer it was handed (__ibuf + __ilen), not from
+// pptr(). In unbuffered mode pptr() is null, so the old code called
+// codecvt::out with from_end == __iresume + (pptr() - __iend) == pptr(),
+// i.e. a null `from_end` for a non-null `from`.
+
+#include <fstream>
+#include <locale>
+#include <testsuite_hooks.h>
+
+typedef wchar_t C;
+
+// Emits one external char per do_out() call without consuming input, and
+// under-reports max_length(), so that _M_convert_to_external is forced to
+// take its partial-conversion retry branch. The retry is where the bogus
+// (null) from_end is passed.
+// This is probably not a conforming codecvt facet and is intended only to
+// test the libstdc++ basic_filebuf::_M_convert_to_external behaviour.
+struct retry_forcing_codecvt : std::codecvt<C, char, std::mbstate_t>
+{
+protected:
+ result
+ do_out(state_type&, const intern_type* from, const intern_type* from_end,
+ const intern_type*& from_next, extern_type* to, extern_type* to_end,
+ extern_type*& to_next) const
+ {
+ // The _M_convert_to_external bug passed non-null from and null from_end.
+ if (from != 0)
+ VERIFY( from_end != 0 );
+
+ from_next = from;
+ to_next = to;
+ if (from < from_end && to < to_end)
+ {
+ *to_next++ = *from; // emit a character; do not advance from_next
+ return partial;
+ }
+ return from == from_end ? ok : partial;
+ }
+
+ bool do_always_noconv() const throw() { return false; }
+ int do_max_length() const throw() { return 1; }
+};
+
+static void
+test_pr118099_unbuffered()
+{
+ std::basic_filebuf<C> fb;
+ fb.pubimbue(std::locale(std::locale::classic(), new retry_forcing_codecvt));
+ fb.pubsetbuf(0, 0); // unbuffered: pptr() == epptr() == null
+ fb.open("tmp_118099_unbuffered", std::ios_base::out | std::ios_base::trunc);
+ VERIFY( fb.is_open() );
+
+ // Each put goes straight to _M_convert_to_external(&__conv, 1); the codecvt
+ // above asserts it is never called with the bogus null from_end.
+ fb.sputc(L'a');
+ fb.sputc(L'b');
+ fb.sputc(L'c');
+}
+
+int main()
+{
+ test_pr118099_unbuffered();
+}
--
2.56.0
