[LLVMdev] Building sanitizers for Android

[Greg Fitzgerald]

> 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
.

On Tue, Apr 15, 2014 at 5:08 PM, Alexey Samsonov <samsonov at google.com> wrote:
> Hi Greg,
>
> First of all, sorry for the late response (I'm in the process of moving to
> California).
>
> On Fri, Apr 4, 2014 at 6:13 PM, Greg Fitzgerald <garious at gmail.com> wrote:
>>
>> 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?
>
>>
>> Do you want these patches?
>>
>> -Greg
>>
>>
>> On Fri, Apr 4, 2014 at 10:22 AM, Greg Fitzgerald <garious at gmail.com>
>> wrote:
>> >>>> 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
>> >
>> >
>> > On Fri, Apr 4, 2014 at 5:57 AM, Alexey Samsonov <samsonov at google.com>
>> > wrote:
>> >>
>> >> On Thu, Apr 3, 2014 at 9:23 PM, Greg Fitzgerald <garious at gmail.com>
>> >> wrote:
>> >>>
>> >>> > 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 :)
>> >>>
>> >>> 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
>> >>>
>> >>>
>> >>> On Thursday, April 3, 2014, Alexey Samsonov <samsonov at google.com>
>> >>> wrote:
>> >>>>
>> >>>>
>> >>>> On Thu, Apr 3, 2014 at 5:04 AM, Greg Fitzgerald <garious at gmail.com>
>> >>>> wrote:
>> >>>>>
>> >>>>> > 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 :)
>> >>>>
>> >>>>>
>> >>>>> I can stop suggesting that the clang build should
>> >>>>> invoke the compiler-rt test suite directly.  :-)
>> >>>>
>> >>>>
>> >>>> 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?
>> >>
>> >>
>> >>>>
>> >>>> What do you mean by "invoking the test-suite directly"?
>> >>>>
>> >>>>
>> >>>>
>> >>>>
>> >>>> > 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
>> >>>>
>> >>>>
>> >>>>
>> >>>> On Wed, Apr 2, 2014 at 7:56 AM, Alexey Samsonov <samsonov at google.com>
>> >>>> wrote:
>> >>>> > Hi Greg,
>> >>>> >
>> >>>> > On Wed, Apr 2, 2014 at 2:50 AM, Greg Fitzgerald <garious at gmail.com>
>> >>>> > wrote:
>> >>>> >>
>> >>>> >> 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?
>> >>>> >
>> >>>> >>
>> >>>> >>
>> >>>> >> Thoughts?
>> >>>> >>
>> >>>> >> Thanks,
>> >>>> >> Greg
>> >>>> >>
>> >>>> >> On Tue, Apr 1, 2014 at 12:58 AM, Evgeniy Stepanov
>> >>>> >> <eugeni.stepanov at gmail.com> wrote:
>> >>>> >> > 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)?
>> >>>> >> >
>> >>>> >> > On Mon, Mar 31, 2014 at 6:46 PM, Alexey Samsonov
>> >>>> >> > <samsonov at google.com>
>> >>>> >> > wrote:
>> >>>> >
>> >>>>
>> >>>>
>> >>>>
>> >>>>
>> >>>> --
>> >>>> Alexey Samsonov, MSK
>> >>
>> >>
>> >>
>> >>
>> >> --
>> >> Alexey Samsonov, MSK
>
>
>
>
> --
> Alexey Samsonov, MSK