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

macOS: incorrect rpath handling for dependencies #3077

Closed
siriobalmelli opened this issue Feb 16, 2018 · 8 comments
Closed

macOS: incorrect rpath handling for dependencies #3077

siriobalmelli opened this issue Feb 16, 2018 · 8 comments
Labels
bug compilers dependencies OS:macos Issues specific to Apple Operating Systems like MacOS and iOS

Comments

@siriobalmelli
Copy link

rpath handling on macOS is incorrect in 2 ways:

  • LC_RPATH pointing to the build dir is NOT stripped on ninja install
  • LC_RPATH for dependencies is OMITTED

The first problem is trivial, but conceivably a security concern if an attacker can manipulate the filesystem at runtime and put a compromised library in the "build rpath" not sripped.

The second problem is critical, but only shows up when dependencies are in non-standard paths (e.g.: Nix).

reproduce with intra-project dependency

A straightforward is the ncp utility of nonlibc:

~ $ git clone https://github.com/siriobalmelli/nonlibc.git
~ $ cd nonlibc
nonlibc $ git checkout tags/v0.1.9b
nonlibc $ nix-build

You will see it builds and tests successfully, including the test for ncp.
For context, ncp is declared in util/meson.build:

ncp = executable('ncp', 'ncp.c',
		include_directories : inc,
		link_with : nonlibc,
		install : true)

test('ncp test',
	find_program('test_ncp.py'),
	args : ncp.full_path())

However, trying to execute the result throws a linker error:

nonlibc$ result/bin/ncp
dyld: Library not loaded: @rpath/libnonlibc.dylib
  Referenced from: /Users/siriobalmelli/nonlibc/result/bin/ncp
  Reason: image not found

NOTE that result is actually a symlink to the Nix build:

nonlibc$ readlink result
/nix/store/p7mmc0i21967k7a2hmxdm67mx36766a2-nonlibc

However, the linking dependency IS correctly declared and exported to pkgconfig by meson:

nonlibc$ pkg-config --libs result/lib/pkgconfig/nonlibc.pc 
-L/nix/store/p7mmc0i21967k7a2hmxdm67mx36766a2-nonlibc/lib -lnonlibc

By looking at the "load commands" of ncp we see the problem (some output elided for clarity):

nonlibc$ otool -l result/bin/ncp | grep -E '\s+(name|path)'
         name @rpath/libnonlibc.dylib (offset 24)
         path /private/tmp/nix-build-nonlibc.drv-0/nonlibc/build/src (offset 12)

Here we see a (correct) @rpath name for nonlibc, but then we see:

  • a path directive pointing to the build directory (should have been stripped)
  • no path directive pointing to /nix/store/p7mmc0i21967k7a2hmxdm67mx36766a2-nonlibc/lib

hack for intra-project dependencies

The can be fixed hacked by:

  1. forcing a linker argument when we declare the dependecy in meson:
# Force dependers to have an rpath entry pointing to where we KNOW we'll install.
# This fixes both Nix and non-nix projects (providing 'prefix' is set correctly),
#+  and does OPAQUELY (depender doesn't have to add an 'install_rpath' directive).
rpath = get_option('prefix') + '/' + get_option('libdir')
nonlibc_dep_shared = declare_dependency(link_with : nonlibc,
					include_directories : inc,
					dependencies : deps,
					link_args : '-Wl,-rpath,' + rpath)
  1. changing the ncp definition to use dependencies instead of link_with:
ncp = executable('ncp', 'ncp.c',
		include_directories : inc,
		dependencies: nonlibc_dep_shared,
		install : true)

Here is what the build looks like (some output elided for clarity):

~ $ git clone https://github.com/siriobalmelli/nonlibc.git
~ $ cd nonlibc
nonlibc $ nix-build
nonlibc $ result/bin/ncp 
Usage: result/bin/ncp [OPTION]... SOURCE_FILE DEST_FILE
  or:  result/bin/ncp [OPTION]... SOURCE_FILE... DEST_DIR/
nonlibc $ otool -l result/bin/ncp | grep -E '\s+(name|path)'
         name @rpath/libnonlibc.dylib (offset 24)
         path /nix/store/0xvwwzz02ngn63hiqv825fg0kyh5vldv-nonlibc/lib (offset 12)
         path /private/tmp/nix-build-nonlibc.drv-0/nonlibc/build/src (offset 12)

reproduce with inter-project (cannot fix as above)

The above would be all well and good, but when importing a dependency which is NOT a meson subproject, using pkgconfig, there is no straightforward way to force meson to generate an rpath.

An example is memorywell which depends on nonlibc:

