[LLVMdev] Building sanitizers for Android

[Greg Fitzgerald]

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
_______________________________________________
LLVM Developers mailing list
LLV...@cs.uiuc.edu http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

[Evgeniy Stepanov]

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.

On Thu, Mar 27, 2014 at 5:00 AM, Greg Fitzgerald <gar...@gmail.com> wrote:
> 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.

[Greg Fitzgerald]

> 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

[Evgeniy Stepanov]

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.

[Greg Fitzgerald]

> 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






On Fri, Mar 28, 2014 at 12:39 AM, Evgeniy Stepanov

[Alexey Samsonov]

On Fri, Mar 28, 2014 at 10:56 PM, Greg Fitzgerald <gar...@gmail.com> wrote:

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

--

Alexey Samsonov, MSK

[Evgeniy Stepanov]

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

[Greg Fitzgerald]

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

On Tue, Apr 1, 2014 at 12:58 AM, Evgeniy Stepanov

[Alexey Samsonov]

Hi Greg,

On Wed, Apr 2, 2014 at 2:50 AM, Greg Fitzgerald <gar...@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?

--

Alexey Samsonov, MSK

[Greg Fitzgerald]

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

> 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

[Yury Gribov]

> 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

[Alexey Samsonov]

On Thu, Apr 3, 2014 at 5:04 AM, Greg Fitzgerald <gar...@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 do you mean by "invoking the test-suite directly"?

--

Alexey Samsonov, MSK

[Greg Fitzgerald]

> 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

[Greg Fitzgerald]

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

> 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

--

Alexey Samsonov, MSK

[Alexey Samsonov]

On Thu, Apr 3, 2014 at 8:55 PM, Greg Fitzgerald <gar...@gmail.com> wrote:

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

 

Greg

On Wednesday, April 2, 2014, Yury Gribov <y.gr...@samsung.com> wrote:

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

--

Alexey Samsonov, MSK

[Alexey Samsonov]

On Thu, Apr 3, 2014 at 9:23 PM, Greg Fitzgerald <gar...@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 <sams...@google.com> wrote:

On Thu, Apr 3, 2014 at 5:04 AM, Greg Fitzgerald <gar...@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?

--

Alexey Samsonov, MSK

[Greg Fitzgerald]

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

[Greg Fitzgerald]

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

[Alexey Samsonov]

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 <gar...@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?

--

Alexey Samsonov, MSK

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

[Yury Gribov]

Greg,

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

Does %run support ssh-based testing? qemu-arm is cool but it does not
emulate threads and other low-level stuff well which may be important
for some users.

-Y

[Greg Fitzgerald]

> Does %run support ssh-based testing?

Yes, %run is configured by the user. I configure it to call a shell
script that calls qemu-arm. It could just as easily ssh or "adb push
&& adb shell".

-Greg

[Alexey Samsonov]

Hi Greg,

On Wed, Apr 16, 2014 at 4:58 PM, Greg Fitzgerald <gar...@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.

--

Alexey Samsonov, Mountain View, CA

[Alexey Samsonov]

On Thu, Apr 17, 2014 at 12:24 PM, Greg Fitzgerald <gar...@gmail.com> wrote:

> Does %run support ssh-based testing?

Yes, %run is configured by the user.  I configure it to call a shell
script that calls qemu-arm.  It could just as easily ssh or "adb push
&& adb shell".

Please discuss this with Evgeniy. We considered adding "%run" to all binary invocations,

but dropped this idea. I don't remember the details, but IIRC %run is just not general enough.

Some tests are not hermetic enough - for example, suppose that the test dlopen()'s a shared object - how

would you copy this file to the device/emulator? Same holds for arguments of -fsanitize-blacklist=.

 

-Greg

On Wed, Apr 16, 2014 at 11:44 PM, Yury Gribov <y.gr...@samsung.com> wrote:
> Greg,
>
>
>> 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.
>
> Does %run support ssh-based testing? qemu-arm is cool but it does not
> emulate threads and other low-level stuff well which may be important for
> some users.
>
> -Y
>

[Yury Gribov]

On 04/17/2014 11:24 PM, Greg Fitzgerald wrote:
>> Does %run support ssh-based testing?
>
> Yes, %run is configured by the user. I configure it to call a shell
> script that calls qemu-arm. It could just as easily ssh or "adb push
> && adb shell".

