<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Apr 22, 2014 at 12:22 PM, Greg Fitzgerald <span dir="ltr"><<a href="mailto:garious@gmail.com" target="_blank">garious@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">> This is a public interface. But ASan runtime (and test-suite) strongly depends on<br>
> the instrumentation pass in Clang.<br>
<br>
</div>How do you feel about adding a runtime init check of a version number<br>
defined by asan_interface.h?<br></blockquote><div><br></div><div>We sometimes bump __asan_init_vX function name for important incompatible changes, that is, use</div><div>a link-time check. Not sure why we would need to maintain an ASan version in public header.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888"><br>
-Greg<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
<br>
On Tue, Apr 22, 2014 at 11:16 AM, Alexey Samsonov <<a href="mailto:samsonov@google.com">samsonov@google.com</a>> wrote:<br>
><br>
> On Tue, Apr 22, 2014 at 10:43 AM, Greg Fitzgerald <<a href="mailto:garious@gmail.com">garious@gmail.com</a>> wrote:<br>
>><br>
>> Sorry for the slow replies.  I'm out on vacation this week.<br>
>><br>
>><br>
>> Alexey wrote:<br>
>> > If you want to test the sanitizer runtiume library "during development",<br>
>> > you should verify that it works with the Clang at hand.<br>
>><br>
>> I want to test an implementation of libraries, not that clang links a<br>
>> library in its install directory.  We only need one clang test for the<br>
>> latter (not 100) and that test already exists in the clang test suite.<br>
><br>
><br>
> No. The test in the clang test suite only calls "clang -fsanitize=address<br>
> -###" and matches<br>
> the command Clang will call linker with. It doesn't verify that the link<br>
> will succeed, or that<br>
> the resulting binary would run and produce expected output.<br>
><br>
>><br>
>><br>
>><br>
>> > Sanitizer runtime and the compiler are tightly<br>
>> > coupled, why would you want to test the former in isolation?<br>
>><br>
>> They aren't that tightly-coupled though.  There's an interface and an<br>
>> implementation.  That interface rarely changes relative to the number<br>
>> of changes to the implementation.<br>
>><br>
>><br>
>> <a href="https://github.com/llvm-mirror/compiler-rt/commits/master/include/sanitizer" target="_blank">https://github.com/llvm-mirror/compiler-rt/commits/master/include/sanitizer</a><br>
><br>
><br>
> This is a public interface. But ASan runtime (and test-suite) strongly<br>
> depends on the instrumentation<br>
> pass in Clang. The latter can define hidden experimental flags we are<br>
> testing. Instrumentation pass<br>
> and compiler-rt library depend on each other. There were _several_ changes<br>
> in instrumentation pass<br>
> last week, most of which required a corresponding change in compiler-rt:<br>
><br>
> <a href="https://github.com/llvm-mirror/llvm/commits/master/lib/Transforms/Instrumentation/AddressSanitizer.cpp" target="_blank">https://github.com/llvm-mirror/llvm/commits/master/lib/Transforms/Instrumentation/AddressSanitizer.cpp</a><br>

><br>
>><br>
>><br>
>> <a href="https://github.com/llvm-mirror/compiler-rt/commits/master/lib" target="_blank">https://github.com/llvm-mirror/compiler-rt/commits/master/lib</a><br>
><br>
><br>
>><br>
>><br>
>><br>
>><br>
>> > Modifying Clang driver for testing-only purposes, especially for testing<br>
>> > in a non-default<br>
>> > configuration doesn't sound good to me.<br>
>><br>
>> Agreed and that's not what's happening.  The optional part is adding a<br>
>> -L flag to the invocation of clang within the compiler-rt test suite.<br>
>> If using the monolithic build, those flags are not added and clang<br>
>> finds the libs via its hardcoded '-L' flag.<br>
>><br>
>><br>
>> Yury wrote:<br>
>> > It's not that easy. Some tests require passing environment variables<br>
>><br>
>> As Evgeniy mentioned, it's a short enough list of variables that you<br>
>> can whitelist them.  LD_PRELOAD, LD_LIBRARY_PATH, ASAN_OPTIONS, etc.<br>
>><br>
>><br>
>> Evgeniy wrote:<br>
>> > Greg, do you copy binaries to the device on %clang or on %run?<br>
>><br>
>> %run.  Can you point me to a case where you needed to override %clang for<br>
>> that?<br>
>><br>
>> Thanks,<br>
>> Greg<br>
>><br>
>> On Mon, Apr 21, 2014 at 12:54 AM, Evgeniy Stepanov <<a href="mailto:eugenis@google.com">eugenis@google.com</a>><br>
>> wrote:<br>
>> > Both %run and my symlink approach add certain (yet undocumented)<br>
>> > requirements on the tests, but they should be 100% robust if those<br>
>> > requirements are followed.<br>
>> > Greg, do you copy binaries to the device on %clang or on %run? The<br>
>> > latter would miss shared libraries that are not executed directly.<br>
>> > Environment can be recreated on the device, my script attempts to do<br>
>> > it (for several whitelisted variables).<br>
>> > We don't preserve ulimit setting, I modified one or two tests to not<br>
>> > rely on that.<br>
>> ><br>
>> > I don't mind switching to a combined approach - copy to device on<br>
>> > %clang, and replace symlink hacks with %run.<br>
>> ><br>
>> ><br>
>> ><br>
>> > On Mon, Apr 21, 2014 at 9:09 AM, Yury Gribov <<a href="mailto:y.gribov@samsung.com">y.gribov@samsung.com</a>><br>
>> > wrote:<br>
>> >>> We considered adding "%run" to all binary invocations,<br>
>> >>> but dropped this idea. I don't remember the details, but IIRC %run is<br>
>> >>> just not general enough.<br>
>> >><br>
>> >><br>
>> >> IMHO this is where simplicity of lit approach starts to fail -<br>
>> >> important<br>
>> >> information (environment variables, dependent shared libs, expected<br>
>> >> test<br>
>> >> status, etc.) is buried inside arbitrarily complex runstrings.<br>
>> >><br>
>> >> -Y<br>
><br>
><br>
><br>
><br>
> --<br>
> Alexey Samsonov, Mountain View, CA<br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div dir="ltr"><div>Alexey Samsonov, Mountain View, CA</div></div>
</div></div>