~ $ git clone https://github.com/siriobalmelli/nonlibc.git
~ $ git clone --branch=sirio https://github.com/siriobalmelli/memorywell.git
~ $ cd memorywell
memorywell $ nix-build -I ~
dyld: Library not loaded: @rpath/libnonlibc.dylib
  Referenced from: /private/tmp/nix-build-memorywell.drv-0/memorywell/build/test/well_test_shared
  Reason: image not found

This is easier to see building step-by-step inside a nix-shell:

memorywell $ nix-shell -I ~ --pure
[nix-shell:~/memorywell]$ CC=clang meson --buildtype=release build-release-clang
The Meson build system
Version: 0.44.0
Source dir: /Users/siriobalmelli/memorywell
Build dir: /Users/siriobalmelli/memorywell/build-release-clang
Build type: native build
Project name: memorywell
Native C compiler: clang (clang 4.0.1)
Build machine cpu family: x86_64
Build machine cpu: x86_64
Found pkg-config: /nix/store/7p159gq5b79cxwjcpw1z3wjfcw0bjwdi-pkg-config-0.29.2/bin/pkg-config (0.29.2)
Native dependency nonlibc found: YES 0.1.9
Configuring well_config.h using configuration
Dependency threads found: YES
Build targets in project: 26
Found ninja-1.8.2 at /nix/store/yxpay4bx8b8fccgvk21fnn15kglkfllk-ninja-1.8.2/bin/ninja
[nix-shell:~/memorywell]$ cd build-release-clang/
[nix-shell:~/memorywell/build-release-clang]$ ninja 
[nix-shell:~/memorywell/build-release-clang]$ otool -l test/well_test_shared | grep -E '\s+(name|path)'
         name @rpath/libmemorywell.dylib (offset 24)
         name @rpath/libnonlibc.dylib (offset 24)
         path /Users/siriobalmelli/memorywell/build-release-clang/src (offset 12)
[nix-shell:~/memorywell/build-release-clang]$ pkg-config --libs nonlibc 
-L/nix/store/dbxywb352yx39j15hk7kmlsxqwc520nm-nonlibc/lib -lnonlibc

As you can see above:

  • meson finds the nonlibc dependency using pkgconfig
  • pkgconfig gives the right link path
  • ... this path is absent from the well_test_shared executable

proposal

I propose that rpath logic for macOS be modified to:

  1. At build-time: dereference all dependencies objects into a (unique) list of all LC_RPATH entries to add into the binary's "load commands" section:
  • for intra-project dependencies these point to the build dir
  • for pkgconfig (and any other) dependencies, these point to the system tree
  • don't take any paths for granted, even if /usr/lib etc - always insert an LC_RPATH entry
  1. At install-time, rewrite all intra-project LC_RPATH entries to point to get_option('prefix') + '/' + get_option('libdir')
@jpakkane
Copy link
Member

Regarding option 2, we should probably do the same thing that CMake does. AFAIK they clear the rpath entry and only write one in if it has been specified with their equivalent of install_rpath.

@nirbheek nirbheek added bug compilers OS:macos Issues specific to Apple Operating Systems like MacOS and iOS dependencies labels Feb 18, 2018
@tp-m tp-m changed the title macOS: incorrect rpath handling for depedencies macOS: incorrect rpath handling for dependencies May 23, 2018
@nirbheek
Copy link
Member

LC_RPATH pointing to the build dir is NOT stripped on ninja install

This is fixed with meson 0.46.1 and newer.

LC_RPATH for dependencies is OMITTED

This is still a problem, doubly so because you can't set DYLD_LIBRARY_PATH or DYLD_FALLBACK_LIBRARY_PATH in new shells on macOS, so build environments can't set it at all.

This means that the rpath entries are all wrong, and you can't run any executables:

$ gtester
dyld: Library not loaded: @rpath/libglib-2.0.0.dylib
  Referenced from: /Library/Frameworks/GStreamer.framework/Versions/1.0/bin/gtester
  Reason: image not found

On macOS, we need to always set the absolute rpath to all libraries that are used by a binary. This is what Autotools does, and also what XCode does. Without this, meson is basically useless on macOS.

@jpakkane
Copy link
Member

But is it also what CMake does?

@nirbheek
Copy link
Member

But is it also what CMake does?

Yes, and cmake also gives you overrides to not run install_name_tool to rewrite the @rpath even after install. Not sure yet if we need that.

@ePirat
Copy link
Contributor

ePirat commented May 24, 2018

