[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [libcxx] [lld] [lldb] [llvm] [mlir] Fix SyntaxWarning messages from python 3.12 (PR #86806)
ldionne wrote: Can we either finish this up or close it, please? I'd like to tidy up the libc++ review queue. https://github.com/llvm/llvm-project/pull/86806 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [libcxx] [lldb] [llvm] Add clang basic module directory (PR #93388)
ldionne wrote: For tidying up the libc++ review queue, please remove the `libc++` tag once you rebase this (the small changes in `libcxx` will go away). https://github.com/llvm/llvm-project/pull/93388 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [compiler-rt] [libcxx] [lld] [lldb] [llvm] [mlir] [polly] python: use raw strings for regex (PR #105990)
https://github.com/ldionne requested changes to this pull request. https://github.com/llvm/llvm-project/pull/105990 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [compiler-rt] [libcxx] [lld] [lldb] [llvm] [mlir] [polly] python: use raw strings for regex (PR #105990)
@@ -284,7 +284,7 @@ def sync_csv(rows: List[Tuple], from_github: List[PaperInfo]) -> List[Tuple]: results.append(gh.for_printing()) continue elif paper.status != gh.status: -print(f"We found a CSV row and a Github issue with different statuses:\nrow: {row}\Github issue: {gh}") +print(rf"We found a CSV row and a Github issue with different statuses:\nrow: {row}\Github issue: {gh}") ldionne wrote: This is not a regex -- this change looks wrong. https://github.com/llvm/llvm-project/pull/105990 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [compiler-rt] [libcxx] [lld] [lldb] [llvm] [mlir] [polly] python: use raw strings for regex (PR #105990)
@@ -284,7 +284,7 @@ def sync_csv(rows: List[Tuple], from_github: List[PaperInfo]) -> List[Tuple]: results.append(gh.for_printing()) continue elif paper.status != gh.status: -print(f"We found a CSV row and a Github issue with different statuses:\nrow: {row}\Github issue: {gh}") +print(rf"We found a CSV row and a Github issue with different statuses:\nrow: {row}\Github issue: {gh}") ldionne wrote: Ah, yeah that's the issue. The `\G` is unintended. I'll fix this right now. https://github.com/llvm/llvm-project/pull/105990 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [compiler-rt] [libcxx] [lld] [lldb] [llvm] [mlir] [polly] python: use raw strings for regex (PR #105990)
https://github.com/ldionne edited https://github.com/llvm/llvm-project/pull/105990 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [compiler-rt] [libcxx] [lldb] [llvm] Rename Sanitizer Coverage => Coverage Sanitizer (PR #106505)
@@ -257,7 +257,7 @@ General purpose options A semicolon list of arguments to pass when running the libc++ benchmarks using the ``check-cxx-benchmarks`` rule. By default we run the benchmarks for a very short amount of time, - since the primary use of ``check-cxx-benchmarks`` is to get test and sanitizer coverage, not to + since the primary use of ``check-cxx-benchmarks`` is to get test and coverage sanitizer, not to ldionne wrote: This change is wrong. This documentation relates to "coverage for sanitizers", not a specific kind of "sanitizer for coverage". https://github.com/llvm/llvm-project/pull/106505 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][FrameRecognizer] Display the first non-std frame on verbose_trap (PR #108825)
https://github.com/ldionne commented: I'm not qualified to review the implementation, but I like the result! https://github.com/llvm/llvm-project/pull/108825 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [libcxx] [lldb] [lldb][libc++] Hide all libc++ implementation details from stacktraces (PR #108870)
https://github.com/ldionne commented: This feels like a good idea, but I am also a bit scared of hiding too much stuff and making it difficult to figure out what's going wrong from a stack trace. It's not rational, I have no evidence to back this up and perhaps this is only fear of unknown. Factually, the heuristic of using `__` makes sense to me since all implementation-detail functions start with `__`, and nothing in the public libc++ API starts with `__` (off the top of my head). Overall, I think I like this patch but like I said I'm a bit scared of it being too aggressive. https://github.com/llvm/llvm-project/pull/108870 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [compiler-rt] [libc] [flang] [llvm] [libcxx] [clang-tools-extra] [lld] [lldb] [clang] [libc++][ranges] P2116R9: Implements `views::enumerate` (PR #73617)
https://github.com/ldionne ready_for_review https://github.com/llvm/llvm-project/pull/73617 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [libcxx] [clang-tools-extra] [flang] [libc] [lld] [llvm] [compiler-rt] [clang] [libc++][variant] P2637R3: Member `visit` (`std::variant`) (PR #76447)
https://github.com/ldionne ready_for_review https://github.com/llvm/llvm-project/pull/76447 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [libcxx] [clang-tools-extra] [libcxxabi] [mlir] [flang] [libc] [lld] [libclc] [libunwind] [llvm] [openmp] [compiler-rt] [clang] [libc++][ranges] Implement ranges::contains_subran
https://github.com/ldionne ready_for_review https://github.com/llvm/llvm-project/pull/66963 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [NFC][CodeGen] Change CodeGenOpt::Level/CodeGenFileType into enum classes (PR #66295)
ldionne wrote: There shouldn't be any changes under libcxxabi. Those changes are not necessary. https://github.com/llvm/llvm-project/pull/66295 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [libcxx] [lld] [lldb] [llvm] [mlir] Fix SyntaxWarning messages from python 3.12 (PR #86806)
https://github.com/ldionne approved this pull request. Libc++ parts LGTM. https://github.com/llvm/llvm-project/pull/86806 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [libcxx] [lldb] [libc++][CI] Tests LLDB libc++ data formatters. (PR #88312)
https://github.com/ldionne approved this pull request. LGTM, thanks for picking this up! https://github.com/llvm/llvm-project/pull/88312 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [compiler-rt] [libc] [libclc] [libcxxabi] [lld] [lldb] [llvm] [mlir] llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:3804: lacking () for c… (PR #90391)
https://github.com/ldionne approved this pull request. The libc++abi changes look fine to me. https://github.com/llvm/llvm-project/pull/90391 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] d2e8686 - [libc++] Re-apply the use of ABI tags to provide per-TU insulation
Author: Louis Dionne Date: 2022-07-08T08:38:36-04:00 New Revision: d2e86866be0f17295c5f677333c2a9952e259dd7 URL: https://github.com/llvm/llvm-project/commit/d2e86866be0f17295c5f677333c2a9952e259dd7 DIFF: https://github.com/llvm/llvm-project/commit/d2e86866be0f17295c5f677333c2a9952e259dd7.diff LOG: [libc++] Re-apply the use of ABI tags to provide per-TU insulation This commit re-applies 9ee97ce3b830, which was reverted by 61d417ce because it broke the LLDB data formatter tests. It also re-applies 6148c79a (the manual GN change associated to it). Differential Revision: https://reviews.llvm.org/D127444 Added: Modified: libcxx/CMakeLists.txt libcxx/cmake/caches/Apple.cmake libcxx/docs/BuildingLibcxx.rst libcxx/docs/DesignDocs/VisibilityMacros.rst libcxx/docs/ReleaseNotes.rst libcxx/include/__config libcxx/include/__config_site.in lldb/packages/Python/lldbsuite/test/make/Makefile.rules llvm/utils/gn/secondary/libcxx/include/BUILD.gn Removed: libcxx/test/libcxx/strings/basic.string/PR42676.sh.cpp diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt index b118007eacbca..3d63bb0c8fbf8 100644 --- a/libcxx/CMakeLists.txt +++ b/libcxx/CMakeLists.txt @@ -205,7 +205,6 @@ if (NOT ("${LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION}" IN_LIST TYPEINFO_COMPARI LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION") endif() -option(LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT "Enable per TU ABI insulation by default. To be used by vendors." OFF) set(LIBCXX_ABI_DEFINES "" CACHE STRING "A semicolon separated list of ABI macros to define in the site config header.") option(LIBCXX_EXTRA_SITE_DEFINES "Extra defines to add into __config_site") option(LIBCXX_USE_COMPILER_RT "Use compiler-rt instead of libgcc" OFF) @@ -847,7 +846,6 @@ config_define(${LIBCXX_ABI_VERSION} _LIBCPP_ABI_VERSION) config_define(${LIBCXX_ABI_NAMESPACE} _LIBCPP_ABI_NAMESPACE) config_define_if(LIBCXX_ABI_FORCE_ITANIUM _LIBCPP_ABI_FORCE_ITANIUM) config_define_if(LIBCXX_ABI_FORCE_MICROSOFT _LIBCPP_ABI_FORCE_MICROSOFT) -config_define_if(LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT _LIBCPP_HIDE_FROM_ABI_PER_TU_BY_DEFAULT) config_define_if_not(LIBCXX_ENABLE_THREADS _LIBCPP_HAS_NO_THREADS) config_define_if_not(LIBCXX_ENABLE_MONOTONIC_CLOCK _LIBCPP_HAS_NO_MONOTONIC_CLOCK) if (NOT LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION STREQUAL "default") diff --git a/libcxx/cmake/caches/Apple.cmake b/libcxx/cmake/caches/Apple.cmake index db567bd394b12..ea701a43f96a3 100644 --- a/libcxx/cmake/caches/Apple.cmake +++ b/libcxx/cmake/caches/Apple.cmake @@ -8,7 +8,6 @@ set(LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY OFF CACHE BOOL "") set(LIBCXX_ENABLE_STATIC ON CACHE BOOL "") set(LIBCXX_ENABLE_SHARED ON CACHE BOOL "") set(LIBCXX_CXX_ABI libcxxabi CACHE STRING "") -set(LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT ON CACHE BOOL "") set(LIBCXX_ENABLE_BACKWARDS_COMPATIBILITY_DEBUG_MODE_SYMBOLS OFF CACHE BOOL "") set(LIBCXX_ENABLE_VENDOR_AVAILABILITY_ANNOTATIONS ON CACHE BOOL "") set(LIBCXX_ENABLE_INCOMPLETE_FEATURES OFF CACHE BOOL "") diff --git a/libcxx/docs/BuildingLibcxx.rst b/libcxx/docs/BuildingLibcxx.rst index 3f4e314382d82..5069c7fe06928 100644 --- a/libcxx/docs/BuildingLibcxx.rst +++ b/libcxx/docs/BuildingLibcxx.rst @@ -412,15 +412,6 @@ libc++ Feature Options Use the specified GCC toolchain and standard library when building the native stdlib benchmark tests. -.. option:: LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT:BOOL - - **Default**: ``OFF`` - - Pick the default for whether to constrain ABI-unstable symbols to - each individual translation unit. This setting controls whether - `_LIBCPP_HIDE_FROM_ABI_PER_TU_BY_DEFAULT` is defined by default -- - see the documentation of that macro for details. - libc++ ABI Feature Options -- diff --git a/libcxx/docs/DesignDocs/VisibilityMacros.rst b/libcxx/docs/DesignDocs/VisibilityMacros.rst index a165fc49afcf4..d1c162682b666 100644 --- a/libcxx/docs/DesignDocs/VisibilityMacros.rst +++ b/libcxx/docs/DesignDocs/VisibilityMacros.rst @@ -65,41 +65,6 @@ Visibility Macros ABI, we should create a new _LIBCPP_HIDE_FROM_ABI_AFTER_XXX macro, and we can use it to start removing symbols from the ABI after that stable version. -**_LIBCPP_HIDE_FROM_ABI_PER_TU** - This macro controls whether symbols hidden from the ABI with `_LIBCPP_HIDE_FROM_ABI` - are local to each translation unit in addition to being local to each final - linked image. This macro is defined to either 0 or 1. When it is defined to - 1, translation units compiled with diff erent versions of libc++ can be linked - together, since all non ABI-facing functions are local to each translation unit. - This allows static archives built with diff erent versions of libc++ to be linked - together. This also means that functions marked with `_LIBCPP_HIDE_FROM_ABI` - are not guaranteed to hav
[Lldb-commits] [llvm] [libcxx] [libcxxabi] [clang] [lldb] [libc] [clang-tools-extra] [libc++] Fix the behavior of throwing `operator new` under -fno-exceptions (PR #69498)
https://github.com/ldionne updated https://github.com/llvm/llvm-project/pull/69498 >From 6f89b118ed56ad7a3af1996e19ccd30cc893c51e Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Wed, 14 Jun 2023 17:49:22 -0700 Subject: [PATCH 1/7] [libc++] Fix the behavior of throwing `operator new` under -fno-exceptions In D144319, Clang tried to land a change that would cause some functions that are not supposed to return nullptr to optimize better. As reported in https://reviews.llvm.org/D144319#4203982, libc++ started seeing failures in its CI shortly after this change was landed. As explained in D146379, the reason for these failures is that libc++'s throwing `operator new` can in fact return nullptr when compiled with exceptions disabled. However, this contradicts the Standard, which clearly says that the throwing version of `operator new(size_t)` should never return nullptr. This is actually a long standing issue. I've previously seen a case where LTO would optimize incorrectly based on the assumption that `operator new` doesn't return nullptr, an assumption that was violated in that case because libc++.dylib was compiled with -fno-exceptions. Unfortunately, fixing this is kind of tricky. The Standard has a few requirements for the allocation functions, some of which are impossible to satisfy under -fno-exceptions: 1. `operator new(size_t)` must never return nullptr 2. `operator new(size_t, nothrow_t)` must call the throwing version and return nullptr on failure to allocate 3. We can't throw exceptions when compiled with -fno-exceptions In the case where exceptions are enabled, things work nicely. `new(size_t)` throws and `new(size_t, nothrow_t)` uses a try-catch to return nullptr. However, when compiling the library with -fno-exceptions, we can't throw an exception from `new(size_t)`, and we can't catch anything from `new(size_t, nothrow_t)`. The only thing we can do from `new(size_t)` is actually abort the program, which does not make it possible for `new(size_t, nothrow_t)` to catch something and return nullptr. This patch makes the following changes: 1. When compiled with -fno-exceptions, the throwing version of `operator new` will now abort on failure instead of returning nullptr on failure. This resolves the issue that the compiler could mis-compile based on the assumption that nullptr is never returned. This constitutes an API and ABI breaking change for folks compiling the library with -fno-exceptions (which is not the general public, who merely uses libc++ headers but use a shared library that has already been compiled). This should mostly impact vendors and other folks who compile libc++.dylib themselves. 2. When the library is compiled with -fexceptions, the nothrow version of `operator new` has no change. When the library is compiled with -fno-exceptions, the nothrow version of `operator new` will now check whether the throwing version of `operator new` has been overridden. If it has not been overridden, then it will use an implementation equivalent to that of the throwing `operator new`, except it will return nullptr on failure to allocate (instead of terminating). However, if the throwing `operator new` has been overridden, it is now an error NOT to also override the nothrow `operator new`. Indeed, there is no way for us to implement a valid nothrow `operator new` without knowing the exact implementation of the throwing version. rdar://103958777 Differential Revision: https://reviews.llvm.org/D150610 --- libcxx/docs/ReleaseNotes/18.rst | 23 +++ libcxx/include/CMakeLists.txt | 1 + libcxx/include/__overridable_function | 119 + libcxx/src/new.cpp| 99 --- ...new_not_overridden_fno_exceptions.pass.cpp | 59 ++ .../new_dont_return_nullptr.pass.cpp | 37 libcxx/test/support/check_assertion.h | 6 + libcxx/utils/generate_iwyu_mapping.py | 2 + libcxxabi/src/stdlib_new_delete.cpp | 168 +++--- 9 files changed, 426 insertions(+), 88 deletions(-) create mode 100644 libcxx/include/__overridable_function create mode 100644 libcxx/test/libcxx/language.support/support.dynamic/assert.nothrow_new_not_overridden_fno_exceptions.pass.cpp create mode 100644 libcxx/test/libcxx/language.support/support.dynamic/new_dont_return_nullptr.pass.cpp diff --git a/libcxx/docs/ReleaseNotes/18.rst b/libcxx/docs/ReleaseNotes/18.rst index 9e509db6359c4aa..405f1e172893bc1 100644 --- a/libcxx/docs/ReleaseNotes/18.rst +++ b/libcxx/docs/ReleaseNotes/18.rst @@ -142,6 +142,29 @@ LLVM 20 ABI Affecting Changes - +- When the shared/static library is built with ``-fno-exceptions``, the behavior of ``operator new`` was changed + to make it standards-conforming. In LLVM 17 and before, the throwing versions of ``operator new`` would return + ``nullptr`` upon failure to allocate, when the sh
[Lldb-commits] [libc] [libcxxabi] [lldb] [llvm] [clang] [clang-tools-extra] [libcxx] [libc++] Fix the behavior of throwing `operator new` under -fno-exceptions (PR #69498)
https://github.com/ldionne edited https://github.com/llvm/llvm-project/pull/69498 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lld] [lldb] [compiler-rt] [libc] [llvm] [libcxx] [libclc] [flang] [clang-tools-extra] [clang] [libc++] Ensure that `std::expected` has no tail padding (PR #69673)
Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= , Jan =?utf-8?q?Kokemüller?= ,Louis Dionne Message-ID: In-Reply-To: https://github.com/ldionne closed https://github.com/llvm/llvm-project/pull/69673 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [libcxxabi] [lldb] [libcxx] [libc] [llvm] [clang-tools-extra] [libc++] Fix the behavior of throwing `operator new` under -fno-exceptions (PR #69498)
https://github.com/ldionne updated https://github.com/llvm/llvm-project/pull/69498 >From 6f89b118ed56ad7a3af1996e19ccd30cc893c51e Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Wed, 14 Jun 2023 17:49:22 -0700 Subject: [PATCH 1/8] [libc++] Fix the behavior of throwing `operator new` under -fno-exceptions In D144319, Clang tried to land a change that would cause some functions that are not supposed to return nullptr to optimize better. As reported in https://reviews.llvm.org/D144319#4203982, libc++ started seeing failures in its CI shortly after this change was landed. As explained in D146379, the reason for these failures is that libc++'s throwing `operator new` can in fact return nullptr when compiled with exceptions disabled. However, this contradicts the Standard, which clearly says that the throwing version of `operator new(size_t)` should never return nullptr. This is actually a long standing issue. I've previously seen a case where LTO would optimize incorrectly based on the assumption that `operator new` doesn't return nullptr, an assumption that was violated in that case because libc++.dylib was compiled with -fno-exceptions. Unfortunately, fixing this is kind of tricky. The Standard has a few requirements for the allocation functions, some of which are impossible to satisfy under -fno-exceptions: 1. `operator new(size_t)` must never return nullptr 2. `operator new(size_t, nothrow_t)` must call the throwing version and return nullptr on failure to allocate 3. We can't throw exceptions when compiled with -fno-exceptions In the case where exceptions are enabled, things work nicely. `new(size_t)` throws and `new(size_t, nothrow_t)` uses a try-catch to return nullptr. However, when compiling the library with -fno-exceptions, we can't throw an exception from `new(size_t)`, and we can't catch anything from `new(size_t, nothrow_t)`. The only thing we can do from `new(size_t)` is actually abort the program, which does not make it possible for `new(size_t, nothrow_t)` to catch something and return nullptr. This patch makes the following changes: 1. When compiled with -fno-exceptions, the throwing version of `operator new` will now abort on failure instead of returning nullptr on failure. This resolves the issue that the compiler could mis-compile based on the assumption that nullptr is never returned. This constitutes an API and ABI breaking change for folks compiling the library with -fno-exceptions (which is not the general public, who merely uses libc++ headers but use a shared library that has already been compiled). This should mostly impact vendors and other folks who compile libc++.dylib themselves. 2. When the library is compiled with -fexceptions, the nothrow version of `operator new` has no change. When the library is compiled with -fno-exceptions, the nothrow version of `operator new` will now check whether the throwing version of `operator new` has been overridden. If it has not been overridden, then it will use an implementation equivalent to that of the throwing `operator new`, except it will return nullptr on failure to allocate (instead of terminating). However, if the throwing `operator new` has been overridden, it is now an error NOT to also override the nothrow `operator new`. Indeed, there is no way for us to implement a valid nothrow `operator new` without knowing the exact implementation of the throwing version. rdar://103958777 Differential Revision: https://reviews.llvm.org/D150610 --- libcxx/docs/ReleaseNotes/18.rst | 23 +++ libcxx/include/CMakeLists.txt | 1 + libcxx/include/__overridable_function | 119 + libcxx/src/new.cpp| 99 --- ...new_not_overridden_fno_exceptions.pass.cpp | 59 ++ .../new_dont_return_nullptr.pass.cpp | 37 libcxx/test/support/check_assertion.h | 6 + libcxx/utils/generate_iwyu_mapping.py | 2 + libcxxabi/src/stdlib_new_delete.cpp | 168 +++--- 9 files changed, 426 insertions(+), 88 deletions(-) create mode 100644 libcxx/include/__overridable_function create mode 100644 libcxx/test/libcxx/language.support/support.dynamic/assert.nothrow_new_not_overridden_fno_exceptions.pass.cpp create mode 100644 libcxx/test/libcxx/language.support/support.dynamic/new_dont_return_nullptr.pass.cpp diff --git a/libcxx/docs/ReleaseNotes/18.rst b/libcxx/docs/ReleaseNotes/18.rst index 9e509db6359c4aa..405f1e172893bc1 100644 --- a/libcxx/docs/ReleaseNotes/18.rst +++ b/libcxx/docs/ReleaseNotes/18.rst @@ -142,6 +142,29 @@ LLVM 20 ABI Affecting Changes - +- When the shared/static library is built with ``-fno-exceptions``, the behavior of ``operator new`` was changed + to make it standards-conforming. In LLVM 17 and before, the throwing versions of ``operator new`` would return + ``nullptr`` upon failure to allocate, when the sh
[Lldb-commits] [clang-tools-extra] [libc] [lldb] [compiler-rt] [flang] [libcxxabi] [llvm] [lld] [libcxx] [mlir] [clang] [libc++] Fix the behavior of throwing `operator new` under -fno-exceptions (PR #
https://github.com/ldionne updated https://github.com/llvm/llvm-project/pull/69498 >From 6f89b118ed56ad7a3af1996e19ccd30cc893c51e Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Wed, 14 Jun 2023 17:49:22 -0700 Subject: [PATCH 01/11] [libc++] Fix the behavior of throwing `operator new` under -fno-exceptions In D144319, Clang tried to land a change that would cause some functions that are not supposed to return nullptr to optimize better. As reported in https://reviews.llvm.org/D144319#4203982, libc++ started seeing failures in its CI shortly after this change was landed. As explained in D146379, the reason for these failures is that libc++'s throwing `operator new` can in fact return nullptr when compiled with exceptions disabled. However, this contradicts the Standard, which clearly says that the throwing version of `operator new(size_t)` should never return nullptr. This is actually a long standing issue. I've previously seen a case where LTO would optimize incorrectly based on the assumption that `operator new` doesn't return nullptr, an assumption that was violated in that case because libc++.dylib was compiled with -fno-exceptions. Unfortunately, fixing this is kind of tricky. The Standard has a few requirements for the allocation functions, some of which are impossible to satisfy under -fno-exceptions: 1. `operator new(size_t)` must never return nullptr 2. `operator new(size_t, nothrow_t)` must call the throwing version and return nullptr on failure to allocate 3. We can't throw exceptions when compiled with -fno-exceptions In the case where exceptions are enabled, things work nicely. `new(size_t)` throws and `new(size_t, nothrow_t)` uses a try-catch to return nullptr. However, when compiling the library with -fno-exceptions, we can't throw an exception from `new(size_t)`, and we can't catch anything from `new(size_t, nothrow_t)`. The only thing we can do from `new(size_t)` is actually abort the program, which does not make it possible for `new(size_t, nothrow_t)` to catch something and return nullptr. This patch makes the following changes: 1. When compiled with -fno-exceptions, the throwing version of `operator new` will now abort on failure instead of returning nullptr on failure. This resolves the issue that the compiler could mis-compile based on the assumption that nullptr is never returned. This constitutes an API and ABI breaking change for folks compiling the library with -fno-exceptions (which is not the general public, who merely uses libc++ headers but use a shared library that has already been compiled). This should mostly impact vendors and other folks who compile libc++.dylib themselves. 2. When the library is compiled with -fexceptions, the nothrow version of `operator new` has no change. When the library is compiled with -fno-exceptions, the nothrow version of `operator new` will now check whether the throwing version of `operator new` has been overridden. If it has not been overridden, then it will use an implementation equivalent to that of the throwing `operator new`, except it will return nullptr on failure to allocate (instead of terminating). However, if the throwing `operator new` has been overridden, it is now an error NOT to also override the nothrow `operator new`. Indeed, there is no way for us to implement a valid nothrow `operator new` without knowing the exact implementation of the throwing version. rdar://103958777 Differential Revision: https://reviews.llvm.org/D150610 --- libcxx/docs/ReleaseNotes/18.rst | 23 +++ libcxx/include/CMakeLists.txt | 1 + libcxx/include/__overridable_function | 119 + libcxx/src/new.cpp| 99 --- ...new_not_overridden_fno_exceptions.pass.cpp | 59 ++ .../new_dont_return_nullptr.pass.cpp | 37 libcxx/test/support/check_assertion.h | 6 + libcxx/utils/generate_iwyu_mapping.py | 2 + libcxxabi/src/stdlib_new_delete.cpp | 168 +++--- 9 files changed, 426 insertions(+), 88 deletions(-) create mode 100644 libcxx/include/__overridable_function create mode 100644 libcxx/test/libcxx/language.support/support.dynamic/assert.nothrow_new_not_overridden_fno_exceptions.pass.cpp create mode 100644 libcxx/test/libcxx/language.support/support.dynamic/new_dont_return_nullptr.pass.cpp diff --git a/libcxx/docs/ReleaseNotes/18.rst b/libcxx/docs/ReleaseNotes/18.rst index 9e509db6359c4aa..405f1e172893bc1 100644 --- a/libcxx/docs/ReleaseNotes/18.rst +++ b/libcxx/docs/ReleaseNotes/18.rst @@ -142,6 +142,29 @@ LLVM 20 ABI Affecting Changes - +- When the shared/static library is built with ``-fno-exceptions``, the behavior of ``operator new`` was changed + to make it standards-conforming. In LLVM 17 and before, the throwing versions of ``operator new`` would return + ``nullptr`` upon failure to allocate, when the
[Lldb-commits] [clang-tools-extra] [lldb] [openmp] [compiler-rt] [libclc] [flang] [mlir] [llvm] [libc] [libcxx] [lld] [clang] [libc++][memory] P1132R8: `out_ptr` - a scalable output pointer abstractio
https://github.com/ldionne milestoned https://github.com/llvm/llvm-project/pull/73618 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [libc++] Use -nostdlib++ on GCC unconditionally (PR #68832)
https://github.com/ldionne updated https://github.com/llvm/llvm-project/pull/68832 >From 9ca88d1a30ec1bcaa9c3c943c32649c36ad136d0 Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Wed, 11 Oct 2023 12:21:39 -0700 Subject: [PATCH 1/3] [libc++] Use -nostdlib++ on GCC unconditionally We support GCC 13, which supports the flag. This allows simplifying the CMake logic around the use of -nostdlib++. Note that there are other places where we don't assume -nostdlib++ yet in the build, but this patch is intentionally trying to be small because this part of our CMake is pretty tricky. --- libcxx/CMakeLists.txt| 33 - libcxx/cmake/config-ix.cmake | 15 --- 2 files changed, 4 insertions(+), 44 deletions(-) diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt index 16540caf68eaf04..55ad0f49cbdb6fd 100644 --- a/libcxx/CMakeLists.txt +++ b/libcxx/CMakeLists.txt @@ -642,18 +642,11 @@ get_sanitizer_flags(SANITIZER_FLAGS "${LLVM_USE_SANITIZER}") # Link system libraries === function(cxx_link_system_libraries target) - -# In order to remove just libc++ from the link step -# we need to use -nostdlib++ whenever it is supported. -# Unfortunately this cannot be used universally because for example g++ supports -# only -nodefaultlibs in which case all libraries will be removed and -# all libraries but c++ have to be added in manually. - if (CXX_SUPPORTS_NOSTDLIBXX_FLAG) -target_add_link_flags_if_supported(${target} PRIVATE "-nostdlib++") + if (MSVC) +target_add_compile_flags(${target} PRIVATE "/Zl") +target_link_libraries(${target} PRIVATE "/nodefaultlib") else() -target_add_link_flags_if_supported(${target} PRIVATE "-nodefaultlibs") -target_add_compile_flags_if_supported(${target} PRIVATE "/Zl") -target_add_link_flags_if_supported(${target} PRIVATE "/nodefaultlib") +target_link_libraries(${target} PRIVATE "-nostdlib++") endif() if (CXX_SUPPORTS_UNWINDLIB_EQ_NONE_FLAG AND LIBCXXABI_USE_LLVM_UNWINDER) @@ -663,24 +656,6 @@ function(cxx_link_system_libraries target) target_add_link_flags_if_supported(${target} PRIVATE "--unwindlib=none") endif() - if (NOT APPLE) # On Apple platforms, we always use -nostdlib++ so we don't need to re-add other libraries -if (LIBCXX_HAS_PTHREAD_LIB) - target_link_libraries(${target} PRIVATE pthread) -endif() - -if (LIBCXX_HAS_C_LIB) - target_link_libraries(${target} PRIVATE c) -endif() - -if (LIBCXX_HAS_M_LIB) - target_link_libraries(${target} PRIVATE m) -endif() - -if (LIBCXX_HAS_RT_LIB) - target_link_libraries(${target} PRIVATE rt) -endif() - endif() - if (LIBCXX_USE_COMPILER_RT) find_compiler_rt_library(builtins LIBCXX_BUILTINS_LIBRARY) if (LIBCXX_BUILTINS_LIBRARY) diff --git a/libcxx/cmake/config-ix.cmake b/libcxx/cmake/config-ix.cmake index 9962d848d85e846..9fed861a4e193c5 100644 --- a/libcxx/cmake/config-ix.cmake +++ b/libcxx/cmake/config-ix.cmake @@ -14,14 +14,6 @@ include(CheckCSourceCompiles) # link with --uwnindlib=none. Check if that option works. llvm_check_compiler_linker_flag(C "--unwindlib=none" CXX_SUPPORTS_UNWINDLIB_EQ_NONE_FLAG) -if(WIN32 AND NOT MINGW) - # NOTE(compnerd) this is technically a lie, there is msvcrt, but for now, lets - # let the default linking take care of that. - set(LIBCXX_HAS_C_LIB NO) -else() - check_library_exists(c fopen "" LIBCXX_HAS_C_LIB) -endif() - if (NOT LIBCXX_USE_COMPILER_RT) if(WIN32 AND NOT MINGW) set(LIBCXX_HAS_GCC_S_LIB NO) @@ -54,9 +46,6 @@ else() endif() if (CXX_SUPPORTS_NOSTDLIBXX_FLAG OR C_SUPPORTS_NODEFAULTLIBS_FLAG) - if (LIBCXX_HAS_C_LIB) -list(APPEND CMAKE_REQUIRED_LIBRARIES c) - endif () if (LIBCXX_USE_COMPILER_RT) include(HandleCompilerRT) find_compiler_rt_library(builtins LIBCXX_BUILTINS_LIBRARY @@ -108,22 +97,18 @@ if(WIN32 AND NOT MINGW) # TODO(compnerd) do we want to support an emulation layer that allows for the # use of pthread-win32 or similar libraries to emulate pthreads on Windows? set(LIBCXX_HAS_PTHREAD_LIB NO) - set(LIBCXX_HAS_M_LIB NO) set(LIBCXX_HAS_RT_LIB NO) set(LIBCXX_HAS_ATOMIC_LIB NO) elseif(APPLE) set(LIBCXX_HAS_PTHREAD_LIB NO) - set(LIBCXX_HAS_M_LIB NO) set(LIBCXX_HAS_RT_LIB NO) set(LIBCXX_HAS_ATOMIC_LIB NO) elseif(FUCHSIA) - set(LIBCXX_HAS_M_LIB NO) set(LIBCXX_HAS_PTHREAD_LIB NO) set(LIBCXX_HAS_RT_LIB NO) check_library_exists(atomic __atomic_fetch_add_8 "" LIBCXX_HAS_ATOMIC_LIB) else() check_library_exists(pthread pthread_create "" LIBCXX_HAS_PTHREAD_LIB) - check_library_exists(m ccos "" LIBCXX_HAS_M_LIB) check_library_exists(rt clock_gettime "" LIBCXX_HAS_RT_LIB) check_library_exists(atomic __atomic_fetch_add_8 "" LIBCXX_HAS_ATOMIC_LIB) endif() >From db65842e8ef437152a59cb72c9b690dd85bc10df Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Thu, 12 Oct 2023 16:20:32 -0700 Subject: [PATCH 2/3
[Lldb-commits] [lldb] [libc++] Use -nostdlib++ on GCC unconditionally (PR #68832)
@@ -642,18 +642,8 @@ get_sanitizer_flags(SANITIZER_FLAGS "${LLVM_USE_SANITIZER}") # Link system libraries === function(cxx_link_system_libraries target) - -# In order to remove just libc++ from the link step -# we need to use -nostdlib++ whenever it is supported. -# Unfortunately this cannot be used universally because for example g++ supports -# only -nodefaultlibs in which case all libraries will be removed and -# all libraries but c++ have to be added in manually. - if (CXX_SUPPORTS_NOSTDLIBXX_FLAG) -target_add_link_flags_if_supported(${target} PRIVATE "-nostdlib++") - else() -target_add_link_flags_if_supported(${target} PRIVATE "-nodefaultlibs") -target_add_compile_flags_if_supported(${target} PRIVATE "/Zl") -target_add_link_flags_if_supported(${target} PRIVATE "/nodefaultlib") ldionne wrote: @mstorsjo FYI, I checked a recent Clang-CL (DLL) job like https://buildkite.com/llvm-project/libcxx-ci/builds/30715#018b257a-9457-4020-a4d0-3d22d4704e25 and it seems like `/nodefaultlib` and `/Zl` are never detected as supported on Windows. Hence, we were never actually enabling those flags. I'm not certain how the Windows build works without that but it seems like it does. https://github.com/llvm/llvm-project/pull/68832 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [libc++] Move the check-generated-files job to Github Actions (PR #68920)
https://github.com/ldionne updated https://github.com/llvm/llvm-project/pull/68920 >From d1c371b9783777d90647b5d93b16266fe053287f Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Thu, 12 Oct 2023 10:29:40 -0700 Subject: [PATCH 1/4] [libc++] Move the check-generated-files job to Github Actions This allows running these quick checks faster than in our Buildkite pipeline, which has much more latency. This will also avoid blocking the rest of the testing pipeline in case the generated-files checks are failing. --- .../libcxx-check-generated-files.yml | 25 +++ libcxx/utils/ci/buildkite-pipeline.yml| 19 -- 2 files changed, 25 insertions(+), 19 deletions(-) create mode 100644 .github/workflows/libcxx-check-generated-files.yml diff --git a/.github/workflows/libcxx-check-generated-files.yml b/.github/workflows/libcxx-check-generated-files.yml new file mode 100644 index 000..1e81d06af266db2 --- /dev/null +++ b/.github/workflows/libcxx-check-generated-files.yml @@ -0,0 +1,25 @@ +name: "Check libc++ generated files" +on: + pull_request: +paths: + - 'libcxx/**' + +jobs: + check_generated_files: +runs-on: ubuntu-latest +steps: + - name: Fetch LLVM sources +uses: actions/checkout@v4 +with: + fetch-depth: 2 + + - name: Install clang-format +uses: aminya/setup-cpp@v1 +with: + clangformat: 17.0.1 + + - name: Install Ninja +uses: llvm/actions/install-ninja + + - name: Check generated files +run: libcxx/utils/ci/run-buildbot check-generated-output diff --git a/libcxx/utils/ci/buildkite-pipeline.yml b/libcxx/utils/ci/buildkite-pipeline.yml index 7a125d16af59493..460c5a8c4301dc4 100644 --- a/libcxx/utils/ci/buildkite-pipeline.yml +++ b/libcxx/utils/ci/buildkite-pipeline.yml @@ -26,31 +26,12 @@ env: # LLVM POST-BRANCH bump version # LLVM POST-BRANCH add compiler test for ToT - 1, e.g. "Clang 17" # LLVM RELEASE bump remove compiler ToT - 3, e.g. "Clang 15" -LLVM_STABLE_VERSION: "17" # Used for tooling, update after the RELEASE. LLVM_HEAD_VERSION: "18" # Used compiler, update POST-BRANCH. GCC_STABLE_VERSION: "13" steps: # # Light pre-commit tests for things like forgetting to update generated files. # - - label: "Generated output" -command: "libcxx/utils/ci/run-buildbot check-generated-output" -artifact_paths: - - "**/generated_output.patch" - - "**/generated_output.status" -env: -CC: "clang-${LLVM_HEAD_VERSION}" -CXX: "clang++-${LLVM_HEAD_VERSION}" -CLANG_FORMAT: "/usr/bin/clang-format-${LLVM_STABLE_VERSION}" -agents: - queue: "libcxx-builders" - os: "linux" -retry: - automatic: -- exit_status: -1 # Agent was lost - limit: 2 -timeout_in_minutes: 120 - - label: "Documentation" command: "libcxx/utils/ci/run-buildbot documentation" artifact_paths: >From 19a6b12473c8214622aaef2f765ebd6247c5d65c Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Thu, 12 Oct 2023 15:14:45 -0700 Subject: [PATCH 2/4] Use apt-get to install Ninja because using the LLVM action seems not to work --- .github/workflows/libcxx-check-generated-files.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/libcxx-check-generated-files.yml b/.github/workflows/libcxx-check-generated-files.yml index 1e81d06af266db2..26b43b5774575d9 100644 --- a/.github/workflows/libcxx-check-generated-files.yml +++ b/.github/workflows/libcxx-check-generated-files.yml @@ -19,7 +19,7 @@ jobs: clangformat: 17.0.1 - name: Install Ninja -uses: llvm/actions/install-ninja +run: sudo apt-get install -y ninja-build - name: Check generated files run: libcxx/utils/ci/run-buildbot check-generated-output >From 1cc21583a9a299a546b16c762d06fb67428f5c6c Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Fri, 13 Oct 2023 07:47:33 -0700 Subject: [PATCH 3/4] Install Ninja using aminya/setup-cpp@v1 --- .github/workflows/libcxx-check-generated-files.yml | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/.github/workflows/libcxx-check-generated-files.yml b/.github/workflows/libcxx-check-generated-files.yml index 26b43b5774575d9..65b331e0bd51cd6 100644 --- a/.github/workflows/libcxx-check-generated-files.yml +++ b/.github/workflows/libcxx-check-generated-files.yml @@ -13,13 +13,11 @@ jobs: with: fetch-depth: 2 - - name: Install clang-format + - name: Install dependencies uses: aminya/setup-cpp@v1 with: clangformat: 17.0.1 - - - name: Install Ninja -run: sudo apt-get install -y ninja-build + ninja: true - name: Check generated files run: libcxx/utils/ci/run-buildbot check-generated-output >From 0ff052b7827f41f77d3174b115fce020fa9c35cf Mon Sep 17 00:00:00 2001 From: Louis
[Lldb-commits] [lldb] [libc++] Use -nostdlib++ on GCC unconditionally (PR #68832)
@@ -642,18 +642,8 @@ get_sanitizer_flags(SANITIZER_FLAGS "${LLVM_USE_SANITIZER}") # Link system libraries === function(cxx_link_system_libraries target) - -# In order to remove just libc++ from the link step -# we need to use -nostdlib++ whenever it is supported. -# Unfortunately this cannot be used universally because for example g++ supports -# only -nodefaultlibs in which case all libraries will be removed and -# all libraries but c++ have to be added in manually. - if (CXX_SUPPORTS_NOSTDLIBXX_FLAG) -target_add_link_flags_if_supported(${target} PRIVATE "-nostdlib++") - else() -target_add_link_flags_if_supported(${target} PRIVATE "-nodefaultlibs") -target_add_compile_flags_if_supported(${target} PRIVATE "/Zl") -target_add_link_flags_if_supported(${target} PRIVATE "/nodefaultlib") ldionne wrote: So it's OK to remove that `/nodefaultlib` and `/Zl` or should we look into building with that again? (Preferably in a separate patch since this is a pre-existing condition) https://github.com/llvm/llvm-project/pull/68832 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [libc++] Use -nostdlib++ on GCC unconditionally (PR #68832)
https://github.com/ldionne closed https://github.com/llvm/llvm-project/pull/68832 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [libc++] Move the check-generated-files job to Github Actions (PR #68920)
https://github.com/ldionne closed https://github.com/llvm/llvm-project/pull/68920 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [compiler-rt] [clang] [lldb] [llvm] [libcxx] [lld] [mlir] [clang-tools-extra] [libc++][ranges] Implement ranges::contains_subrange (PR #66963)
ldionne wrote: Gentle ping. There's outstanding feedback to address on this review. @ZijunZhaoCCK if you don't think you'll have time to pursue this PR anymore, it's all good but please let us know so we can assign it to someone else! https://github.com/llvm/llvm-project/pull/66963 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][CMake] Add single target that runs libc++ tests (PR #110856)
@@ -267,6 +267,21 @@ add_lit_testsuite(check-lldb "Running lldb lit test suite" lldb-shell-test-deps lldb-unit-test-deps) +# This target covers all targets that are tied to implementation details +# of libc++, intended to be run by the libc++ pre-merge CI. +add_lit_testsuite(check-lldb-libcxx-integration "Running lldb libc++ support test suite" ldionne wrote: As part of this patch, you could also change https://github.com/llvm/llvm-project/blob/main/libcxx/utils/ci/run-buildbot#L374 to call this target, that way everything is already plumbed together! https://github.com/llvm/llvm-project/pull/110856 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [libcxx] [lldb] [lldb][CMake] Add single target that runs libc++ tests (PR #110856)
https://github.com/ldionne approved this pull request. Libc++ changes LGTM. Thank you! https://github.com/llvm/llvm-project/pull/110856 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap][test] Set disableASLR to False for tests (PR #113593)
https://github.com/ldionne approved this pull request. Thank you for the prompt fix! https://github.com/llvm/llvm-project/pull/113593 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [libcxx] [lldb] [lldb][libc++] Hide all libc++ implementation details from stacktraces (PR #108870)
ldionne wrote: > Do you have an idea how we could de-risk this change? E.g., would do you have > a couple more scenarios in mind for which I should add test cases? Are there > similar patterns to the `operator()` which I should also cover in test cases? What I could imagine is to drive this via attributes instead of a heuristic. We could potentially mark implementation details of libc++ as such to control what the debugging experience is like at a much finer grain if we used attributes, but obviously this would also increase complexity in the code and it would be yet another thing that we have to slap on almost every declaration. > Do we have some libc++ developers which are using lldb top-of-tree for their > day-to-day work on libc++ and who would make us aware in case this commit > turns out to be too aggressive? I'm not aware of anyone. I just use `lldb` provided by the platform usually. https://github.com/llvm/llvm-project/pull/108870 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [libcxx] [lldb] [lldb][CMake] Add single target that runs libc++ tests (PR #110856)
ldionne wrote: Just for my understanding, is `--category` a Lit argument? I have never seen this before and I can't find documentation about tit. https://github.com/llvm/llvm-project/pull/110856 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [compiler-rt] [libcxx] [lld] [lldb] [llvm] [mlir] [polly] python: use raw strings for regex (PR #105990)
ldionne wrote: Closing as stale, please feel free to reopen if you want to pursue this. https://github.com/llvm/llvm-project/pull/105990 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [compiler-rt] [libcxx] [lld] [lldb] [llvm] [mlir] [polly] python: use raw strings for regex (PR #105990)
https://github.com/ldionne closed https://github.com/llvm/llvm-project/pull/105990 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [libcxx] [libcxxabi] [lldb] [libc++] Stop copying headers to the build directory (PR #115380)
https://github.com/ldionne edited https://github.com/llvm/llvm-project/pull/115380 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [libcxx] [libcxxabi] [lldb] [libc++] Stop copying headers to the build directory (PR #115380)
ldionne wrote: Apologies for the spam everyone, I rebased this onto `main` and didn't realize the merge base was set to something else, causing this. https://github.com/llvm/llvm-project/pull/115380 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [libcxx] [libcxxabi] [lldb] [libc++] Stop copying headers to the build directory (PR #115380)
https://github.com/ldionne closed https://github.com/llvm/llvm-project/pull/115380 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [libcxx] [libcxxabi] [lldb] [libc++] Stop copying headers to the build directory (PR #115380)
ldionne wrote: A bunch of CI jobs were triggered because of my faulty rebase, but the necessary jobs seems to have passed. What's still pending are some release jobs which normally don't run on PRs, and the documentation job which fails for an unrelated reason (`clang-tools-extra`). Merging. https://github.com/llvm/llvm-project/pull/115380 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [libcxx] [libcxxabi] [lldb] [libc++] Stop copying headers to the build directory (PR #115380)
ldionne wrote: LLDB, please ping us if you encounter problems after we merge this patch. https://github.com/llvm/llvm-project/pull/115380 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [libcxx] [libcxxabi] [lldb] [libc++] Stop copying headers to the build directory (PR #115380)
ldionne wrote: @kstoimenov @vitalybuka Could you folks show us how the sanitizer builds you run are different from the ones we already run in our CI pipeline? It would be important to align both since we often run into issues with your post-commit CI jobs failing but our pre-commit CI not catching it. https://github.com/llvm/llvm-project/pull/115380 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [libcxx] [libcxxabi] [lldb] [libc++] Stop copying headers to the build directory (PR #115380)
ldionne wrote: IIUC, the build that encountered issues in this patch ([in the comment above](https://github.com/llvm/llvm-project/pull/115380#issuecomment-2590801533)) is a flavour of (2), right? That seems surprising to me though since we also use the same flags when configuring libc++: https://github.com/llvm/llvm-project/blob/main/libcxx/CMakeLists.txt#L596 So the difference would be that you folks are building libc++ instrumented via the bootstrapping build instead of the "runtimes" build that we use for the rest of libc++ CI. I also fail to understand why our own bootstrapping build pre-commit CI didn't trip this wire. https://github.com/llvm/llvm-project/pull/115380 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [compiler-rt] [libcxx] [libcxxabi] [libunwind] [lldb] [llvm] [compiler-rt] Disable LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON on AIX. (PR #131200)
@@ -223,6 +223,13 @@ endif() # This can be used to detect whether we're in the runtimes build. set(LLVM_RUNTIMES_BUILD ON) +if (LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "AIX") + # Set LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF as AIX doesn't support it + message(WARNING + "LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON is not supported on AIX. LLVM_ENABLE_PER_TARGET_RUNTIME_DIR is set to OFF.") + set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR OFF CACHE BOOL "" FORCE) +endif() ldionne wrote: What we do for Apple is also bad, and we should fix that. In fact we want to make it work on Apple platforms but we haven't gotten to it yet. The reason for my push back is that we don't want to have configuration logic, especially when it's imperative, in the CMake files. We're working really hard to get away from that and that's why I care so strongly about it. https://github.com/llvm/llvm-project/pull/131200 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [compiler-rt] [libcxx] [libcxxabi] [libunwind] [lldb] [llvm] [compiler-rt] Disable LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON on AIX. (PR #131200)
@@ -223,6 +223,13 @@ endif() # This can be used to detect whether we're in the runtimes build. set(LLVM_RUNTIMES_BUILD ON) +if (LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "AIX") + # Set LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF as AIX doesn't support it + message(WARNING + "LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON is not supported on AIX. LLVM_ENABLE_PER_TARGET_RUNTIME_DIR is set to OFF.") + set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR OFF CACHE BOOL "" FORCE) +endif() ldionne wrote: Right, but the mistake in that case is at the level of the CMake invocation. Whoever configured their CMake invocation passed `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON` mistakenly, right? We should tell the user that their invocation is wrong with a `FATAL_ERROR`, not ignore their setting and continue. https://github.com/llvm/llvm-project/pull/131200 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits