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 variablesAs 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 On Mon, Apr 21, 2014 at 12:54 AM, Evgeniy Stepanov <eugenis at google.com> wrote:> 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. > > > > On Mon, Apr 21, 2014 at 9:09 AM, Yury Gribov <y.gribov at samsung.com> wrote: >>> 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. >> >> -Y
On Tue, Apr 22, 2014 at 10:43 AM, Greg Fitzgerald <garious at 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/sanitizerThis 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> > 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 > > On Mon, Apr 21, 2014 at 12:54 AM, Evgeniy Stepanov <eugenis at google.com> > wrote: > > 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. > > > > > > > > On Mon, Apr 21, 2014 at 9:09 AM, Yury Gribov <y.gribov at samsung.com> > wrote: > >>> 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. > >> > >> -Y >-- Alexey Samsonov, Mountain View, CA -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140422/e718cb96/attachment.html>
> 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 On Tue, Apr 22, 2014 at 11:16 AM, Alexey Samsonov <samsonov at google.com> wrote:> > On Tue, Apr 22, 2014 at 10:43 AM, Greg Fitzgerald <garious at 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 > >> >> >> 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 >> >> On Mon, Apr 21, 2014 at 12:54 AM, Evgeniy Stepanov <eugenis at google.com> >> wrote: >> > 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. >> > >> > >> > >> > On Mon, Apr 21, 2014 at 9:09 AM, Yury Gribov <y.gribov at samsung.com> >> > wrote: >> >>> 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. >> >> >> >> -Y > > > > > -- > Alexey Samsonov, Mountain View, CA
>> 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. -Y
On Wed, Apr 23, 2014 at 9:08 AM, Yury Gribov <y.gribov at 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.> > -Y
> 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.