New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 630322 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

GN ASan build does not work with bundled targets

Project Member Reported by rsesek@chromium.org, Jul 21 2016

Issue description

On Mac, if you set is_asan=true and build browser_tests or or chrome, the program crashes because the ASan library is not found:

% ./out/asan/Chromium.app/Contents/MacOS/Chromium 
dyld: Library not loaded: @executable_path/libclang_rt.asan_osx_dynamic.dylib
  Referenced from: /Volumes/Build/src/out/asan/Chromium.app/Contents/Versions/54.0.2803.0/Chromium Framework.framework/Helpers/crashpad_handler
  Reason: image not found
[0721/121500:ERROR:file_io.cc(30)] read: expected 8, observed 0
dyld: Library not loaded: @executable_path/libclang_rt.asan_osx_dynamic.dylib
  Referenced from: /Volumes/Build/src/out/asan/Chromium.app/Contents/Versions/54.0.2803.0/Chromium Helper.app/Contents/MacOS/Chromium Helper
  Reason: image not found

The libclang_rt.asan_osx_dynamic.dylib has an install_name based on @executable_path, which does not work with bundled executables. We need to copy the runtime into the bundle and fix up the install name.
 

Comment 1 by rsesek@chromium.org, Jul 22 2016

Cc: dpranke@chromium.org glider@chromium.org thakis@chromium.org
This turns out to be a little trickier than anticipated in GN. We cannot use install_name_tool on bundled outputs, because it only operates on the file in-place, which causes overbuild (e.g., https://bugs.chromium.org/p/chromium/issues/detail?id=431177#c135).

I've thought of a few options, and none of them are great.

1) Copy the runtime dylib into Foo.bundle/Contents/MacOS/ for each bundled executable.
2) Copy the runtime dylib into the bundle, modify linker_driver.py (our linker wrapper to do multiple operations atomically) to support changing install_name_tool via GN ldflags, and adjust every bundled executable's install_name for the runtime dylib (keeping it @executable_path-based).
3) Keep the runtime lib in $root_out_dir, change its install_name to be @rpath-based, and just set @rpaths for the bundled executables (this is already done by virtue of the component build).