I think it is fine to have the @rpath in the install_name of the libraries for the uninstalled version in the meson folder, and the correct rpath values in the binaries. But when installing the install_name has to be adjusted and the rpaths in the binaries removed, no?

(Optionally users probably should have a way to set the rpath and install_name for the installed stuff, similar to what CMake allows to do)

@nirbheek
Copy link
Member

nirbheek commented May 24, 2018

I think it is fine to have the @rpath in the install_name of the libraries for the uninstalled version in the meson folder, and the correct rpath values in the binaries. But when installing the install_name has to be adjusted and the rpaths in the binaries removed, no?

Yes. Specifically, the builddir rpaths are removed (which we already do) and new ones are added based on the external and internal dependencies (which we do not do). I have a WIP patch for this.

(Optionally users probably should have a way to set the rpath and install_name for the installed stuff, similar to what CMake allows to do)

One step at a time, please. We keep getting stuck trying to 'fix everything on macOS' (the exact meaning of which everyone disagrees on) and getting nothing done.

@ePirat
Copy link
Contributor

ePirat commented May 24, 2018

One step at a time, please.

Sure, just wanted to make sure to point it out.

BTW is the build folder created by meson relocatable? I guess not?

@nirbheek
Copy link
Member

BTW is the build folder created by meson relocatable? I guess not?

It is not.

nirbheek added a commit that referenced this issue Jun 5, 2018
This allows us to more aggressively de-dup them, and also sets RPATHs
to all libraries that are not in the system linker paths so that
binaries can be run uninstalled without any special steps.

These RPATHs will be wiped on install, so they do not affect
reproducible builds.

De-duping:
Fixes #2150
Fixes #2118

RPATHs:
Fixes #314
Fixes #2881

Also fixes the uninstalled usage portions of:
#3038
#3077
nirbheek added a commit that referenced this issue Jun 5, 2018
This allows us to more aggressively de-dup them, and also sets RPATHs
to all libraries that are not in the system linker paths so that
binaries can be run uninstalled without any special steps.

These RPATHs will be wiped on install, so they do not affect
reproducible builds.

De-duping:
Fixes #2150
Fixes #2118

RPATHs:
Fixes #314
Fixes #2881

Also fixes the uninstalled usage portions of:
#3038
#3077
nirbheek added a commit that referenced this issue Jun 7, 2018
This allows us to more aggressively de-dup them, and also sets RPATHs
to all libraries that are not in the system linker paths so that
binaries can be run uninstalled without any special steps.

These RPATHs will be wiped on install, so they do not affect
reproducible builds.

De-duping:
Fixes #2150
Fixes #2118

RPATHs:
Fixes #314
Fixes #2881

Also fixes the uninstalled usage portions of:
#3038
#3077
nirbheek added a commit that referenced this issue Jun 7, 2018
On macOS, we set the install_name for built libraries to
@rpath/libfoo.dylib, and when linking to the library, we set the RPATH
to its path in the build directory. This allows all built binaries to
be run as-is from the build directory (uninstalled).

However, on install, we have to strip all the RPATHs because they
point to the build directory, and we change the install_name of all
built libraries to the absolute path to the library. This causes the
install name in binaries to be out of date.

We now change that install name to point to the absolute path to each
built library after installation.

Fixes #3038
Fixes #3077

With this, the default workflow on macOS matches what everyone seems
to do, including Autotools and CMake. The next step is providing a way
for build files to override the install_name that is used after
installation for use with, f.ex., private libraries when combined with
the install_rpath: kwarg on targets.
nirbheek added a commit that referenced this issue Jun 7, 2018
On macOS, we set the install_name for built libraries to
@rpath/libfoo.dylib, and when linking to the library, we set the RPATH
to its path in the build directory. This allows all built binaries to
be run as-is from the build directory (uninstalled).

However, on install, we have to strip all the RPATHs because they
point to the build directory, and we change the install_name of all
built libraries to the absolute path to the library. This causes the
install name in binaries to be out of date.

We now change that install name to point to the absolute path to each
built library after installation.

Fixes #3038
Fixes #3077

With this, the default workflow on macOS matches what everyone seems
to do, including Autotools and CMake. The next step is providing a way
for build files to override the install_name that is used after
installation for use with, f.ex., private libraries when combined with
the install_rpath: kwarg on targets.
nirbheek added a commit that referenced this issue Jun 7, 2018
On macOS, we set the install_name for built libraries to
@rpath/libfoo.dylib, and when linking to the library, we set the RPATH
to its path in the build directory. This allows all built binaries to
be run as-is from the build directory (uninstalled).

However, on install, we have to strip all the RPATHs because they
point to the build directory, and we change the install_name of all
built libraries to the absolute path to the library. This causes the
install name in binaries to be out of date.

