Switch to the "new" way of ASan deployment on Android devices.

Switch to the "new" way of ASan deployment on Android devices.

This will elimitate startup slowdown in ASan/Android tests, and
improve overall run time very significantly.
For the idea behind this change, refer to
https://code.google.com/p/address-sanitizer/wiki/Android#Running_NDK_applications_with_ASan

R=bulach@chromium.org, frankf@chromium.org, glider@chromium.org, navabi@chromium.org
BUG=

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=246847

[eugenis, 2014-01-17 12:32:09]

I'm adding a new copy of libclang_rt.asan-arm-android.so, because the one in
third_party/llvm-build is broken ATM, and out-of-date anyway. I'll change the
script to use the llvm-build copy when this issues are resolved.

https://codereview.chromium.org/138153003/diff/1/build/android/pylib/valgrind...
File build/android/pylib/valgrind_tools.py (right):

https://codereview.chromium.org/138153003/diff/1/build/android/pylib/valgrind...
build/android/pylib/valgrind_tools.py:102: self._adb.Reboot()
Reboot is required after asan_device_setup.sh.
Unfortunately, this way we reboot the device twice before running the tests.
Not a big deal, probably.

[eugenis, 2014-01-17 12:38:39]

Oh, and I want to use asan_device_setup.sh as standalone script, potentially
outside of chromium, that's why it's in shell and not part of
build/android/pylib.

[Alexander Potapenko, 2014-01-17 12:43:49]

LGTM if others are ok with this temporary .so in the repository.

https://codereview.chromium.org/138153003/diff/1/build/android/pylib/valgrind...
File build/android/pylib/valgrind_tools.py (right):

https://codereview.chromium.org/138153003/diff/1/build/android/pylib/valgrind...
build/android/pylib/valgrind_tools.py:101:
"tools/android/asan/asan_device_setup.sh"))
Looks like this script uses single quotes for strings.

https://codereview.chromium.org/138153003/diff/1/build/android/pylib/valgrind...
build/android/pylib/valgrind_tools.py:102: self._adb.Reboot()
On 2014/01/17 12:32:10, eugenis wrote:
> Reboot is required after asan_device_setup.sh.
> Unfortunately, this way we reboot the device twice before running the tests.
> Not a big deal, probably.

Please put this into a comment.

https://codereview.chromium.org/138153003/diff/1/tools/android/asan/asan_devi...
File tools/android/asan/asan_device_setup.sh (right):

https://codereview.chromium.org/138153003/diff/1/tools/android/asan/asan_devi...
tools/android/asan/asan_device_setup.sh:17: if [[ x$1 != x ]]; then
Better quote both operands.

[eugenis, 2014-01-17 12:48:08]

https://codereview.chromium.org/138153003/diff/1/build/android/pylib/valgrind...
File build/android/pylib/valgrind_tools.py (right):

https://codereview.chromium.org/138153003/diff/1/build/android/pylib/valgrind...
build/android/pylib/valgrind_tools.py:101:
"tools/android/asan/asan_device_setup.sh"))
On 2014/01/17 12:43:50, Alexander Potapenko wrote:
> Looks like this script uses single quotes for strings.

Done.

https://codereview.chromium.org/138153003/diff/1/build/android/pylib/valgrind...
build/android/pylib/valgrind_tools.py:102: self._adb.Reboot()
On 2014/01/17 12:43:50, Alexander Potapenko wrote:
> On 2014/01/17 12:32:10, eugenis wrote:
> > Reboot is required after asan_device_setup.sh.
> > Unfortunately, this way we reboot the device twice before running the tests.
> > Not a big deal, probably.
> 
> Please put this into a comment.

Done.

[bulach, 2014-01-17 13:27:52]

lgtm, this is neat! please wait for armand and frank.

also, I think doing the extra script in python would be more inline with the
rest of the scripts, but no strong feelings..

https://codereview.chromium.org/138153003/diff/120001/build/android/pylib/val...
File build/android/pylib/valgrind_tools.py (right):

https://codereview.chromium.org/138153003/diff/120001/build/android/pylib/val...
build/android/pylib/valgrind_tools.py:103: # Reboot is required after
asan_device_setup.sh.
is it always required? can we detect if the property that requires reboot has
actually changed and then reboot after that?
also... does it need a full reboot or would adb shell stop && adb shell start be
enough?

https://codereview.chromium.org/138153003/diff/120001/tools/android/asan/asan...
File tools/android/asan/asan_device_setup.sh (right):

https://codereview.chromium.org/138153003/diff/120001/tools/android/asan/asan...
tools/android/asan/asan_device_setup.sh:14: HERE="$(cd "$(dirname "$0")" &&
pwd)"
every time we start with bash scripts, we end up converting them to python,
since there are *tons* of utilities there already.
I understand this is asan-specific, but would there be any chance of rewriting
it?
would be better with proper options, say, --device, and perhaps --revert?

https://codereview.chromium.org/138153003/diff/120001/tools/android/asan/asan...
tools/android/asan/asan_device_setup.sh:85: rm -r "$TMPDIR"
nit: if people would be running this as a standalone script, then it should say
something about "Please reboot device now"...
if it's not meant to be used standalone, let's do it in python ;)

[frankf, 2014-01-18 01:55:56]

lgtm

https://codereview.chromium.org/138153003/diff/120001/build/android/pylib/val...
File build/android/pylib/valgrind_tools.py (right):

https://codereview.chromium.org/138153003/diff/120001/build/android/pylib/val...
build/android/pylib/valgrind_tools.py:1: # Copyright (c) 2012 The Chromium
Authors. All rights reserved.
You should also move this into pylib/utils in a future CL

https://codereview.chromium.org/138153003/diff/120001/tools/android/asan/asan...
File tools/android/asan/asan_device_setup.sh (right):

https://codereview.chromium.org/138153003/diff/120001/tools/android/asan/asan...
tools/android/asan/asan_device_setup.sh:14: HERE="$(cd "$(dirname "$0")" &&
pwd)"
On 2014/01/17 13:27:53, bulach_ooo_until_feb_2 wrote:
> every time we start with bash scripts, we end up converting them to python,
> since there are *tons* of utilities there already.
> I understand this is asan-specific, but would there be any chance of rewriting
> it?
> would be better with proper options, say, --device, and perhaps --revert?

Seconded. But I'm OK with doing the conversion as a follow up.

[eugenis, 2014-01-20 13:51:17]

https://codereview.chromium.org/138153003/diff/120001/build/android/pylib/val...
File build/android/pylib/valgrind_tools.py (right):

https://codereview.chromium.org/138153003/diff/120001/build/android/pylib/val...
build/android/pylib/valgrind_tools.py:103: # Reboot is required after
asan_device_setup.sh.
On 2014/01/17 13:27:53, bulach_ooo_until_feb_2 wrote:
> is it always required? can we detect if the property that requires reboot has
> actually changed and then reboot after that?

Yes, definitely.

> also... does it need a full reboot or would adb shell stop && adb shell start
be
> enough?

I think start & stop would be enough. All we need is to restart Zygote process.
It can be done as part of asan_device_setup.sh, too, if we can reliably detect
when it is needed.

https://codereview.chromium.org/138153003/diff/120001/tools/android/asan/asan...
File tools/android/asan/asan_device_setup.sh (right):

https://codereview.chromium.org/138153003/diff/120001/tools/android/asan/asan...
tools/android/asan/asan_device_setup.sh:14: HERE="$(cd "$(dirname "$0")" &&
pwd)"
On 2014/01/18 01:55:57, frankf wrote:
> On 2014/01/17 13:27:53, bulach_ooo_until_feb_2 wrote:
> > every time we start with bash scripts, we end up converting them to python,
> > since there are *tons* of utilities there already.
> > I understand this is asan-specific, but would there be any chance of
rewriting
> > it?
> > would be better with proper options, say, --device, and perhaps --revert?
> 
> Seconded. But I'm OK with doing the conversion as a follow up.

This is definitely meant to be used as a standalone script, too. People would
need it, for example, to run an asan-instrumented APK (browser or any other
app), and asking them to bring the entire chromium source is way too much.
I'll commit the script to LLVM repository, and this will be a chromium version,
the same as asan_symbolize.py.

We can rewrite it in python, of course, but then we will end up with 2 places
that must be updated if we decide to change the process.

--device and --revert are great ideas, I'll implement them before commit.

[eugenis, 2014-01-20 14:51:11]

PTAL

[eugenis, 2014-01-21 11:33:09]

Removed ASAN_OPTIONS from asanwrapper. They are not necessary and can be set in
the environment by the caller.

[eugenis, 2014-01-21 15:09:07]

On 2014/01/21 11:33:09, eugenis wrote:
> Removed ASAN_OPTIONS from asanwrapper. They are not necessary and can be set
in
> the environment by the caller.

Moved all chromium-specific ASAN_OPTIONS to valgrind_tools.py

[eugenis, 2014-01-22 13:53:32]

On 2014/01/21 15:09:07, eugenis wrote:
> On 2014/01/21 11:33:09, eugenis wrote:
> > Removed ASAN_OPTIONS from asanwrapper. They are not necessary and can be set
> in
> > the environment by the caller.
> 
> Moved all chromium-specific ASAN_OPTIONS to valgrind_tools.py

Anyone wants to take another look? There have been significant changes since the
last lgtm.

[bulach, 2014-01-24 05:51:21]

lgtm, thanks! tiny suggestion:

https://codereview.chromium.org/138153003/diff/250002/tools/android/asan/asan...
File tools/android/asan/asan_device_setup.sh (right):

https://codereview.chromium.org/138153003/diff/250002/tools/android/asan/asan...
tools/android/asan/asan_device_setup.sh:6: # Prepare Android device to run ASan
application.
nit: since this is going to be a standalone tool, may want to add a note say
that it's asynchronous, and the caller should somehow wait for the device to
fully come back up (i.e., WaitForDevicePM)

[eugenis, 2014-01-24 07:52:00]

https://codereview.chromium.org/138153003/diff/250002/tools/android/asan/asan...
File tools/android/asan/asan_device_setup.sh (right):

https://codereview.chromium.org/138153003/diff/250002/tools/android/asan/asan...
tools/android/asan/asan_device_setup.sh:6: # Prepare Android device to run ASan
application.
On 2014/01/24 05:51:22, bulach_ooo_until_feb_2 wrote:
> nit: since this is going to be a standalone tool, may want to add a note say
> that it's asynchronous, and the caller should somehow wait for the device to
> fully come back up (i.e., WaitForDevicePM)

Done.

[eugenis, 2014-01-24 12:35:15]

Committed patchset #12 manually as r246847 (presubmit successful).