[libcxx-commits] [libcxx] [libc++] Fix ungetc failing after xsgetn (PR #210951)

Nikolas Klauser via libcxx-commits libcxx-commits at lists.llvm.org
Tue Jul 21 04:16:32 PDT 2026


https://github.com/philnik777 created https://github.com/llvm/llvm-project/pull/210951

After #206453 we don't correctly handle `unget()` anymore. This fixes the issue by updating the internal buffer to contain the tail of the read data.


>From 24cc1dfc9e792231143e8e5f8cf0826794aa9b27 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 +++++++++++++------
 .../fstreams/filebuf.members/test.dat         |  1 +
 .../filebuf.members/xsgetn.buffer.pass.cpp    | 59 ++++++++++++++
 .../fstreams/ifstream.members/xsgetn.pass.cpp | 14 ++++
 4 files changed, 126 insertions(+), 25 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..bd94bb4bf4b90 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;
     }
@@ -383,7 +392,41 @@ private:
   bool __owns_ib_;
   bool __always_noconv_;
 
-  bool __read_mode();
+  struct [[__nodiscard__]] __read_guard {
+    basic_filebuf& __self_;
+    size_t __unget_size_;
+    char_type& __1buf_;
+
+    __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);
+    }
+
+    size_t __unget_size() {
+      return __unget_size_;
+    }
+
+    ~__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 +845,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 +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/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..d97d09e4c0ce9
--- /dev/null
+++ b/libcxx/test/extensions/libcxx/input.output/file.streams/fstreams/filebuf.members/xsgetn.buffer.pass.cpp
@@ -0,0 +1,59 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+
+#include <cassert>
+#include <fstream>
+
+int main(int, char**) {
+  std::ifstream is("test.dat");
+  char buffer[12];
+  is.rdbuf()->pubsetbuf(buffer, 12);
+  assert(is.is_open());
+  { // Check that we can unget() when reading a single character
+    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
+    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
+    char buf[33];
+    buf[32] = '\0';
+    is.read(buf, 32);
+    assert(buf == std::string("of data so the test can read som"));
+    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(" som"));
+  }
+
+  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