We now change that install name to point to the absolute path to each
built library after installation.

Fixes #3038
Fixes #3077

With this, the default workflow on macOS matches what everyone seems
to do, including Autotools and CMake. The next step is providing a way
for build files to override the install_name that is used after
installation for use with, f.ex., private libraries when combined with
the install_rpath: kwarg on targets.
nirbheek added a commit that referenced this issue Jun 7, 2018
This allows us to more aggressively de-dup them, and also sets RPATHs
to all libraries that are not in the system linker paths so that
binaries can be run uninstalled without any special steps.

These RPATHs will be wiped on install, so they do not affect
reproducible builds.

De-duping:
Fixes #2150
Fixes #2118
Fixes #3071

RPATHs:
Fixes #314
Fixes #2881

Also fixes the uninstalled usage portions of:
#3038
#3077
nirbheek added a commit that referenced this issue Jun 7, 2018
On macOS, we set the install_name for built libraries to
@rpath/libfoo.dylib, and when linking to the library, we set the RPATH
to its path in the build directory. This allows all built binaries to
be run as-is from the build directory (uninstalled).

However, on install, we have to strip all the RPATHs because they
point to the build directory, and we change the install_name of all
built libraries to the absolute path to the library. This causes the
install name in binaries to be out of date.

We now change that install name to point to the absolute path to each
built library after installation.

Fixes #3038
Fixes #3077

With this, the default workflow on macOS matches what everyone seems
to do, including Autotools and CMake. The next step is providing a way
for build files to override the install_name that is used after
installation for use with, f.ex., private libraries when combined with
the install_rpath: kwarg on targets.
nirbheek added a commit that referenced this issue Jun 7, 2018
On macOS, we set the install_name for built libraries to
@rpath/libfoo.dylib, and when linking to the library, we set the RPATH
to its path in the build directory. This allows all built binaries to
be run as-is from the build directory (uninstalled).

However, on install, we have to strip all the RPATHs because they
point to the build directory, and we change the install_name of all
built libraries to the absolute path to the library. This causes the
install name in binaries to be out of date.

We now change that install name to point to the absolute path to each
built library after installation.

Fixes #3038
Fixes #3077

With this, the default workflow on macOS matches what everyone seems
to do, including Autotools and CMake. The next step is providing a way
for build files to override the install_name that is used after
installation for use with, f.ex., private libraries when combined with
the install_rpath: kwarg on targets.
nirbheek added a commit that referenced this issue Jun 7, 2018
On macOS, we set the install_name for built libraries to
@rpath/libfoo.dylib, and when linking to the library, we set the RPATH
to its path in the build directory. This allows all built binaries to
be run as-is from the build directory (uninstalled).

However, on install, we have to strip all the RPATHs because they
point to the build directory, and we change the install_name of all
built libraries to the absolute path to the library. This causes the
install name in binaries to be out of date.

We now change that install name to point to the absolute path to each
built library after installation.

Fixes #3038
Fixes #3077

With this, the default workflow on macOS matches what everyone seems
to do, including Autotools and CMake. The next step is providing a way
for build files to override the install_name that is used after
installation for use with, f.ex., private libraries when combined with
the install_rpath: kwarg on targets.
nirbheek added a commit that referenced this issue Jun 8, 2018
This allows us to more aggressively de-dup them, and also sets RPATHs
to all libraries that are not in the system linker paths so that
binaries can be run uninstalled without any special steps.

These RPATHs will be wiped on install, so they do not affect
reproducible builds.

De-duping:
Fixes #2150
Fixes #2118
Fixes #3071

RPATHs:
Fixes #314
Fixes #2881

Also fixes the uninstalled usage portions of:
#3038
#3077
nirbheek added a commit that referenced this issue Jun 8, 2018
On macOS, we set the install_name for built libraries to
@rpath/libfoo.dylib, and when linking to the library, we set the RPATH
to its path in the build directory. This allows all built binaries to
be run as-is from the build directory (uninstalled).

However, on install, we have to strip all the RPATHs because they
point to the build directory, and we change the install_name of all
built libraries to the absolute path to the library. This causes the
install name in binaries to be out of date.

We now change that install name to point to the absolute path to each
built library after installation.

Fixes #3038
Fixes #3077

With this, the default workflow on macOS matches what everyone seems
to do, including Autotools and CMake. The next step is providing a way
for build files to override the install_name that is used after
installation for use with, f.ex., private libraries when combined with
the install_rpath: kwarg on targets.
nirbheek added a commit that referenced this issue Jun 10, 2018
This allows us to more aggressively de-dup them, and also sets RPATHs
to all libraries that are not in the system linker paths so that
binaries can be run uninstalled without any special steps.

