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

Trouble including .a and .so in sandbox with linkopts #818

Closed
mikedanese opened this issue Jan 27, 2016 · 18 comments
Closed

Trouble including .a and .so in sandbox with linkopts #818

mikedanese opened this issue Jan 27, 2016 · 18 comments
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Rules-CPP Issues for C++ rules
Milestone

Comments

@mikedanese
Copy link

I have a rule:

cc_library(
    name = "base",
    hdrs = [
        "logging.h",
        "scope_guard.h",
    ],
    srcs = [
        "logging.cc",
    ],
    deps = [
        "@glog//:glog",
        "@gperftools//:tcmalloc",
    ],
    linkopts = [
        "@gperftools//:tcmalloc",
    ],
)

When I build:

$ bazel build @gperftools//:tcmalloc
INFO: Found 1 target...
Target @gperftools//:tcmalloc up-to-date:
  bazel-bin/external/gperftools/libtcmalloc.a
  bazel-bin/external/gperftools/libtcmalloc.so
INFO: Elapsed time: 0.107s, Critical Path: 0.00s

It looks like everything worked. But when I run:

$ bazel build //base    
INFO: Found 1 target...
INFO: From Linking base/libbase.so:
/usr/bin/ld: cannot find bazel-out/local_linux-fastbuild/bin/external/gperftools/libtcmalloc.a: No such file or directory
/usr/bin/ld: cannot find bazel-out/local_linux-fastbuild/bin/external/gperftools/libtcmalloc.so: No such file or directory
collect2: error: ld returned 1 exit status
ERROR: /home/mikedanese/jailer/base/BUILD:3:1: Linking of rule '//base:base' failed: gcc failed: error executing command /usr/bin/gcc -shared -o bazel-out/local_linux-fastbuild/bin/base/libbase.so -B/usr/bin/ -Wl,-z,relro,-z,now -no-canonical-prefixes -pass-exit-codes '-Wl,--build-id=md5' '-Wl,--hash-style=gnu' -Wl,-S ... (remaining 1 argument(s) skipped).
Target //base:base failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 0.168s, Critical Path: 0.07s

It looks like those files are not included in the sandbox. I checked and those paths exist on the host.
This is my tcmalloc BUILD:

package(default_visibility = ["//visibility:public"])
licenses(["notice"])

AM_CXXFLAGS = [
    "-Wall",
    "-Wwrite-strings",
    "-Woverloaded-virtual",
    "-Wno-sign-compare",
    "-fno-builtin-malloc",
    "-fno-builtin-free",
    "-fno-builtin-realloc",
    "-fno-builtin-calloc",
    "-fno-builtin-cfree",
    "-fno-builtin-memalign",
    "-fno-builtin-posix_memalign",
    "-fno-builtin-valloc",
    "-fno-builtin-pvalloc",
    "-fno-builtin",
]

genrule(
    name = "gperftools-configure",
    srcs = glob(["**/*"]),
    outs = [
        "src/config.h",
        "src/gperftools/tcmalloc.h",
    ],
    cmd = ""
    + "gperf=\"external/gperftools\"; "
    + "outs=($(OUTS)); "
    + "cp -r $${gperf} build/; "
    + "( cd build; ./autogen.sh; ./configure; ); "
    + "cp build/src/config.h              \"$${outs[0]}\"; "
    + "cp build/src/gperftools/tcmalloc.h \"$${outs[1]}\"; "
    ,
)

cc_library(
    name = "tcmalloc",
    hdrs = [
        "src/gperftools/heap-checker.h",
        "src/gperftools/heap-profiler.h",
        "src/gperftools/malloc_extension.h",
        "src/gperftools/malloc_extension_c.h",
        "src/gperftools/malloc_hook.h",
        "src/gperftools/malloc_hook_c.h",
        "src/gperftools/profiler.h",
        "src/gperftools/stacktrace.h",
        "src/gperftools/tcmalloc.h",
    ],
    srcs = glob([
            "src/*.cc",
            "src/*.h",
            "src/base/*.cc",
            "src/base/*.h",
        ],
        exclude = [
            "src/debugallocation.cc",
        ],
    ) + [
        ":gperftools-configure"
    ],
    includes = [ "src/" ],
    copts = AM_CXXFLAGS,
)

How do I link tcmalloc to another cc library?

@kchodorow kchodorow added P2 We'll consider working on this in future. (Assignee optional) external repositories category: sandboxing labels Jan 27, 2016
@bsilver8192
Copy link
Contributor

Is there some reason you have "@gperftools//:tcmalloc" in both deps and linkopts? Bazel should handle linking it in correctly if you just put it in deps.

@mikedanese
Copy link
Author

From linkopts doc: Each element of this list that does not start with $ or - is assumed to be the label of a target in deps. The list of files generated by that target is appended to the linker options. An error is reported if the label is invalid, or is not declared in deps.

@bsilver8192
Copy link
Contributor

I can see how that's confusing, but linkopts is intended for things like linker scripts, not other libraries.

Bazel-built libraries which should get linked into another cc_* rule go in deps, and external .so/.a/.o files go in srcs. It's a bit complicated because cc_library rules don't have the .so and .a file as normal outputs like eg the binary from cc_binary because Bazel chooses between the .a and the .so, and the .so has to be added to the runfiles of dependent rules if it's used.

@mikedanese
Copy link
Author

@bsilver8192 adding in only deps seems to do the right thing here.

What is the usecase of specifying a cc_library in a linkopt? Is it to control order of '-l's? I'm still pretty sure it's not working as intended with sandboxing.

@bsilver8192
Copy link
Contributor

I don't think there is a use case for doing putting a cc_library in linkopts. Might make sense to add a warning for that like some other common misuses of the cc_* rules have.

For ordering the libraries during the final link, Bazel handles it based on the dependency edges established by deps. Also, Bazel knows about things like alwayslink which require doing special things with where the libraries show up on the link command line, so it's really not possible to pull the libraries out separately in any kind of simple list.

@philwo
Copy link
Member

philwo commented Feb 1, 2016

Sorry that I didn't reply earlier. I'm currently overwhelmed with other issues that I have to work on.

I can't comment on what the right thing to do is regarding the files of a cc_library that shows up on linkopts, but if we decide that they should be mounted by the sandbox, they should probably be added to the action inputs by CppCompileActionBuilder and its friends. The sandbox would then automatically mount them.

@ulfjack
Copy link
Contributor

ulfjack commented Feb 5, 2016

For tcmalloc and other malloc libraries specifically, you should use the --custom_malloc flag or set cc_binary.malloc.

@mikedanese
Copy link
Author

Thanks for the suggestions. I like @bsilver8192's suggestion because if I define the tcmalloc cc_library with alwayslink = 1 and set it as a dependency of my "base" library, all binaries that directly or indirectly depend on base (read all binaries :) will use tcmalloc. One question is unanswered though. The docs for cc_library.linkopts says:

Each element of this list that does not start with $ or - is assumed to be the label of a target in deps. The list of files generated by that target is appended to the linker options. An error is reported if the label is invalid, or is not declared in deps.

This leads me to believe that at some point in time someone thought it was a good idea to specify cc_libraries in linkopts as opposed to just in dependencies. The open question is, what is the usecase for that?

Also this feature appears to be broken by sandboxing since the .a and .so are not included in the build sandbox.

@ahyangyi
Copy link

Occasionally, one wants to insert ["-Wl,--start-group", some_label, some_other_label, "-Wl,--end-group"] if some existing code has screwed up interdependencies. I wonder if this is the reason why labels are supported (but broken) in linkopts.

It could be argued that one should use a genrule or something to merge the static libraries and then include the resultant thing in srcs. But this solution comes with a caveat:

  • In an environment where multiple platforms and multiple toolchains (including cross-compiling ones) exist, writing a correct genrule to merge archives is hard.

