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

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug

Blocking:
issue 473837



Sign in to add a comment

Android binaries must now be PIE (but ICS doesn't support them)

Project Member Reported by primiano@chromium.org, May 14 2014

Issue description

Recent versions of Android require our native binaries (forwarder, md5sum, adb_reboot, purge_ashmem, memdump) to be PIE.
However, the same binaries must be also able to run on our bots running previous versions of Android all the way down to ICS.
Sadly, ICS doesn't seem to support PIE (see b/6587214 and crbug.com/147832).

Summarizing, the situation is the following:
Android (latest AOSP master): supports only PIE binaries.
Android JB and KK: support both
Android ICS: supports only non-PIE binaries.

AFAICT, these are the possible strategies to address this problem:

1) Make the android executables PIE by default and have the Android ICS bots pass some extra gyp define to build the binaries for ICS as non-PIE.

2) Change the gyp files for build both the PIE and non PIE version of each executable and then, at upload time (i.e. in the telemetry and test runner scripts) decide which one to upload.

3) Make the android executables PIE by default and have a wrapper for ICS to help the linker to load them (see later).

In the light of this:

1) Seems a no-go, since the infra team is planning to split builders from testers, which in a nutshell means that the builders (which will build the binaries) will not be aware of the target Android version of the tester.

2) Ends up making the Android gyp files more messy and confusing, since for each binary we'd end up having 4 targets (pie, pie-stripped, nonpie, nonpie-stripped) + the dist folders for component build.
Also, all the call sites in the python scripts should be somehow aware of that and push the right binary.
This also means that we should then duplicate (and maintain) the binaries checked in in GCS for telemetry.
IMHO this would end up looking pretty much horrible and hard to maintain, just to support a case (ICS) which one day will be no more.

3) I managed to write a "run_pie" wrapper (cl coming soon) for supporting PIE on ICS. The idea is to just wrap commands with run_pie /actual/binary args.
The (small) price to pay is two extra gyp flags (only for android tools exe targets) to force the PIE executable to export "main", so the pie wrapper can dlsym it (at least, as long as we'll support ICS).

 
Issue 372744 has been merged into this issue.
Cc: tonyg@chromium.org jbudorick@chromium.org
Summary: Android binaries must now be PIE (but ICS doesn't support them) (was: Android binaries must now be PIE (but ICS doensn't support them))
Project Member

Comment 4 by bugdroid1@chromium.org, May 14 2014

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

commit 7d2b7cf3bb5bed35857222d349267accd2872635
Author: primiano@chromium.org <primiano@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Wed May 14 15:15:19 2014

[Android] Build android tools as PIE and add a wrapper for ICS

- Make Android tools build as position independent executable by
  default, as it now required by Android.
- Introduce a wrapper for running PIE on Android ICS (its linker
  doesn't support PIE).
- Add the plumbing for running the PIE wrapper in the test/telemetry
  python scripts.

BUG= 373219 
NOTRY=true

Review URL: https://codereview.chromium.org/287513002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@270406 0039d316-1c4b-4281-b951-d872f2087c98


Project Member

Comment 5 by bugdroid1@chromium.org, May 14 2014

------------------------------------------------------------------
r270406 | primiano@chromium.org | 2014-05-14T15:15:19.263686Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/build/android/pylib/android_commands.py?r1=270406&r2=270405&pathrev=270406
   M http://src.chromium.org/viewvc/chrome/trunk/src/tools/android/forwarder2/forwarder.gyp?r1=270406&r2=270405&pathrev=270406
   M http://src.chromium.org/viewvc/chrome/trunk/src/tools/android/memdump/memdump.gyp?r1=270406&r2=270405&pathrev=270406
   M http://src.chromium.org/viewvc/chrome/trunk/src/tools/telemetry/bin/android/file_poller.sha1?r1=270406&r2=270405&pathrev=270406
   M http://src.chromium.org/viewvc/chrome/trunk/src/build/common.gypi?r1=270406&r2=270405&pathrev=270406
   A http://src.chromium.org/viewvc/chrome/trunk/src/tools/telemetry/bin/android/run_pie.sha1?r1=270406&r2=270405&pathrev=270406
   M http://src.chromium.org/viewvc/chrome/trunk/src/tools/telemetry/bin/android/md5sum_bin.sha1?r1=270406&r2=270405&pathrev=270406
   M http://src.chromium.org/viewvc/chrome/trunk/src/build/android/provision_devices.py?r1=270406&r2=270405&pathrev=270406
   A http://src.chromium.org/viewvc/chrome/trunk/src/tools/android/run_pie/run_pie.c?r1=270406&r2=270405&pathrev=270406
   M http://src.chromium.org/viewvc/chrome/trunk/src/tools/android/file_poller/file_poller.gyp?r1=270406&r2=270405&pathrev=270406
   M http://src.chromium.org/viewvc/chrome/trunk/src/tools/android/purge_ashmem/purge_ashmem.gyp?r1=270406&r2=270405&pathrev=270406
   M http://src.chromium.org/viewvc/chrome/trunk/src/tools/telemetry/bin/android/device_forwarder.sha1?r1=270406&r2=270405&pathrev=270406
   M http://src.chromium.org/viewvc/chrome/trunk/src/tools/telemetry/bin/android/purge_ashmem.sha1?r1=270406&r2=270405&pathrev=270406
   M http://src.chromium.org/viewvc/chrome/trunk/src/tools/android/android_tools.gyp?r1=270406&r2=270405&pathrev=270406
   A http://src.chromium.org/viewvc/chrome/trunk/src/tools/android/run_pie/run_pie.gyp?r1=270406&r2=270405&pathrev=270406
   M http://src.chromium.org/viewvc/chrome/trunk/src/build/android/pylib/forwarder.py?r1=270406&r2=270405&pathrev=270406
   A http://src.chromium.org/viewvc/chrome/trunk/src/tools/android/run_pie?r1=270406&r2=270405&pathrev=270406
   M http://src.chromium.org/viewvc/chrome/trunk/src/tools/android/ps_ext/ps_ext.gyp?r1=270406&r2=270405&pathrev=270406
   M http://src.chromium.org/viewvc/chrome/trunk/src/tools/telemetry/telemetry/core/platform/android_platform_backend.py?r1=270406&r2=270405&pathrev=270406
   M http://src.chromium.org/viewvc/chrome/trunk/src/tools/telemetry/telemetry/core/backends/adb_commands.py?r1=270406&r2=270405&pathrev=270406

[Android] Build android tools as PIE and add a wrapper for ICS

- Make Android tools build as position independent executable by
  default, as it now required by Android.
- Introduce a wrapper for running PIE on Android ICS (its linker
  doesn't support PIE).
- Add the plumbing for running the PIE wrapper in the test/telemetry
  python scripts.

BUG= 373219 
NOTRY=true

Review URL: https://codereview.chromium.org/287513002
-----------------------------------------------------------------
Project Member

Comment 6 by bugdroid1@chromium.org, May 19 2014

The following revision refers to this bug:
  http://goto.ext.google.com/viewvc/chrome-internal?view=rev&revision=53625

------------------------------------------------------------------
r53625 | primiano@google.com | 2014-05-19T09:24:09.676911Z

-----------------------------------------------------------------
Status: Fixed
Project Member

Comment 8 by bugdroid1@chromium.org, Feb 13 2015

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

commit 2dca4da271cbab6f53832d98cc79a874a4e4244c
Author: raphael.kubo.da.costa <raphael.kubo.da.costa@intel.com>
Date: Fri Feb 13 17:30:49 2015

[Android] Do not overwrite command when using the run_pie wrapper.

Some change starting in M41 has started causing an existing bug in
GetAndroidToolsStatusAndOutput() to fail in ICS devices. When the
run_pie wrapper needs to be used, the call to `adb push' ends up
overwriting the original command that was going to be run on the device,
and the instrumentation tests fail like this:

> LD_LIBRARY_PATH=/data/local/tmp/forwarder/ /data/local/tmp/run_pie push /path/in/host/out/Release/run_pie /data/local/tmp/run_pie; echo %$?
[...]
[PIE Loader] dlopen() failed: Cannot load library: load_library[1095]: Library 'push' not found.

Fix it by using a different variable name for the `adb push' command
that must be run on the host.

BUG= 373219 
R=primiano@chromium.org, torne@chromium.org

Review URL: https://codereview.chromium.org/926023002

Cr-Commit-Position: refs/heads/master@{#316233}

[modify] http://crrev.com/2dca4da271cbab6f53832d98cc79a874a4e4244c/build/android/pylib/android_commands.py

Status: Assigned
primiano: Since ICS was deprecated can we remove all the workarounds that we added in this?

e.g. -fvisibility and -rdynamic flags in common.gypi
Sorry for the delay on this. Yes please, let's dismantle the pile of dark magic I had to do to make this work on ICS to L in the first place!
The only think I need get more info now is the priority of this, but let's discuss this offline.

Blocking: chromium:473837
Came across this today as well.
Status: Fixed
Fixed in https://codereview.chromium.org/1497743004 

Sign in to add a comment