android / ndk

The Android Native Development Kit
2.02k stars 259 forks source link

Make it easier to use asan with cmake #840

Open enh opened 6 years ago

enh commented 6 years ago

at ADS18 we had someone asking about asan. enh said it was hard, danalbert said it just worked with ndk-build, but the questioner asked "what about cmake?"...

apparently cmake doesn't like the idea that you need to bundle a couple of extra libraries sometimes. (we have similar problems with wrap.sh and cmake.)

DanAlbert commented 5 years ago

I'm still not totally sure what this should look like. Currently I think the best path is to ship an Android.cmake module that includes helper functions for the NDK. One of those helper functions would add rules to copy the ASan runtime and wrap.sh to the out directory. However, that would require that the CMakeLists.txt differ for Android, and typically we want a CMakeLists.txt made for a different platform to Just Work for Android. Alternatively, this could be handled by Gradle (or maybe have both options).

In other words, since I'm still not sure what form this should take, moving to r21. If anyone has opinions on which of those options would be best (or better ideas), lmk.

sonicdebris commented 5 years ago

Hello, I finally took some time and managed to enable address sanitizer with cmake, and documented the process in a small sample app: https://github.com/sonicdebris/asan-android-test

In short, I have cmake grab the asan shared libraries from the ndk toolchain directory and put them in CMAKE_LIBRARY_OUTPUT_DIRECTORY, so they can be packaged in the APK.

As for the wrap.sh scripts, I put them in a java resource directory, so they get automatically picked and packaged in the APK, too. I actually use a custom resources location, that's added to a new sanitize gradle build type, so the release and debug build are not affected.

These are the major pain points in setting up the whole thing: 1) Configuration is split between gradle (c++ runtime selection, wrap scripts directory setup) and cmake (compiler flags setup, asan libraries copy) 2) The asan libraries are retrieved from the toolchain directory. I tried to keep that part flexible using wildcards and GLOB, but it feels quite fragile 3) There's apparently no way to easily map the ANDROID_ABI to the architecture (as used in the asan library names) 4) I had to deal with putting the wrap scripts in the right place and making sure they are packaged only for the build type I wanted, even though I just copied the ones available in the NDK distribution.

I understand you are trying to minimize the android-specific configuration in cmakelists but in our case, with multi-platform development, there's already quite a few android-specific things in our cmakelists anyway. As I came to "see" it, the android cmakelists ends up defining an android-only target (with its configuration flags, additional android-only and jni glue code), that can in turn depend on additional, "generic" targets defined in another cmakelist. So I would really consider the idea of moving more of the configuration logic to the cmake side (at least putting it on par with ndk-build).

Regarding the wrap scripts, the build system could at least package the default ones if there's none in the resources directory and the sanitizer is enabled.

DanAlbert commented 5 years ago

Thanks for looking at this. I'm hoping that we can make the interface for these more or less identical to how sanitizers are used on any other platform. For example:

target_compile_options(foo
  PRIVATE
    -fsanitize=address
)

and that's it... That's all we have to do to use ASan in ndk-build, so that should be our goal with CMake. This isn't currently possible because the toolchain file has no ability to inspect targets.

Another option would be to add handling for sanitizers directly to CMake in such a way that each target can override the behavior if needed. Something like:

# Possibly android_sanitize, but this seems like it would be generally useful.
sanitize(foo address) 

Either of these approaches would then set up the installation of the sanitizer library and wrap.sh files as needed automatically.

Unfortunately, I suspect that nothing this clean is doable until we have the time to redo our CMake implementation to match the upstream interface (https://github.com/android-ndk/ndk/issues/463).

DanAlbert commented 5 years ago

Probably something we'll tackle post (or as part of) the CMake implementation cleanup. Don't know exactly which release that will be, so moving into the unplanned bucket (it's planned, just not a known release).

rpattabi commented 4 years ago

Any update on this? The documentation is decent. However, it is such a pain to set it up. We don't always want this to be enabled. It is difficult to disable it as it involves removal of files and reverting at different places.

DanAlbert commented 4 years ago

No update. The CMake cleanup I mention above hasn't happened yet. It might be something we can start on this year, but there's other work that needs to be finished first and it's not clear how long that will take.

It is difficult to disable it as it involves removal of files and reverting at different places.

You can set the various flags dependent on your cmake build type, and I believe you can use packagingOptions in your build.gradle to avoid including the runtimes in that situation (there might be a better way, I'm far from an expert on the gradle plugin).

rpattabi commented 4 years ago

I see this is more for AGP while the initiative is from NDK. The challenge in getting it done is about coordinating across teams across priorities, I guess. Thanks for the update.