It's not that easy. Some tests require passing environment variables
(which aren't transparently forwarded by ssh/adb) and some require
several objects to run (e.g. executable + dynamic library).
It sounds like the only way would be to support this in %run would be to
parse all lit commands (not just %run-command btw), detect environments
variables and all objects that executable depends on, copy them to
device, etc.

Frankly I'm not sure %run will be robust solution for anything but qemu-arm.

[Yury Gribov]

> We considered adding "%run" to all binary invocations,
> but dropped this idea. I don't remember the details, but IIRC %run is
> just not general enough.

IMHO this is where simplicity of lit approach starts to fail - important
information (environment variables, dependent shared libs, expected test
status, etc.) is buried inside arbitrarily complex runstrings.

[Evgeniy Stepanov]

Both %run and my symlink approach add certain (yet undocumented)
requirements on the tests, but they should be 100% robust if those
requirements are followed.
Greg, do you copy binaries to the device on %clang or on %run? The
latter would miss shared libraries that are not executed directly.
Environment can be recreated on the device, my script attempts to do
it (for several whitelisted variables).
We don't preserve ulimit setting, I modified one or two tests to not
rely on that.

I don't mind switching to a combined approach - copy to device on
%clang, and replace symlink hacks with %run.

[Greg Fitzgerald]

Sorry for the slow replies. I'm out on vacation this week.

Alexey wrote:
> If you want to test the sanitizer runtiume library "during development",
> you should verify that it works with the Clang at hand.

I want to test an implementation of libraries, not that clang links a
library in its install directory. We only need one clang test for the
latter (not 100) and that test already exists in the clang test suite.

> Sanitizer runtime and the compiler are tightly
> coupled, why would you want to test the former in isolation?

They aren't that tightly-coupled though. There's an interface and an
implementation. That interface rarely changes relative to the number
of changes to the implementation.

https://github.com/llvm-mirror/compiler-rt/commits/master/include/sanitizer
https://github.com/llvm-mirror/compiler-rt/commits/master/lib

> Modifying Clang driver for testing-only purposes, especially for testing in a non-default
> configuration doesn't sound good to me.

Agreed and that's not what's happening. The optional part is adding a
-L flag to the invocation of clang within the compiler-rt test suite.
If using the monolithic build, those flags are not added and clang
finds the libs via its hardcoded '-L' flag.

Yury wrote:
> It's not that easy. Some tests require passing environment variables

As Evgeniy mentioned, it's a short enough list of variables that you
can whitelist them. LD_PRELOAD, LD_LIBRARY_PATH, ASAN_OPTIONS, etc.

Evgeniy wrote:
> Greg, do you copy binaries to the device on %clang or on %run?

%run. Can you point me to a case where you needed to override %clang for that?

Thanks,
Greg

[Alexey Samsonov]

On Tue, Apr 22, 2014 at 10:43 AM, Greg Fitzgerald <gar...@gmail.com> wrote:

Sorry for the slow replies.  I'm out on vacation this week.

Alexey wrote:
> If you want to test the sanitizer runtiume library "during development",
> you should verify that it works with the Clang at hand.

I want to test an implementation of libraries, not that clang links a
library in its install directory.  We only need one clang test for the
latter (not 100) and that test already exists in the clang test suite.

No. The test in the clang test suite only calls "clang -fsanitize=address -###" and matches

the command Clang will call linker with. It doesn't verify that the link will succeed, or that

the resulting binary would run and produce expected output.

 

> Sanitizer runtime and the compiler are tightly
> coupled, why would you want to test the former in isolation?

They aren't that tightly-coupled though.  There's an interface and an
implementation.  That interface rarely changes relative to the number
of changes to the implementation.

https://github.com/llvm-mirror/compiler-rt/commits/master/include/sanitizer

This is a public interface. But ASan runtime (and test-suite) strongly depends on the instrumentation

pass in Clang. The latter can define hidden experimental flags we are testing. Instrumentation pass

and compiler-rt library depend on each other. There were _several_ changes in instrumentation pass

last week, most of which required a corresponding change in compiler-rt:

https://github.com/llvm-mirror/llvm/commits/master/lib/Transforms/Instrumentation/AddressSanitizer.cpp

[Greg Fitzgerald]

> This is a public interface. But ASan runtime (and test-suite) strongly depends on
> the instrumentation pass in Clang.

How do you feel about adding a runtime init check of a version number
defined by asan_interface.h?

-Greg

[Alexey Samsonov]

On Tue, Apr 22, 2014 at 12:22 PM, Greg Fitzgerald <gar...@gmail.com> wrote:

> This is a public interface. But ASan runtime (and test-suite) strongly depends on
> the instrumentation pass in Clang.

How do you feel about adding a runtime init check of a version number
defined by asan_interface.h?

We sometimes bump __asan_init_vX function name for important incompatible changes, that is, use

a link-time check. Not sure why we would need to maintain an ASan version in public header.

[Yury Gribov]

>> It's not that easy. Some tests require passing environment variables
>
> As Evgeniy mentioned, it's a short enough list of variables that you
> can whitelist them. LD_PRELOAD, LD_LIBRARY_PATH, ASAN_OPTIONS, etc.

Still sounds somewhat hacky. E.g. what if you've sanitized Clang itself
and then run it with custom ASAN_OPTIONS?
Surely you won't want to propagate them to ssh unless the test
explicitly asks for them.

[Evgeniy Stepanov]

On Wed, Apr 23, 2014 at 9:08 AM, Yury Gribov <y.gr...@samsung.com> wrote:
>>> It's not that easy. Some tests require passing environment variables
>>
>>
>> As Evgeniy mentioned, it's a short enough list of variables that you
>> can whitelist them. LD_PRELOAD, LD_LIBRARY_PATH, ASAN_OPTIONS, etc.
>
>
> Still sounds somewhat hacky. E.g. what if you've sanitized Clang itself and
> then run it with custom ASAN_OPTIONS?
> Surely you won't want to propagate them to ssh unless the test explicitly
> asks for them.

I think we want to propagate them anyway. We are talking of a %run
macro, which is a replacement for directly running the test binary -
if the binary would get ASAN_OPTIONS in the "normal" (native) testing
mode, we would like it to get the same ASAN_OPTIONS in the
cross-testing mode as well. We just need to filter out stuff that does
not make sense on a remote machine, like host PATH setting.

[Evgeniy Stepanov]

> Evgeniy wrote:
>> Greg, do you copy binaries to the device on %clang or on %run?
>
> %run. Can you point me to a case where you needed to override %clang for that?

Shared libraries. Grep -shared in ASan tests. They are not %run
directly, and sometimes not even mentioned on the %run line at all -
some tests dlopen() libraries by a relative path.

[Greg Fitzgerald]

Thanks for your feedback Alexey. I've decided to abandon my effort to
include the sanitizer lit tests in the compiler-rt component. They
needs to be part of the clang component because the clang-generated
instrumentation is nontrivial.

-Greg