That said, by default a cc_library label actually resolves to the shared library it generates, as opposed to the static one, so what I said is probably wrong.

@ahyangyi
Copy link

@kchodorow I don't think this is a sandbox bug actually. If one replace that "@gperftools//:tcmalloc" to "external/gperftools/libtcmalloc.a" one should actually be able to build it. So the problem seems to be the labels being replaced with wrong paths, instead of sandbox problem.

Also notice that @mikedanese included the same thing in deps, so the needed files should actually be accessible in the sandbox.

@philwo
Copy link
Member

philwo commented Oct 13, 2016

@hermione521 Yue, could you try to find a good owner for this bug? I don't think it's about sandboxing and I don't know anything about linkopts / C++ rules...

@hermione521
Copy link
Contributor

Done. Not sure if I'm right..

@kchodorow kchodorow added this to the 0.5 milestone Dec 9, 2016
@kchodorow kchodorow modified the milestones: 0.5, 0.6 Dec 21, 2016
@ulfjack
Copy link
Contributor

ulfjack commented Jul 26, 2017

Our internal cc_library rules actually allow individual files in deps, not just cc_library rules, and this is used to refer to linker scripts. We should either allow linker scripts in srcs, or something, and also allow linkopts to refer to those. I think there's also a case to be made to support --start-group --end-group in linkopts, with the .a files in srcs (?), or maybe in a special cc_import rule, except we'd need to make sure that cc_library doesn't also put the .a files on the link command line.

@ulfjack
Copy link
Contributor

ulfjack commented Jul 26, 2017

@hlopko
Copy link
Member

hlopko commented Jul 27, 2017

Feel free to read and comment on cc_*_library_import proposal: https://docs.google.com/document/d/1hK2mWl3TYNL9oJYX_S020TKkXZvBw1aBoYERvTHVyfg/edit

@siddharthab
Copy link
Contributor

Slightly unrelated to this thread, but is there a good way of using gperftools/tcmalloc with bazel? I could not find a good up to date BUILD file for gperftools, and reverse engineering their make process is hacky.

@hlopko
Copy link
Member

hlopko commented Sep 11, 2017

@siddharthab I'm afraid this is not the right place to ask this question. Maybe somebody on bazel-discuss will know?

@siddharthab
Copy link
Contributor

Thank you for the reply. I saw it being used in the example above and I was tempted. I will take my question to the Google group.

@hlopko hlopko added team-Rules-CPP Issues for C++ rules and removed category: rules > C++ labels Oct 11, 2018
@hlopko hlopko added P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed P2 We'll consider working on this in future. (Assignee optional) labels Jun 6, 2019
@hlopko hlopko removed their assignment Jun 6, 2019
CodingCanuck added a commit to 3rdparty/eventuals that referenced this issue Jul 19, 2022
This is recommended here: bazelbuild/bazel#818 (comment)

But more importantly, this fixes issues when trying to include eventuals
in targets that are built as shared libraries.

Fixes #470.
CodingCanuck added a commit to 3rdparty/eventuals that referenced this issue Jul 19, 2022
This is recommended here: bazelbuild/bazel#818 (comment)

But more importantly, this fixes issues when trying to include eventuals
in targets that are built as shared libraries.

Fixes #470.
CodingCanuck added a commit to 3rdparty/eventuals that referenced this issue Jul 19, 2022
This is recommended here: bazelbuild/bazel#818 (comment)

But more importantly, this fixes issues when trying to include eventuals
in targets that are built as shared libraries.

Fixes #470.
aviator-app bot pushed a commit to 3rdparty/eventuals that referenced this issue Jul 19, 2022
This is recommended here: bazelbuild/bazel#818 (comment)

But more importantly, this fixes issues when trying to include eventuals
in targets that are built as shared libraries.

Fixes #470.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Rules-CPP Issues for C++ rules
Projects
None yet
Development

No branches or pull requests