From 1906af8a337c4448eb5103810b3e24cf27b4ee8f Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Thu, 3 Jul 2025 11:47:45 -0400 Subject: [PATCH 1/5] [libc++] Ensure that we restore invariants in basic_filebuf::overflow In rare circumstances, the invariants could fail to be restored. --- libcxx/include/fstream | 35 ++++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/libcxx/include/fstream b/libcxx/include/fstream index c86f709bedb80..283e31294cc73 100644 --- a/libcxx/include/fstream +++ b/libcxx/include/fstream @@ -821,6 +821,14 @@ typename basic_filebuf<_CharT, _Traits>::int_type basic_filebuf<_CharT, _Traits> template typename basic_filebuf<_CharT, _Traits>::int_type basic_filebuf<_CharT, _Traits>::overflow(int_type __c) { + auto __failure = [this] { + if (this->pptr() == this->epptr() + 1) { + this->pbump(-1); // lose the character we overflowed above -- we don't really have a + // choice since we couldn't commit the contents of the put area + } + return traits_type::eof(); + }; + if (__file_ == nullptr) return traits_type::eof(); __write_mode(); @@ -841,8 +849,9 @@ typename basic_filebuf<_CharT, _Traits>::int_type basic_filebuf<_CharT, _Traits> if (__always_noconv_) { size_t __n = static_cast(this->pptr() - this->pbase()); - if (std::fwrite(this->pbase(), sizeof(char_type), __n, __file_) != __n) - return traits_type::eof(); + if (std::fwrite(this->pbase(), sizeof(char_type), __n, __file_) != __n) { + return __failure(); + } } else { if (!__cv_) std::__throw_bad_cast(); @@ -854,34 +863,38 @@ typename basic_filebuf<_CharT, _Traits>::int_type basic_filebuf<_CharT, _Traits> char* __extbuf_end = __extbuf_; do { codecvt_base::result __r = __cv_->out(__st_, __b, __p, __end, __extbuf_, __extbuf_ + __ebs_, __extbuf_end); - if (__end == __b) - return traits_type::eof(); + if (__end == __b) { + return __failure(); + } // No conversion needed: output characters directly to the file, done. if (__r == codecvt_base::noconv) { size_t __n = static_cast(__p - __b); - if (std::fwrite(__b, 1, __n, __file_) != __n) - return traits_type::eof(); + if (std::fwrite(__b, 1, __n, __file_) != __n) { + return __failure(); + } break; // Conversion successful: output the converted characters to the file, done. } else if (__r == codecvt_base::ok) { size_t __n = static_cast(__extbuf_end - __extbuf_); - if (std::fwrite(__extbuf_, 1, __n, __file_) != __n) - return traits_type::eof(); + if (std::fwrite(__extbuf_, 1, __n, __file_) != __n) { + return __failure(); + } break; // Conversion partially successful: output converted characters to the file and repeat with the // remaining characters. } else if (__r == codecvt_base::partial) { size_t __n = static_cast(__extbuf_end - __extbuf_); - if (std::fwrite(__extbuf_, 1, __n, __file_) != __n) - return traits_type::eof(); + if (std::fwrite(__extbuf_, 1, __n, __file_) != __n) { + return __failure(); + } __b = const_cast(__end); continue; } else { - return traits_type::eof(); + return __failure(); } } while (true); } From ae2baa9ddd48c1eace69b6960220a51819c661f1 Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Tue, 8 Jul 2025 14:39:06 -0400 Subject: [PATCH 2/5] Add test --- .../overflow.writefail.pass.cpp | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 libcxx/test/std/input.output/file.streams/fstreams/filebuf.virtuals/overflow.writefail.pass.cpp diff --git a/libcxx/test/std/input.output/file.streams/fstreams/filebuf.virtuals/overflow.writefail.pass.cpp b/libcxx/test/std/input.output/file.streams/fstreams/filebuf.virtuals/overflow.writefail.pass.cpp new file mode 100644 index 0000000000000..e0da976d4478d --- /dev/null +++ b/libcxx/test/std/input.output/file.streams/fstreams/filebuf.virtuals/overflow.writefail.pass.cpp @@ -0,0 +1,70 @@ +//===----------------------------------------------------------------------===// +// +// 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 +// +//===----------------------------------------------------------------------===// + +// UNSUPPORTED: no-filesystem +// REQUIRES: locale.en_US.UTF-8 + +// + +// Make sure that we properly handle the case where we try to write content to a file +// but we fail to do so because std::fwrite fails. + +#include +#include +#include +#include +#include + +#include "platform_support.h" +#include "test_macros.h" + +#if __has_include() +# include +void limit_file_size_to(std::size_t bytes) { + rlimit lim = {bytes, bytes}; + assert(setrlimit(RLIMIT_FSIZE, &lim) == 0); + + std::signal(SIGXFSZ, [](int) {}); // ignore SIGXFSZ to ensure std::fwrite fails +} +#else +# error No known way to limit the amount of filesystem space available +#endif + +template +void test() { + std::string temp = get_temp_file_name(); + std::basic_filebuf fbuf; + assert(fbuf.open(temp, std::ios::out | std::ios::trunc)); + + std::size_t const limit = 100000; + limit_file_size_to(limit); + + std::basic_string large_block(limit / 10, CharT(42)); + + std::streamsize ret; + std::size_t bytes_written = 0; + while ((ret = fbuf.sputn(large_block.data(), large_block.size())) != 0) { + bytes_written += ret; + + // In theory, it's possible for an implementation to allow writing arbitrarily more bytes than + // set by setrlimit, but in practice if we bust 100x our limit, something else is wrong with the + // test and we'd end up looping forever. + assert(bytes_written < 100 * limit); + } + + fbuf.close(); +} + +int main(int, char**) { + test(); +#ifndef TEST_HAS_NO_WIDE_CHARACTERS + test(); +#endif + + return 0; +} From 78a95355851e684f1d339ab63b206f283866b93e Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Wed, 9 Jul 2025 09:31:20 -0400 Subject: [PATCH 3/5] Only run the new test on Apple --- .../fstreams/filebuf.virtuals/overflow.writefail.pass.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libcxx/test/std/input.output/file.streams/fstreams/filebuf.virtuals/overflow.writefail.pass.cpp b/libcxx/test/std/input.output/file.streams/fstreams/filebuf.virtuals/overflow.writefail.pass.cpp index e0da976d4478d..27e06982d749b 100644 --- a/libcxx/test/std/input.output/file.streams/fstreams/filebuf.virtuals/overflow.writefail.pass.cpp +++ b/libcxx/test/std/input.output/file.streams/fstreams/filebuf.virtuals/overflow.writefail.pass.cpp @@ -7,7 +7,9 @@ //===----------------------------------------------------------------------===// // UNSUPPORTED: no-filesystem -// REQUIRES: locale.en_US.UTF-8 + +// setrlimit(RLIMIT_FSIZE) seems to only work as intended on Apple platforms +// REQUIRES: target={{.+}}-apple-{{.+}} // From 966801d0b51c2dd29034a1cdfe4127a649dcab06 Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Thu, 10 Jul 2025 16:45:03 -0400 Subject: [PATCH 4/5] Try to work around Windows error --- libcxx/include/fstream | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcxx/include/fstream b/libcxx/include/fstream index 283e31294cc73..28cb115cf9216 100644 --- a/libcxx/include/fstream +++ b/libcxx/include/fstream @@ -821,7 +821,7 @@ typename basic_filebuf<_CharT, _Traits>::int_type basic_filebuf<_CharT, _Traits> template typename basic_filebuf<_CharT, _Traits>::int_type basic_filebuf<_CharT, _Traits>::overflow(int_type __c) { - auto __failure = [this] { + auto __failure = [this]() { if (this->pptr() == this->epptr() + 1) { this->pbump(-1); // lose the character we overflowed above -- we don't really have a // choice since we couldn't commit the contents of the put area From 5daa9f38272b96a1c96da425fe78e5783f0515d3 Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Fri, 11 Jul 2025 12:15:04 -0400 Subject: [PATCH 5/5] Is there a weird macro named __failure on Windows? --- libcxx/include/fstream | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/libcxx/include/fstream b/libcxx/include/fstream index 28cb115cf9216..dc5c47304f014 100644 --- a/libcxx/include/fstream +++ b/libcxx/include/fstream @@ -821,7 +821,7 @@ typename basic_filebuf<_CharT, _Traits>::int_type basic_filebuf<_CharT, _Traits> template typename basic_filebuf<_CharT, _Traits>::int_type basic_filebuf<_CharT, _Traits>::overflow(int_type __c) { - auto __failure = [this]() { + auto __failed = [this]() { if (this->pptr() == this->epptr() + 1) { this->pbump(-1); // lose the character we overflowed above -- we don't really have a // choice since we couldn't commit the contents of the put area @@ -850,7 +850,7 @@ typename basic_filebuf<_CharT, _Traits>::int_type basic_filebuf<_CharT, _Traits> if (__always_noconv_) { size_t __n = static_cast(this->pptr() - this->pbase()); if (std::fwrite(this->pbase(), sizeof(char_type), __n, __file_) != __n) { - return __failure(); + return __failed(); } } else { if (!__cv_) @@ -864,14 +864,14 @@ typename basic_filebuf<_CharT, _Traits>::int_type basic_filebuf<_CharT, _Traits> do { codecvt_base::result __r = __cv_->out(__st_, __b, __p, __end, __extbuf_, __extbuf_ + __ebs_, __extbuf_end); if (__end == __b) { - return __failure(); + return __failed(); } // No conversion needed: output characters directly to the file, done. if (__r == codecvt_base::noconv) { size_t __n = static_cast(__p - __b); if (std::fwrite(__b, 1, __n, __file_) != __n) { - return __failure(); + return __failed(); } break; @@ -879,7 +879,7 @@ typename basic_filebuf<_CharT, _Traits>::int_type basic_filebuf<_CharT, _Traits> } else if (__r == codecvt_base::ok) { size_t __n = static_cast(__extbuf_end - __extbuf_); if (std::fwrite(__extbuf_, 1, __n, __file_) != __n) { - return __failure(); + return __failed(); } break; @@ -888,13 +888,13 @@ typename basic_filebuf<_CharT, _Traits>::int_type basic_filebuf<_CharT, _Traits> } else if (__r == codecvt_base::partial) { size_t __n = static_cast(__extbuf_end - __extbuf_); if (std::fwrite(__extbuf_, 1, __n, __file_) != __n) { - return __failure(); + return __failed(); } __b = const_cast(__end); continue; } else { - return __failure(); + return __failed(); } } while (true); }