llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.33k stars 12.13k forks source link

[flang][libc++][regression] Flang runtime cannot be built after merging PR #109151 #113059

Open pawosm-arm opened 1 month ago

pawosm-arm commented 1 month ago

The following change is needed to make the flang runtime build again after #109151 has been merged:

diff --git a/flang/runtime/io-api-minimal.cpp b/flang/runtime/io-api-minimal.cpp
index ad76fe3de032..a06d3d12f8f0 100644
--- a/flang/runtime/io-api-minimal.cpp
+++ b/flang/runtime/io-api-minimal.cpp
@@ -150,7 +150,7 @@ bool IODEF(OutputLogical)(Cookie cookie, bool truth) {
 // Provide own definition for `std::__libcpp_verbose_abort` to avoid dependency
 // on the version provided by libc++.

-void std::__libcpp_verbose_abort(char const *format, ...) {
+void std::__libcpp_verbose_abort(char const *format, ...) noexcept {
   va_list list;
   va_start(list, format);
   std::vfprintf(stderr, format, list);
mstorsjo commented 1 month ago

The following change is needed to make the flang runtime build again after #109151 has been merged:

diff --git a/flang/runtime/io-api-minimal.cpp b/flang/runtime/io-api-minimal.cpp
index ad76fe3de032..a06d3d12f8f0 100644
--- a/flang/runtime/io-api-minimal.cpp
+++ b/flang/runtime/io-api-minimal.cpp
@@ -150,7 +150,7 @@ bool IODEF(OutputLogical)(Cookie cookie, bool truth) {
 // Provide own definition for `std::__libcpp_verbose_abort` to avoid dependency
 // on the version provided by libc++.

-void std::__libcpp_verbose_abort(char const *format, ...) {
+void std::__libcpp_verbose_abort(char const *format, ...) noexcept {
   va_list list;
   va_start(list, format);
   std::vfprintf(stderr, format, list);

To make the solution work with both older and newer libc++ versions, can we provide the function in two overloaded forms - both with and without noexcept?

ldionne commented 1 month ago

Moving the discussion from https://github.com/llvm/llvm-project/pull/109151#issuecomment-2427534878

In fact, we want to move away from supporting overriding that function. The way of customizing the assertion handler is when configuring the library, as explained here: https://libcxx.llvm.org/Hardening.html#overriding-the-assertion-failure-handler

@ldionne - The issue here from the Flang perspective, is not that flang want to override the function, they don't really care. The thing is that Flang uses some classes from the C++ standard library (from std::variant iirc) but don't want to link against the C++ standard library. When using the libc++ implementation of std::variant, they end up with undefined references to std::__libcpp_verbose_abort unless they actually override/provide it themselves.

Understood. But why don't they want to link against the standard library? Using the libc++ headers without linking against libc++ is not supported, and it fails in various ways. For example, we externally instantiate various templates like member functions of std::string or some specializations of std::sort, and you'll get linker errors today if you try to use these functions without linking against libc++.

mstorsjo commented 1 month ago

Understood. But why don't they want to link against the standard library? Using the libc++ headers without linking against libc++ is not supported, and it fails in various ways. For example, we externally instantiate various templates like member functions of std::string or some specializations of std::sort, and you'll get linker errors today if you try to use these functions without linking against libc++.

Indeed, that's understandably not supported. See https://reviews.llvm.org/D158957 for earlier discussion on this - as far as I understand, the flang runtime wants to take their chances and use some subsets of the C++ standard library as a convenience, expecting to use only some, header-only classes, while not linking against it.

That's of course nothing that libc++ can support, but explains the current situation.

ldionne commented 1 month ago

CC @mmuetzel

Is there a reason why you try to control how you compile and link against the C++ standard library? If you just use whatever comes by default with your compiler (i.e. you drop -nostdinc++ and -nostdlib++), you'll end up using the right headers and linking against the right C++ standard library. Is there a reason why Flang is making its life more complicated by dropping the way the compiler handles the C++ stdlib by default? I think that's the root cause leading to all this complexity.

mmuetzel commented 1 month ago

Is there a reason why you try to control how you compile and link against the C++ standard library?

If I understand correctly, that is something that the flang driver does. Iirc, I was told that that must not change and any dependency of the flang runtime on the C++ runtime must be avoided. I can't give the reasoning for that though and I'm just echoing what I seem to recall.

mmuetzel commented 1 month ago

Is there a reason why you try to control how you compile and link against the C++ standard library? If you just use whatever comes by default with your compiler (i.e. you drop -nostdinc++ and -nostdlib++), you'll end up using the right headers and linking against the right C++ standard library. Is there a reason why Flang is making its life more complicated by dropping the way the compiler handles the C++ stdlib by default? I think that's the root cause leading to all this complexity.

Maybe, @klausler can give feedback on how the flang runtime is supposed to use the C++ runtime and the motivation for that?

pawosm-arm commented 2 weeks ago

It seems to build now, after #113310 yet it seems it's a temporary measure and this still needs to be solved before LLVM21