[libcxx-commits] [libcxx] [libc++] Fix ungetc failing after xsgetn (PR #210951)
Nikolas Klauser via libcxx-commits
libcxx-commits at lists.llvm.org
Thu Jul 23 23:28:03 PDT 2026
https://github.com/philnik777 updated https://github.com/llvm/llvm-project/pull/210951
>From c428a9ada3080af9cf20df99cf09062a5b328605 Mon Sep 17 00:00:00 2001
From: Nikolas Klauser <nikolasklauser at berlin.de>
Date: Tue, 21 Jul 2026 13:14:55 +0200
Subject: [PATCH] [libc++] Fix ungetc failing after xsgetn
---
libcxx/include/fstream | 77 ++++++---
libcxx/src/ios.instantiations.cpp | 16 ++
.../fstreams/filebuf.members/test.dat | 1 +
.../filebuf.members/xsgetn.buffer.pass.cpp | 156 ++++++++++++++++++
.../fstreams/ifstream.members/xsgetn.pass.cpp | 14 ++
5 files changed, 240 insertions(+), 24 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..eebe2c879edfb 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,42 @@ 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,12 +847,12 @@ 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);
- int_type __c = traits_type::eof();
+ __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));
if (__always_noconv_) {
@@ -849,8 +894,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 +1184,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..2f885688c60ee 100644
--- a/libcxx/src/ios.instantiations.cpp
+++ b/libcxx/src/ios.instantiations.cpp
@@ -15,6 +15,21 @@
#include <streambuf>
_LIBCPP_BEGIN_NAMESPACE_STD
+_LIBCPP_BEGIN_EXPLICIT_ABI_ANNOTATIONS
+
+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;
+}
// Original explicit instantiations provided in the library
template class _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS basic_ios<char>;
@@ -45,4 +60,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..0b7f3b04fa219
--- /dev/null
+++ b/libcxx/test/extensions/libcxx/input.output/file.streams/fstreams/filebuf.members/xsgetn.buffer.pass.cpp
@@ -0,0 +1,156 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+
+// <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.data(), 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.data(), 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
More information about the libcxx-commits
mailing list