Building sanitizers for Android

The build for the Android sanitizers is unique in that it needs to
link against the Android system libraries to create a shared object
and its test suites. The current solution to build ASan is to drop
the compiler-rt repo into the llvm source tree and cross-compile the
llvm build for Android. This is a bit awkward for few reasons:

1) Not all of llvm can be cross-compiled for Android. "ninja all"
causes expected build errors.
2) The sanitizers don't depend on any LLVM libraries, only llvm-config
for its build configuration.
3) No "install" rule. Instead, you cherry-pick files from the build directory.

Building against the LLVM install directory cleans this up nicely and
as it turns out, this mostly works today (see CMake configuration
below). The only missing pieces are that the shared object is not
added to the install directory and the test suites are not built. Is
this a build configuration you'd consider using?

$ cmake -G Ninja .. \
    -DCMAKE_INSTALL_PREFIX=ship \
    -DCMAKE_C_COMPILER=arm-linux-androideabi-gcc \
    -DCMAKE_CXX_COMPILER=arm-linux-androideabi-g++ \
    -DCMAKE_PREFIX_PATH=`pwd`/../../llvm/out/ship \
    -DANDROID=1 \
    -DCMAKE_SYSTEM_NAME=Linux \
    -DCMAKE_C_FLAGS=--sysroot=$(ndkDir)/platforms/android-19/arch-arm \
    -DCMAKE_CXX_FLAGS=--sysroot=$(ndkDir)/platforms/android-19/arch-arm
...
$ ninja install
...
[68/68]
-- Install configuration: "Release"
-- Installing: ship/include/sanitizer/asan_interface.h
-- Installing: ship/include/sanitizer/common_interface_defs.h
-- Installing: ship/include/sanitizer/dfsan_interface.h
-- Installing: ship/include/sanitizer/linux_syscall_hooks.h
-- Installing: ship/include/sanitizer/lsan_interface.h
-- Installing: ship/include/sanitizer/msan_interface.h
-- Installing: ship/include/sanitizer/tsan_interface_atomic.h
-- Installing: ship/asan_blacklist.txt
-- Installing: ship/bin/asan_device_setup

Thanks,
Greg

I'd prefer something based on Alexey's approach with CMake
sub-projects. I think it already copies compiler-rt build products to
the parent build directory, right?

Requiring that llvm is installed complicates things.

The build for the Android sanitizers is unique in that it needs to
link against the Android system libraries to create a shared object
and its test suites. The current solution to build ASan is to drop
the compiler-rt repo into the llvm source tree and cross-compile the
llvm build for Android. This is a bit awkward for few reasons:

1) Not all of llvm can be cross-compiled for Android. "ninja all"
causes expected build errors.

This should not be too hard to fix.

Alexey's approach with CMake sub-projects.

I prefer that direction as well, but what I've proposed is a solution
that works today. To support cross-compilation, we'll need to loop
over each supported arch (llvm-config --targets-built), then loop over
each supported triple for each arch (hard-coded map?), and then pair
up each triple with a sysroot (system paths provided by the user).

I think it already copies
compiler-rt build products to the parent build directory, right?

The Android compiler-rt build does this, but in an unnecessarily
complex way. My proposal is a simplification of that process. What
is described in "llvm/cmake/platforms/Android.cmake" is to build
llvm+clang for the host architecture and then run a second build to
cross-compile llvm+compiler-rt to get a the ASan shared object and
test suite. Instead, I propose building only llvm for the host
architecture (for the purpose of building llvm-config) and then only
compile compiler-rt for Android. No need to build clang or to
cross-compile llvm.

Requiring that llvm is installed complicates things.

Sorry for the confusion. By "llvm install directory", I don't mean to
install llvm to the root directory. I'm referring to the intermediary
directory pointed to by CMAKE_INSTALL_PREFIX. When you run "ninja
install", the build populates this directory with only the libs,
headers, docs, and executables that the LLVM build intends customers
to use directly. In the case of compiler-rt, we need LLVM's
"bin/llvm-config" to determine the build mode, flags, and targets
available.

This should not be too hard to fix.

Porting the LLVM build to Android? That seems like a more challenging
solution than to cut out the unused cross-compiled llvm build.

-Greg

Alexey's approach with CMake sub-projects.

I prefer that direction as well, but what I've proposed is a solution
that works today. To support cross-compilation, we'll need to loop
over each supported arch (llvm-config --targets-built), then loop over
each supported triple for each arch (hard-coded map?), and then pair
up each triple with a sysroot (system paths provided by the user).

I think it already copies
compiler-rt build products to the parent build directory, right?

The Android compiler-rt build does this, but in an unnecessarily
complex way. My proposal is a simplification of that process. What
is described in "llvm/cmake/platforms/Android.cmake" is to build
llvm+clang for the host architecture and then run a second build to
cross-compile llvm+compiler-rt to get a the ASan shared object and
test suite. Instead, I propose building only llvm for the host
architecture (for the purpose of building llvm-config) and then only
compile compiler-rt for Android. No need to build clang or to
cross-compile llvm.

Requiring that llvm is installed complicates things.

Sorry for the confusion. By "llvm install directory", I don't mean to
install llvm to the root directory. I'm referring to the intermediary
directory pointed to by CMAKE_INSTALL_PREFIX. When you run "ninja
install", the build populates this directory with only the libs,
headers, docs, and executables that the LLVM build intends customers
to use directly. In the case of compiler-rt, we need LLVM's
"bin/llvm-config" to determine the build mode, flags, and targets
available.

Yes, so "ninja install" is optional. This should work with LLVM build
directory just as well.

This should not be too hard to fix.

Porting the LLVM build to Android? That seems like a more challenging
solution than to cut out the unused cross-compiled llvm build.

Note that ASan tests on Android require llvm-symbolizer binary.
Building just compiler-rt for the target would not be enough.

Note that ASan tests on Android require llvm-symbolizer binary.

That's a really good point. And I see that llvm-symbolizer can't just
be pulled into compiler-rt because it has dependencies on DebugInfo,
Object, and Support libraries.

This throws a big wrench in Alexey's plan to have the native
compiler-rt build generate the cross-compiled binaries for all
supported targets. We would need to do the same in the llvm repo.
The alternative is to run a separate cross-compiled build for each
architecture. :-/

Yes, so "ninja install" is optional.

But the 'install' depends on the 'all' target.

-Greg

> Note that ASan tests on Android require llvm-symbolizer binary.

That's a really good point. And I see that llvm-symbolizer can't just
be pulled into compiler-rt because it has dependencies on DebugInfo,
Object, and Support libraries.

This throws a big wrench in Alexey's plan to have the native
compiler-rt build generate the cross-compiled binaries for all
supported targets. We would need to do the same in the llvm repo.
The alternative is to run a separate cross-compiled build for each
architecture. :-/

Yes, this llvm-symbolizer part is frustrating... I though we can get away
with it at first, as llvm-symbolizer is optional, and we generally won't be
able
to produce working *binaries* for architectures different from the host one
(we might not have a linker, for instance).
Android with its NDK looks like an exception, though.

It does sound like Android is better suited for "honest"
cross-compilation, rather than "build compiler-rt for all targets we
can find" model.

I'm still not convinced that we must require the "ninja install" step.
Could we just "ninja clang" and then build the second stage against
the first stage build directory? Will this "find_package" thing not
work that way, or do you mean it as a way to copy target asan runtime
where the first stage compiler will find it (or probably both)?

Alexey, Evgeniy,

I propose the following steps to unify multi-arch support in compiler-rt:

1) The compiler-rt test suite adds "-L${CMAKE_CURRENT_BINARY_DIR}/lib"
to its 'clang' variables. This way we can test the sanitizers without
installing any libs to the just-built-clang install directory.

2) The "clang/runtime" build calls ExternalProject once for each arch
it needs of compiler-rt. So once to create x86_64 libs and once for
i386 libs.

3) The compiler-rt build drops the ${arch} suffix from its libs, and
the "clang/runtime" build uses CMAKE_INSTALL_PREFIX to control where
the compiler-rt libs go.
"CMAKE_INSTALL_PREFIX=${CMAKE_CURRENT_BINARY_DIR}/lib/clang/${CLANG_VERSION}/x86_64-linux"

