llvm / llvm-project

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

[libc++] modules: missing "initializer for module std" symbol in `libc++.so` #80639

Open jiixyj opened 5 months ago

jiixyj commented 5 months ago

I'm testing the new support for installed modules by created an "imported target" in CMake that points to libc++.so and the installed importable module units (std.cppm) with the Findcxx.cmake script listed below.

The reason I want to use an imported target is: I don't want every consumer of the std module to have to build its own std CMake target with add_library(std). This won't scale I think. Of course, CMake will probably figure out a way to make using the std module easier for consumers, but I guess they will stumble upon this issue as well.

When I link against that imported target, I'm running into the issue that the "initializer for module std" symbol is missing from libc++.so, resulting in linker errors:

ld.lld: error: undefined symbol: initializer for module std

This means I have to hack in this symbol in some way, for example by adding this file to my build:

extern "C" void _ZGIW3std()
{
}

Could libc++.so provide _ZGIW3std, or are there reasons against it?

Findcxx.cmake:

if(TARGET cxx::std)
  return()
endif()

set(PATH_TO_LIBCXX "...")

add_library(cxx::std STATIC IMPORTED)

set_target_properties(
  cxx::std
  PROPERTIES CXX_EXTENSIONS "OFF"
             IMPORTED_CXX_MODULES_COMPILE_FEATURES "cxx_std_23"
             INTERFACE_COMPILE_FEATURES "cxx_std_23"
             IMPORTED_LINK_INTERFACE_LANGUAGES "CXX"
             IMPORTED_LOCATION
             "${PATH_TO_LIBCXX}/lib/x86_64-unknown-linux-gnu/libc++.so")

target_sources(
  cxx::std INTERFACE FILE_SET CXX_MODULES BASE_DIRS "${PATH_TO_LIBCXX}" FILES
                     "${PATH_TO_LIBCXX}/share/libc++/v1/std.cppm")
set_property(
  SOURCE "${PATH_TO_LIBCXX}/share/libc++/v1/std.cppm"
  APPEND
  PROPERTY COMPILE_OPTIONS
           "-Wno-reserved-module-identifier;-Wno-reserved-user-defined-literal")

# This is needed to provide the "initializer for module std" symbol:
#
#     extern "C" void _ZGIW3std()
#     {
#     }
#
target_sources(cxx::std INTERFACE "${CMAKE_CURRENT_LIST_DIR}/std.cpp")

Eventually, I want to soup up this find script by parsing the new libc++.modules.json, but this is what I got so far.

ChuanqiXu9 commented 5 months ago

The "initializer for module std" symbol should live in the object file corresponding to std.cppm. So from the perspective of the compiler, it should be fine to link that object file.

jiixyj commented 5 months ago

Yes, and that object file should live in libc++.{a,so}, I think.

One way of doing it would be to compile std.cppm and std.compat.cppm as part of libc++'s build:

diff --git a/libcxx/src/CMakeLists.txt b/libcxx/src/CMakeLists.txt
index 44a088663463..9709553bcfd3 100644
--- a/libcxx/src/CMakeLists.txt
+++ b/libcxx/src/CMakeLists.txt
@@ -196,6 +196,12 @@ split_list(LIBCXX_LINK_FLAGS)
 # Build the shared library.
 if (LIBCXX_ENABLE_SHARED)
   add_library(cxx_shared SHARED ${exclude_from_all} ${LIBCXX_SOURCES} ${LIBCXX_HEADERS})
+  if (LIBCXX_INSTALL_MODULES)
+    target_sources(cxx_shared PUBLIC FILE_SET CXX_MODULES
+      BASE_DIRS "${LIBCXX_GENERATED_MODULE_DIR}"
+      FILES "${LIBCXX_GENERATED_MODULE_DIR}/std.cppm"
+            "${LIBCXX_GENERATED_MODULE_DIR}/std.compat.cppm")
+  endif ()
   target_include_directories(cxx_shared PRIVATE ${CMAKE_CURRENT_SOURCE_DIR})
   target_link_libraries(cxx_shared PUBLIC cxx-headers
                                    PRIVATE ${LIBCXX_LIBRARIES})

Then, the initializer symbols (which are actually the only symbols generated from building the .cppm files) would pop up in libc++.so:

$ diff old-syms.txt new-syms.txt
508a509,510
> _ZGIW3std
> _ZGIW3stdW6compat

My mental model so far is that .cppm files are compiled into .o files and BMI files. The .o files (including the initializer symbols) should live in the artifacts provided by the library, and the BMI's are thrown away. Instead, the .cppm files are delivered to the user, who compiles them, but in turn throws away the .o files (or provides flags to the compiler so those aren't even generated), leaving just the BMI's. I hope this made sense...

philnik777 commented 5 months ago

@jiixyj The module symbols can't live in the dylib, since the module symbols change depending on how you build the module (e.g. C++ version, exceptions, hardening mode). Especially once we put in library symbols and not just compiler-generated symbols.

jiixyj commented 5 months ago

If I build std.cppm/std.compat.cppm with different compilers, language standards, sanitizers etc., the BMIs change of course, but the symbols (right now just _ZGIW3std and _ZGIW3stdW6compat) should be ABI compatible, I thought. Are you saying that this might change in the future?

philnik777 commented 5 months ago

Yes, that will almost certainly change. I expect that we'll make use of the additional flexibility of having an archive that is compiled by the user in the future, since it would allow us to not commit to any ABI while still providing better QoI by providing some external class instantiations and functions.

jiixyj commented 5 months ago

Ah, I see! I hadn't considered this use case. I always assumed from reading papers such as this and following SG15 discussions that the metadata file (i.e. .modules.json) is placed alongside the library artifact that contains all symbols needed to link against it, in particular the initializer symbol. That way, the users would have to only build an "interface" BMI but not the .o file(s). This is also the reason why there always exists a library artifact to put the metadata file next to: there is always at least the initializer symbol.

Would having another, ABI unstable, library file such as libc++modules.a (and moving the metadata file to this library, i.e. libc++modules.modules.json) be acceptable? Or reusing libc++experimental.a for this, resulting in libc++experimental.modules.json?

ChuanqiXu9 commented 5 months ago

On the one hand, as far as I know, there is no plan to change the ABI interfaces (renaming _ZGIW3std or _ZGIW3stdW6compat). On the other hand, the std modules feature is not stable enough. It is still marked as experimental now. So it may not be too good to put it into the mature libc++.so now from the perspective of system libraries.

Would having another, ABI unstable, library file such as libc++modules.a (and moving the metadata file to this library, i.e. libc++modules.modules.json) be acceptable? Or reusing libc++experimental.a for this, resulting in libc++experimental.modules.json?

While I think all of the suggestions are technically acceptable, it depends on the maintainers to make the choice. From the users' perspective, this doesn't really matter since the compilation of std modules should be wrapped by the build systems. So the compilation process should be transparent to users in the end of the day.

It is just because CMake haven't supported it and you need to do it yourselves... But I believe this won't be a problem for users later.

jiixyj commented 5 months ago

On the one hand, as far as I know, there is no plan to change the ABI interfaces (renaming _ZGIW3std or _ZGIW3stdW6compat). On the other hand, the std modules feature is not stable enough. It is still marked as experimental now. So it may not be too good to put it into the mature libc++.so now from the perspective of system libraries.

That makes sense to me.

It is just because CMake haven't supported it and you need to do it yourselves... But I believe this won't be a problem for users later.

This is a very good point -- It's still early days, and a lot of this stuff will get easier as time goes on.

However, I think it would pay off in the long term if libc++ would behave just like a "normal" modules enabled C++ library, "owning" all symbols resulting from building the importable module units.

With the current CMake support for modules, requiring the users to build the whole module (including the initializer and possibly other symbols) themselves actually makes a huge difference.

I think at the very least, the libc++.modules.json file would need another key that it actually expects the user to build both BMIs and .o, not just the BMIs.

