Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix linker error when compiling with clang 9+ #2592

Merged
merged 1 commit into from
May 28, 2020

Conversation

rkunze
Copy link
Contributor

@rkunze rkunze commented May 26, 2020

Clang 9 changed the linkage definition for variable templates with const-qualified type
(see llvm/llvm-project@bb75068).

This leads to the following linker error in "make test" when compiling with clang versions >= 9:

clang++ -c -std=c++1z -stdlib=libc++ -O3 -g -fPIC -fexceptions -ferror-limit=1000 -fno-omit-frame-pointer -Wall -Wpedantic -Wno-char-subscripts -Wno-gnu-anonymous-struct -Wno-gnu-zero-variadic-macro-arguments -Wno-nested-anon-types -Wno-unknown-pragmas -DPROJECT_DIR='std::filesystem::path("ksp_plugin_adapter/")' -DSOLUTION_DIR='std::filesystem::path("./")' -DTEMP_DIR='std::filesystem::path("/tmp")' -DNDEBUG -m64 -I. -Ideps/glog/src -Ideps/protobuf/src -Ideps/gipfeli/include -Ideps/abseil-cpp -Ideps/zfp/include base/version.generated.cc -o obj/base/version.generated.o
clang++ -std=c++1z -stdlib=libc++ -O3 -g -fPIC -fexceptions -ferror-limit=1000 -fno-omit-frame-pointer -Wall -Wpedantic -Wno-char-subscripts -Wno-gnu-anonymous-struct -Wno-gnu-zero-variadic-macro-arguments -Wno-nested-anon-types -Wno-unknown-pragmas -DPROJECT_DIR='std::filesystem::path("ksp_plugin_adapter/")' -DSOLUTION_DIR='std::filesystem::path("./")' -DTEMP_DIR='std::filesystem::path("/tmp")' -DNDEBUG -m64 obj/tools/generate_configuration.o obj/tools/generate_kopernicus.o obj/tools/generate_profiles.o obj/tools/journal_proto_processor.o obj/tools/main.o obj/serialization/astronomy.pb.o obj/serialization/geometry.pb.o obj/serialization/integrators.pb.o obj/serialization/journal.pb.o obj/serialization/ksp_plugin.pb.o obj/serialization/numerics.pb.o obj/serialization/physics.pb.o obj/serialization/quantities.pb.o obj/serialization/testing_utilities.pb.o obj/numerics/cbrt.o obj/numerics/elliptic_functions.o obj/numerics/elliptic_integrals.o obj/numerics/fast_sin_cos_2π.o obj/base/version.generated.o deps/protobuf/src/.libs/libprotobuf.a deps/gipfeli/libgipfeli.a deps/abseil-cpp/absl/strings/libabsl_strings.a deps/abseil-cpp/absl/synchronization/libabsl_synchronization.a deps/abseil-cpp/absl/time/libabsl_*.a deps/abseil-cpp/absl/debugging/libabsl_*.a deps/abseil-cpp/absl/numeric/libabsl_*.a deps/abseil-cpp/absl/base/libabsl_*.a deps/zfp/build/lib/libzfp.a deps/glog/.libs/libglog.a -lpthread -lc++ -lc++abi -lsupc++ -lc++fs -o bin/tools
/usr/bin/ld: obj/tools/generate_kopernicus.o:(.rodata+0x8): multiple definition of `principia::quantities::si::Unit<double>'; obj/tools/generate_configuration.o:(.rodata+0x8): first defined here
/usr/bin/ld: obj/tools/generate_kopernicus.o:(.rodata+0x0): multiple definition of `principia::base::internal_traits::all_different_v<>'; obj/tools/generate_configuration.o:(.rodata+0x0): first defined here
/usr/bin/ld: obj/tools/main.o:(.rodata+0x8): multiple definition of `principia::quantities::si::Unit<double>'; obj/tools/generate_configuration.o:(.rodata+0x8): first defined here
/usr/bin/ld: obj/tools/main.o:(.rodata+0x0): multiple definition of `principia::base::internal_traits::all_different_v<>'; obj/tools/generate_configuration.o:(.rodata+0x0): first defined here
/usr/bin/ld: obj/numerics/elliptic_functions.o:(.rodata+0x8): multiple definition of `principia::quantities::si::Unit<double>'; obj/tools/generate_configuration.o:(.rodata+0x8): first defined here
/usr/bin/ld: obj/numerics/elliptic_functions.o:(.rodata+0x0): multiple definition of `principia::base::internal_traits::all_different_v<>'; obj/tools/generate_configuration.o:(.rodata+0x0): first defined here
/usr/bin/ld: obj/numerics/elliptic_integrals.o:(.rodata+0x8): multiple definition of `principia::quantities::si::Unit<double>'; obj/tools/generate_configuration.o:(.rodata+0x8): first defined here
/usr/bin/ld: obj/numerics/elliptic_integrals.o:(.rodata+0x0): multiple definition of `principia::base::internal_traits::all_different_v<>'; obj/tools/generate_configuration.o:(.rodata+0x0): first defined here
/usr/bin/ld: obj/numerics/fast_sin_cos_2π.o:(.rodata+0x18): multiple definition of `principia::quantities::si::Unit<double>'; obj/tools/generate_configuration.o:(.rodata+0x8): first defined here
/usr/bin/ld: obj/numerics/fast_sin_cos_2π.o:(.rodata+0x10): multiple definition of `principia::base::internal_traits::all_different_v<>'; obj/tools/generate_configuration.o:(.rodata+0x0): first defined here

