From ee471383cd46bcb4686e849f777d9caba45fb92f Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Tue, 15 Jul 2025 10:40:54 -0400 Subject: [PATCH 1/3] [libc++] Ensure that we restore invariants in basic_filebuf::overflow (#147389) In rare circumstances, the invariants could fail to be restored. (cherry picked from commit 6291b63a9a104fe93f8e4e279ef2237dc081304f) --- libcxx/include/fstream | 35 ++++++--- .../overflow.writefail.pass.cpp | 72 +++++++++++++++++++ 2 files changed, 96 insertions(+), 11 deletions(-) create mode 100644 libcxx/test/std/input.output/file.streams/fstreams/filebuf.virtuals/overflow.writefail.pass.cpp diff --git a/libcxx/include/fstream b/libcxx/include/fstream index c86f709bedb80..dc5c47304f014 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 __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 + } + 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 __failed(); + } } 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 __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 traits_type::eof(); + if (std::fwrite(__b, 1, __n, __file_) != __n) { + return __failed(); + } 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 __failed(); + } 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 __failed(); + } __b = const_cast(__end); continue; } else { - return traits_type::eof(); + return __failed(); } } while (true); } 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..27e06982d749b --- /dev/null +++ b/libcxx/test/std/input.output/file.streams/fstreams/filebuf.virtuals/overflow.writefail.pass.cpp @@ -0,0 +1,72 @@ +//===----------------------------------------------------------------------===// +// +// 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 + +// setrlimit(RLIMIT_FSIZE) seems to only work as intended on Apple platforms +// REQUIRES: target={{.+}}-apple-{{.+}} + +// + +// 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 b16bde274c8587d0e53d61bb903b7f5638631e70 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Thu, 17 Jul 2025 23:50:39 +0100 Subject: [PATCH 2/3] [libcxx][fstream][NFC] Make __failed helper lambda a member function (#149390) This patch makes the `__failed` lambda a member function on `fstream`. This fixes two LLDB expression evaluation test failures that got introduced with https://github.com/llvm/llvm-project/pull/147389: ``` 16:22:51 ******************** 16:22:51 Unresolved Tests (2): 16:22:51 lldb-api :: commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py 16:22:51 lldb-api :: commands/expression/import-std-module/list/TestListFromStdModule.py ``` The expression evaluator is asserting in the Clang parser: ``` Assertion failed: (capture_size() == Class->capture_size() && "Wrong number of captures"), function LambdaExpr, file ExprCXX.cpp, line 1277. PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace. ``` Ideally we'd figure out why LLDB is falling over on this lambda. But to unblock CI for now, make this a member function. In the long run we should figure out the LLDB bug here so libc++ doesn't need to care about whether it uses lambdas like this or not. (cherry picked from commit 8f4deff5d51ac190e056a6738018fc8aa3114151) --- libcxx/include/fstream | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/libcxx/include/fstream b/libcxx/include/fstream index dc5c47304f014..6d3f20fff688f 100644 --- a/libcxx/include/fstream +++ b/libcxx/include/fstream @@ -401,6 +401,14 @@ private: } } } + + _LIBCPP_HIDE_FROM_ABI typename traits_type::int_type __overflow_failed() { + 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(); + } }; template @@ -821,14 +829,6 @@ 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 __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 - } - return traits_type::eof(); - }; - if (__file_ == nullptr) return traits_type::eof(); __write_mode(); @@ -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 __failed(); + return __overflow_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 __failed(); + return __overflow_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 __failed(); + return __overflow_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 __failed(); + return __overflow_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 __failed(); + return __overflow_failed(); } __b = const_cast(__end); continue; } else { - return __failed(); + return __overflow_failed(); } } while (true); } From d8dbddcb88ec47cbdfde7a941c116dded4b161dc Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Fri, 22 Aug 2025 20:33:59 +0100 Subject: [PATCH 3/3] [lldb][ClangASTImporter] Don't ASTImport LambdaExpr nodes (#154962) This patch works around an assertion that we hit in the `LambdaExpr` constructor when we call it from `ASTNodeImporter::VisitLambdaExpr` (see https://github.com/llvm/llvm-project/issues/149477). The lambda that we imported doesn't have the `NumCaptures` field accurately set to the one on the source decl. This is because in `MinimalImport` mode, we skip importing of lambda definitions: https://github.com/llvm/llvm-project/blob/e21b0dd81928a3266df0e3ede008fb7a6676ff95/clang/lib/AST/ASTImporter.cpp#L2499 In practice we have seen this assertion occur in our `import-std-module` test-suite when libc++ headers decide to use lambdas inside inline function bodies (the latest failure being caused by https://github.com/llvm/llvm-project/pull/144602). To avoid running into this whenever libc++ decides to use lambdas in headers, this patch skips `ASTImport` of lambdas alltogether. Ideally this would bubble up to the user or log as an error, but we swallow the `ASTImportError`s currently. The only way this codepath is hit is when lambdas are used inside functions in defined in the expression evaluator, or when importing AST nodes from Clang modules. Both of these are very niche use-cases (for now), so a workaround seemed appropriate. (cherry picked from commit 0bbb79475432f72ee0e7d99cc5dc48bb8f8ff443) --- .../Clang/ClangASTImporter.cpp | 10 +++++++ lldb/test/API/lang/cpp/lambdas/TestLambdas.py | 10 ++++++- .../test/Shell/Expr/TestLambdaExprImport.test | 26 +++++++++++++++++++ 3 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 lldb/test/Shell/Expr/TestLambdaExprImport.test diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp index 2529e78f78bca..633b860b5784c 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp @@ -1053,6 +1053,16 @@ ClangASTImporter::MapCompleter::~MapCompleter() = default; llvm::Expected ClangASTImporter::ASTImporterDelegate::ImportImpl(Decl *From) { + // FIXME: The Minimal import mode of clang::ASTImporter does not correctly + // import Lambda definitions. Work around this for now by not importing + // lambdas at all. This is most likely encountered when importing decls from + // the `std` module (not from debug-info), where lambdas can be defined in + // inline function bodies. Those will be imported by LLDB. + if (const auto *CXX = llvm::dyn_cast(From)) + if (CXX->isLambda()) + return llvm::make_error( + ASTImportError::UnsupportedConstruct); + if (m_std_handler) { std::optional D = m_std_handler->Import(From); if (D) { diff --git a/lldb/test/API/lang/cpp/lambdas/TestLambdas.py b/lldb/test/API/lang/cpp/lambdas/TestLambdas.py index c8308c16011e0..112f375c0ef2f 100644 --- a/lldb/test/API/lang/cpp/lambdas/TestLambdas.py +++ b/lldb/test/API/lang/cpp/lambdas/TestLambdas.py @@ -1,4 +1,12 @@ from lldbsuite.test import lldbinline from lldbsuite.test import decorators -lldbinline.MakeInlineTest(__file__, globals()) +lldbinline.MakeInlineTest( + __file__, + globals(), + [ + decorators.expectedFailureAll( + bugnumber="https://github.com/llvm/llvm-project/issues/149477" + ) + ], +) diff --git a/lldb/test/Shell/Expr/TestLambdaExprImport.test b/lldb/test/Shell/Expr/TestLambdaExprImport.test new file mode 100644 index 0000000000000..c57ce06453fe2 --- /dev/null +++ b/lldb/test/Shell/Expr/TestLambdaExprImport.test @@ -0,0 +1,26 @@ +# Test that we can successfully ASTImport clang::LambdaExpr nodes. +# Currently this is not supported in MinimalImport mode (which LLDB +# uses always). + +# RUN: split-file %s %t +# RUN: %clang_host -g -gdwarf %t/main.cpp -o %t.out +# RUN: %lldb -o "settings set interpreter.stop-command-source-on-error false" \ +# RUN: -x -b -s %t/commands.input %t.out 2>&1 \ +# RUN: | FileCheck %s + +#--- main.cpp + +int main() { + __builtin_debugtrap(); +} + +#--- commands.input + +run +expression --top-level -- void method(int x) { [x=x] { ; }; } +target dump typesystem + +# CHECK: expression +# CHECK: target dump typesystem +# CHECK-NOT: FunctionDecl +# CHECK-NOT: LambdaExpr