These RPATHs will be wiped on install, so they do not affect
reproducible builds.

De-duping:
Fixes #2150
Fixes #2118
Fixes #3071

RPATHs:
Fixes #314
Fixes #2881

Also fixes the uninstalled usage portions of:
#3038
#3077
nirbheek added a commit that referenced this issue Jun 10, 2018
On macOS, we set the install_name for built libraries to
@rpath/libfoo.dylib, and when linking to the library, we set the RPATH
to its path in the build directory. This allows all built binaries to
be run as-is from the build directory (uninstalled).

However, on install, we have to strip all the RPATHs because they
point to the build directory, and we change the install_name of all
built libraries to the absolute path to the library. This causes the
install name in binaries to be out of date.

We now change that install name to point to the absolute path to each
built library after installation.

Fixes #3038
Fixes #3077

With this, the default workflow on macOS matches what everyone seems
to do, including Autotools and CMake. The next step is providing a way
for build files to override the install_name that is used after
installation for use with, f.ex., private libraries when combined with
the install_rpath: kwarg on targets.
nirbheek added a commit that referenced this issue Jun 10, 2018
This allows us to more aggressively de-dup them, and also sets RPATHs
to all libraries that are not in the system linker paths so that
binaries can be run uninstalled without any special steps.

These RPATHs will be wiped on install, so they do not affect
reproducible builds.

De-duping:
Fixes #2150
Fixes #2118
Fixes #3071

RPATHs:
Fixes #314
Fixes #2881

Also fixes the uninstalled usage portions of:
#3038
#3077
nirbheek added a commit that referenced this issue Jun 10, 2018
On macOS, we set the install_name for built libraries to
@rpath/libfoo.dylib, and when linking to the library, we set the RPATH
to its path in the build directory. This allows all built binaries to
be run as-is from the build directory (uninstalled).

However, on install, we have to strip all the RPATHs because they
point to the build directory, and we change the install_name of all
built libraries to the absolute path to the library. This causes the
install name in binaries to be out of date.

We now change that install name to point to the absolute path to each
built library after installation.

Fixes #3038
Fixes #3077

With this, the default workflow on macOS matches what everyone seems
to do, including Autotools and CMake. The next step is providing a way
for build files to override the install_name that is used after
installation for use with, f.ex., private libraries when combined with
the install_rpath: kwarg on targets.
nirbheek added a commit that referenced this issue Jun 18, 2018
This allows us to more aggressively de-dup them, and also sets RPATHs
to all libraries that are not in the system linker paths so that
binaries can be run uninstalled without any special steps.

These RPATHs will be wiped on install, so they do not affect
reproducible builds.

De-duping:
Fixes #2150
Fixes #2118
Fixes #3071

RPATHs:
Fixes #314
Fixes #2881

Also fixes the uninstalled usage portions of:
#3038
#3077
noverby pushed a commit to noverby/meson that referenced this issue Jun 20, 2018
This allows us to more aggressively de-dup them, and also sets RPATHs
to all libraries that are not in the system linker paths so that
binaries can be run uninstalled without any special steps.

These RPATHs will be wiped on install, so they do not affect
reproducible builds.

De-duping:
Fixes mesonbuild#2150
Fixes mesonbuild#2118
Fixes mesonbuild#3071

RPATHs:
Fixes mesonbuild#314
Fixes mesonbuild#2881

Also fixes the uninstalled usage portions of:
mesonbuild#3038
mesonbuild#3077
noverby pushed a commit to noverby/meson that referenced this issue Jun 20, 2018
On macOS, we set the install_name for built libraries to
@rpath/libfoo.dylib, and when linking to the library, we set the RPATH
to its path in the build directory. This allows all built binaries to
be run as-is from the build directory (uninstalled).

However, on install, we have to strip all the RPATHs because they
point to the build directory, and we change the install_name of all
built libraries to the absolute path to the library. This causes the
install name in binaries to be out of date.

We now change that install name to point to the absolute path to each
built library after installation.

Fixes mesonbuild#3038
Fixes mesonbuild#3077

With this, the default workflow on macOS matches what everyone seems
to do, including Autotools and CMake. The next step is providing a way
for build files to override the install_name that is used after
installation for use with, f.ex., private libraries when combined with
the install_rpath: kwarg on targets.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug compilers dependencies OS:macos Issues specific to Apple Operating Systems like MacOS and iOS
Projects
None yet
Development

No branches or pull requests

4 participants