4) Remove multi-arch support from the compiler-rt build. Instead,
declare compiler-rt as an "any-arch" build, configured with:
    CMAKE_CXX_COMPILER (defaults to just-built-clang)
    CMAKE_CXX_FLAGS (defaults to llvm's default target)
    COMPILER_RT_TEST_COMPILER (defaults to CMAKE_CXX_COMPILER)
    COMPILER_RT_TEST_FLAGS (defaults to CMAKE_CXX_FLAGS)

Thoughts?

Thanks,
Greg

Hi Greg,

Alexey, Evgeniy,

I propose the following steps to unify multi-arch support in compiler-rt:

1) The compiler-rt test suite adds "-L${CMAKE_CURRENT_BINARY_DIR}/lib"
to its 'clang' variables. This way we can test the sanitizers without
installing any libs to the just-built-clang install directory.

This is possible, but please keep in mind that:
1) we would still want to use compiler-rt test-suite in a standalone mode,
to test fully built/installed toolchains,
and even GCC.
2) Adding -L... wouldn't work: Clang driver links the static xsan runtimes
from a hardcoded paths in Clang resource directory,
and doesn't add flags like "-lasan -L/path/to/clang/resource/dir". I find
this behavior reasonable.

I don't see a problem with the current approach - we can make "run
sanitizer test suite" command in the
top-level build tree depend on "build/install compiler-rt ExternalProject"
steps (see how it's done currently).

2) The "clang/runtime" build calls ExternalProject once for each arch
it needs of compiler-rt. So once to create x86_64 libs and once for
i386 libs.

3) The compiler-rt build drops the ${arch} suffix from its libs, and
the "clang/runtime" build uses CMAKE_INSTALL_PREFIX to control where
the compiler-rt libs go.

"CMAKE_INSTALL_PREFIX=${CMAKE_CURRENT_BINARY_DIR}/lib/clang/${CLANG_VERSION}/x86_64-linux"