Below follow more details about how CMake's module support works and why expecting the users to build the whole module is not a good idea in my opinion. But I believe that the general points apply to all "non-monorepo" build systems since it basically boils down to C++'s compilation model.

\ \ \ \ _

One common way of building complex projects right now in the CMake/Conan universe is to have a CMake build system for each library and then exporting (in CMake terms) this library. Then, consumers would pick up that prebuilt library by using imported CMake targets: add_library(foo::foo STATIC IMPORTED). Since CMake 3.28 this also works for targets containing modules:

# foo's build system
add_library(foo SHARED foo.cpp)
target_sources(foo PUBLIC FILE_SET CXX_MODULES foo.cppm)

If you "export" that target, consumers can "import" it, CMake will build the BMIs from the .cppm files (though not the object files!) and everything basically works just like with "normal" C++ libraries. The object files created from foo.cppm live in libfoo.{a,so} and are "owned" by the "foo" library.

Now, if the users actually had to build another libfoo-module.a from foo.cppm, this would break down very quickly. Say we had a library "bar" that requires "foo". We'd have to build libfoo-module.a that contains the objects from foo.cppm like this in "bar"'s build system:

# bar's build system

add_library(foo-module STATIC)
target_sources(foo-module PUBLIC FILE_SET CXX_MODULES foo.cppm)

add_library(bar STATIC bar.cpp)
target_link_libraries(bar PRIVATE foo::foo foo-module)

Then, when trying to only "export" the "bar" library, CMake would rightly complain: You would also have to export the "foo-module" library because "bar" depends on it, and your build system "owns" "foo-module" now. That means that every library that depends on "foo" would have its own "foo-module" library that it'd have to export, leading to chaos.

One workaround would be for the user to first build a "foo-module" library in a separate CMake build system, building the libfoo-module.a from the foo.cppm file, and then "export" that library. Then, all dependent projects would need to depend on "foo-module", additionally to "foo". What this means is that all the "usual" "foo" symbols would be "owned" by the "foo" package, and the "foo" module initializer symbol and any additional symbols from foo.cppm would be "owned" by the "foo-module" package. But I strongly believe that this isn't how projects will produce and consume module enabled C++ libraries in the future. They would simply provide all .o files in their package, including the initializer symbol(s).

ChuanqiXu9 commented 5 months ago

Pretty good thoughts. But I feel this may not be the right place to discuss this.

The reason why it get into the shape today is that when libc++ guys don't have a good idea about how to handle std modules building correctly, cmake says this should be the job of cmake (build systems?). So I feel it may be better to discuss this in cmake community or discuss this in SG15, where is a place for vendors to get consensuses.

mordante commented 5 months ago

+1 for discussing this in SG15. The current design of the modules.json was based on discussions on SG15. Note that this file is aiming at usage in build systems. The goal is that you write import std; and the build system does its magic to provide this module from the recipe in the modules.json. If the current json indeed contains too little information I'll be happy to add more information, but again this should be discussed in SG15.

jiixyj commented 5 months ago

Thank you for the comments. I agree that SG15 is a better place for this discussion. I wrote to the list here: https://lists.isocpp.org/sg15/2024/02/2353.php

ChuanqiXu9 commented 4 months ago

Thank you for the comments. I agree that SG15 is a better place for this discussion. I wrote to the list here: https://lists.isocpp.org/sg15/2024/02/2353.php

According to the discussion in SG15, it looks like the libc++ should provide the initializer in the library (probably in libc++.so). @mordante

mathstuf commented 4 months ago

Note that if there are variants of the symbol, they can all be provided by some refinement on the sketch provided in this message.

probably in libc++.so

If ABI committal is a problem, libc++experimental is fine until that is hammered out.

ChuanqiXu9 commented 4 months ago

I believe currently the std.o may only contain an empty initializer since all the meaningful entities are inline now.

mathstuf commented 4 months ago

Is that something to fix before 18.0.0 (or during the 18 release cycle)?

ChuanqiXu9 commented 4 months ago

Is that something to fix before 18.0.0 (or during the 18 release cycle)?

I think it should be the case already.

mathstuf commented 4 months ago

Sorry, I was referring to providing the initializer in libc++.so, not having it be a no-op function.

ChuanqiXu9 commented 4 months ago

cc: @mordante

I am not optimistic since the first version of clang18 will be released in the next week.

philnik777 commented 4 months ago

Note that if there are variants of the symbol, they can all be provided by some refinement on the sketch provided in this message.

probably in libc++.so

If ABI committal is a problem, libc++experimental is fine until that is hammered out.

I don't think we should ever commit to an ABI for these symbols. That's the whole problem. The only thing it does is paint us into a corner. There is literally no upside to committing to an ABI for this symbol. IMO the only solution is to make it a linkonce_odr symbol that is ABI-tagged.

FWIW I also think that this is really a lost opportunity. Build systems already have to compile stuff to get the BMI, and adding a static archive really isn't that far away. Being able to add things to a static archive that is compiled by the user would be invaluable for QoI, since we wouldn't be bound by ABI commitments when adding symbols.

mathstuf commented 4 months ago

FWIW I also think that this is really a lost opportunity. Build systems already have to compile stuff to get the BMI, and adding a static archive really isn't that far away. Being able to add things to a static archive that is compiled by the user would be invaluable for QoI, since we wouldn't be bound by ABI commitments when adding symbols.

I explained in the thread, but CMake has 1 target that represents "the standard library". This means that there is one object file. If there are multiple ABI flavors (ASAN? Debug?) within the same target, only one can be satisfied in a given build. Well, per configuration; multi-config generators can do it, but within a configuration, there is only one kind.

philnik777 commented 4 months ago

@mathstuf I don't understand what the problem you see is. If CMake has only a single target, then you can't use multiple standard library flavours anyways, thus requiring only a single static archive.

mathstuf commented 4 months ago

We can make BMIs per consumer flag set. It is objects where that is not possible.

philnik777 commented 4 months ago

We can make BMIs per consumer flag set. It is objects where that is not possible.

Ok, now I'm completely confused. What is the fundamental difference between BMIs and static archives that you can generate multiple of one kind, but not of the other?

mordante commented 4 months ago

At the moment we don't do anything special for initializing modules. For the tests in libc++ clang generates an object file with the initializer. The generated initializer only contains a return instruction. If this initializer depends on the build flags we would run into ODR violations.

mathstuf commented 4 months ago

So the thing that concerns me is this:

Note that we need to deal with the situation where existing libraries already have the symbol due to this and we use a new compiler/libc++ for a consuming library. How is this expected to work without a single "home" for the initializer (like all other symbols with "comes from another object" linkage)?

philnik777 commented 4 months ago

So the thing that concerns me is this:

* CMake builds this object as a static library

* It gets linked from multiple shared libraries (therefore copied)

* How is this not also an ODR violation?

I'm not sure what ODR violation you see. If you actually have only a single definition, there is obviously no problem. If the definitions may be different, you make things hidden (or another of the usual mitigations - different symbol name, internal linkage, ...). A static archive doesn't change anything here compared to header files AFAICT.

Note that we need to deal with the situation where existing libraries already have the symbol due to this and we use a new compiler/libc++ for a consuming library. How is this expected to work without a single "home" for the initializer (like all other symbols with "comes from another object" linkage)?

I think you're asking for weak linkage maybe? I'm honestly not sure. Could you maybe be a bit more specific about the situation you're thinking about? Are you concerned about initializing things multiple times?

Just to be clear: I don't expect the build system to magically fix ODR violations. The library still has to be engineered to allow this sort of mixing and matching. libc++ would still have to add an ABI tag to every symbol in the archive and make it hidden. The only difference is that other TUs know that the symbol already exists somewhere.

jiixyj commented 4 months ago

Just to be clear: I don't expect the build system to magically fix ODR violations. The library still has to be engineered to allow this sort of mixing and matching. libc++ would still have to add an ABI tag to every symbol in the archive and make it hidden. The only difference is that other TUs know that the symbol already exists somewhere.

