facebook / buck2

Build system, successor to Buck
https://buck2.build/
Apache License 2.0
3.53k stars 215 forks source link

buck2: linker does not respect order for static libraries #506

Closed hutm closed 10 months ago

hutm commented 10 months ago

Liker fails with undefined symbols. Target c++ binary depends on libcurl, which depends on libssl. //common:init depends on libfolly, which also depends on libssl.

cxx_binary(
    name = "curl_test",
    srcs = [
        "curl_test.cpp",
    ],
    deps = [
        "//common:init",
        '//third-party/build/curl:curl',
    ],
)

inspecting argfile for linker suggests that dependency graph is not respected: libssl appears before libcurl and causes linker error.

-static-libstdc++
third-party/build/gcc/lib-linux-x86_64/libstdc++.a
-o
buck-out/v2/gen/root/524f8da68ea2a374/experimental/network/__curl_test__/curl_test
buck-out/v2/gen/root/524f8da68ea2a374/experimental/network/__curl_test__/__objects__/curl_test.cpp.o
third-party/build/gflags/lib-linux-x86_64/libgflags.a
third-party/build/glog/lib-linux-x86_64/libglog.a
third-party/build/folly/lib-linux-x86_64/libfolly.a
third-party/build/double-conversion/lib-linux-x86_64/libdouble-conversion.a
third-party/build/boost/lib-linux-x86_64/libboost_context.a
third-party/build/boost/lib-linux-x86_64/libboost_filesystem.a
third-party/build/boost/lib-linux-x86_64/libboost_program_options.a
third-party/build/boost/lib-linux-x86_64/libboost_regex.a
third-party/build/boost/lib-linux-x86_64/libboost_system.a
third-party/build/boost/lib-linux-x86_64/libboost_thread.a
third-party/build/boost/lib-linux-x86_64/libboost_chrono.a
third-party/build/boost/lib-linux-x86_64/libboost_atomic.a
third-party/build/boost/lib-linux-x86_64/libboost_date_time.a
third-party/build/libevent/lib-linux-x86_64/libevent.a
third-party/build/lz4/lib-linux-x86_64/liblz4.a
third-party/build/lzma/lib-linux-x86_64/liblzmadec.a
third-party/build/snappy/lib-linux-x86_64/libsnappy.a
third-party/build/openssl/lib-linux-x86_64/libssl.a
third-party/build/openssl/lib-linux-x86_64/libcrypto.a
-ldl
third-party/build/libdwarf/lib-linux-x86_64/libdwarf.a
third-party/build/libaio/lib-linux-x86_64/libaio.a
third-party/build/libelf/lib-linux-x86_64/libelf.a
third-party/build/libiberty/lib-linux-x86_64/libiberty.a
third-party/build/libunwind/lib-linux-x86_64/libunwind.a
third-party/build/liburcu/lib-linux-x86_64/liburcu.a
third-party/build/xz/lib-linux-x86_64/liblzma.a
third-party/build/jemalloc/lib-linux-x86_64/libjemalloc.a
-lpthread
third-party/build/zlib/lib-linux-x86_64/libz.a
third-party/build/zstd/lib-linux-x86_64/libzstd.a
third-party/build/fmt/lib-linux-x86_64/libfmt.a
third-party/build/liburing/lib-linux-x86_64/liburing.a
third-party/build/libsodium/lib-linux-x86_64/libsodium.a
third-party/build/curl/lib-linux-x86_64/libcurl.a
third-party/build/nghttp2/lib-linux-x86_64/libnghttp2.a

Buck1 handles dependency graph correctly and the binary can be linked successfully;

Is there a configuration parameter in cxx toolchain to resolve the issue?

krallin commented 10 months ago

Yeah, you should be able to do this by tweaking the system_cxx_toolchain rule. I'll put up a change for this internally, but this is what you need:

diff --git a/fbcode/buck2/prelude/toolchains/cxx.bzl b/fbcode/buck2/prelude/toolchains/cxx.bzl
--- a/fbcode/buck2/prelude/toolchains/cxx.bzl
+++ b/fbcode/buck2/prelude/toolchains/cxx.bzl
@@ -18,7 +18,7 @@
 )
 load("@prelude//cxx:headers.bzl", "HeaderMode")
 load("@prelude//cxx:linker.bzl", "is_pdb_generated")
-load("@prelude//linking:link_info.bzl", "LinkStyle")
+load("@prelude//linking:link_info.bzl", "LinkOrdering", "LinkStyle")
 load("@prelude//linking:lto.bzl", "LtoMode")
 load("@prelude//toolchains/msvc:tools.bzl", "VisualStudio")
 load("@prelude//utils:cmd_script.bzl", "ScriptOs", "cmd_script")
@@ -111,6 +111,7 @@
                 force_full_hybrid_if_capable = False,
                 is_pdb_generated = is_pdb_generated(linker_type, ctx.attrs.link_flags),
                 produce_interface_from_stub_shared_library = True,
+                link_ordering = ctx.attrs.link_ordering,
             ),
             bolt_enabled = False,
             binary_utilities_info = BinaryUtilitiesInfo(
@@ -182,6 +183,7 @@
         "cxx_compiler": attrs.string(default = "cl.exe" if host_info().os.is_windows else "clang++"),
         "cxx_flags": attrs.list(attrs.string(), default = []),
         "link_flags": attrs.list(attrs.string(), default = []),
+        "link_ordering": attrs.option(attrs.enum(LinkOrdering.values()), default = None),
         "link_style": attrs.string(default = "shared"),
         "linker": attrs.string(default = "link.exe" if host_info().os.is_windows else "clang++"),
         "linker_wrapper": attrs.default_only(attrs.exec_dep(providers = [RunInfo], default = "prelude//cxx/tools:linker_wrapper")),

Then when you instantiate it (in toolchains/BUCK), add link_ordering = "topological" to the attrs.

hutm commented 10 months ago

thanks @krallin , that indeed resolved the issue. Closing this