https://github.com/philnik777 created https://github.com/llvm/llvm-project/pull/213627
Backport 455ef3684fae3273b3423bb0229e33584745c6cc and 76140a666d18b940597bf73362d4101af0247ea6 >From 5e1e6776b62760ede98be29d5c6fa4de126b6faf Mon Sep 17 00:00:00 2001 From: Nikolas Klauser <[email protected]> Date: Tue, 28 Jul 2026 16:27:10 +0200 Subject: [PATCH] release/23.x: [libc++] Fix ungetc failing after xsgetn (#210951) Backport 455ef3684fae3273b3423bb0229e33584745c6cc and 76140a666d18b940597bf73362d4101af0247ea6 --- libcxx/include/fstream | 73 +++++--- libcxx/src/ios.instantiations.cpp | 21 +++ .../fstreams/filebuf.members/test.dat | 1 + .../filebuf.members/xsgetn.buffer.pass.cpp | 159 ++++++++++++++++++ .../fstreams/ifstream.members/xsgetn.pass.cpp | 14 ++ 5 files changed, 245 insertions(+), 23 deletions(-) create mode 100644 libcxx/test/extensions/libcxx/input.output/file.streams/fstreams/filebuf.members/test.dat create mode 100644 libcxx/test/extensions/libcxx/input.output/file.streams/fstreams/filebuf.members/xsgetn.buffer.pass.cpp diff --git a/libcxx/include/fstream b/libcxx/include/fstream index 7b84bf6609086..92c79e81d4906 100644 --- a/libcxx/include/fstream +++ b/libcxx/include/fstream @@ -311,6 +311,8 @@ protected: _LIBCPP_HIDE_FROM_ABI_VIRTUAL streamsize xsgetn(char_type* __str, streamsize __len) override { if (__file_ && __always_noconv_) { + char_type __1buf; + __read_guard __guard(__1buf, *this); const streamsize __n = std::min(this->egptr() - this->gptr(), __len); if (__n != 0) { traits_type::copy(__str, this->gptr(), __n); @@ -319,9 +321,16 @@ protected: const streamsize __remainder = __len - __n; const streamsize __buffer_space = this->egptr() - this->eback(); - if (__remainder >= __buffer_space) - return std::fread(__str + __n, sizeof(char_type), __remainder, __file_) + __n; - else if (__remainder > 0) + if (__remainder >= __buffer_space) { + auto __res = std::fread(__str + __n, sizeof(char_type), __remainder, __file_) + __n; + + // Copy the buffer-sized tail into the buffer so that `unget`ting works correctly + auto __buf_size = std::min<size_t>(__res, this->egptr() - this->eback()); + traits_type::copy(this->eback(), __str + __res - __buf_size, __buf_size); + this->setg(this->eback(), this->eback() + __buf_size, this->eback() + __buf_size); + + return __res; + } else if (__remainder > 0) return basic_streambuf<_CharT, _Traits>::xsgetn(__str + __n, __remainder) + __n; return __n; } @@ -384,6 +393,40 @@ private: bool __always_noconv_; bool __read_mode(); + + struct [[__nodiscard__]] __read_guard { + basic_filebuf& __self_; + size_t __unget_size_; + char_type& __1buf_; + + _LIBCPP_HIDE_FROM_ABI __read_guard(char_type& __1buf, basic_filebuf& __self) : __self_(__self), __1buf_(__1buf) { + if (__self.__cm_ & ios_base::in) { + __unget_size_ = std::min<size_t>((__self.egptr() - __self.eback()) / 2, 4); + } else { + __self.setp(nullptr, nullptr); + if (__self.__always_noconv_) { + __self.setg(reinterpret_cast<char_type*>(__self.__extbuf_), + reinterpret_cast<char_type*>(__self.__extbuf_) + __self.__ebs_, + reinterpret_cast<char_type*>(__self.__extbuf_) + __self.__ebs_); + } else { + __self.setg(__self.__intbuf_, __self.__intbuf_ + __self.__ibs_, __self.__intbuf_ + __self.__ibs_); + } + __self.__cm_ = ios_base::in; + __unget_size_ = 0; + } + + if (__self.gptr() == nullptr) + __self.setg(std::addressof(__1buf_), std::addressof(__1buf_) + 1, std::addressof(__1buf_) + 1); + } + + _LIBCPP_HIDE_FROM_ABI size_t __unget_size() { return __unget_size_; } + + _LIBCPP_HIDE_FROM_ABI ~__read_guard() { + if (__self_.eback() == std::addressof(__1buf_)) + __self_.setg(nullptr, nullptr, nullptr); + } + }; + void __write_mode(); _LIBCPP_HIDE_FROM_ABI static int __fseek(FILE* __file, pos_type __offset, int __whence); @@ -802,11 +845,11 @@ template <class _CharT, class _Traits> typename basic_filebuf<_CharT, _Traits>::int_type basic_filebuf<_CharT, _Traits>::underflow() { if (__file_ == nullptr) return traits_type::eof(); - bool __initial = __read_mode(); + char_type __1buf; - if (this->gptr() == nullptr) - this->setg(std::addressof(__1buf), std::addressof(__1buf) + 1, std::addressof(__1buf) + 1); - const size_t __unget_sz = __initial ? 0 : std::min<size_t>((this->egptr() - this->eback()) / 2, 4); + __read_guard __guard(__1buf, *this); + + const size_t __unget_sz = __guard.__unget_size(); int_type __c = traits_type::eof(); if (this->gptr() == this->egptr()) { std::memmove(this->eback(), this->egptr() - __unget_sz, __unget_sz * sizeof(char_type)); @@ -849,8 +892,6 @@ typename basic_filebuf<_CharT, _Traits>::int_type basic_filebuf<_CharT, _Traits> } } else __c = traits_type::to_int_type(*this->gptr()); - if (this->eback() == std::addressof(__1buf)) - this->setg(nullptr, nullptr, nullptr); return __c; } @@ -1141,20 +1182,6 @@ void basic_filebuf<_CharT, _Traits>::imbue(const locale& __loc) { } } -template <class _CharT, class _Traits> -bool basic_filebuf<_CharT, _Traits>::__read_mode() { - if (!(__cm_ & ios_base::in)) { - this->setp(nullptr, nullptr); - if (__always_noconv_) - this->setg((char_type*)__extbuf_, (char_type*)__extbuf_ + __ebs_, (char_type*)__extbuf_ + __ebs_); - else - this->setg(__intbuf_, __intbuf_ + __ibs_, __intbuf_ + __ibs_); - __cm_ = ios_base::in; - return true; - } - return false; -} - template <class _CharT, class _Traits> void basic_filebuf<_CharT, _Traits>::__write_mode() { if (!(__cm_ & ios_base::out)) { diff --git a/libcxx/src/ios.instantiations.cpp b/libcxx/src/ios.instantiations.cpp index a8d267f7cfd42..a50d3b745944a 100644 --- a/libcxx/src/ios.instantiations.cpp +++ b/libcxx/src/ios.instantiations.cpp @@ -15,6 +15,26 @@ #include <streambuf> _LIBCPP_BEGIN_NAMESPACE_STD +_LIBCPP_BEGIN_EXPLICIT_ABI_ANNOTATIONS + +#if _LIBCPP_HAS_FILESYSTEM +// This is provided for ABI compatiblity with programs compiled against old headers. +// TODO: Is this actually required? If so, this should be guarded with +// `_LIBCPP_AVAILABILITY_MINIMUM_HEADER_VERSION < 24`. Otherwise this should be removed. +template <class _CharT, class _Traits> +bool basic_filebuf<_CharT, _Traits>::__read_mode() { + if (!(__cm_ & ios_base::in)) { + this->setp(nullptr, nullptr); + if (__always_noconv_) + this->setg((char_type*)__extbuf_, (char_type*)__extbuf_ + __ebs_, (char_type*)__extbuf_ + __ebs_); + else + this->setg(__intbuf_, __intbuf_ + __ibs_, __intbuf_ + __ibs_); + __cm_ = ios_base::in; + return true; + } + return false; +} +#endif // Original explicit instantiations provided in the library template class _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS basic_ios<char>; @@ -45,4 +65,5 @@ template class _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS basic_filebuf<char>; // Add more here if needed... +_LIBCPP_END_EXPLICIT_ABI_ANNOTATIONS _LIBCPP_END_NAMESPACE_STD diff --git a/libcxx/test/extensions/libcxx/input.output/file.streams/fstreams/filebuf.members/test.dat b/libcxx/test/extensions/libcxx/input.output/file.streams/fstreams/filebuf.members/test.dat new file mode 100644 index 0000000000000..6fe86c31590f8 --- /dev/null +++ b/libcxx/test/extensions/libcxx/input.output/file.streams/fstreams/filebuf.members/test.dat @@ -0,0 +1 @@ +This is a bunch of data so the test can read some stuff and not instantly run out of data to read. diff --git a/libcxx/test/extensions/libcxx/input.output/file.streams/fstreams/filebuf.members/xsgetn.buffer.pass.cpp b/libcxx/test/extensions/libcxx/input.output/file.streams/fstreams/filebuf.members/xsgetn.buffer.pass.cpp new file mode 100644 index 0000000000000..baa6de2b04c34 --- /dev/null +++ b/libcxx/test/extensions/libcxx/input.output/file.streams/fstreams/filebuf.members/xsgetn.buffer.pass.cpp @@ -0,0 +1,159 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +// FILE_DEPENDENCIES: test.dat + +// UNSUPPORTED: no-localization, no-filesystem + +// XFAIL: using-built-library-before-llvm-23 + +// <fstream> + +// streamsize basic_filebuf::xsgetn(char_type*, streamsize); + +// Test that xsgetn buffers properly. Specifically, we guarantee that `unget()` can be called at least four times. + +#include <cassert> +#include <fstream> +#include <string> + +#include "platform_support.h" + +void small_file_tests() { + char buffer[12]; + + { // Check that we can unget() when reading a single character + std::ifstream is("test.dat"); + assert(is.is_open()); + char buf[1]; + is.read(buf, 1); + assert(buf[0] == 'T'); + is.unget(); + assert(is.good()); + } + + { // Check that we can unget() when reading a single character with a user-provided buffer + std::ifstream is("test.dat"); + assert(is.is_open()); + is.rdbuf()->pubsetbuf(buffer, 12); + char buf[1]; + is.read(buf, 1); + assert(buf[0] == 'T'); + is.unget(); + assert(is.good()); + } + + { // Check that unget() works as expected when the remainder is smaller than the buffer + std::ifstream is("test.dat"); + assert(is.is_open()); + is.rdbuf()->pubsetbuf(buffer, 12); + (void)is.rdbuf()->sgetc(); // Make sure there is data in the buffer + char buf[17]; + buf[16] = '\0'; + is.read(buf, 16); + assert(buf == std::string("This is a bunch ")); + for (size_t i = 0; i != 4; ++i) + is.unget(); + assert(is.good()); + is.read(buf, 4); + buf[4] = '\0'; + assert(buf == std::string("nch ")); + } + + { // Check that unget() works as expected when the remainder is larger than the buffer + std::ifstream is("test.dat"); + assert(is.is_open()); + is.rdbuf()->pubsetbuf(buffer, 12); + char buf[33]; + buf[32] = '\0'; + is.read(buf, 32); + assert(buf == std::string("This is a bunch of data so the t")); + for (size_t i = 0; i != 4; ++i) + is.unget(); + assert(is.good()); + is.read(buf, 4); + buf[4] = '\0'; + assert(is.good()); + assert(buf == std::string("he t")); + } + + { // read an empty file + std::string empty_file = get_temp_file_name(); + { + std::ofstream os(empty_file); + } + std::ifstream is(empty_file); + is.rdbuf()->pubsetbuf(nullptr, 64); + + char buf[100]; + is.read(buf, 100); + assert(is.eof()); + assert(is.gcount() == 0); + + is.clear(); + is.unget(); + assert(is.fail()); + std::remove(empty_file.c_str()); + } +} + +static std::string make_pattern(std::size_t n) { + std::string s(n, '\0'); + for (std::size_t i = 0; i != n; ++i) + s[i] = static_cast<char>('0' + (i % 10)); + return s; +} + +void large_file_tests() { + const std::string data = make_pattern(10000); + std::string file = get_temp_file_name(); + { // Prepare the file + std::ofstream os(file); + assert(os.write(data.data(), data.size())); + } + + { // default buffer with a read larger than the buffer + std::ifstream is(file); + assert(is.is_open()); + std::string buf; + buf.resize(8000); + + is.read(&*buf.begin(), 8000); + assert(is.gcount() == 8000); + assert(std::string(buf.data(), 8000) == data.substr(0, 8000)); + is.unget(); + assert(is.good()); + assert(is.get() == data[7999]); // the ungotten character + assert(is.get() == data[8000]); // reading forward continues from the right place + } + + { // EOF before the buffer is full + std::ifstream is(file); + assert(is.is_open()); + is.rdbuf()->pubsetbuf(nullptr, 64); + + std::string buf; + buf.resize(data.size() + 100); + + is.read(&*buf.begin(), data.size() + 100); + assert(is.gcount() == static_cast<std::streamsize>(data.size())); + assert(is.rdstate() == (std::ios::eofbit | std::ios::failbit)); + is.clear(); // Clear the failbit due to trying to read more data than available + is.unget(); + assert(is.good()); + assert(is.get() == data.back()); + } + + std::remove(file.c_str()); +} + +int main(int, char**) { + small_file_tests(); + large_file_tests(); + return 0; +} diff --git a/libcxx/test/std/input.output/file.streams/fstreams/ifstream.members/xsgetn.pass.cpp b/libcxx/test/std/input.output/file.streams/fstreams/ifstream.members/xsgetn.pass.cpp index d9ccc2fe62914..35c4787e2a29f 100644 --- a/libcxx/test/std/input.output/file.streams/fstreams/ifstream.members/xsgetn.pass.cpp +++ b/libcxx/test/std/input.output/file.streams/fstreams/ifstream.members/xsgetn.pass.cpp @@ -25,6 +25,14 @@ #include "test_macros.h" +void check_unget(std::filebuf* fb, char expected) { + auto c = fb->sungetc(); + if (c != EOF) { + assert(c == expected); + assert(fb->sbumpc() == expected); + } +} + int main(int, char**) { std::vector<char> stream_buffer(10); std::ifstream fs("xsgetn.test.dat"); @@ -40,33 +48,39 @@ int main(int, char**) { { // Check that a read smaller than the buffer works fine assert(fb->sgetn(test_buffer.data(), 5) == 5); assert(std::string(test_buffer.data(), 5) == "this "); + check_unget(fb, ' '); } { // Check that reading up to the buffer end works fine assert(fb->sgetn(test_buffer.data(), 5) == 5); assert(std::string(test_buffer.data(), 5) == "is so"); + check_unget(fb, 'o'); } { // Check that reading from an empty buffer, but more than the buffer can // hold works fine test_buffer.resize(12); assert(fb->sgetn(test_buffer.data(), 12) == 12); assert(std::string(test_buffer.data(), 12) == "me random da"); + check_unget(fb, 'a'); } { // Check that reading from a non-empty buffer, and more than the buffer can // hold works fine Fill the buffer up test_buffer.resize(2); assert(fb->sgetn(test_buffer.data(), 2) == 2); assert(std::string(test_buffer.data(), 2) == "ta"); + check_unget(fb, 'a'); // Do the actual check test_buffer.resize(12); assert(fb->sgetn(test_buffer.data(), 12) == 12); assert(std::string(test_buffer.data(), 12) == " to be able "); + check_unget(fb, ' '); } { // Check that trying to read more than the file size works fine test_buffer.resize(30); assert(fb->sgetn(test_buffer.data(), 30) == 24); test_buffer.resize(24); assert(std::string(test_buffer.data(), 24) == "to test buffer behaviour"); + check_unget(fb, 'r'); } { // Ensure that the read fails gracefully with an unopened ifstream // See https://llvm.org/PR168628 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
