[compiler-rt] initial support for cmake builds of lit-style tests

[chandlerc, 2012-07-25 18:51:03]

http://codereview.appspot.com/6443046/diff/1/lib/asan/output_tests/CMakeLists...
File lib/asan/output_tests/CMakeLists.txt (right):

http://codereview.appspot.com/6443046/diff/1/lib/asan/output_tests/CMakeLists...
lib/asan/output_tests/CMakeLists.txt:1: set(ASAN_SOURCE_DIR
"${CMAKE_CURRENT_SOURCE_DIR}/..")
What's this used for?

http://codereview.appspot.com/6443046/diff/1/lib/asan/output_tests/CMakeLists...
lib/asan/output_tests/CMakeLists.txt:10: clang-rt.asan-x86_64
clang-rt.asan-i386)
Closing paren on the next line for consistency please.

http://codereview.appspot.com/6443046/diff/1/lib/asan/output_tests/deep_tail_...
File lib/asan/output_tests/deep_tail_call.cc (right):

http://codereview.appspot.com/6443046/diff/1/lib/asan/output_tests/deep_tail_...
lib/asan/output_tests/deep_tail_call.cc:2: // RUN: %t 2>&1 | %symbolizer |
FileCheck %s --check-prefix=Check-Common
I think it would be useful to switch to the default FileCheck prefix of 'CHECK'
just to make these tests more approachable.

http://codereview.appspot.com/6443046/diff/1/lib/asan/output_tests/lit.cfg
File lib/asan/output_tests/lit.cfg (right):

http://codereview.appspot.com/6443046/diff/1/lib/asan/output_tests/lit.cfg#ne...
lib/asan/output_tests/lit.cfg:25: # Setup clang binary.
I feel like all of the clang binary setup should be sunk into lit.common.cfg

[samsonov, 2012-07-30 06:36:09]

Thanks for comments! I've addressed them and moved new files to asan/lit_tests
directory to avoid breaking down current testing machinery.

http://codereview.appspot.com/6443046/diff/1/lib/asan/output_tests/CMakeLists...
File lib/asan/output_tests/CMakeLists.txt (right):

http://codereview.appspot.com/6443046/diff/1/lib/asan/output_tests/CMakeLists...
lib/asan/output_tests/CMakeLists.txt:1: set(ASAN_SOURCE_DIR
"${CMAKE_CURRENT_SOURCE_DIR}/..")
On 2012/07/25 18:51:04, chandlerc wrote:
> What's this used for?
Removed.

http://codereview.appspot.com/6443046/diff/1/lib/asan/output_tests/CMakeLists...
lib/asan/output_tests/CMakeLists.txt:10: clang-rt.asan-x86_64
clang-rt.asan-i386)
On 2012/07/25 18:51:04, chandlerc wrote:
> Closing paren on the next line for consistency please.

Done.

http://codereview.appspot.com/6443046/diff/1/lib/asan/output_tests/deep_tail_...
File lib/asan/output_tests/deep_tail_call.cc (right):

http://codereview.appspot.com/6443046/diff/1/lib/asan/output_tests/deep_tail_...
lib/asan/output_tests/deep_tail_call.cc:2: // RUN: %t 2>&1 | %symbolizer |
FileCheck %s --check-prefix=Check-Common
On 2012/07/25 18:51:04, chandlerc wrote:
> I think it would be useful to switch to the default FileCheck prefix of
'CHECK'
> just to make these tests more approachable.

Done.

http://codereview.appspot.com/6443046/diff/1/lib/asan/output_tests/lit.cfg
File lib/asan/output_tests/lit.cfg (right):

http://codereview.appspot.com/6443046/diff/1/lib/asan/output_tests/lit.cfg#ne...
lib/asan/output_tests/lit.cfg:25: # Setup clang binary.
On 2012/07/25 18:51:04, chandlerc wrote:
> I feel like all of the clang binary setup should be sunk into lit.common.cfg

Done.

[chandlerc, 2012-07-31 08:40:41]

Generally LGTM, see comments on the mailing list thread for high-level stuff.
only small stuff here...

All of these comments are fine to address in follow-up changes if they're more
invasive than you'd like...

http://codereview.appspot.com/6443046/diff/5001/lib/asan/lit_tests/lit.cfg
File lib/asan/lit_tests/lit.cfg (right):

http://codereview.appspot.com/6443046/diff/5001/lib/asan/lit_tests/lit.cfg#ne...
lib/asan/lit_tests/lit.cfg:25: # Setup default compiler flags used with
-faddress-sanitizer option.
Are all of these really required?

http://codereview.appspot.com/6443046/diff/5001/lib/asan/lit_tests/lit.cfg#ne...
lib/asan/lit_tests/lit.cfg:39: lit.fatal("Can't find symbolizer script on path
%r" % symbolizer)
Any reason why not to pull this from the .site.cfg?

Shouldn't the cmake scripts be making a binary form of it some where in the
build tree that we can point at?

[samsonov, 2012-07-31 15:45:50]

On 2012/07/31 08:40:41, chandlerc wrote:
> Generally LGTM, see comments on the mailing list thread for high-level stuff.
> only small stuff here...
> 
> All of these comments are fine to address in follow-up changes if they're more
> invasive than you'd like...

Moved comments to FIXMEs and committed as r161050.

>http://codereview.appspot.com/6443046/diff/5001/lib/asan/lit_tests/lit.cfg
> File lib/asan/lit_tests/lit.cfg (right):
> 
>
http://codereview.appspot.com/6443046/diff/5001/lib/asan/lit_tests/lit.cfg#ne...
> lib/asan/lit_tests/lit.cfg:25: # Setup default compiler flags used with
> -faddress-sanitizer option.
> Are all of these really required?
> 
>
http://codereview.appspot.com/6443046/diff/5001/lib/asan/lit_tests/lit.cfg#ne...
> lib/asan/lit_tests/lit.cfg:39: lit.fatal("Can't find symbolizer script on path
> %r" % symbolizer)
> Any reason why not to pull this from the .site.cfg?
> 
> Shouldn't the cmake scripts be making a binary form of it some where in the
> build tree that we can point at?