I was initially leaning towards (2), but there are several executables that get copied into the bundle that are not linked in the same build file as where the bundle is created. (E.g., crashpad_handler and blink_test_plugin are compiled far away from where they are bundled, so they can't really know about a path to inside the bundle, which needs to be set at link-time).

Option (1) is attractive because it is very simple, but it would copy the runtime several times. It could maybe be done with symlinks, too. At the moment, I'm leaning towards (3).

glider: Do you have any thoughts/preferences?
I think #3 is more-or-less what we do on Linux (i.e., adjust the rpath to pick up the asan lib).
I think this is what you had in mind (or I had in mind, or something): https://codereview.chromium.org/2180653002

Comment 4 by glider@chromium.org, Jul 25 2016

In GYP builds there's a special script, build/mac/copy_asan_runtime_dylib.sh, that fixes bundled targets. We just need to make it work with GN.
@glider, thanks for the pointer. I took a look at the script, and it looks like it runs as a postbuild action on every link, which is roughly the implementation of rsesek's choice #1. 

GN doesn't have postbuild actions, so, the direct analog doesn't really work. I suppose we could maybe incorporate the copy_asan_runtime_dylib.sh script into the linker_driver.py script?

But option #3 seems easier ...

Comment 6 by rsesek@chromium.org, Jul 25 2016

copy_asan_runtime_dylib.sh is not going to work in GN because it operates in-place (it just wraps install_name_tool, which doesn't work for the reasons in #1). Furthermore, as Dirk mentions, GN has no concept of postbuild actions, which is how this script is meant to be run.

Of the options I listed above, I do think #3 is the simplest. But talking to Mark today, we came up with another option:

Create a static library stub target that provides symbols/symbol resolvers for the functions declared in libclang_rt.asan_osx_dynamic.dylib. The stubs would be responsible for determining how to locate the asan dylib, dlopen it, and then invoke the real implementation. The stub implementations would be able to locate the dylib either through examining and manipulating the executable's path, or through an environment variable. This is pretty flexible, and it means we wouldn't need to do any rpath manipulation at all.

I think the stub solution is the cleanest option and it should be pretty easy to implement.

Comment 7 by rsesek@chromium.org, Jul 27 2016

I spent yesterday looking at the solutions I outlined in this bug. Some notes:

The most promising idea was to use a special dyld feature called symbol resolvers. These are equivalent to the dyld binding stubs, but they allow you to use your own function to return the address of the symbol to be called. But using a .symbol_resolver does not work because ld only lets certain output types use those stubs (and executables aren't one of them), per Pass::process() http://opensource.apple.com/source/ld64/ld64-264.3.102/src/ld/passes/stubs/stubs.cpp. So in order to use this, we'd need to have a dylib to find the asan dylib... nope!

Next I tried creating a stubs source_set that uses a static initializer to locate the library at runtime and then initialize some function pointers, which the stubs would indirect through. But because initialization order is not deterministic, the asan.module_ctor can get called first, and so the pointers are NULL in the stubs. I thought about doing the initialization at call-time, but then the stubs change from being simple thunks to needing to spill register state, and that gets more complicated than I'd like.

So I wrote up #1 and it's really quite simple: https://codereview.chromium.org/2185833002/. It's also similar to how the GYP copy_asan_runtime_dylib.sh script works, except it unconditionally places the dylib next to the executable, rather than conditionally putting it in Executable/../Libraries/.
Project Member

Comment 8 by bugdroid1@chromium.org, Jul 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1cfe20c37872faa5e4d94b9a3b35959294de9462

commit 1cfe20c37872faa5e4d94b9a3b35959294de9462
Author: rsesek <rsesek@chromium.org>
Date: Wed Jul 27 17:44:51 2016

[Mac/GN] Specify the ASan dynamic runtime as a bundle_data deps.

This causes the ASan library to be copied into a bundle's Contents/MacOS/
directory, so it can be loaded via its @executable_path install name.

BUG= 630322 

Review-Url: https://codereview.chromium.org/2185833002
Cr-Commit-Position: refs/heads/master@{#408174}

[modify] https://crrev.com/1cfe20c37872faa5e4d94b9a3b35959294de9462/build/config/sanitizers/BUILD.gn
[modify] https://crrev.com/1cfe20c37872faa5e4d94b9a3b35959294de9462/chrome/BUILD.gn
[modify] https://crrev.com/1cfe20c37872faa5e4d94b9a3b35959294de9462/content/shell/BUILD.gn

Comment 9 by rsesek@chromium.org, Jul 27 2016

Status: Fixed (was: Started)
Chromium and Content Shell now build and run under ASan, with most tests passing. NaCl is not working, though, and I cannot figure out why. I've filed bug 632059 to investigate that.

For posterity, these are the CLs for the dead ends in #7:
https://codereview.chromium.org/2188473002/
https://codereview.chromium.org/2187063003/
Project Member

Comment 10 by bugdroid1@chromium.org, Jul 28 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aff2a7f0744e477a728d0cacd467445df5b4d4e5

commit aff2a7f0744e477a728d0cacd467445df5b4d4e5
Author: dpranke <dpranke@chromium.org>
Date: Thu Jul 28 06:17:11 2016

Flip the last Mac GYP bots to GN (the ASAN bots):

This flips the remaining Mac builders to GN. A number of
browser_tests that use NaCl are failing under ASAN, so
we disable NaCl for now.

This affects:

- chromium.fyi
  - ClangToTMacASan
- chromium.lkgr
  - Mac ASAN Debug
  - Mac ASAN Release Media
  - Mac ASAN Release
- chromium.memory:
  - Mac ASAN 64 Builder
- tryserver.chromium.mac:
  - mac_chromium_asan_rel_ng

TBR=rsesek@chromium.org, brettw@chromium.org, jyasskin@chromium.org
BUG= 618468 ,  630322 , 632059
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.mac:mac_chromium_asan_rel_ng

Review-Url: https://codereview.chromium.org/2168713003
Cr-Commit-Position: refs/heads/master@{#408345}

[modify] https://crrev.com/aff2a7f0744e477a728d0cacd467445df5b4d4e5/chrome/chrome_tests.gypi
[modify] https://crrev.com/aff2a7f0744e477a728d0cacd467445df5b4d4e5/chrome/test/BUILD.gn
[modify] https://crrev.com/aff2a7f0744e477a728d0cacd467445df5b4d4e5/tools/mb/mb_config.pyl

Project Member

Comment 11 by bugdroid1@chromium.org, Jul 28 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/788943c15aa5614f8b34e1933d4c1b367e4656a2

commit 788943c15aa5614f8b34e1933d4c1b367e4656a2
Author: benwells <benwells@chromium.org>
Date: Thu Jul 28 07:00:53 2016

Revert of Flip the last Mac GYP bots to GN (the ASAN bots) (patchset #4 id:60001 of https://codereview.chromium.org/2168713003/ )

Reason for revert:
The Mac ASAN bot is not compiling after this.

Example stdio: https://build.chromium.org/p/chromium.memory/builders/Mac%20ASan%2064%20Builder/builds/40502/steps/compile/logs/stdio

Original issue's description:
> Flip the last Mac GYP bots to GN (the ASAN bots):
>
> This flips the remaining Mac builders to GN. A number of
> browser_tests that use NaCl are failing under ASAN, so
> we disable NaCl for now.
>
> This affects:
>
> - chromium.fyi
>   - ClangToTMacASan
> - chromium.lkgr
>   - Mac ASAN Debug
>   - Mac ASAN Release Media
>   - Mac ASAN Release
> - chromium.memory:
>   - Mac ASAN 64 Builder
> - tryserver.chromium.mac:
>   - mac_chromium_asan_rel_ng
>
> TBR=rsesek@chromium.org, brettw@chromium.org, jyasskin@chromium.org
> BUG= 618468 ,  630322 , 632059
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.mac:mac_chromium_asan_rel_ng
>
> Committed: https://crrev.com/aff2a7f0744e477a728d0cacd467445df5b4d4e5
> Cr-Commit-Position: refs/heads/master@{#408345}

TBR=rsesek@chromium.org,brettw@chromium.org,jyasskin@chromium.org,kinuko@chromium.org,dpranke@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 618468 ,  630322 , 632059

Review-Url: https://codereview.chromium.org/2184363003
Cr-Commit-Position: refs/heads/master@{#408348}

[modify] https://crrev.com/788943c15aa5614f8b34e1933d4c1b367e4656a2/chrome/chrome_tests.gypi
[modify] https://crrev.com/788943c15aa5614f8b34e1933d4c1b367e4656a2/chrome/test/BUILD.gn
[modify] https://crrev.com/788943c15aa5614f8b34e1933d4c1b367e4656a2/tools/mb/mb_config.pyl

Status: Assigned (was: Fixed)
Looks like the problem is still not fixed, correct?

BTW, Robert, thanks a lot for looking into this! I don't have a Mac OS machine these days.
Status: Fixed (was: Assigned)
No, this is fixed. The bot config was wrong.
Project Member

Comment 14 by bugdroid1@chromium.org, Jul 28 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8c07d4e0af781b15ad65c50f8a9127d4ab5db01f

commit 8c07d4e0af781b15ad65c50f8a9127d4ab5db01f
Author: dpranke <dpranke@chromium.org>
Date: Thu Jul 28 17:32:42 2016

Re-land "Flip the last Mac GYP bots to GN (the ASAN bots)"

This re-lands r408345 with the needed fix to the Mac ASAN 64
Builder on chromium.memory (by turning off lsan).

TBR=rsesek@chromium.org
BUG= 618468 ,  630322 , 632059

Review-Url: https://codereview.chromium.org/2191473006
Cr-Commit-Position: refs/heads/master@{#408422}

[modify] https://crrev.com/8c07d4e0af781b15ad65c50f8a9127d4ab5db01f/chrome/chrome_tests.gypi
[modify] https://crrev.com/8c07d4e0af781b15ad65c50f8a9127d4ab5db01f/chrome/test/BUILD.gn
[modify] https://crrev.com/8c07d4e0af781b15ad65c50f8a9127d4ab5db01f/tools/mb/mb_config.pyl

Project Member

Comment 15 by bugdroid1@chromium.org, Jul 28 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3d6e18f08b7abe178e7abd6babee202b5a763e45

commit 3d6e18f08b7abe178e7abd6babee202b5a763e45
Author: rsesek <rsesek@chromium.org>
Date: Thu Jul 28 20:20:20 2016

[Mac/GN] Fix gn gen when using_sanitizer=true use_custom_libcxx=true.

BUG= 630322 
R=dpranke@chromium.org

Review-Url: https://codereview.chromium.org/2192923003
Cr-Commit-Position: refs/heads/master@{#408459}

[modify] https://crrev.com/3d6e18f08b7abe178e7abd6babee202b5a763e45/build/config/sanitizers/BUILD.gn

Sign in to add a comment