Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

copy ASAN library to app install directory #540

Closed
DanAlbert opened this issue Oct 6, 2017 · 8 comments
Closed

copy ASAN library to app install directory #540

DanAlbert opened this issue Oct 6, 2017 · 8 comments
Milestone

Comments

@DanAlbert
Copy link
Member

Now that Android has wrap.sh, we need to ship the ASAN runtime (or whatever sanitizer runtime) with the other shared libraries that are a part of the app. ndk-build should install this in the out directory with the rest of the shared libs.

cc @eugenis

@DanAlbert DanAlbert added this to the r17 milestone Oct 6, 2017
@eugenis
Copy link
Collaborator

eugenis commented Oct 6, 2017

Yes, please! And it should also generate wrap.sh script.

@DanAlbert
Copy link
Member Author

I was uncertain about that. I was thinking maybe it would be better to let the user do that so they can add any of their own behaviors, but I suppose as long as we allow them to do that as well then there's no problem with us providing a sensible default.

Probably will add an APP_WRAP_SH_SCRIPT or something. Will need to confirm that ndk-build is the sensible place to handle this though, since ndk-build doesn't do any of the app packaging. Might make more sense to do it in gradle.

@DanAlbert
Copy link
Member Author

This actually ended up being a lot easier than I expected: https://android-review.googlesource.com/#/c/platform/ndk/+/618842

Too late for beta 1, but it will be in beta 2.

Note that I haven't looked into the wrap.sh generation yet, but this handles the more involved part of the work.

@DanAlbert
Copy link
Member Author

Will be in beta 1 after all since we're still waiting for gradle fixes.

miodragdinic pushed a commit to MIPS/ndk that referenced this issue Apr 17, 2018
If the app uses any sanitizers, we can install the library to the out
directory to allow them to use wrap.sh rather than root their devices.

This only works for ndk-build due to the nature of CMake toolchain
files.

Test: Removed the sanitizer runtime libraries from my test devices
Test: ./run_tests.py --rebuild --filter asan-smoke
Bug: android/ndk#540
Change-Id: I2944c468a1b34161d792e689c68ad2c391bdd409
(cherry picked from commit d7e5e76ed836ed0c4093d5558d64fabecdfdb1eb)
miodragdinic pushed a commit to MIPS/ndk that referenced this issue Apr 17, 2018
Test: ./checkbuild.py && ./run_tests.py
Bug: android/ndk#540
Change-Id: I36990c3f905611e2c56a5f89c1cee4bd54c8bdb1
(cherry picked from commit 87bda31b284539a2e1e4455fcfbbf6aa4e706168)
miodragdinic pushed a commit to MIPS/ndk that referenced this issue Apr 17, 2018
I used the wrong variable to check the sanitizer flags, so it was
always using ASAN, causing the asan wrap.sh to be installed unless
the user specified their own.

Fix the problem and add a test case to make sure this is only
installed if it's requested.

Also clean up some debug logging that was accidentally left in.

Test: ./run_tests.py --rebuild
Bug: android/ndk#540
Change-Id: I0e6d4b0222a4aeb1b491f6869c338011b74e2e8f
(cherry picked from commit 38b57e4af408b0df9288f110dd6cffa0edef30ba)
miodragdinic pushed a commit to MIPS/ndk that referenced this issue Apr 17, 2018
Deal with the `-fsanitize=address,undefined` syntax, as well as cases
like `-fsanitize=address -fno-sanitize=address`.

Test: ./run_tests.py --rebuild
Test: Manually checked a few cases with more complicated flags
Test: nose2 build
Bug: android/ndk#540
Change-Id: Ib3e740cce53f5007e1d1218898c4c15a19b94922
(cherry picked from commit 4d958275a1026b6d1f3e9c6dd0ff45b6c6d57b8d)
miodragdinic pushed a commit to MIPS/ndk that referenced this issue Apr 17, 2018
The Python tests were passing because we only tested the individual
functions, not the program as a whole.

The run_tests.py tests were passing because errors from $(shell) do
not cause the build to fail; ndk-build just ended up parsing the
error message and not finding any sanitizers.

The asan-smoke test passed because A) I hadn't run `./run_tests.py
--clean-device` to purge the old tests, so the libraries were still
on the device and B) running the tests in the r17 branch had led to
asan_device_setup running again, installing the libraries to the
system partition once again.

Test: nose2 build
Test: ./run_tests.py --clean-device  # After purging asan /system libs
Bug: android/ndk#540
Change-Id: I85a30cc198be4a0f39548271f56112835805ca76
@sonicdebris
Copy link

The docs at https://github.com/google/sanitizers/wiki/AddressSanitizerOnAndroid are still indicating the need for a rooted phone. Could we get updated info and tips on that page about how to setup asan with gradle and cmake?

@eugenis
Copy link
Collaborator

eugenis commented May 21, 2018

I'd be happy to update that page or accept a pull request, but I don't know much about Gradle.

@enh
Copy link
Contributor

enh commented May 22, 2018

@yabinc is probably our wrap.sh + gradle expert...

@yabinc
Copy link
Collaborator

yabinc commented May 22, 2018

Here is the part of doc in simpleperf showing how to add wrap.sh, https://android.googlesource.com/platform/prebuilts/simpleperf/+/master/doc/#prepare-an-android-application.
Here is an example of gradle code adding wrap.sh, https://android.googlesource.com/platform/system/extras/+/master/simpleperf/demo/SimpleperfExampleOfKotlin/app/profiling.gradle.
Hopefully that can give some help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants