Page MenuHomePhabricator

[Driver] Add flag to request arch-specific-subdir in -rpath
ClosedPublic

Authored by pirama on Mar 7 2017, 9:47 AM.

Details

Summary

This patch adds -f[no-]rtlib-add-rpath, which if enabled, embeds the
arch-specific subdirectory in resource directory using -rpath (instead
of doing so only during native compilation).

This patch also re-enables test arch-specific-libdir.c which was
silently unsupported because of the REQUIRES tag 'linux'.

Event Timeline

pirama created this revision.Mar 7 2017, 9:47 AM
rnk edited edge metadata.Mar 7 2017, 10:12 AM

Can you add a flag pair to control the insertion of rpath into the final binary? As things are currently, clang is basically leaking paths from the machine doing the linking into the final binary, which often will not run on the same machine. I was thinking -f[no-]compiler-rt-rpath or something, but openmp is not part of compiler-rt. Name recommendations welcome. We might also want to reconsider the default setting.

pirama added a comment.Mar 7 2017, 2:29 PM
In D30700#694511, @rnk wrote:

I was thinking -f[no-]compiler-rt-rpath or something, but openmp is not part of compiler-rt. Name recommendations welcome. \

Maybe -f[no-]rtlib-add-rpath?

We might also want to reconsider the default setting.

I also feel defaulting to not adding rpath makes sense.

rnk added a comment.Mar 7 2017, 2:37 PM

Both suggestions sound good to me. Thanks!

No build system will ever set -frtlib-add-rpath to enable this "feature". I'm for keeping this opt-out until we have configuration files to set this by default. Making it opt-in would weaken its main reason of existence: Not to break simple binaries for the user, and we can just drop it.

pirama updated this revision to Diff 91633.Mar 13 2017, 3:33 PM
  • Rebase
  • Added command line flag and updated tests.
rnk added a comment.Mar 13 2017, 3:37 PM

No build system will ever set -frtlib-add-rpath to enable this "feature". I'm for keeping this opt-out until we have configuration files to set this by default. Making it opt-in would weaken its main reason of existence: Not to break simple binaries for the user, and we can just drop it.

I don't agree. If we want to be good citizens on Linux, what's supposed to happen is that we install our shared libraries into /usr/lib/${distro_target}, which is what GCC does with its shared compiler runtime libraries. GCC does not add system-specific absolute rpaths to the binaries it produces.

pirama updated this revision to Diff 91635.Mar 13 2017, 3:41 PM

*Actually* add the command line flags.

rnk accepted this revision.Mar 13 2017, 6:07 PM

lgtm, thanks!

This revision is now accepted and ready to land.Mar 13 2017, 6:07 PM
pirama updated this revision to Diff 91743.Mar 14 2017, 10:08 AM

Update commit message

pirama retitled this revision from [Driver] Always add arch-specific-subdir to -rpath to 36079897.Mar 14 2017, 10:09 AM
pirama edited the summary of this revision. (Show Details)
pirama retitled this revision from 36079897 to [Driver] Add flag to request arch-specific-subdir in -rpath.
This revision was automatically updated to reflect the committed changes.