4) Remove multi-arch support from the compiler-rt build. Instead,
declare compiler-rt as an "any-arch" build, configured with:
    CMAKE_CXX_COMPILER (defaults to just-built-clang)
    CMAKE_CXX_FLAGS (defaults to llvm's default target)
    COMPILER_RT_TEST_COMPILER (defaults to CMAKE_CXX_COMPILER)
    COMPILER_RT_TEST_FLAGS (defaults to CMAKE_CXX_FLAGS)

Unfortunately, this could be problematic for Mac - we use "universal
binaries" there - a single static/dynamic runtime
which contains runtimes for all the possible architectures. We might work
around this, however, by adding a custom
Mac-specific step that would merge all single-arch binaries into a
multi-arch one - I think this is fine as long as we
greatly simplify the rest of the ugly build system.

Do you want to give it a try?

we would still want to use compiler-rt test-suite in a standalone mode, to test fully built/installed toolchains,

and even GCC.

Sounds good.

Clang driver links the static xsan runtimes from a hardcoded
paths in Clang resource directory, and doesn't add flags like
"-lasan -L/path/to/clang/resource/dir". I find this behavior reasonable.

Can we change the flags to "-lasan -L/path/to/clang/resource/dir"? If
we make that change, I can stop suggesting that the clang build should
invoke the compiler-rt test suite directly. :slight_smile:

Do you want to give it a try?

I'll take a crack it, sure.

By the way, locally, I now have just over half the ASan test suite
passing ARM-Linux via QEMU. It's been really convenient to build
compiler-rt in standalone mode. The edit-compile-run cycle is much
shorter this way. Thanks for your help in making this work!

-Greg

By the way, locally, I now have just over half the ASan test suite
passing ARM-Linux via QEMU.

Greg,

Do you mean that you've added support for QEMU-based testing to sanitizer CMakeLists? That would be super-cool.

-Y

> we would still want to use compiler-rt test-suite in a standalone mode,
to test fully built/installed toolchains,
and even GCC.

Sounds good.

> Clang driver links the static xsan runtimes from a hardcoded
> paths in Clang resource directory, and doesn't add flags like
> "-lasan -L/path/to/clang/resource/dir". I find this behavior reasonable.

Can we change the flags to "-lasan -L/path/to/clang/resource/dir"? If
we make that change,

I don't think it's a good idea to let user hijack the driver and stuff in
custom version of
ASan runtime instead the one installed/built with compiler :slight_smile:

I can stop suggesting that the clang build should
invoke the compiler-rt test suite directly. :slight_smile:

I haven't yet fully understood your complaints. Note that if you
build/configure compiler-rt
in a standalone mode, then "make check-compiler-rt" in that tree wouldn't
rebuild runtimes,
and instead assume that they are provided by toolchain (that is, in
standalone mode runtimes and
test suites are effectively independent). What do you mean by "invoking the
test-suite directly"?

Do you mean that you’ve added support for
QEMU-based testing to sanitizer CMakeLists?

Yes. I’m up to 75% of ASan’s test suite passing now. Most of the remaining failures are 1-off line numbers in the stack traces.

Greg

I don’t think it’s a good idea to let user hijack
the driver and stuff in custom version of
ASan runtime instead the one installed/built
with compiler :slight_smile:

I’m okay with it. This is open source. If someone wants to put the sanitizers on a shorter release cycle than the full toolchain, who are we to hold them back?

What do you mean by “invoking the test-suite directly”?

In Clang’s CMakeLists, add_subdirectory() of compiler-rt’s ‘test’ directory. I suggested we do this earlier, because of clang’s hardcoded dependency on the path to ASan. Instead, I think a better solution is to break the hardcoded dependency, allowing the test suite to point clang to compiler-rt’s local copy of ASan using a ‘-L’ parameter.

Greg

> Do you mean that you've added support for
> QEMU-based testing to sanitizer CMakeLists?

Yes. I'm up to 75% of ASan's test suite passing now. Most of the
remaining failures are 1-off line numbers in the stack traces.

Weird. Does it mean that debug info is inaccurate?

> I don't think it's a good idea to let user hijack
> the driver and stuff in custom version of
> ASan runtime instead the one installed/built
> with compiler :slight_smile:

I'm okay with it. This is open source. If someone wants to put the
sanitizers on a shorter release cycle than the full toolchain, who are we
to hold them back?

They can always rebuild the fresh ASan from source and replace the ASan
runtime in toolchain with the new one.

> What do you mean by "invoking the test-suite directly"?

In Clang's CMakeLists, add_subdirectory() of compiler-rt's 'test'
directory. I suggested we do this earlier, because of clang's hardcoded
dependency on the path to ASan. Instead, I think a better solution is to
break the hardcoded dependency, allowing the test suite to point clang to
compiler-rt's local copy of ASan using a '-L' parameter.

I don't believe it's needed anywhere except in the testing infrastructure
you suggest. And, again, we have to ensure that -L/path/to/new/asan
provided by user "wins" over -L/path/to/clang/resource/dir, and we have
strong reasons to put the latter as early in the link line as possible.
That said, ....

Greg

> we would still want to use compiler-rt test-suite in a standalone
mode, to test fully built/installed toolchains,
and even GCC.

Sounds good.

> Clang driver links the static xsan runtimes from a hardcoded
> paths in Clang resource directory, and doesn't add flags like
> "-lasan -L/path/to/clang/resource/dir". I find this behavior
reasonable.

Can we change the flags to "-lasan -L/path/to/clang/resource/dir"? If
we make that change,

I don't think it's a good idea to let user hijack the driver and stuff in
custom version of
ASan runtime instead the one installed/built with compiler :slight_smile:

I can stop suggesting that the clang build should
invoke the compiler-rt test suite directly. :slight_smile:

I haven't yet fully understood your complaints.

Note that if you build/configure compiler-rt

in a standalone mode, then "make check-compiler-rt" in that tree wouldn't
rebuild runtimes,
and instead assume that they are provided by toolchain (that is, in
standalone mode runtimes and
test suites are effectively independent).

^^^^
What is wrong with that approach?

in a standalone mode, then "make check-compiler-rt" in that tree
wouldn't rebuild runtimes, and instead assume that they are provided by toolchain

What is wrong with that approach?

Not wrong, just inconsistent. I'm trying to test the
just-built-runtime before running the 'install' step. That's the way
all the other projects work, "all check-all install" in that order.
In other words, "build, verify, commit".

And, again, we have to ensure that -L/path/to/new/asan provided by
user "wins" over -L/path/to/clang/resource/dir, and we have strong
reasons to put the latter as early in the link line as possible.

Some good news, the drivers (both gcc and clang) allow us to put the
'-L' parameters after the '-l' parameters. So we can put -lasan at
the start of the parameter list to get the desired link order and -L
at the end so that the user-specified runtime is always chosen.

-Greg

Alexey,

Some good news, the drivers (both gcc and clang) allow us to put the
'-L' parameters after the '-l' parameters.

I made these changes locally and it went really well. The patch to
clang is quite small and only one unit-test needed updating. In
compiler-rt, I updated the flags passed to clang to include a
'-L${COMPILER_RT_BINARY_DIR}/lib' and
'-I${COMPILER_RT_BINARY_DIR}/include' flag. All tests pass except for
the default blacklist tests. I had to move those into the clang test
suite.

As for cross-compilation and test ARM Linux, I needed to update all
tests to include '%sim' before invoking cross-compiled executables.
For x86, the variable is empty. For cross-compiling, it's a
user-configured ${COMPILER_RT_EMULATOR}, which for ARM Linux, is QEMU.
There are currently a handful of test failures. If you'd like, I can
XFAIL them and commit this stuff sooner than later.

Do you want these patches?

-Greg

Hi Greg,

First of all, sorry for the late response (I'm in the process of moving to
California).

Alexey,

>> Some good news, the drivers (both gcc and clang) allow us to put the
>> '-L' parameters after the '-l' parameters.

I made these changes locally and it went really well. The patch to
clang is quite small and only one unit-test needed updating. In
compiler-rt, I updated the flags passed to clang to include a
'-L${COMPILER_RT_BINARY_DIR}/lib' and
'-I${COMPILER_RT_BINARY_DIR}/include' flag. All tests pass except for
the default blacklist tests. I had to move those into the clang test
suite.

I've looked at the patch to Clang. I'll try to explain why I'm opposed to
this approach.

You are making the test suite configuration more and more dependent on the
specific
version of the specific Compiler (ToT Clang), while recently we were moving
in the opposite direction.
You even hardcode the knowledge of how the sanitizer headers and sanitizer
libraries are used (by adding -I and -L flags).

I treat sanitizer test-suite as toolchain tests, not a library tests. We
need to verify that simple command
"clang -fsanitize=address foo.cc" works, and produce a binary that works as
expected. As an illustration, suppose
the following sequence of events
1) we land your changes to Clang driver, so that it uses -lasan
-L/path/to/clang/resource/dir
2) we add additional -L/path/to/compiler-rt/libs to all %clang invocations
in lit test-suite.
3) someone accidentally breaks the Clang dirver, and now it uses -L/*wrong/*
path/to/clang/resource/dir
We will not be able to detect this error. The test-suite is green, but the
functionality is broken.

IMO we should just deal with the fact that sanitizer test-suite depends not
only on sanitizer libraries themselves,
but also on LLVM/Clang libs and tools.

As for cross-compilation and test ARM Linux, I needed to update all
tests to include '%sim' before invoking cross-compiled executables.
For x86, the variable is empty. For cross-compiling, it's a
user-configured ${COMPILER_RT_EMULATOR}, which for ARM Linux, is QEMU.
There are currently a handful of test failures. If you'd like, I can
XFAIL them and commit this stuff sooner than later.

This looks familiar to what Evgeniy did for running lit test suite on
Android. This is now possible
w/o modifying any RUN-lines, see the hacky scripts in
compiler-rt/test/asan/android_commands/
folder. Can you use the similar approach for QEMU?

First of all, sorry for the late response (I'm in the process of moving to California).

Welcome!

We need to verify that simple command "clang -fsanitize=address foo.cc" works

Agreed. The test suite is useful in many different scenarios:

1) Verifying the integrated clang.
2) Verifying the integrated gcc.
3) Verifying the libraries during development.

I'm working to improve #3 and in a way that does not interfere with #1 or #2.

we add additional -L/path/to/compiler-rt/libs to all %clang invocations in lit test-suite.

Yes, that's the next patch I have for you. But adding the '-L' flag
is optional. If you want to use the test suite to verify an
integrated clang or gcc, you disable that.

This looks familiar to what Evgeniy did for running lit test suite on
Android. This is now possible w/o modifying any RUN-lines, see
the hacky scripts in compiler-rt/test/asan/android_commands/folder.
Can you use the similar approach for QEMU?

Clever, but I hope we can try to avoid the symlink hackery. Locally,
I've renamed "%sim %t" to "%run %t" so it reads quite nicely, IMHO.
It was a bunch of work to go change all the tests, but I think it's
the right approach. Each cross-compiled variation configures "%run"
from the command-line when invoking CMake. No hacks.

-Greg
.