bazel-contrib / bazel-contrib/toolchains_llvm
Compiling with `-install_path` and `-rpath` flags containing `@` macros fails
- Dominant language
- Starlark
- Stars
- 371
- Forks
- 283
- Avg merge
- 1d 55m
- Merged PRs (30d)
- 25
Description
H/t to @vinistock for finding [this issue](https://github.com/grailbio/bazel-toolchain/issues/88#issuecomment-1021579070).
The below is mostly copied from #88; the issue is essentially that we're parsing `-install_path @executable_path/...` and similar flags as param files, incorrectly.
-----
> [That snippet](https://github.com/grailbio/bazel-toolchain/blob/619f31df5e1fd1f769c406012963f9655ae31adf/toolchain/osx_cc_wrapper.sh.tpl#L53-L63) was added to support [parameter files](https://stackoverflow.com/questions/7139685/does-gcc-support-command-files).
>
> The macOS cc wrapper script inspects the full command line in order to remap libraries that are being linked against to their fully resolved paths, taking into account the [`rpath`](https://developer.apple.com/library/archive/documentation/DeveloperTools/Conceptual/DynamicLibraries/100-Articles/RunpathDependentLibraries.html)s added to the binary. I don't have first-hand experience with this but this is [allegedly](https://github.com/bazelbuild/bazel/blob/6b32f227f21c9a9e5dbff713ac1ba3f662b47007/tools/cpp/osx_cc_wrapper.sh#L17-L23) because of some [oddness](https://stackoverflow.com/questions/39601359/osx-gcc-clang-how-to-set-a-dynamic-non-standard-library-framework-path-wit) having to do with runpaths added to binaries ([this](https://github.com/bazelbuild/bazel/commit/28ac615402bfbba56fbcbc6b4f3f51fca8bc9612) has some context; I _think_ it's that the paths are relative to the build dir and not the workspace that's causing the issue but I have no idea what's introducing the `-Wl,-rpath`s in the first place).
>
> Anyways, for that reason we need to actually read what's in the parameter file. The PR in this repo you linked to was essentially copied from upstream ([this commit](https://github.com/bazelbuild/bazel/commit/f08819b731f8bb5d95c196c2781e54908b846b74)); in general the logic in the macOS wrapper mostly comes from upstream.
>
> The issue here, of course, is that the `@` in `-install_name @execution_path/...` does _not_ signify a parameter file!
>
> What's peculiar to me is that upstream Bazel seems to fail on this in the exact same way ([here's a minimal test case](https://gist.github.com/rrbutani/a0a52776d87b026ef1339445070353fd)). Perhaps it's simply not common for users to want to generate dylibs with `install_path`s from Bazel and it hasn't come up? Not sure.
----
Extending the logic in the macOS wrapper to skip processing args starting with `@` (like `@execution_path/...`, `@executable_path/...`, `@load_path/..`, `@rpath/...`, etc.) when the preceeding arg is `-install_name` or `-rpath` is the obvious fix, I think.
I have a few concerns though:
- is this genuinely an error with the default Bazel toolchain?
+ edit: it appears so: bazelbuild/bazel#13044
- are `-install_name` and `-rpath` the only args that accept `@` macro form args? might be worth checking the `clang` arg parsing source to be sure
+ edit: it's probably better to just check that the file exists instead of maintaining a list. FWIW clang and the linker don't seem to actually even have a list of `@` macros that are acceptable; passing in `@some_random_thing` will compile successfully. grepping through [`dyld` source](https://opensource.apple.com/source/dyld/) and peering through the `dyld` man pages does indicate that it's really just `@rpath`, `@executable_path`, and `@loader_path` though. (`@execution_path` seems to be just a typo that a few people independently made over the years)
- what kind of path remapping should we be doing for `-install_name` and `-rpath`?
+ I think the answer is "none" for `-install_name` but what about `-rpath`? Don't we have the same issues as with `-Wl,-rpath`? `-rpath` certainly does seem to just expand out into `-rpath` to the linker, experimentally. Are we just banking on users always using the not-macOS-specific `-Wl` form?
* edit: going to add `-rpath @loader_path/...` to `$RPATHS` too; this misses `-Xlinker -rpath -Xlinker @loader_path/...` but I think that's okay for now.
- why does the logic in the wrapper only check for `@loader_path`?
+ edit: not really sure but assuming this is the path of the directory the `dylib` being loaded was found at at link time it makes sense that this doesn't match up with the runtime search path, for Bazel. if you don't think about it too hard.
Contributor guide
Research direction
Start with toolchain/osx_cc_wrapper.sh.tpl, especially the parameter-file parsing around lines 53–63, and reproduce the failure with the linked minimal test case. Compare the wrapper logic with upstream Bazel and verify how -install_name and -rpath arguments are handled. Done means @ macro paths are not incorrectly treated as parameter files while genuine parameter-file processing still works.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- shell
- Domain
- build-system
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100