Adding "inline" to the definitions for principia::quantities::si::Unit<double> and principia::base::internal_traits::all_different_v<> fixes this error.

With these changes, compilation worked on my system (Arch Linux, clang 10, libc++ 10) with one additional change: Since LLVM version 9, libc++fs is integrated into the main library (see https://libcxx.llvm.org/docs/UsingLibcxx.html#using-filesystem), so it needs to be removed from the linker flags.

Running make test on this version failed one test for me, though:

astronomy/orbital_elements_test.cpp:205: Failure
Value of: elements.mean_semimajor_axis_interval().measure()
Expected: is < +1.00000000000000002e-03 m
  Actual: +1.09284557402133942e-03 m
[  FAILED  ] OrbitalElementsTest.KeplerOrbit (2829 ms)

Clang 9 changed the linkage definition for variable templates with const-qualified type
(see llvm/llvm-project@bb75068).

This leads to the following linker error in "make test" when compiling with clang versions >= 9:

clang++ -c -std=c++1z -stdlib=libc++ -O3 -g -fPIC -fexceptions -ferror-limit=1000 -fno-omit-frame-pointer -Wall -Wpedantic -Wno-char-subscripts -Wno-gnu-anonymous-struct -Wno-gnu-zero-variadic-macro-arguments -Wno-nested-anon-types -Wno-unknown-pragmas -DPROJECT_DIR='std::filesystem::path("ksp_plugin_adapter/")' -DSOLUTION_DIR='std::filesystem::path("./")' -DTEMP_DIR='std::filesystem::path("/tmp")' -DNDEBUG -m64 -I. -Ideps/glog/src -Ideps/protobuf/src -Ideps/gipfeli/include -Ideps/abseil-cpp -Ideps/zfp/include base/version.generated.cc -o obj/base/version.generated.o
clang++ -std=c++1z -stdlib=libc++ -O3 -g -fPIC -fexceptions -ferror-limit=1000 -fno-omit-frame-pointer -Wall -Wpedantic -Wno-char-subscripts -Wno-gnu-anonymous-struct -Wno-gnu-zero-variadic-macro-arguments -Wno-nested-anon-types -Wno-unknown-pragmas -DPROJECT_DIR='std::filesystem::path("ksp_plugin_adapter/")' -DSOLUTION_DIR='std::filesystem::path("./")' -DTEMP_DIR='std::filesystem::path("/tmp")' -DNDEBUG -m64 obj/tools/generate_configuration.o obj/tools/generate_kopernicus.o obj/tools/generate_profiles.o obj/tools/journal_proto_processor.o obj/tools/main.o obj/serialization/astronomy.pb.o obj/serialization/geometry.pb.o obj/serialization/integrators.pb.o obj/serialization/journal.pb.o obj/serialization/ksp_plugin.pb.o obj/serialization/numerics.pb.o obj/serialization/physics.pb.o obj/serialization/quantities.pb.o obj/serialization/testing_utilities.pb.o obj/numerics/cbrt.o obj/numerics/elliptic_functions.o obj/numerics/elliptic_integrals.o obj/numerics/fast_sin_cos_2π.o obj/base/version.generated.o deps/protobuf/src/.libs/libprotobuf.a deps/gipfeli/libgipfeli.a deps/abseil-cpp/absl/strings/libabsl_strings.a deps/abseil-cpp/absl/synchronization/libabsl_synchronization.a deps/abseil-cpp/absl/time/libabsl_*.a deps/abseil-cpp/absl/debugging/libabsl_*.a deps/abseil-cpp/absl/numeric/libabsl_*.a deps/abseil-cpp/absl/base/libabsl_*.a deps/zfp/build/lib/libzfp.a deps/glog/.libs/libglog.a -lpthread -lc++ -lc++abi -lsupc++ -lc++fs -o bin/tools
/usr/bin/ld: obj/tools/generate_kopernicus.o:(.rodata+0x8): multiple definition of `principia::quantities::si::Unit<double>'; obj/tools/generate_configuration.o:(.rodata+0x8): first defined here
/usr/bin/ld: obj/tools/generate_kopernicus.o:(.rodata+0x0): multiple definition of `principia::base::internal_traits::all_different_v<>'; obj/tools/generate_configuration.o:(.rodata+0x0): first defined here
/usr/bin/ld: obj/tools/main.o:(.rodata+0x8): multiple definition of `principia::quantities::si::Unit<double>'; obj/tools/generate_configuration.o:(.rodata+0x8): first defined here
/usr/bin/ld: obj/tools/main.o:(.rodata+0x0): multiple definition of `principia::base::internal_traits::all_different_v<>'; obj/tools/generate_configuration.o:(.rodata+0x0): first defined here
/usr/bin/ld: obj/numerics/elliptic_functions.o:(.rodata+0x8): multiple definition of `principia::quantities::si::Unit<double>'; obj/tools/generate_configuration.o:(.rodata+0x8): first defined here
/usr/bin/ld: obj/numerics/elliptic_functions.o:(.rodata+0x0): multiple definition of `principia::base::internal_traits::all_different_v<>'; obj/tools/generate_configuration.o:(.rodata+0x0): first defined here
/usr/bin/ld: obj/numerics/elliptic_integrals.o:(.rodata+0x8): multiple definition of `principia::quantities::si::Unit<double>'; obj/tools/generate_configuration.o:(.rodata+0x8): first defined here
/usr/bin/ld: obj/numerics/elliptic_integrals.o:(.rodata+0x0): multiple definition of `principia::base::internal_traits::all_different_v<>'; obj/tools/generate_configuration.o:(.rodata+0x0): first defined here
/usr/bin/ld: obj/numerics/fast_sin_cos_2π.o:(.rodata+0x18): multiple definition of `principia::quantities::si::Unit<double>'; obj/tools/generate_configuration.o:(.rodata+0x8): first defined here
/usr/bin/ld: obj/numerics/fast_sin_cos_2π.o:(.rodata+0x10): multiple definition of `principia::base::internal_traits::all_different_v<>'; obj/tools/generate_configuration.o:(.rodata+0x0): first defined here

Adding "inline" to the definitions for principia::quantities::si::Unit<double> and principia::base::internal_traits::all_different_v<> fixes this error.
@pleroy
Copy link
Member

pleroy commented May 27, 2020

The test failure probably tells us that the Clang mathematical libraries have changed. I would just make it pass by setting the expectation on line 205 to Lt(1.1 * Milli(Metre)). It would be good if you could add that to this PR.

@pleroy
Copy link
Member

pleroy commented May 27, 2020

[Automated message from GitHub Pull Request Builder] Answer "ok to test" to trigger testing of this PR.

@eggrobin
Copy link
Member

[Automated message from GitHub Pull Request Builder] Answer "ok to test" to trigger testing of this PR.

It looks like the tests ran; did the silly bot look at the ok to test in its own answer and conclude that it should test?

Clang mathematical libraries have changed

That is libm, and should have nothing to do with clang. Until we deal with #1760 (by having our own libm and shipping it with Principia), the results of some tests will depend on the libm of the machine running the tests, its processor (if the libm behaves differently depending on e.g. the availability of FMA), the phase of the moon, etc.

On the other hand, the expectations should not change with compiler changes (we do not allow contraction to FMAs or any other transformations that change the result).

It would be good if you could add that to this PR.

While the expected value is admittedly not particularly meaningful, let’s not start randomly tweaking test expectations because they fail on some mysterious machine running some mysterious OS.

@eggrobin eggrobin added the LGTM label May 28, 2020
@pleroy pleroy merged commit a9d5e41 into mockingbirdnest:master May 28, 2020
@rkunze rkunze deleted the fix-compile-with-clang-9+ branch May 28, 2020 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants