New issue
Advanced search Search tips

Issue 872278 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Make android copy asan lib to build dir instead of relying on llvm version number at test execution time

Project Member Reported by thakis@chromium.org, Aug 8

Issue description

build/android/pylib/valgrind_tools.py

    libs = glob.glob(os.path.join(DIR_SOURCE_ROOT,
                                  'third_party/llvm-build/Release+Asserts/',
                                  'lib/clang/*/lib/linux/',
                                  'libclang_rt.asan-arm-android.so'))


On Mac and Win, we copy the runtime to the build dir here: https://cs.chromium.org/chromium/src/build/config/sanitizers/BUILD.gn?type=cs&q=clang.version+file:%5C.gn&sq=package:chromium&g=0&l=75

On Android, we instead make the original dylib a data dep here:

https://cs.chromium.org/chromium/src/build/config/android/rules.gni?type=cs&q=clang.version+file:%5C.gn&sq=package:chromium&g=0&l=65

  group(_runtime_deps_target_name) {
    data = _sanitizer_runtimes

If we used the mac/win approach on android, we wouldn't need the glob there.

(todo: how does linux do this)

spun off from  issue 746975 
 
Cc: h...@chromium.org
Labels: -OS-Mac OS-Android
Thanks for filing!
Another reason to fix this, build/android/pylib/valgrind_tools.py will pick up the wrong runtime if one tries to build/run with a custom clang_base_path
Oh, I guess on Linux we just always statically link in the asan runtime?
The recipe (which I suppose we used before using swarming on android?) also refers to the location, albeit via --print-clang-revision: https://cs.chromium.org/chromium/build/scripts/slave/recipe_modules/chromium_android/api.py?q=asan_device_setup.s&sq=package:chromium&dr=C&l=706
Hm, the

    data = _sanitizer_runtimes

line is for the ubsan runtime, not the asan runtime. I guess the android asan bots don't use swarming yet (?).

Do we use ubsan on android at all? I wonder why that's there.
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 8

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

commit 7e545610c67d3d05ec6b0951e480d15c572d904e
Author: Nico Weber <thakis@chromium.org>
Date: Wed Aug 08 20:28:47 2018

android/asan: Don't depend on llvm version number at test time.

Instead, copy the runtime to the build dir at build time and find it there
at test time.

Matches how things work on other platforms with the asan runtime in a shared
library (mac/ios/win).

This also makes things do the right thing with a custom clang_base_path.

Bug: 872278
Change-Id: I930bd88206f973c10eb47bfd2f132e70167235f4
Reviewed-on: https://chromium-review.googlesource.com/1167465
Reviewed-by: Hans Wennborg <hans@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581682}
[modify] https://crrev.com/7e545610c67d3d05ec6b0951e480d15c572d904e/build/android/pylib/valgrind_tools.py
[modify] https://crrev.com/7e545610c67d3d05ec6b0951e480d15c572d904e/build/config/sanitizers/BUILD.gn

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 10

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

commit 5f069582b9f87d7953b03993e9707c569722896e
Author: Dirk Pranke <dpranke@chromium.org>
Date: Fri Aug 10 01:58:44 2018

Revert "android/asan: Don't depend on llvm version number at test time."

This reverts commit 7e545610c67d3d05ec6b0951e480d15c572d904e.

Reason for revert: Speculative revert for  crbug.com/872714 

Original change's description:
> android/asan: Don't depend on llvm version number at test time.
> 
> Instead, copy the runtime to the build dir at build time and find it there
> at test time.
> 
> Matches how things work on other platforms with the asan runtime in a shared
> library (mac/ios/win).
> 
> This also makes things do the right thing with a custom clang_base_path.
> 
> Bug: 872278
> Change-Id: I930bd88206f973c10eb47bfd2f132e70167235f4
> Reviewed-on: https://chromium-review.googlesource.com/1167465
> Reviewed-by: Hans Wennborg <hans@chromium.org>
> Commit-Queue: Nico Weber <thakis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#581682}

TBR=thakis@chromium.org,hans@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 872278
Change-Id: Ie2903e4f2a65dc44abe698f3a26c6d370afd1e89
Reviewed-on: https://chromium-review.googlesource.com/1169933
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582012}
[modify] https://crrev.com/5f069582b9f87d7953b03993e9707c569722896e/build/android/pylib/valgrind_tools.py
[modify] https://crrev.com/5f069582b9f87d7953b03993e9707c569722896e/build/config/sanitizers/BUILD.gn

Sign in to add a comment