New issue
Advanced search Search tips

Issue 853716 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 3
Type: Bug

Blocked on:
issue 850055

Blocking:
issue 843511



Sign in to add a comment

content_shell_crash_test failing again on ToTMac bot

Project Member Reported by thakis@chromium.org, Jun 18 2018

Issue description

Started after my rpath removal CL https://chromium-review.googlesource.com/1102056

Error:
https://chromium-swarm.appspot.com/task?id=3e1e88ddf6fefb10&refresh=10&show_raw=1

ERROR: failed to resolve @rpath/libbindings.dylib, exe_path ./Content Shell.app/Contents/Frameworks/Content Shell Framework.framework/Versions/Current, loader_path , rpaths /., /../../..


So loader_path ends up empty.

  loader_path = os.path.dirname(binary)

probably should be

  loader_path = os.path.dirname(binary) or '.'


But why wasn't this needed earlier?

The rpath stack stuff was added in #566140, https://chromium-review.googlesource.com/c/chromium/src/+/1093569 landed in #566651 . So I guess that added a dep on the rpath stack behavior?
 

Comment 1 by thakis@chromium.org, Jun 18 2018

To run locally:
cd out/gn
../../testing/scripts/content_shell_crash_test.py --isolated-script-test-output foo.json

Comment 2 by thakis@chromium.org, Jun 18 2018

https://chromium-review.googlesource.com/c/chromium/src/+/1093569 did change the rpath from ../../../.. to ../../../../.. which does make standalone ` Content Shell.app/Contents/Frameworks/Content Shell Framework.framework/Versions/Current/Content Shell Framework` symbolize fine, but the shell links to the symlink at  Content Shell.app/Contents/Frameworks/Content Shell Framework.framework/Content Shell Framework for which the old rpath was correct. I guess the generate script needs to look through symlinks.

Comment 3 by thakis@chromium.org, Jun 18 2018

Ah no, that's because I haven't rebuilt locally, the shell no longer links to the framework at all at head.

Comment 4 by thakis@chromium.org, Jun 18 2018

$ otool -L './Content Shell.app/Contents/MacOS/Content Shell'
./Content Shell.app/Contents/MacOS/Content Shell:
	/System/Library/Frameworks/Cocoa.framework/Versions/A/Cocoa (compatibility version 1.0.0, current version 22.0.0)
	/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation (compatibility version 300.0.0, current version 1450.15.0)
	/System/Library/Frameworks/IOKit.framework/Versions/A/IOKit (compatibility version 1.0.0, current version 275.0.0)
	/System/Library/Frameworks/Security.framework/Versions/A/Security (compatibility version 1.0.0, current version 58286.31.2)
	/System/Library/Frameworks/SystemConfiguration.framework/Versions/A/SystemConfiguration (compatibility version 1.0.0, current version 963.30.1)
	/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 400.9.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1252.0.0)

Comment 5 by thakis@chromium.org, Jun 18 2018

Ok, repros locally and the patch in comment 0 works. But why wasn't it needed before #566140?

Comment 6 by thakis@chromium.org, Jun 18 2018

Hm I guess it didn't work locally before https://chromium-review.googlesource.com/1085864 / #564472 either. Maybe that was why it 'worked'?

Comment 7 by thakis@chromium.org, Jun 18 2018

Hm, the error checking is new in https://chromium-review.googlesource.com/1085864 / #564472, so I guess previously we were able to resolve the main app (or now, framework) but then silently failed to resolve stuff in the components -- but almost all of them got symbolized as part of the main app / framework dep symbolization already so it didn't really matter in practice?
Project Member

Comment 9 by bugdroid1@chromium.org, Jun 18 2018

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

commit 1f418de3b71ec13f45955d44e14b260c1fcaea4c
Author: Nico Weber <thakis@chromium.org>
Date: Mon Jun 18 15:50:13 2018

mac: Fix bug in generate_breakpad_symbols.py exposed by rpath stack removal.

The framework depends on libfoo.dylib, and when we symbolize this dependency,
we use dirname(libfoo.dylib) -> '' as @loader_path, turning the rpath
@loader_path/. into the absolute path /.

Call realname() on the file before calling dirname to fix this, and to
fix a second (latent) bug where the rpaths should be relative to the
symlink targets, not to symlinks.

(I think this was always broken, but harmless before I made
GetSharedLibraryDependenciesMac() error out on resolving failures, and
harmless in practice since the framework itself symbolized itself and
its deps fine, just resolving the deps of the deps would silently fail
before I made it fail loudly.)

Bug:  853716 
Change-Id: I519f786f785bcae8b5d64edc8ff1a883edba5c72
Reviewed-on: https://chromium-review.googlesource.com/1104393
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568018}
[modify] https://crrev.com/1f418de3b71ec13f45955d44e14b260c1fcaea4c/components/crash/content/tools/generate_breakpad_symbols.py

Status: Fixed (was: Started)
Bot is happy (well, happier at least) again.

Sign in to add a comment