[LLVMdev] Building sanitizers for Android

[Alexey Samsonov]

Hi Greg,

On Wed, Apr 16, 2014 at 4:58 PM, Greg Fitzgerald <garious at gmail.com> wrote:

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

(1) should be the part of (3). If you want to test the sanitizer runtiume
library "during development",
you should verify that it works with the Clang at hand. Sanitizer runtime
and the compiler are tightly
coupled, why would you want to test the former in isolation?


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

Modifying Clang driver for testing-only purposes, especially for testing in
a non-default configuration
doesn't sound good to me. Also, I still don't get why you would want that
(see comment above).


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

I will reply to that shortly.


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



-- 
Alexey Samsonov, Mountain View, CA
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140418/7c7041ba/attachment.html>