Ah, just for my understanding -- assuming following scenario: libc++ 18.0 ships the .cppm file to a consumer "libA" who also builds the "library interface object file". This object file contains the initializer symbol for module "std". Some time later, libc++ 23.0 ships its .cppm file to "libB" who then also needs to build the library interface object file. Then, you link "libA" and "libB" together. Normally, this would result in a linker error because of the duplicated initializer symbol. So, libc++ would need to ensure all of the following:

...then, all would work out fine. I also found this subthread of the SG15 discussion very illuminating to understand how a "interface only module" ABI could work: https://lists.isocpp.org/sg15/2024/02/2362.php

But if I understand the discussions correctly, something like this would need additional language/compiler/build system support. In the current design of modules, the library interface object files are expected to have one home.

For now, I'm testing with this patch, which does the following:

Could this be a way forward until the details of the "interface only module" ABI(s) are hashed out? It will require CMake >= 3.28, though, at least when LIBCXX_INSTALL_MODULES=ON.

ChuanqiXu9 commented 4 months ago

Ah, just for my understanding -- assuming following scenario: libc++ 18.0 ships the .cppm file to a consumer "libA" who also builds the "library interface object file". This object file contains the initializer symbol for module "std". Some time later, libc++ 23.0 ships its .cppm file to "libB" who then also needs to build the library interface object file. Then, you link "libA" and "libB" together. Normally, this would result in a linker error because of the duplicated initializer symbol. So, libc++ would need to ensure all of the following:

I am not super sure if this violates the design principles of modules: every module should be unique in a program. Let's say we have library C uses libA and libB. Then during the building process of library C, maybe there are two primary module units involved for the std module...

or maybe you only talks about the ABI issue and assuming only one std.cppm is involved during the building process. And the libA.{a, so} and libB.{a, so} are compiled previously.... slightly odd. Can the interface files of libA and libB get compiled correctly with an inappropriate standard library?

So my summary for the two alternaitve options:

  1. Require the build systems to build a standalone static achieve for std modules. This breaks the general working model for modules.
  2. Require the standard library vendors to commit the object fie to standard libraries. Then it may be problematic to mix the use of standard libraries with different versions. Also we must require the compiler to provide a mechanism to add ABI tags to the module's initializer.
mathstuf commented 4 months ago

Here's a test case I was able to craft that shows a problem with "consumers must provide the std initializer symbol".

The test case is set up as follows:

This setup fails with Clang 18 when the impl-using-import std target is static because the end consumer needs to know that the implementation of it used import std somewhere. CMake would have to rummage around in the P1689 output, know what stdlib module names are, and figure this out and force all consumers to also somehow create and link the object despite not actually saying import std anywhere themselves. I do not know how much work this would be but it is…non-trivial (we'd have to conjure a dependency link out of nowhere). I suppose an alternative is to make the __cmake_cxx23 target that makes the object an OBJECT library (rather than STATIC), but then we're definitely putting the object into multiple places and tempting ODR demons.

A real-world case for this would be a library that uses import std internally and exposes a C ABI through headers and extern "C" functions. This means that we'd have to tell consumers "yeah, it looks like C, but you also need to build this C++ object and link it" (rather than just "here's a C++ library you need to link").

It is "fixed" by using shared libraries to "package" the initializer into the library that uses import std in the implementation. I doubt "just use shared libraries" is a suitable answer to this problem.

mathstuf commented 4 months ago

A real-world case for this would be a library that uses import std internally and exposes a C ABI through headers and extern "C" functions. This means that we'd have to tell consumers "yeah, it looks like C, but you also need to build this C++ object and link it" (rather than just "here's a C++ library you need to link").

This gets worse if we have std-specific initializer symbols that need provided as if two libraries do this, one with C++23 internally and the other with C++26, we now need to compile import std twice and link both in.

Heh…and we also need to do the import std compile with clang even if the consumer is using GCC. There's no model in CMake for having two different C++ toolchains loaded at the same time…so this would force "you must use clang too" on anything consuming static library projects using import std even if they don't expose C++ APIs.

mordante commented 4 months ago

Here's a test case I was able to craft that shows a problem with "consumers must provide the std initializer symbol".

Thanks for the test case. I'll have a look at it. (I'm a bit busy so it might be next weekend before I have time.)

mathstuf commented 4 months ago

@mordante Have you had any chance to look at it?

jiixyj commented 3 months ago

I've opened a (draft) PR that adds the initializer symbols to libc++experimental.a and moves the modules.json file alongside: https://github.com/llvm/llvm-project/pull/85832/files

With this, I've been able to drop the dirty hack in my initial post.

mordante commented 3 months ago

@mordante Have you had any chance to look at it?

Not yet, but I expect to have time soon.

mordante commented 3 months ago

I tried to reproduce this issue, but I've not been successful.

jiixyj commented 2 weeks ago

I thought to summarize some recent developments around CMake and SG15 discussions about this issue.

The upcoming CMake 3.30 supports std modules experimentally. They work around the missing symbols by building one static archive lib__cmake_cxx26.a in each build system. This library contains the module object files:

$ nm lib__cmake_cxx26.a

std.cppm.o:
0000000000000000 T _ZGIW3std

std.compat.cppm.o:
0000000000000000 T _ZGIW3stdW6compat

This library is "special": It is not exported/installed alongside the other library files from this "CMake build". Instead, all targets that need std modules are marked as such in the installed CMake config scripts. Then, consumers in other "CMake builds" know that they need to create their own lib__cmake_cxx26.a in turn. So in a big build graph consisting of many "CMake builds" the standard library's modules will be built O(n) times. To the best of my understanding, this is where concerns of ODR violations arise as you have multiple versions of those symbols floating around. The CMake blog entry about the feature mentions this as an explicit reason it is still marked as "experimental". There is still the open question whether those ODR issues are "benign" or not.

The consensus in SG15 seems to be that requiring consumers to build the object files is discouraged. This is (I guess) also the reason why there is no "require_user_to_build_objects" flag in their proposed metadata file.

For the use case of "interface-only" modules, there was some discussion about some to-be-proposed language features which could make this possible. Is this something that libc++ would be interested in?

Finally, I find it interesting that apparently, MSVC STL also does not ship the modules' objects but requires the user to build them. I wonder what their rationale behind this decision is.

ChuanqiXu9 commented 2 weeks ago

BTW, it looks like there are some on-going discussion about this in the in-person ISO C++ meeting. So I'll suggest libc++ dev to reach out SG15 in such cases.

mathstuf commented 1 week ago

Finally, I find it interesting that apparently, MSVC STL also does not ship the modules' objects but requires the user to build them. I wonder what their rationale behind this decision is.

It appeared to not be necessary, but there are symbols that are only realized in the .obj of the module build (internal things, not even the module initializer).

The consensus in SG15 seems to be that requiring consumers to build the object files is discouraged. This is (I guess) also the reason why there is no "require_user_to_build_objects" flag in their proposed metadata file.

Yes. While I suppose the standard library might be able to get away with magic to avoid the ODR, it is not something I expect developers to be so cognizant of/careful about in the wider ecosystem.

mordante commented 1 week ago

BTW, it looks like there are some on-going discussion about this in the in-person ISO C++ meeting. So I'll suggest libc++ dev to reach out SG15 in such cases.

I was not able to attend the SG-15 meetings last week. Do you have a link to the minutes of that discussion?

ChuanqiXu9 commented 1 week ago

BTW, it looks like there are some on-going discussion about this in the in-person ISO C++ meeting. So I'll suggest libc++ dev to reach out SG15 in such cases.

I was not able to attend the SG-15 meetings last week. Do you have a link to the minutes of that discussion?

I didn't take part in either. Maybe @mathstuf have a link

mathstuf commented 1 week ago

There are these minutes and I believe that this Google Doc is related. @druoso was there in person and may know more.